nextflow-io / language-server

The Nextflow language server
Apache License 2.0
9 stars 0 forks source link

Linting error special cases not handled #45

Closed LouisLeNezet closed 3 weeks ago

LouisLeNezet commented 3 weeks ago

Hi,

Here a few example of error given by the linting extension that might not be errors:

Error with iterator

def nb_batch = 0
Channel.of("A", "B")
    .collect{ [[ id: "all", batch: nb_batch++ ], it] }

Definition in script use in output closure

process STITCH {
    input:
    tuple val(meta), path(input)

    output:
    tuple val(meta), path("*.txt") , emit: plots , optional: { generate_input }

    script:
    def generate_input  = task.ext.args.contains( "--generateInputOnly TRUE" )
}

Assertion with comma

assert params.test, "Test argument required"

Math plugin in string closure:

def myval = 3.45
println "My val ceiled is: ${(int) Math.ceil(myval)}"
println "My val2 ceiled is: " + (int) Math.ceil(myval)

And small warning when in a closure one of the argument is not used:

ch1 = Channel.of([[id1:"A"], "my_file.vcf", "my_file.csi"])
ch2 = Channel.of([[id2:"B"], "my_file2.vcf", "my_file2.csi"])
ch1.combine(ch2)
    .map { meta1, file1, index1, meta2, file2, index2 -> [meta.id1 + meta2, [file1, file2]] }
bentsherman commented 3 weeks ago

Thanks Louis for all of the interesting edge cases. In general I will refer you to our new docs which explain the strict syntax that is enforced by the language server (and eventually Nextflow itself) and some common patterns that need to be migrated

Error with iterator

The issue is the nb_batch++. Prefix/postfix increment isn't supported in the strict syntax, instead you'll need to increment in a separate assignment:

    .collect { nb_batch += 1; [[ id: "all", batch: nb_batch ], it] }

Definition in script use in output closure

Wow, I had no idea this worked. However I'm going to say this is an anti-pattern and instead you should declare the variable without def so that it can be used in the output block without a closure:

    output:
    tuple val(meta), path("*.txt") , emit: plots , optional: generate_input

    script:
    generate_input  = task.ext.args.contains( "--generateInputOnly TRUE" )

This is also a hack but it seems to be more common and it's easier to support in the language server. In the future I aim to make it work without either hack

Assertion with comma

Groovy allows a colon or comma here, I decided it would be better to support only one for consistency and colon seems to be the more common usage by far. Sorry :/

Math plugin in string closure

The issue is actually the static cast (int). Groovy supports both (int) x and x as int, again I opted for only one, and the latter is much simpler to implement.

And small warning when in a closure one of the argument is not used

This warning will be more clear in 1.0.1, you can prefix the parameter name with _ to suppress the warning

bentsherman commented 3 weeks ago

Forgot to link the docs:

https://nextflow.io/docs/latest/vscode.html#syntax-guide https://nextflow.io/docs/latest/reference/syntax.html#syntax-page

LouisLeNezet commented 3 weeks ago

Thanks a lot for all the explanations. This plugin will be really usefull for the whole community !