Fix GString equality checks with String (#6330)

---------

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

Author: bentsherman

docs/migrations/25-10.md

@@ -29,29 +29,45 @@ workflow {
 
 This syntax is simpler and easier to use with the {ref}`strict syntax <strict-syntax-page>`. See {ref}`workflow-handlers` for details.
 
-<h3>Configurable date and time formatting</h3>
+<h3>Improved handling of dynamic directives</h3>
 
-:::{versionadded} 25.07.0-edge
-:::
+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.
+
+<h3>Configurable date formatting</h3>
 
-You can now customize the date and time format used in notifications, reports, and console output using the `NXF_DATE_FORMAT` environment variable. This addresses previous inconsistencies in timestamp representations:
+You can now customize the date and time format used in notifications, reports, and console output using the `NXF_DATE_FORMAT` environment variable:
 
 ```bash
+# Default format
+# e.g., 11-Aug-2016 09:40:20
+
 # Use ISO format with timezone
-export NXF_DATE_FORMAT=iso
-nextflow run workflow.nf
-# Output: 2016-08-11T09:40:20+02:00
+export NXF_DATE_FORMAT="iso"
+# e.g., 2016-08-11T09:40:20+02:00
 
-# Use custom format  
+# Use custom format
 export NXF_DATE_FORMAT="yyyy-MM-dd HH:mm"
-nextflow run workflow.nf
-# Output: 2016-08-11 09:40
-
-# Default format includes seconds
-nextflow run workflow.nf
-# Output: 11-Aug-2016 09:40:20
+# e.g., 2016-08-11 09:40
 ```
 
+This feature addresses previous inconsistencies in timestamp representations.
+
 ## 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

@@ -1276,15 +1276,16 @@ 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 can be specified without 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.
-:::
+Directives will be evaluated dynamically if they reference task inputs. Process configuration options must still be specified with a closure to be dynamic.
 
 (dynamic-task-resources)=
 

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());
+    }
+
+}