Description

SUZUKI Sosuke (sosukesuzuki)

opened on Apr 17, 2022

Issue body actions

Bug Report

🔎 Search Terms

• conditional
• 4.7 beta
• syntax error

🕗 Version & Regression Information

• This is a crash
• This changed between versions 4.6.3 and 4.7.0-beta

⏯ Playground Link

Playground link with relevant code

💻 Code

(false ? (param): string => param :  null);

🙁 Actual behavior

Parsing failure for arrow function expr that has type annotations for return type, but doesn't have type annotations for parameters, in conditional expression.

🙂 Expected behavior

Parsing successfull.

Activity

1. MartinJohns commented on Apr 17, 2022

   

   Martin Johns (MartinJohns)

   on Apr 17, 2022

   Contributor

   More actions

   This is a crash

   I can't reproduce this. Neither locally nor in the playground it crashes.
2. sosukesuzuki commented on Apr 17, 2022

   

   SUZUKI Sosuke (sosukesuzuki)

   on Apr 17, 2022

   Author

   More actions

   I can't reproduce this. Neither locally nor in the playground it crashes.

   Sorry I forgot remove this from template.

   Reacted by Martin Johns
3. jmcscript commented on Apr 18, 2022

   

   James McDonald (jmcscript)

   on Apr 18, 2022

   More actions

   The OP's statement is accurate as the playground proves that there are no parsing errors in the version he mentions. However, if it helps resolve the problem, changing his code as follows resolves the error in the current version, leaving you with just the any type inference for param.

   TS Playground Example

   (false ? ((param): string => param) :  null);
4. Ryan Cavanaugh (RyanCavanaugh)

   added

   Working as IntendedThe behavior described is the intended behavior; this is not a bugThe behavior described is the intended behavior; this is not a bug

   and removed

   Working as IntendedThe behavior described is the intended behavior; this is not a bugThe behavior described is the intended behavior; this is not a bug

   on Apr 18, 2022
5. Ryan Cavanaugh (RyanCavanaugh)

   assigned

   Jake Bailey (jakebailey)

   on Apr 18, 2022
6. Ryan Cavanaugh (RyanCavanaugh)

   added this to the TypeScript 4.8.0 milestone on Apr 18, 2022
7. Ryan Cavanaugh (RyanCavanaugh)

   added

   Needs InvestigationThis issue needs a team member to investigate its status.This issue needs a team member to investigate its status.

   on Apr 18, 2022
8. RyanCavanaugh commented on Apr 18, 2022

   

   Ryan Cavanaugh (RyanCavanaugh)

   on Apr 18, 2022

   Member

   More actions

   I'd like Jake Bailey (@jakebailey) to confirm but this seems like it might be intentional. Your code might terminate at the legal JS expression

   false ? (param) : string

   so parsing the : string as a type annotation might require "too much" lookahead.
9. jakebailey commented on Apr 19, 2022

   

   Jake Bailey (jakebailey)

   on Apr 19, 2022

   Member

   More actions

   No doubt this is #16241 / #47550, yes. The reason why the workaround in typescript-eslint/typescript-eslint#4829 works is because the parser can see the : inside of the parens and says "that's definitely a parameter", and allows the colon before the return type.

   When the (what I can only describe as) heuristics don't "prove" that it's an arrow function, my change disallows the : when parsing, preferring it to end the conditional.

   The test case in the issue is similar to one I wrote in my PR, but then throws an extra case on the end.

   I'd have to think about how I might fix this; the "heuristic" I described previously mainly checks things to do with parameters. It doesn't go and parse all the way past an error expression.

   The gotcha with lookahead is when conditionals are nested, though I can't seem to produce an example that actually parses in JS. I thought the following would be a counter example, but this fails to parse when I run it in node.

   true ? false ? (a) : b => c : null : null
   

   (If anyone can come up with an example, let me know. This is tricky.)
10. jakebailey commented on Apr 19, 2022

    

    Jake Bailey (jakebailey)

    on Apr 19, 2022

    Member

    More actions

    Regardless, I think all of this is going to have to be figured out for the types-in-JS proposal, since all of these parsing choices will have to be made and our parser changed to fix any differences.
11. sosukesuzuki commented on Apr 19, 2022

    

    SUZUKI Sosuke (sosukesuzuki)

    on Apr 19, 2022

    Author

    More actions

    FYI here is a case of how this bug caused problems in a real product https://github.com/typescript-eslint/typescript-eslint/pull/4829/files#diff-d9d4dc5af184a093e1caab204aa0ed6da56d008309ff90294561a64bec2e6663
12. TypeScript Bot (typescript-bot)

    added

    Fix AvailableA PR has been opened for this issueA PR has been opened for this issue

    on Apr 20, 2022
13. jakebailey commented on Apr 20, 2022

    

    Jake Bailey (jakebailey)

    on Apr 20, 2022

    Member

    More actions

    SUZUKI Sosuke (@sosukesuzuki) Can you try taking a peek using the build on my PR here? #48788 (comment)
14. jakebailey commented on Apr 21, 2022

    

    Jake Bailey (jakebailey)

    on Apr 21, 2022

    Member

    More actions

    To link it here, here are various variations of "fixes" to this:

    • Permissive (Allow return type in conditional if body is followed by a colon #48788): Playground Link
    • Strict (Demo: make colon always terminate in conditional expression #48791): Playground Link
    • Strict only on the left (Demo: make colon always terminate in conditional expression on true side #48792): Playground Link
15. Jake Bailey (jakebailey)

    closed this as completedin #48940on May 4, 2022
16. Daniel Rosenwasser (DanielRosenwasser)

    reopened this on May 6, 2022
17. Daniel Rosenwasser (DanielRosenwasser)

    closed this as completedon May 6, 2022

Issue actions