Skip to content

fix(parser): consume semicolon after let.x and let?.x#16857

Merged
overlookmotel merged 3 commits intomainfrom
copilot/fix-parser-empty-statements
Dec 14, 2025
Merged

fix(parser): consume semicolon after let.x and let?.x#16857
overlookmotel merged 3 commits intomainfrom
copilot/fix-parser-empty-statements

Conversation

Copy link
Contributor

Copilot AI commented Dec 14, 2025

let.x; and let?.x; were parsed as two statements (ExpressionStatement + EmptyStatement) instead of one. This caused codegen idempotency failures.

Root cause: In parse_let(), the branch handling member access and optional chaining on let directly constructed the statement without consuming the semicolon:

} else if matches!(peeked, Kind::Dot | Kind::QuestionDot | Kind::LParen) {
    let expr = self.parse_expr();
    self.ast.statement_expression(self.end_span(span), expr)  // semicolon left unparsed

Fix: Call parse_expression_statement() to apply ASI:

} else if matches!(peeked, Kind::Dot | Kind::QuestionDot | Kind::LParen) {
    let expr = self.parse_expr();
    self.parse_expression_statement(span, expr)  // consumes semicolon

This matches the pattern used in other branches of the same function.

Original prompt

This section details on the original issue you should resolve

<issue_title>Parser: let.x; or let?.x; is parsed as 2 statements</issue_title>
<issue_description>Fixing #16839 surfaced another obscure bug.

let.x;
let?.x;

These are each parsed as 2 statements - 1 x ExpressionStatement + 1 x EmptyStatement.

Playground

Once fixed, this idempotency test failure for oxc_codegen should be resolved:

Normal: tasks/coverage/misc/pass/let-optional-chaining.cjs
</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Fix parser for let.x and let?.x statements fix(parser): consume semicolon after let.x and let?.x Dec 14, 2025
Copilot AI requested a review from overlookmotel December 14, 2025 19:45
Copilot AI and others added 3 commits December 14, 2025 21:08
Co-authored-by: overlookmotel <557937+overlookmotel@users.noreply.github.com>
@overlookmotel overlookmotel force-pushed the copilot/fix-parser-empty-statements branch from 56170d7 to c8426ed Compare December 14, 2025 21:08
@overlookmotel overlookmotel marked this pull request as ready for review December 14, 2025 21:08
Copilot AI review requested due to automatic review settings December 14, 2025 21:08
@github-actions github-actions bot added the A-parser Area - Parser label Dec 14, 2025
@github-actions github-actions bot added the C-bug Category - Bug label Dec 14, 2025
Copy link

@charliecreates charliecreates bot left a comment

Choose a reason for hiding this comment

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

No issues found in the modified lines. The change correctly routes the let member/optional-chain/call branch through parse_expression_statement(...), aligning it with other branches and fixing the semicolon/ASI consumption bug described in the PR context.

Summary of changes

What changed

  • Parser fix in crates/oxc_parser/src/js/declaration.rs:

    • In the parse_let() branch for member access / optional chaining / calls (e.g. let.x, let?.x, let()), replaced direct construction of an ExpressionStatement via self.ast.statement_expression(self.end_span(span), expr) with self.parse_expression_statement(span, expr).
    • This ensures the statement is finalized using the shared expression-statement path (including semicolon consumption / ASI behavior), preventing let.x; / let?.x; from being split into an ExpressionStatement + EmptyStatement.
  • Snapshot update in tasks/coverage/snapshots/codegen_misc.snap:

    • codegen_misc positive pass count moved from 52/53 to 53/53 and removed the previously failing let-optional-chaining.cjs entry.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a parser bug where let.x; and let?.x; were incorrectly parsed as two separate statements (ExpressionStatement + EmptyStatement) instead of a single ExpressionStatement. The fix ensures proper Automatic Semicolon Insertion (ASI) by using parse_expression_statement() instead of directly constructing the statement.

Key changes:

  • Modified parse_let() to call parse_expression_statement() for member access and optional chaining on let, ensuring semicolons are properly consumed
  • Resolved codegen idempotency test failure for let-optional-chaining.cjs

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
crates/oxc_parser/src/js/declaration.rs Changed line 30 to use parse_expression_statement() instead of ast.statement_expression() to properly consume semicolons via ASI
tasks/coverage/snapshots/codegen_misc.snap Updated test results showing the idempotency test now passes (53/53 instead of 52/53)

The implementation is correct and consistent with the other branches in the same function. All expression statement branches in parse_let() now properly handle semicolon consumption through parse_expression_statement(), which internally calls self.asi().


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 14, 2025

CodSpeed Performance Report

Merging #16857 will not alter performance

Comparing copilot/fix-parser-empty-statements (c8426ed) with main (d402242)

Summary

✅ 42 untouched
⏩ 3 skipped1

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@overlookmotel overlookmotel merged commit cb88414 into main Dec 14, 2025
35 checks passed
@overlookmotel overlookmotel deleted the copilot/fix-parser-empty-statements branch December 14, 2025 21:14
graphite-app bot pushed a commit that referenced this pull request Dec 14, 2025
Follow-on after #16840 and #16857. We had a few test cases relating to the same thing, and one of them called the undescriptive `oxc.js`. Combine them, and rename the test case files with a more accurate name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-parser Area - Parser C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parser: let.x; or let?.x; is parsed as 2 statements

3 participants