Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect error message on missing comma #4085

Merged
merged 3 commits into from
Jul 29, 2023

Conversation

bentsherman
Copy link
Member

Close #3185

This PR fixes a particularly nasty error message that occurs with missing commas in process inputs/outputs.

Consider the following example:

process foo {
  debug true

  input:
  tuple val(x) val(y) // missing comma

  script:
  """
  echo "foo ${task.index} ${task.workDir}"
  """
}

workflow {
  foo(['x', 'y'])
}

Currently, the script compiles despite the missing comma but then Nextflow reports that the process has been used twice:

 nextflow run gh-3185/bad-error-message-4.nf
N E X T F L O W  ~  version 23.06.0-edge
Launching `gh-3185/bad-error-message-4.nf` [loving_keller] DSL2 - revision: 63eb6918e1
Process 'foo' has been already used -- If you need to reuse the same component, include it with a different name or include it in a different workflow context

 -- Check script 'gh-3185/bad-error-message-4.nf' at line: 14 or see '.nextflow.log' file for more details

The transpiled Groovy code looks like this:

this._in_tuple(
    new nextflow.script.TokenValCall(new nextflow.script.TokenVar('x'))
).val(new nextflow.script.TokenVar('y'))

So I just add a methodMissing() implementation to BaseParam to catch the incorrect .val() call. With that change, the error now looks like this:

$ ../launch.sh run gh-3185/bad-error-message-4.nf
N E X T F L O W  ~  version 23.06.0-edge
Launching `gh-3185/bad-error-message-4.nf` [nasty_bohr] DSL2 - revision: 63eb6918e1
Invalid method call `val([nextflow.script.TokenVar(y)])` -- possible syntax error

 -- Check script 'gh-3185/bad-error-message-4.nf' at line: 5 or see '.nextflow.log' file for more details

The error message is correctly called a syntax error and it gives the exact line where it happened.

Note however that it doesn't catch all missing comma errors. For example if you forget a comma before emit: ..., the script fails to compile so we can't catch the error.

@netlify
Copy link

netlify bot commented Jul 10, 2023

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit ad5eda0
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/64c53756dcf0d30008be0d56

@bentsherman
Copy link
Member Author

If we remove old DSL1 syntax (#3664 and #3671), this change should also catch things like from and into.

@pditommaso
Copy link
Member

What about adding a unit test for capture this behavior

Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Copy link
Member Author

I added a unit test for process inputs.

As for outputs, I modified the example to have a tuple output with missing comma:

process foo {
  debug true

  output:
  tuple val(x) val(y) // missing comma

  script:
  """
  echo "foo ${task.index} ${task.workDir}"
  """
}

workflow {
  foo()
}

Which produces this syntax:

this._out_tuple(new nextflow.script.TokenValCall(new nextflow.script.TokenVar('x'))).val(y)

And finally this error:

N E X T F L O W  ~  version 23.07.0-edge
Launching `gh-3185/main.nf` [elegant_stone] DSL2 - revision: 6f14ffa021
ERROR ~ No such variable: y

 -- Check script 'gh-3185/main.nf' at line: 8 or see '.nextflow.log' file for more details

Since outputs are not wrapped in a TokenVar, the Groovy shell encounters a missing variable before it reaches the methodMissing(). This error is already specific enough so I left it alone.

Signed-off-by: Paolo Di Tommaso <[email protected]>
@pditommaso pditommaso merged commit a59af39 into master Jul 29, 2023
@pditommaso pditommaso deleted the 3185-error-improvement-missing-comma branch July 29, 2023 16:35
abhi18av pushed a commit to abhi18av/nextflow that referenced this pull request Oct 28, 2023
…ast]



Signed-off-by: Ben Sherman <[email protected]>
Signed-off-by: Paolo Di Tommaso <[email protected]>
Co-authored-by: Paolo Di Tommaso <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wrong error when forgetting comma
2 participants