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

[bug] Ternary + arrow function is not correctly parsed #16241

Closed
nicolo-ribaudo opened this issue Jun 3, 2017 · 11 comments · Fixed by #47550 or #49531
Closed

[bug] Ternary + arrow function is not correctly parsed #16241

nicolo-ribaudo opened this issue Jun 3, 2017 · 11 comments · Fixed by #47550 or #49531
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@nicolo-ribaudo
Copy link

TypeScript Version: Online repl

Code

var a = b ? (c) : d => e;

https://www.typescriptlang.org/play/index.html#src=var%20a%20%3D%20b%20%3F%20(c)%3A%20d%20%3D%3E%20e%3B

Expected behavior:
It is valid javascript, thus it should be parsed correctly

Actual behavior:
It is parsed as

var consequent = (c) : d => e
var a = b ? consequent
@j-oliveras
Copy link
Contributor

I have the same problem.

Using playground, the code:

var b = 0, c = 1, e = 2;
var a = b ? (c): d => e;

Is transpiled to:

var b = 0, c = 1, e = 2;
var a = b ? function (c) { return e; } : ;

With the errors:
Cannot find name 'd'. at d.
':' expected. at second semi-colon.

As reference, using node console:

var b = 0, c = 1, e = 2;
var a = b ? (c) : d => e;
console.log(a);

outputs [Function]

Changing to b = 1:

var b = 1, c = 1, e = 2;
var a = b ? (c) : d => e;
console.log(a);

outputs 1.

@nicolo-ribaudo
Copy link
Author

nicolo-ribaudo commented Jun 7, 2017

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
    }
}

@j-oliveras
Copy link
Contributor

@mhegazy, @DanielRosenwasser
Looks like the compiler has some ambiguity errors if the true branch of ternary operator is wrapped with parenthesis and the false branch is a lambda expression.

Another example:

var b = 0, c = 1, e = 2;
var a = b ? (c + e) : d => c+e;

is transpiled as:

var b = 0, c = 1, e = 2;
var a = b ? function (c) {
    if (c === void 0) { c = +e; }
    return c + e;
} : ;

I expect that the transpiled output be the same as input (with es6). Or like this (with es5):

var b = 0, c = 1, e = 2;
var a = b ? (c + e) : function(d) { return c+e; };

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Spec Issues related to the TypeScript language specification labels Jun 7, 2017
@DanielRosenwasser
Copy link
Member

We need to disambiguate this in the spec. We'll potentially

  1. Add non-trivial lookahead
  2. Create a new production that breaks existing code that uses this.
  3. Keep things the same

@j-oliveras
Copy link
Contributor

@DanielRosenwasser a problem is that the compiler show errors in javascript files with the sample of my previous response:

var b = 0, c = 1, e = 2;
var a = b ? (c + e) : d => c+e;

'=' expected.', at first + symbol.
'types' can only be used in a .ts file.', at symbol d.
':' expected.', at last semi-colon.

This code has no errors at node and works as expected.

In this case, removing the parenthesis solve the error.

@nicolo-ribaudo
Copy link
Author

nicolo-ribaudo commented Jul 10, 2017

I have opened a PR to fix this same issue (but for Flow, not TypeScript) in Babylon (the Babel's parser). It basically tries to parse the middle part of a conditional expression up to three times, which should be enough to find the correct way of parsing.
I don't know how the TypeScript parser works, but maybe it could follow a similar approach.

ref: babel/babylon#596

@tjjfvi
Copy link
Contributor

tjjfvi commented Jan 19, 2022

This is still a bug in v4.5.4, and occurs even when parsing JavaScript files.

ts.transpileModule("x ? x => ({ x }) : x => ({ x })", {
  fileName: "file.js",
  compilerOptions: {
    target: ts.ScriptTarget.ESNext,
    allowJs: true,
  },
})

outputs

x ? x => ({ x }) => ({ x }) : ;

This pattern occurs in real world files; for example, tsc currently outputs malformed js when run on monaco.

@jakebailey
Copy link
Member

The trouble with the current parser seems to be that when it parses out:

x ? y => ({ y }) : z => ({ z })

It tries to parse ({ y }) : z => ({ z }) as an expression, but tries to parse it as an arrow expression to see if it should be one, which it is, because : z can be a return type. Probably needs some extra context passed down to tell it to reject on the : if we're in a conditional expression.

@ahejlsberg
Copy link
Member

Right, we need a context flag that prevents us from parsing a type annotation on an arrow function immediately nested in one of the branches of a conditional expression. Sort of similar to the flag that prevents us from parsing an in expression in a for-in loop.

@jakebailey
Copy link
Member

My fix in #47550 doesn't use a context flag, but a boolean that gets passed down via the few call sites where it matters. I'll have to look at how that for-in flag works, as my impression was that it would be difficult to later unset the flag where it needed to be unset (when parens/nesting happens), but maybe I'm wrong.

@jakebailey
Copy link
Member

The fix I merged for this has been reverted in #48940 due to some issues on #48733. We plan to try again for 4.8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment