wdl icon indicating copy to clipboard operation
wdl copied to clipboard

Error parsing ternary operator assignment

Open rickymagner opened this issue 10 months ago • 4 comments

Hi folks, I think I found a subtle bug in parsing a particular type of ternary operator assignment. Here are the steps to reproduce:

  1. Make the following file called MinimalParse.wdl:
version 1.0

workflow MinimalParse {
    input {
        Int num = 8
    }

    Int new_num=if(num>5) then 1 else 2  # Add space after `if` to parse with wdl crate
}
  1. Try to parse using the wdl crate. Here is a Rust snippet:
// Inside main
    let src = std::env::args().nth(1).expect("missing src");
    let contents = std::fs::read_to_string(src).expect("could not read file contents");

    // Generate the parse tree from the document source.
    let parse_result = grammar::v1::parse(&contents);
    let parse_tree = match parse_result {
        Ok(_) => {
            let inner = parse_result.unwrap();
            println!("Inner is: {:?}", &inner);
            let tree = inner.into_tree().expect("Cannot make tree");
            tree
        }
        Err(e) => {
            println!("{}", e);
            panic!()
        }
    };
  1. The printed output is:
Inner is: Result { concerns: Some(Concerns(NonEmpty { head: ParseError(Error { message: "The following tokens are required: singular_identifier.", location: Position(Position { line_no: 8, col_no: 32, byte_no: 107 }) }), tail: [] })), tree: None }
  1. Add a space after if in the original WDL, i.e. Int new_num=if (num>5) then 1 else 2 and rerun. It now parses successfully.

I'm not defending the gross-looking syntax where there's no space after if, but womtool validate passes and cromwell does in fact run the original WDL, so this seems to be a parsing bug.

Curiously enough, the bug doesn't seem to care about the other shifty looking lack of spaces. Adding a space before or after the = does nothing to fix this. It also doesn't seem to care if you use a space or not after if in normal blocks outside of a ternary assignment.

rickymagner avatar Apr 23 '24 14:04 rickymagner