Implicit workflow syntax

[bentsherman]

One of the restrictions I added to the script grammar was that imperative code is not allowed outside the workflow body. Essentially, the only valid top-level statements are:

• shebang
• feature flags
• includes
• processes
• workflows
• output definition

I added this restriction because arbitrary code would make parsing more difficult and it makes the script harder to read. This code is essentially part of the entry workflow, so it should be placed there. The nf-core pipelines are slowly moving towards this pattern as well.

Of course, this restriction would break most of our code examples in the Nextflow docs. Most notably, the basic "hello world" wouldn't work:

println "Hello, World!"

To support this use case, I added an alternate rule to the parser called an "implicit workflow". Basically, if the script consists only of imperative code (no includes, processes, workflows, etc), then the parser wraps the entire script in an entry workflow under the hood:

workflow {
  println "Hello, World!"
}

If the script does contain some processes or workflows, the parser tries to accept them and then raise an error along the lines of "processes and workflows are not allowed with an implicit workflow script". That way users can get some guidance from the editor rather than something more cryptic like "unexpected token".

But now the problem is that certain syntax errors are handled incorrectly. For example:

process foo {
  publishDir = params.outdir
  // ...
}

The equals sign is invalid, so the parser falls back to interpreting the entire script as an implicit workflow. In this scenario, processes and workflows just look like function calls, so the parser accepts the script and immediately declares every process/workflow invalid because it's actually an implicit workflow 😆

If it had been something like a stray comma or curly brace, the syntax error remains correct because the implicit workflow rule also fails to parse it.

In this particular example, I could extend the parser rule for processes to accept the equals sign and raise an error later, but I fear that there are endless edge cases which could still cause this problem.

There are a few other ways I can think of to deal with this, each with their own trade-offs. Need to ponder this question more to see if there is a better way out.

[pditommaso]

I agree as mid/long-term goals. In the short term, we should privilege backward compatibility with the existing code base.

[bentsherman]

Fixed with 9789208. I made the process syntax more flexible and then validate the AST in the next stage. That way, if there is a simple syntax error in the process definition, it won't try to parse the whole script as an implicit workflow.

[bentsherman]

I am still seeing some cases where a simple syntax error causes an explosion because the parser tries to parse the entire script as an implicit workflow. Saving an idea here so that I don't forget:

1. move the implicit workflow rule into a separate grammar that extends the main grammar
2. always try to parse the script as an implicit workflow first
3. if a process/workflow/etc is detected, fall back to main grammar
4. otherwise, emit a syntax error and stop

This way, if the script is not an implicit workflow but there is a syntax error, it will be treated as such. If the user tries to mix Groovy code and DSL code, the Groovy code should be highlighted with a suggestion to move it into the entry workflow

[bentsherman]

Looks like we can remove this ambiguity by removing statement labels from the grammar. Statement labels creates an ambiguity with process and workflow definitions which leads to very unhelpful error messages for certain kinds of syntax mistakes (because a process / workflow also looks like a closure with labels).

This would require us to drop the branch and multiMap operators because they rely on labels to define the multiple output channels. But these operators are conveniences over map and filter and contribute to the issue of "multiple ways to do the same thing". So I think we can phase them out over time.

For now I have saved the proposed changes in this PR: #16

[bentsherman]

multiMap can just be replaced with multiple map operations, it is not much different

branch can be replaced with multiple filter / map combos, however we could provide a filterMap operator which provides the same convenience of branch but for a single output, that way it doesn't depend on statement labels

[bentsherman]

Another option is to provide an alternate syntax for branch and multiMap, for example:

// branch
Channel.of(1, 2, 3, 40, 50)
    .branch(
        small: { it -> it < 10 },
        large: { it -> it > 10 }
    )
    .set { result }

// multiMap
Channel.of( 1, 2, 3, 4 )
    .multiMap(
        foo: { it -> it + 1 }
        bar: { it -> it * it }
    )
    .set { result }

Slightly more verbose but avoids the syntax ambiguity.

I still think we can phase out these operators in the long-term, once we make some deeper improvements to the language that make them less necessary. But that might take a while, and nf-core devs still use these operators a lot and are not keen to lose them. So I think this alternate syntax could be a nice bridge in the interim.

[bentsherman]

This loose end likely won't be addressed in the 1.0.0 release, since we want the first iteration to be as smooth as possible. We can try to deprecate the closure-label syntax and add the new syntax for Nextflow 24.10, but this specific issue (removing the ambiguity in the grammar) can't be addressed until the closure-label syntax is removed entirely.