Skip to content

fix(linter/no-else-return): preserve statement boundary in fixer#22687

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/fix-no-else-return-fixer
May 28, 2026
Merged

fix(linter/no-else-return): preserve statement boundary in fixer#22687
graphite-app[bot] merged 1 commit into
mainfrom
codex/fix-no-else-return-fixer

Conversation

@camc314

@camc314 camc314 commented May 23, 2026

Copy link
Copy Markdown
Contributor

Summary

eslint/no-else-return could remove an else block in a way that glued the moved statement to the next token. In minified code, this turned a valid pattern like:

if (foo) { return bar } else { baz = qux } while (baz) {}

into invalid syntax because the moved assignment and following while were no longer separated.

This fixes that by adding an explicit semicolon only when the moved statement needs a boundary before the next token. The check uses the existing line-terminator utility and is limited to the expression/return statement shapes that can run into the following token. I also added a focused regression case for the minified shape that triggered the bug.

Ecosystem CI

This came from the failing vercel/next.js ecosystem CI job:
https://github.com/oxc-project/oxc-ecosystem-ci/actions/runs/26298001101/job/77416984230

The job panicked while running oxlint --fix ... -D all because the fixer produced invalid syntax after applying no-else-return.

@github-actions github-actions Bot added the A-linter Area - Linter label May 23, 2026
@codspeed-hq

codspeed-hq Bot commented May 23, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 5 untouched benchmarks
⏩ 55 skipped benchmarks1


Comparing codex/fix-no-else-return-fixer (1f71ba9) with main (26b9396)2

Open in CodSpeed

Footnotes

  1. 55 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.

  2. No successful run was found on main (779e769) during the generation of this report, so 26b9396 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@camc314 camc314 force-pushed the codex/fix-no-else-return-fixer branch from 222b26e to 3fd1c8b Compare May 23, 2026 17:03
@camc314 camc314 self-assigned this May 24, 2026
@camc314 camc314 force-pushed the codex/fix-no-else-return-fixer branch from 3fd1c8b to a6188ca Compare May 24, 2026 11:53
@camc314 camc314 marked this pull request as ready for review May 24, 2026 12:04
Copilot AI review requested due to automatic review settings May 24, 2026 12:04
@camc314 camc314 changed the title fix(linter): preserve no-else-return statement boundary fix(linter/no-else-return): preserve statement boundary in fixer May 24, 2026
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 24, 2026

camc314 commented May 24, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • May 24, 12:04 PM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 24, 12:05 PM UTC: camc314 added this pull request to the Graphite merge queue.
  • May 24, 12:08 PM UTC: The Graphite merge queue couldn't merge this PR because it failed for an unknown reason (All comments in this PR must be resolved before merging. Once you've resolved all open comment threads, you can retry your merge.).
  • May 28, 10:16 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • May 28, 10:16 AM UTC: camc314 added this pull request to the Graphite merge queue.
  • May 28, 10:20 AM UTC: Merged by the Graphite merge queue.

graphite-app Bot pushed a commit that referenced this pull request May 24, 2026
)

## Summary

`eslint/no-else-return` could remove an `else` block in a way that glued the moved statement to the next token. In minified code, this turned a valid pattern like:

```js
if (foo) { return bar } else { baz = qux } while (baz) {}
```

into invalid syntax because the moved assignment and following `while` were no longer separated.

This fixes that by adding an explicit semicolon only when the moved statement needs a boundary before the next token. The check uses the existing line-terminator utility and is limited to the expression/return statement shapes that can run into the following token. I also added a focused regression case for the minified shape that triggered the bug.

## Ecosystem CI

This came from the failing `vercel/next.js` ecosystem CI job:
https://github.com/oxc-project/oxc-ecosystem-ci/actions/runs/26298001101/job/77416984230

The job panicked while running `oxlint --fix ... -D all` because the fixer produced invalid syntax after applying `no-else-return`.
@graphite-app graphite-app Bot force-pushed the codex/fix-no-else-return-fixer branch from 8786840 to 71d2a3b Compare May 24, 2026 12:06

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.

Pull request overview

Fixes an eslint/no-else-return autofix edge case where removing an else block could concatenate the moved statement with the next token (notably in minified code), producing invalid JavaScript.

Changes:

  • Adds logic to optionally append a trailing semicolon when moving else contents out of an if and a statement boundary is required.
  • Introduces helper functions to determine whether a separator is needed and to fetch the last statement of an else branch.
  • Adds a regression fix test covering the minified ...}while(...)... shape that previously broke.

Comment thread crates/oxc_linter/src/rules/eslint/no_else_return.rs
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 24, 2026
@camc314 camc314 requested a review from Copilot May 24, 2026 12:21

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread crates/oxc_linter/src/rules/eslint/no_else_return.rs

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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread crates/oxc_linter/src/rules/eslint/no_else_return.rs
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 28, 2026
)

## Summary

`eslint/no-else-return` could remove an `else` block in a way that glued the moved statement to the next token. In minified code, this turned a valid pattern like:

```js
if (foo) { return bar } else { baz = qux } while (baz) {}
```

into invalid syntax because the moved assignment and following `while` were no longer separated.

This fixes that by adding an explicit semicolon only when the moved statement needs a boundary before the next token. The check uses the existing line-terminator utility and is limited to the expression/return statement shapes that can run into the following token. I also added a focused regression case for the minified shape that triggered the bug.

## Ecosystem CI

This came from the failing `vercel/next.js` ecosystem CI job:
https://github.com/oxc-project/oxc-ecosystem-ci/actions/runs/26298001101/job/77416984230

The job panicked while running `oxlint --fix ... -D all` because the fixer produced invalid syntax after applying `no-else-return`.
@graphite-app graphite-app Bot force-pushed the codex/fix-no-else-return-fixer branch from 1f71ba9 to 27268a0 Compare May 28, 2026 10:17
@graphite-app graphite-app Bot merged commit 27268a0 into main May 28, 2026
29 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 28, 2026
@graphite-app graphite-app Bot deleted the codex/fix-no-else-return-fixer branch May 28, 2026 10:20
camc314 pushed a commit that referenced this pull request Jun 1, 2026
# Oxlint
### 🚀 Features

- e4b1f46 linter/typescript: Implement `method-signature-style` rule
(#22679) (Mikhail Baev)
- bc462ca linter/vue: Implement no-reserved-component-names rule
(#22741) (bab)
- ef9e751 linter/vue: Implement component-definition-name-casing rule
(#22818) (bab)
- d67f51a linter/vue: Implement require-prop-type-constructor rule
(#22708) (bab)
- 1444f82 linter/promise/spec-only: Add `Promise.try` to `Promise`
static methods (#22812) (Ben Saufley)
- 8422e8b linter/jsdoc: Implement `require-yields-description` rule
(#22805) (Mikhail Baev)
- fe93f97 linter/eslint: Implement `prefer-named-capture-group` rule
(#22759) (Sebastian Poxhofer)
- 1a7798b linter: Add suggestion for `unicorn/no-new-array` (#22682)
(Sysix)

### 🐛 Bug Fixes

- 760a9f9 linter: Report errors when writing to the filesystem (#22881)
(camc314)
- e5a2748 linter: Avoid no-unreachable false positive after conditional
loop (#22869) (camc314)
- 39d92d6 linter/arrow-body-style: Preserve comments within function
(#22854) (Sysix)
- 3d13e29 parser: Reject `declare` in an already-ambient context
(TS1038) (#22850) (Boshen)
- 5152854 parser: Reject statements in ambient contexts (TS1036)
(#22849) (Boshen)
- 2eafea6 parser: Reject function implementations in ambient contexts
(TS1183) (#22845) (Boshen)
- c645615 parser: Reject incompatible class member modifiers (#22843)
(Boshen)
- 4a1ca4a linter/export: Detect duplicate explicit exports (#22798)
(camc314)
- 0a9a735 linter/no-loop-func: Allow safe let closures (#22811)
(camc314)
- 1599f11 linter: Align lsp extends default plugins (#22788) (camc314)
- db32ec9 linter/no-accumulating-spread: Use loop as primary span
(#22800) (camc314)
- 33ec6b4 linter/consistent-test-it: Avoid adjacent describe leakage
(#22796) (camc314)
- 2606069 linter/no-array-sort: Unwrap parenthesized sort args (#22794)
(camc314)
- 9f2f709 linter/no-array-sort: Skip non compare fn sort arguments
(#22752) (Gaurav Dubey)
- 27268a0 linter/no-else-return: Preserve statement boundary in fixer
(#22687) (camc314)
- d9cb6d8 linter/no-empty-function: Allow functions callbacks with
`allow: functions` (#22764) (camc314)
- a40a314 linter/no-shadow-restricted-names: Ignore enum members
(#22762) (camc314)
- 82366d9 linter/no-cond-assign: Align ternary handling (#22761)
(camc314)

### 📚 Documentation

- 5e113ba linter: Add license notices for ported ESLint plugins (#22768)
(Boshen)
# Oxfmt
### 🚀 Features

- d75cbbf oxfmt: Format `parser:json` files by `oxc_formatter_json`
(#22709) (leaysgur)
- 49db054 formatter_json: Implement `oxc_formatter_json` (json variant
only) (#22641) (leaysgur)
- 9c71f2e ast, codegen, formatter: Add `WithClauseKeyword::as_str`
helper and use it (#22791) (camc314)

### 🐛 Bug Fixes

- d3cdd62 oxfmt: Skip formatting for whitespace-only file (#22780)
(leaysgur)
- 23f0cc8 formatter: Don't move comments inside variable declaration in
for in loop (#22776) (leaysgur)
- f200c40 formatter: Don't move comments inside variable declaration in
for of loop (#22773) (Leonabcd123)

### 📚 Documentation

- 845f393 oxfmt,formatter,formatter_json,formatter_core: Add/update
AGENTS.md (#22873) (leaysgur)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants