chanzuckerberg / miniwdl

Workflow Description Language developer tools & local runner
MIT License
175 stars 54 forks source link

MiniWDL can't parse a brace-enclosed command section with a brace in it #673

Open adamnovak opened 8 months ago

adamnovak commented 8 months ago

I went to exercise the special characters and I found that MiniWDL can't parse this:

task check {
    input {
        Array[File] files
    }

    command {
        set -e
        cp ~{sep=" " files} .
        stat 'GRCh38#0#chm1.fa'
        stat 'GRCh38%230%23chm1.fa'
        stat "'"'"!@#$%^&*()_[]{};<>~`+=?β€”πŸŒ'.fa
        echo "SUCCESS" >"output.txt"
    }

    output {
        Boolean success = read_string("output.txt") == "SUCCESS"
    }
}

The close brace in the quotes in the shell script terminates the brace-enclosed command section, presumably because it isn't part of a ${} substitution. I get this from the parser where this snippet appears in my file:

(/Users/anovak/workspace/toil/wdl-conformance-tests/tests/special_character_files/_version_1.0_special_character_files.wdl Ln 42 Col 34) Unexpected token Token('STRING1_FRAGMENT', ';<>~`+=?β€”πŸŒ') at line 42, column 34.
Expected one of:
    * CNAME
    * OUTPUT
    * INPUT
    * RBRACE
    * META
    * PARAMETER_META
    * RUNTIME
Previous tokens: [Token('RBRACE', '}')]

            stat "'"'"!@#$%^&*()_[]{};<>~`+=?β€”πŸŒ'.fa
                                     ^

I could enclose my command in <<< >>> instead. But if I have a command string that contains both } and >>>, how would I enclose it? And \} doesn't seem to work for escaping the brace.

The WDL spec has an actual grammar definition in some places, but not really for command sections, so it's hard to tell what should happen here.

mlin commented 8 months ago

so it's hard to tell what should happen here.

Agree this isn't crystal clear from the spec, and the current miniwdl implementation takes a hard line in making any } close the command regardless of anything preceding it (such as a nested opening brace, or an attempt to escape it).

In the absence of a clear spec contradiction I'm inclined to leave it be, but certainly leaving this open for further discussion.