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

Wrong AST for conditional that looks like arrow #2006

Closed
danez opened this issue Jun 28, 2016 · 6 comments
Closed

Wrong AST for conditional that looks like arrow #2006

danez opened this issue Jun 28, 2016 · 6 comments
Labels
help wanted Extra attention is needed parsing

Comments

@danez
Copy link
Contributor

danez commented Jun 28, 2016

I noticed a bug in flow with this snippet, while trying to fix the same bug in babel/babylon.

let y = a ? (b) : c => d;

Flow creates an ast that looks like this:

VariableDeclerator
    -> ConditionalExpression
        -> consequent: ArrowFunctionExpression((b) : c => d)
        -> alternate: Literal(null)

but it should be

VariableDeclerator
    -> ConditionalExpression
        -> consequent: Identifier(b)
        -> alternate: ArrowFunctionExpression(c => d)

Tested with flow-parser 0.26 (the latest version on npm)

@samwgoldman
Copy link
Member

Yeah, it looks like we disagree with babylon on this parse. It seems pretty ambiguous to me, but we should at least ensure consistency.

@samwgoldman samwgoldman added help wanted Extra attention is needed parsing labels Jun 28, 2016
@mroch
Copy link
Contributor

mroch commented Jun 28, 2016

Flow actually doesn't parse this successfully. The Literal(null) is flow trying to recover from a parse error. Since it greedily interpreted it as an arrow function with an annotation, there was no : and no alternate.

@Schmavery
Copy link

Looks like both flow (v0.28.0) and babylon (6.8.4, latest on npm) agree now with an unexpected token on the semicolon.

var a, b, c, d;

let y = a ? (b) : c => d; // Fails with "Unexpected token ;"
let y = a ? (b) : (c => d); // Parses as desired
let y = a ? (b) : c => d : "foo"; // Also valid

@danez
Copy link
Contributor Author

danez commented Jul 19, 2016

There is a PR for babylon that makes the first case valid: babel/babylon#59

@nicolo-ribaudo
Copy link
Contributor

Fixing this might introduce an ambiguity in the language:

a ? (b) : c => (d) : e => f

How should that expressions be parsed?

a ? function (b) : c {
    return (d)
} : function (e) {
    return f
}

// or

a ? (b) : function (c) {
    return function (d) : e {
        return f
    }
}

@nicolo-ribaudo
Copy link
Contributor

I'm fixing this in babylon (babel/babylon#573); is it ok to throw in ambiguous cases?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed parsing
Projects
None yet
Development

No branches or pull requests

5 participants