Fix GString equality check with String

Signed-off-by: Ben Sherman <[email protected]>

Author: bentsherman

docs/migrations/25-10.md

@@ -29,6 +29,26 @@ workflow {
 
 This syntax is simpler and easier to use with the {ref}`strict syntax <strict-syntax-page>`. See {ref}`workflow-handlers` for details.
 
+<h3>Improved handling of dynamic directives</h3>
+
+The {ref}`strict syntax <strict-syntax-page>` allows dynamic process directives to be specified without a closure:
+
+```nextflow
+process hello {
+    queue (entries > 100 ? 'long' : 'short')
+
+    input:
+    tuple val(entries), path('data.txt')
+
+    script:
+    """
+    your_command --here
+    """
+}
+```
+
+See {ref}`dynamic-directives` for details.
+
 ## Breaking changes
 
 - The AWS Java SDK used by Nextflow was upgraded from v1 to v2, which introduced some breaking changes to the `aws.client` config options. See {ref}`the guide <aws-java-sdk-v2-page>` for details.

docs/process.md

@@ -1255,16 +1255,16 @@ To be defined dynamically, the directive's value needs to be expressed using a {
 
 ```nextflow
 process hello {
-  executor 'sge'
-  queue { entries > 100 ? 'long' : 'short' }
+    executor 'sge'
+    queue { entries > 100 ? 'long' : 'short' }
 
-  input:
-  tuple val(entries), path('data.txt')
+    input:
+    tuple val(entries), path('data.txt')
 
-  script:
-  """
-  your_command --here
-  """
+    script:
+    """
+    your_command --here
+    """
 }
 ```
 
@@ -1276,14 +1276,19 @@ All directives can be assigned a dynamic value except the following:
 - {ref}`process-label`
 - {ref}`process-maxforks`
 
-:::{tip}
-Assigning a string value with one or more variables is always resolved in a dynamic manner, and therefore is equivalent to the above syntax. For example, the above directive can also be written as:
+:::{versionadded} 25.10
+:::
+
+Dynamic directives do not need to be wrapped in a closure when using the {ref}`strict syntax <strict-syntax-page>`:
 
 ```nextflow
-queue "${ entries > 100 ? 'long' : 'short' }"
+queue (entries > 100 ? 'long' : 'short')
 ```
 
-Note, however, that the latter syntax can be used both for a directive's main argument (as in the above example) and for a directive's optional named attributes, whereas the closure syntax is only resolved dynamically for a directive's main argument.
+Nextflow will evaluate this directive dynamically if it references task inputs. Directives that use an explicit closure are still resolved dynamically.
+
+:::{note}
+Process configuration options must still be specified with a closure in order to be dynamic.
 :::
 
 (dynamic-task-resources)=
@@ -1324,6 +1329,9 @@ disk 375.GB, type: 'local-ssd'
 
 // dynamic request
 disk { [request: 375.GB * task.attempt, type: 'local-ssd'] }
+
+// dynamic request (25.10 or later, NXF_SYNTAX_PARSER=v2)
+disk request: 375.GB * task.attempt, type: 'local-ssd'
 ```
 :::
 

modules/nextflow/src/main/groovy/nextflow/config/parser/v2/ConfigCompiler.java

@@ -31,6 +31,7 @@
 import nextflow.config.control.StripSecretsVisitor;
 import nextflow.config.parser.ConfigParserPluginFactory;
 import nextflow.script.control.Compiler;
+import nextflow.script.control.GStringToStringVisitor;
 import nextflow.script.control.PathCompareVisitor;
 import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.ClassHelper;
@@ -179,6 +180,7 @@ private void analyze(SourceUnit source) {
                 new StripSecretsVisitor(source).visitClass(cn);
             if( renderClosureAsString )
                 new ClosureToStringVisitor(source).visitClass(cn);
+            new GStringToStringVisitor(source).visitClass(cn);
         }
 
         SourceUnit createSourceUnit(String source, Path path) {

modules/nextflow/src/main/groovy/nextflow/script/params/CmdEvalParam.groovy

@@ -45,7 +45,7 @@ class CmdEvalParam extends BaseOutParam implements OptionalParam {
     }
 
     BaseOutParam bind( def obj ) {
-        if( obj !instanceof CharSequence )
+        if( obj !instanceof CharSequence && obj !instanceof Closure )
             throw new IllegalArgumentException("Invalid argument for command output: $this")
         // the target value object
         target = obj
@@ -54,7 +54,9 @@ class CmdEvalParam extends BaseOutParam implements OptionalParam {
 
     @Memoized
     String getTarget(Map<String,Object> context) {
-        return target instanceof GString
+        return target instanceof Closure
+            ? context.with(target)
+            : target instanceof GString
             ? target.cloneAsLazy(context).toString()
             : target.toString()
     }

modules/nextflow/src/main/groovy/nextflow/script/parser/v2/ScriptCompiler.java

@@ -34,6 +34,7 @@
 import nextflow.script.ast.WorkflowNode;
 import nextflow.script.control.CallSiteCollector;
 import nextflow.script.control.Compiler;
+import nextflow.script.control.GStringToStringVisitor;
 import nextflow.script.control.ModuleResolver;
 import nextflow.script.control.OpCriteriaVisitor;
 import nextflow.script.control.PathCompareVisitor;
@@ -290,6 +291,7 @@ private void analyze(SourceUnit source) {
             new ScriptToGroovyVisitor(source).visit();
             new PathCompareVisitor(source).visitClass(cn);
             new OpCriteriaVisitor(source).visitClass(cn);
+            new GStringToStringVisitor(source).visitClass(cn);
         }
 
         SourceUnit createSourceUnit(URI uri) {

modules/nextflow/src/test/groovy/nextflow/config/parser/v2/ConfigParserV2Test.groovy

@@ -428,7 +428,7 @@ class ConfigParserV2Test extends Specification {
                 .parse(configText)
         then:
         config.params.str1 instanceof String
-        config.params.str2 instanceof GString
+        config.params.str2 instanceof String
         config.process.clusterOptions instanceof Closure
         config.process.ext.bar instanceof Closure
 
@@ -438,7 +438,7 @@ class ConfigParserV2Test extends Specification {
                 .parse(configText)
         then:
         config.params.str1 instanceof String
-        config.params.str2 instanceof GString
+        config.params.str2 instanceof String
         config.process.clusterOptions instanceof Closure
         config.process.ext.bar instanceof Closure
 

modules/nextflow/src/test/groovy/nextflow/script/parser/v2/ScriptLoaderV2Test.groovy

@@ -177,4 +177,59 @@ class ScriptLoaderV2Test extends Dsl2Spec {
         noExceptionThrown()
     }
 
+    def 'should eagerly evaluate GStrings' () {
+
+        given:
+        def session = new Session()
+        def parser = new ScriptLoaderV2(session)
+
+        def TEXT = '''
+            workflow { 
+                assert "${'hello'}" == 'hello'
+                assert "${'hello'}" in ['hello']
+            }
+            '''
+
+        when:
+        parser.parse(TEXT)
+        parser.runScript()
+
+        then:
+        noExceptionThrown()
+    }
+
+    def 'should lazily evaluate process inputs/outputs/directives' () {
+
+        given:
+        def session = new Session()
+        session.executorFactory = new MockExecutorFactory()
+        def parser = new ScriptLoaderV2(session)
+
+        def TEXT = '''
+            process HELLO {
+                tag props.name
+
+                input:
+                val props
+
+                output:
+                val props.name
+
+                script:
+                "echo 'Hello ${props.name}'"
+            }
+
+            workflow {
+                HELLO( [name: 'World'] )
+            }
+            '''
+
+        when:
+        parser.parse(TEXT)
+        parser.runScript()
+
+        then:
+        noExceptionThrown()
+    }
+
 }

modules/nf-lang/src/main/java/nextflow/config/control/VariableScopeVisitor.java

@@ -119,8 +119,12 @@ public void visitConfigAssign(ConfigAssignNode node) {
         var scopes = currentConfigScopes();
         inProcess = !scopes.isEmpty() && "process".equals(scopes.get(0));
         inClosure = node.value instanceof ClosureExpression;
-        if( inClosure && !inProcess && !isWorkflowHandler(scopes, node) )
-            vsc.addError("Dynamic config options are only allowed in the `process` scope", node);
+        if( inClosure ) {
+            if( isWorkflowHandler(scopes, node) )
+                vsc.addWarning("The use of workflow handlers in the config is deprecated -- use the entry workflow or a plugin instead", String.join(".", node.names), node);
+            else if( !inProcess )
+                vsc.addError("Dynamic config options are only allowed in the `process` scope", node);
+        }
         if( inClosure ) {
             vsc.pushScope(ScriptDsl.class);
             if( inProcess )

modules/nf-lang/src/main/java/nextflow/script/control/GStringToLazyVisitor.java

@@ -30,7 +30,7 @@
  * from
  *   "${foo} ${bar}"
  * to
- *   "${->foo} ${->bar}
+ *   "${->foo} ${->bar}"
  *
  * @author Paolo Di Tommaso <[email protected]>
  */

modules/nf-lang/src/main/java/nextflow/script/control/GStringToStringVisitor.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2013-2024, Seqera Labs
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package nextflow.script.control;
+
+import org.codehaus.groovy.ast.ClassCodeExpressionTransformer;
+import org.codehaus.groovy.ast.expr.ArgumentListExpression;
+import org.codehaus.groovy.ast.expr.ClosureExpression;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.GStringExpression;
+import org.codehaus.groovy.control.SourceUnit;
+
+import static org.codehaus.groovy.ast.tools.GeneralUtils.*;
+
+/**
+ * Coerce a GString into a String.
+ *
+ * from
+ *   "${foo} ${bar}"
+ * to
+ *   "${foo} ${bar}".toString()
+ *
+ * This enables equality checks between GStrings and Strings,
+ * e.g. `"${'hello'}" == 'hello'`.
+ *
+ * @author Ben Sherman <[email protected]>
+ */
+public class GStringToStringVisitor extends ClassCodeExpressionTransformer {
+
+    private SourceUnit sourceUnit;
+
+    public GStringToStringVisitor(SourceUnit sourceUnit) {
+        this.sourceUnit = sourceUnit;
+    }
+
+    @Override
+    protected SourceUnit getSourceUnit() {
+        return sourceUnit;
+    }
+
+    @Override
+    public Expression transform(Expression node) {
+        if( node instanceof ClosureExpression ce )
+            super.visitClosureExpression(ce);
+
+        if( node instanceof GStringExpression gse )
+            return transformToString(gse);
+
+        return super.transform(node);
+    }
+
+    private Expression transformToString(GStringExpression node) {
+        return callX(node, "toString", new ArgumentListExpression());
+    }
+
+}