Skip to content

fix(64231): avoid parsing async() calls as typed arrows in conditionals - #64233

Open
Oleksandr Tarasiuk (a-tarasyuk) wants to merge 5 commits into
microsoft:mainfrom
a-tarasyuk:fix/64231
Open

Oleksandr Tarasiuk (a-tarasyuk) wants to merge 5 commits into
microsoft:mainfrom
a-tarasyuk:fix/64231

Conversation

@a-tarasyuk

Copy link
Copy Markdown
Contributor

Fixes #64231

Copilot AI balanced review requested due to automatic review settings September 10, 2026 17:39
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Sep 10, 2026
@typescript-automation typescript-automation Bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Sep 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The parser change is narrowly scoped and is backed by targeted regression tests plus updated baselines covering the reported ambiguity and key variants.

Pull request overview

Fixes a long-standing parsing ambiguity where async() inside conditional (?:) expressions could be misread as the start of an async arrow with a return type annotation, producing spurious syntax errors in valid code.

Changes:

  • Adjusts the parser’s parenthesized-arrow lookahead so async () : ... is treated as ambiguous (and therefore speculatively parsed) instead of being assumed to be an arrow.
  • Adds regression coverage for a ? async() : ... patterns in both TS and JS (allowJs/checkJs) test suites, including nested conditionals and mixed arrow branches.
  • Updates the corresponding reference baselines (types/symbols/errors and JS emit) to reflect the corrected parse.
File summaries
File Description
tsc/internal/parser/parser.go Makes async () : return TSUnknown in lookahead to avoid misclassifying ternary : as an arrow return-type :.
tsc/testdata/tests/cases/conformance/async/es2017/asyncArrowFunction/asyncArrowFunction_allowJs.ts Adds allowJs regression cases for conditional expressions involving async() calls.
tsc/testdata/tests/cases/compiler/parenthesizedAsyncArrowFunction.ts Adds compiler regression cases covering the reported conditional parsing scenarios and related edge cases.
tsc/testdata/baselines/reference/conformance/asyncArrowFunction_allowJs.types Baseline updates for the new allowJs conditional parsing cases.
tsc/testdata/baselines/reference/conformance/asyncArrowFunction_allowJs.symbols Symbol baseline updates for the new allowJs cases.
tsc/testdata/baselines/reference/conformance/asyncArrowFunction_allowJs.errors.txt Confirms the new allowJs cases introduce no additional diagnostics.
tsc/testdata/baselines/reference/compiler/parenthesizedAsyncArrowFunction.types Type baseline updates for the new compiler regression cases.
tsc/testdata/baselines/reference/compiler/parenthesizedAsyncArrowFunction.symbols Symbol baseline updates for the new compiler regression cases.
tsc/testdata/baselines/reference/compiler/parenthesizedAsyncArrowFunction.js Emit baseline updates demonstrating correct parsing/emit around the new conditional cases.
Review details
  • Files reviewed: 9/9 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@DanielRosenwasser

Copy link
Copy Markdown
Member

TypeScript Bot (@typescript-bot) test top800

@typescript-automation

typescript-automation Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
test top800 ✅ Started ✅ Results

@typescript-automation

Copy link
Copy Markdown
Contributor

Daniel Rosenwasser (@DanielRosenwasser) Here are the results of running the top 800 repos with tsc comparing main and refs/pull/64233/merge:

Everything looks good!

@typescript-automation typescript-automation Bot added For Milestone Bug PRs that fix a bug with a specific milestone and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Sep 11, 2026
@jakebailey

Copy link
Copy Markdown
Member

I had copilot look at this and it caught some cases I missed:


This looks correct for the reported examples. Could we also cover spread arguments? The (... lookahead currently returns TSTrue even after async, although that can be an ordinary call:

const xs = [1, 2];
const spread1 = a ? async(...xs) : 0;
const spread2 = a ? async(...xs) : x => x;

Both still fail on this branch. It looks like the spread lookahead could similarly return TSUnknown when preceded by async. A typed-rest-arrow control would help ensure that remains recognized:

const restArrow = a
    ? async (...xs: number[]): Promise<number> => xs.length
    : () => 0;

There is also a remaining nested case worth covering specifically in the JavaScript test:

const nested1 = a ? a ? async() : x => x : 0;
const nested2 = a ? a ? async(1) : x => x : 0;

Node and esbuild's JS parser accept both, but this branch reports a spurious type annotation and ':' expected. The speculative inner arrow parse appears to use the outer conditional's colon as confirmation of a return-type annotation.

These are pre-existing failures, not regressions introduced here, so the nested-JS issue could reasonably be a follow-up.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Nested typed async arrows with empty parameter lists can still select the wrong conditional separator and change program behavior.

1 open issue

Review effort: Balanced

Comment thread tsc/internal/parser/parser.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Nested typed-arrow forms can still misparse conditional branches and change emitted program behavior.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread tsc/internal/parser/parser.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Conditional restrictions are still lost inside unambiguous arrow bodies, causing valid JavaScript to produce a syntax error.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread tsc/internal/parser/parser.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Shared parser disambiguation and rollback changes need human confirmation and compiler-suite validation, which was not run.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

}
state := p.mark()
result := p.parsePossibleParenthesizedArrowFunctionExpression(allowReturnTypeInArrowFunction)
result := p.parsePossibleParenthesizedArrowFunctionExpression(flags)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm missing something, but isn't flags here only being used to pass two bools, and this has nothing to do with the existing meaning of parse flags? Should this be a different set of flags? Or not passed as an arg at all? (I remember when I added this bool arg I did an arg on purpose because it was challenging to remember to clear at the right time but maybe that time has passed)

This branch has not been deployed

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

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

Parser misinterprets async() calls in conditional expressions as async arrow functions

4 participants