Merged
Conversation
✅ Deploy Preview for nextflow-docs-staging canceled.
|
pditommaso
reviewed
Jul 6, 2025
Member
pditommaso
left a comment
There was a problem hiding this comment.
Looks like some tests are failing
6519e72 to
87e3464
Compare
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
87e3464 to
e3c54ab
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR moves some AST transforms, those that are used both by the v1 and v2 parsers, into nf-lang.
This is needed to give me better visibility and control over the total sequence of AST transforms in the Nextflow compiler. It also allows me to remove some ugliness in ScriptCompiler and ConfigCompiler.
I left some AST transforms like NextflowDSLImpl alone because they aren't used by the v2 parser and will be deprecated anyway.
Copilot summary
This pull request refactors the AST transformation logic in the Nextflow codebase by replacing several standalone implementation classes with inline implementations defined within their respective annotations. Additionally, it removes unused imports and simplifies the structure of transformation-related files. The changes also include the migration of the
LangHelpersutility class to a new package to better align with its purpose. Below are the key changes grouped by theme:Refactoring AST Transformations:
NextflowXformImplclass with an inline implementation inside theNextflowXformannotation. The new implementation usesPathCompareVisitorfor path comparison logic. (modules/nextflow/src/main/groovy/nextflow/ast/NextflowXform.groovy)OpXformImplclass with an inline implementation inside theOpXformannotation, utilizingOpCriteriaVisitorfor transforming closure arguments into criteria objects. (modules/nextflow/src/main/groovy/nextflow/ast/OpXform.groovy)TaskTemplateVarsXformImplclass with an inline implementation inside theTaskTemplateVarsXformannotation, delegating variable capture toTaskTemplateVisitor. (modules/nextflow/src/main/groovy/nextflow/ast/TaskTemplateVarsXform.groovy)StripSecretsXformImplclass with an inline implementation inside theStripSecretsXformannotation, usingStripSecretsVisitorto handle secret property replacements. (modules/nextflow/src/main/groovy/nextflow/config/StripSecretsXform.groovy) [1] [2]Code Cleanup:
NextflowXformImpl.groovy,OpXformImpl.groovy,TaskTemplateVarsXformImpl.groovy, andStripSecretsXformImpl.groovy. [1] [2] [3]Nextflow.groovy,ConfigCompiler.java, andScriptCompiler.javaafter removing the standalone transformation implementations. [1] [2] [3]Compiler Updates:
ConfigCompilerandScriptCompilerclasses to use the new visitor-based transformation approach (PathCompareVisitor,OpCriteriaVisitor, andStripSecretsVisitor) instead of the removed standalone classes. (modules/nextflow/src/main/groovy/nextflow/config/parser/v2/ConfigCompiler.java) [1] [2]Utility Class Migration:
LangHelpersclass from thenextflow.astpackage tonextflow.utilto better reflect its utility nature. Adjusted its visibility by removing the@PackageScopeannotation. (modules/nextflow/src/main/groovy/nextflow/util/LangHelpers.java) [1] [2] [3] [4]