Skip to content

fix(linter/no-unused-vars): avoid renaming captured vars#22310

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/no-unused-vars-reference-capture
May 11, 2026
Merged

fix(linter/no-unused-vars): avoid renaming captured vars#22310
graphite-app[bot] merged 1 commit into
mainfrom
codex/no-unused-vars-reference-capture

Conversation

@camc314

@camc314 camc314 commented May 11, 2026

Copy link
Copy Markdown
Contributor

When no-unused-vars renames an unused parameter to satisfy argsIgnorePattern, it can accidentally change existing references in the function body.

For example:

const _unused = 1;
function foo(unused) {
  return _unused;
}

Renaming unused to _unused would make the body reference the parameter instead of the outer binding. This now detects that capture risk and chooses _unused0 instead, while still allowing inner-scope bindings that already shadow the name.

@camc314 camc314 self-assigned this May 11, 2026
Copilot AI review requested due to automatic review settings May 11, 2026 09:44
@github-actions github-actions Bot added the A-linter Area - Linter label May 11, 2026
@camc314 camc314 changed the title fix(linter/no-unused-vars): avoid captured argument renames fix(linter/no-unused-vars): avoid renaming captured vars May 11, 2026
@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 11, 2026

camc314 commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • May 11, 9:45 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 11, 9:47 AM UTC: camc314 added this pull request to the Graphite merge queue.
  • May 11, 9:50 AM 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 11, 10:14 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 11, 10:14 AM UTC: camc314 added this pull request to the Graphite merge queue.
  • May 11, 10:17 AM UTC: Merged by the Graphite merge queue.

@graphite-app graphite-app Bot force-pushed the codex/no-unused-vars-reference-capture branch from fe4d8d0 to f2b3469 Compare May 11, 2026 09:47
graphite-app Bot pushed a commit that referenced this pull request May 11, 2026
When `no-unused-vars` renames an unused parameter to satisfy `argsIgnorePattern`, it can accidentally change existing references in the function body.

For example:

```js
const _unused = 1;
function foo(unused) {
  return _unused;
}
```

Renaming `unused` to `_unused` would make the body reference the parameter instead of the outer binding. This now detects that capture risk and chooses `_unused0` instead, while still allowing inner-scope bindings that already shadow the name.

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

This PR improves the no-unused-vars fixer’s parameter-renaming logic so that when it renames an unused parameter to satisfy argsIgnorePattern, it avoids choosing a name that would “capture” existing references inside the function body (changing which binding those references resolve to).

Changes:

  • Extend conflict-avoidance logic to also reject rename candidates that would capture existing resolved or unresolved references in descendant scopes.
  • Add new regression tests covering outer-scope capture and nested-function capture cases, while still allowing inner-scope shadowing.

Reviewed changes

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

File Description
crates/oxc_linter/src/rules/eslint/no_unused_vars/fixers/fix_conflicts.rs Adds capture-risk detection when synthesizing an ignored name, preventing renames that would change identifier resolution.
crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs Adds fix regression tests for capture-risk scenarios and an inner-scope shadowing “allowed” scenario.

Comment thread crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/oxc.rs
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 11, 2026
@camc314 camc314 requested a review from Copilot May 11, 2026 10:08
camc314 added a commit that referenced this pull request May 11, 2026
When `no-unused-vars` renames an unused parameter to satisfy `argsIgnorePattern`, it can accidentally change existing references in the function body.

For example:

```js
const _unused = 1;
function foo(unused) {
  return _unused;
}
```

Renaming `unused` to `_unused` would make the body reference the parameter instead of the outer binding. This now detects that capture risk and chooses `_unused0` instead, while still allowing inner-scope bindings that already shadow the name.
@camc314 camc314 force-pushed the codex/no-unused-vars-reference-capture branch from f16d617 to 1889c36 Compare May 11, 2026 10:11

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 2 out of 2 changed files in this pull request and generated 1 comment.

@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label May 11, 2026
When `no-unused-vars` renames an unused parameter to satisfy `argsIgnorePattern`, it can accidentally change existing references in the function body.

For example:

```js
const _unused = 1;
function foo(unused) {
  return _unused;
}
```

Renaming `unused` to `_unused` would make the body reference the parameter instead of the outer binding. This now detects that capture risk and chooses `_unused0` instead, while still allowing inner-scope bindings that already shadow the name.
@graphite-app graphite-app Bot force-pushed the codex/no-unused-vars-reference-capture branch from 1889c36 to 610f4c7 Compare May 11, 2026 10:15
@graphite-app graphite-app Bot merged commit 610f4c7 into main May 11, 2026
28 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label May 11, 2026
@graphite-app graphite-app Bot deleted the codex/no-unused-vars-reference-capture branch May 11, 2026 10:17
camc314 added a commit that referenced this pull request May 11, 2026
# Oxlint
### 💥 BREAKING CHANGES

- 00ce512 oxlint/lsp: [**BREAKING**] Don't fix suggestions on fixAll
code actions & command (#22195) (Sysix)

### 🚀 Features

- 0eeceaf linter/no-unused-vars: Rename parameter with initializer
(#22308) (camc314)
- fa0232b linter/no-unused-vars: Add param rename suggestion (#22285)
(Ryota Misumi)
- ae59305 linter/promise/no-promise-in-callback: Add
`exemptDeclarations` option (#22275) (Mikhail Baev)
- 60bed4a linter: Extends `no-redundant-roles` and
`prefer-tag-over-role` support roles (#22069) (mehm8128)
- 545c80f linter/eslint: Implement `prefer-regex-literals` rule (#22192)
(Mikhail Baev)
- cf86d7a linter: Bulk suppression (#19328) (Said Atrahouch)
- 23abd22 linter/jsx-a11y: Implement
no-noninteractive-element-to-interactive-role (#21264) (Pedro Tainha)
- fbb8f22 linter: Support `ignores` in overrides (#22148) (camc314)
- 5a4414d oxlint/lsp: Support `rulesCustomization` lsp option (#21858)
(Sysix)

### 🐛 Bug Fixes

- 610f4c7 linter/no-unused-vars: Avoid renaming captured vars (#22310)
(camc314)
- 6b50f23 oxlint/cli: Load root config by searching up parent
directories (#22272) (Sysix)
- 31a5de7 linter: Rename override `ignores` to `excludeFiles` (#22283)
(camc314)
- 26d5d7b linter: Add missing vitest/valid-describe-callback
functionality (#22279) (camchenry)
- 784530f linter: `valid-title`: detect `String.raw` strings (#22271)
(Sysix)
- 080d90e linter: Move `no-debugger` fix to suggestion (#22256) (Sysix)
- 25b7017 linter: Undocument override `ignores` option (#22213)
(camc314)
- 7bb00dd linter: Fix role-has-required-aria-props (#22097) (mehm8128)
- d25279e linter/disable-directives: Improve parsing of names,
descriptions (#22184) (camc314)
- a59e447 linter/disable-directives: Ignore invalid enable suffixes
(#22179) (camc314)
- aafef0f ci: Disable bulk supression test on big endian (#22175)
(camc314)
- 281daec linter/vue/define-props-destructuring: Add
`only-when-assigned` config opt (#22142) (camc314)
- 46ab679 linter/plugins: Trim leading newline for partial sources
(#20928) (bab)
- 29ff6d9 linter: Update docs for no_alias_methods rule to be
Vitest-specific and add toThrowError alias (#22129) (camchenry)

### ⚡ Performance

- 9414bee linter/role-has-required-aria-props: Avoid intermediate vec
(#22212) (camc314)
- 3883ea3 linter/no-useless-escape: Drop unnecessary Vec collect
(#22171) (connorshea)
- 42c3029 linter/check-property-names: Replace split-collect-pop-join
with rfind (#22172) (connorshea)
- 9551d53 linter: Remove unnecessary Vec collect in CFG edge traversal
(#22167) (connorshea)
- 26fa2fc linter/aria-role: Remove unnecessary string allocations in run
method (#22168) (connorshea)
- c9ce045 linter/getter-return: Remove unnecessary Vec collect in CFG
edge traversal (#22166) (connorshea)
- 72bd846 linter/no-this-in-sfc: Reorder cheap name check, avoid String
allocation (#22164) (connorshea)

### 📚 Documentation

- 4da212a linter/no-unused-vars: Add docs to
`rename_unused_function_parameter` (#22311) (camc314)
- 27c4628 linter/forbid-dom-props: Escape jsx examples in lint rule docs
(#22254) (4MBL)
- 3f81147 linter: Improve the `react/jsx-key` rule docs. (#22162)
(connorshea)
- 07f03cc linter/consistent-return: Add note about `noImplicitReturns`
coverage (#22156) (camc314)
- 7c1e049 oxlint/lsp: Improve autogenerated lsp docs (#22154) (Sysix)
- 87b3e38 linter: Update docs to be vitest-specific for
consistent-test-it (#22128) (camchenry)
# Oxfmt
### 💥 BREAKING CHANGES

- 5c6c390 oxfmt: [**BREAKING**] Respect more git ignore options, align
with Oxlint (#22210) (leaysgur)

### 🚀 Features

- 6e8e818 oxfmt: Experimental .svelte support (#21700) (leaysgur)

### 🐛 Bug Fixes

- e2a20b6 formatter: Add space after commas in import attributes
(#22274) (Leonabcd123)

### ⚡ Performance

- b756682 oxfmt: Optimize nested config prescan (#22232) (Jovi De
Croock)
- f14e81e formatter/sort_imports: Skip sort for single import runs
(#22204) (leaysgur)
- 32255b1 formatter: Process `ImportDeclaration`s in a run (#22079)
(overlookmotel)

### 📚 Documentation

- 4da6f4c formatter: Correct comment (#22217) (overlookmotel)
- ef3507d formatter/sort_imports: Refresh docs (#22203) (leaysgur)

Co-authored-by: Cameron <cameron.clark@hey.com>
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