Enable strict syntax by default

[bentsherman]

• Set NXF_SYNTAX_PARSER to v2 by default
• Deprecate the nextflow.enable.strict feature flag
• Update unit tests to work with strict syntax
• Fix a few bugs with the strict syntax along the way
• Consolidate test harness for parsing/loading/running scripts into ScriptHelper

[netlify]

✅ Deploy Preview for nextflow-docs-staging canceled.

Name	Link
🔨 Latest commit	70e4aa3
🔍 Latest deploy log	https://app.netlify.com/projects/nextflow-docs-staging/deploys/695f1a554320bc000870d77a

[bentsherman]

@claude please review

[claude]

Claude finished @bentsherman's task —— View job

---

Code Review: Enable strict syntax by default

Reviewing the changes to enable strict syntax (v2 parser) by default and deprecate the nextflow.enable.strict feature flag.

Review Tasks:

• Understand the scope and impact of the changes
• Review core implementation changes (NF.groovy, ConfigBuilder, etc.)
• Examine parser and script loading modifications
• Review test consolidation and ScriptHelper implementation
• Check documentation updates
• Analyze potential breaking changes and migration path
• Provide comprehensive feedback

Review Summary

This is a major but well-executed change that enables strict syntax by default in Nextflow. The implementation is thorough and the extensive test updates demonstrate careful attention to maintaining compatibility while enforcing stricter parsing rules.

Positive Aspects

1. Clean Implementation of Default Change

• NF.groovy:36: The change from 'v1' to 'v2' as default for NXF_SYNTAX_PARSER is clean and well-placed
• ConfigBuilder.groovy:440-441: Appropriate conditional validation only applies to v1 parser, maintaining backward compatibility
• The change is controlled by environment variable, allowing easy rollback if needed

2. Comprehensive Test Consolidation

• ScriptHelper.groovy: Excellent consolidation of test harness functionality that was scattered across multiple helper classes
• The new ScriptHelper provides unified methods for loadScript(), runScript(), and parseAndReturnProcess()
• Proper session management with MockSession and MockExecutor classes
• Smart result normalization handling ChannelOut and dataflow constructs

3. Documentation Updates

• docs/strict-syntax.md: Clear versioning annotation showing v2 is now default (line 19-21)
• docs/reference/feature-flags.md: Proper deprecation notice for nextflow.enable.strict (lines 25-27)
• Good migration guidance provided for users

4. Extensive Test Coverage

• 64 files modified shows comprehensive test updates
• Tests properly use the new ScriptHelper methods instead of scattered approaches
• Proper handling of syntax differences between v1 and v2 parsers

Areas for Improvement

1. Error Handling in ScriptLoaderV2

// ScriptLoaderV2.groovy:54-55
if( name )
    throw new IllegalArgumentException("The `-entry` option is not supported with the strict syntax -- use a param to run a named workflow from the entry workflow")

Concern: This error message could be more helpful. Consider:

• Providing a concrete example of how to use params instead
• Linking to documentation about strict syntax entry workflows

2. Configuration Parser Strictness

// ConfigParserV2.groovy:48
private boolean strict = true

Concern: The v2 parser defaults to strict mode, but this could cause issues for users with complex config files that haven't been updated yet. Consider:

• Adding a grace period with warnings before making it strictly enforced
• Providing clearer error messages when strict parsing fails

3. Missing Migration Path for Complex Cases

Concern: While documentation covers many cases, some complex Groovy patterns in configs might need more guidance:

• Dynamic config generation patterns
• Complex closure-based configurations
• Third-party library usage in configs

4. Test File Organization

- modules/nextflow/src/test/groovy/nextflow/extension/SplitFastqOp2Test.groovy (DELETED)
- modules/nextflow/src/testFixtures/groovy/test/MockHelpers.groovy (DELETED)  
- modules/nextflow/src/testFixtures/groovy/test/TestParser.groovy (DELETED)

Observation: Good cleanup of obsolete test utilities, but ensure all functionality is preserved in the new ScriptHelper.

Technical Implementation Review

1. Parser Selection Logic

The implementation correctly uses environment variable to control parser selection:

// NF.groovy:36
static String getSyntaxParserVersion() {
    return SysEnv.get('NXF_SYNTAX_PARSER', 'v2')
}

This provides a clean escape hatch for users who need v1 behavior.

2. Backward Compatibility

The changes maintain good backward compatibility:

• V1 parser still available via environment variable
• Existing validation only applies to v1 parser
• Gradual deprecation path for feature flags

3. AST Transformations

The visitor pattern changes in nf-lang module look appropriate for handling the stricter syntax requirements while maintaining compatibility with existing Groovy AST processing.

Recommendations

1. Consider Phased Rollout

• Consider adding a "strict mode warning" phase before full enforcement
• Provide migration tooling to help users identify issues in their pipelines

2. Enhanced Error Messages

• Improve error messages in ScriptLoaderV2 to be more actionable
• Consider providing suggested fixes for common migration patterns

3. Documentation Enhancements

• Add troubleshooting section for common strict syntax issues
• Provide more examples of complex config migrations
• Document performance implications of strict vs non-strict parsing

4. Monitoring Plan

• Consider adding telemetry to understand adoption and failure patterns
• Plan for support channels during the transition period

Breaking Changes Assessment

Impact Level: Medium to High - This is a breaking change for some users, but:

✅ Mitigated by:

• Environment variable escape hatch (NXF_SYNTAX_PARSER=v1)
• Comprehensive documentation and migration guide
• Clear deprecation timeline

⚠️ Potential Issues:

• Existing pipelines with complex Groovy constructs may break
• Config files using unsupported syntax patterns will need updates
• Some community pipelines may need updates

Overall Assessment

Recommendation: APPROVE

This is a well-implemented and necessary step forward for Nextflow. The change:

• Improves language consistency and tooling capabilities
• Provides clear migration path and backward compatibility
• Has comprehensive test coverage and documentation
• Sets foundation for future language improvements

The extensive test modifications (2072 additions, 3888 deletions) indicate thorough validation of the changes across the codebase.

Suggested Follow-up Actions

1. Monitor community feedback and common migration issues
2. Consider creating migration tooling for common patterns
3. Prepare FAQ for common strict syntax questions
4. Plan communication strategy for the broader community

---

[christopher-hakkaart]

Suggested change

	The strict syntax is enabled by default. It can be disabled by setting the environment variable `NXF_SYNTAX_PARSER=v1`.
	The strict syntax is enabled by default. You can disable it by setting the environment variable `NXF_SYNTAX_PARSER=v1`.