Skip to content

fix(linter/no-unused-vars): reportUsedIgnorePattern should not report used rest siblings#20108

Merged
camc314 merged 3 commits intooxc-project:mainfrom
DonIsaac:don/fix/nuv-report-unused-rest-siblings
Mar 9, 2026
Merged

fix(linter/no-unused-vars): reportUsedIgnorePattern should not report used rest siblings#20108
camc314 merged 3 commits intooxc-project:mainfrom
DonIsaac:don/fix/nuv-report-unused-rest-siblings

Conversation

@DonIsaac
Copy link
Contributor

@DonIsaac DonIsaac commented Mar 7, 2026

What This PR Does

Fixes a bug where setting { "ignoreRestSiblings": true, "reportUsedIgnorePattern": true } would report used unpacked properties as a lint rule violation.

/* no-unused-vars: ["error", { "ignoreRestSiblings": true, "reportUsedIgnorePattern": true }] */
export const MyComponent = ({ role = "nav" , ...props }) => (
  <div
    role={role} // <-- Would report "Variable 'role' is marked as ignored but is not used"
    {...props}
  />
)

Changes

  • refactor is_ignored to return Option<IgnoreReason> instead of bool. None is not ignored, Some is ignored for some cause.
  • Only ignored symbols with a "NamePattern" reason (ignored because of a match with varsIgnorePattern, argsIgnorePattern, ...fooIgnorePattern) get reported when reportUsedIgnorePattern is set

@github-actions github-actions bot added A-linter Area - Linter C-bug Category - Bug labels Mar 7, 2026
@DonIsaac DonIsaac marked this pull request as ready for review March 7, 2026 22:50
@DonIsaac DonIsaac requested a review from camc314 as a code owner March 7, 2026 22:50
Copilot AI review requested due to automatic review settings March 7, 2026 22:50
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 7, 2026

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 52 skipped benchmarks1


Comparing DonIsaac:don/fix/nuv-report-unused-rest-siblings (834f413) with main (a3013ae)

Open in CodSpeed

Footnotes

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

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 bug where { "ignoreRestSiblings": true, "reportUsedIgnorePattern": true } incorrectly reports used properties that were only ignored because they are rest-siblings (e.g., role in ({ role = "nav", ...props }) => ...). The fix refactors is_ignored to return an Option<IgnoreReason> (via the new Ignored struct) instead of bool, allowing the diagnostic logic to distinguish why a symbol was ignored and only trigger reportUsedIgnorePattern for NamePattern-based ignores.

Changes:

  • Introduces a new IgnoreReason enum and Ignored newtype wrapper in ignored.rs, with FoundStatus updated to carry a reason.
  • Updates is_ignored and related helper functions to return Ignored with a specific IgnoreReason.
  • Adds a comprehensive test_ignore test function in oxc.rs and re-enables a previously commented-out ESLint conformance test.

Reviewed changes

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

Show a summary per file
File Description
ignored.rs Core refactoring: new IgnoreReason enum, Ignored struct, updated FoundStatus and all is_ignored_* helpers
mod.rs Updated run_on_symbol_internal to use Ignored and match on IgnoreReason
usage.rs Adapted call sites to call .is_some() on Ignored where a bool was previously expected
binding_pattern.rs Adapted is_ignored_array_destructured call to use .is_some()
tests/oxc.rs New test_ignore test covering the fixed behavior and edge cases
tests/eslint.rs Re-enables a previously commented-out foox in IIFE test
snapshots/eslint_no_unused_vars@oxc-ignore.snap New snapshot for test_ignore
snapshots/eslint_no_unused_vars@eslint.snap Updated snapshot for the re-enabled ESLint test

@camc314 camc314 self-assigned this Mar 9, 2026
@camc314 camc314 force-pushed the don/fix/nuv-report-unused-rest-siblings branch from 6475b3a to 834f413 Compare March 9, 2026 11:15
Copy link
Contributor

@camc314 camc314 left a comment

Choose a reason for hiding this comment

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

Thank you Don!

@camc314 camc314 merged commit 5559f0d into oxc-project:main Mar 9, 2026
21 checks passed
leaysgur added a commit that referenced this pull request Mar 11, 2026
# Oxlint
### 🚀 Features

- 04a5ce0 oxlint: Support `vite.config.ts` `.lint` field (#20214)
(leaysgur)
- 1735215 linter: Implement `react/no-clone-element` rule. (#20129)
(connorshea)
- 68e6f6f linter: Implement `react/no-react-children` rule. (#20104)
(connorshea)
- fe3b32e linter/plugins: Add `oxlint-plugin-eslint` package (#20009)
(overlookmotel)

### 🐛 Bug Fixes

- 05f6a09 linter/no-inline-comments: Deserialize rule options with serde
(#20207) (camc314)
- c7eb09d linter/default-case: Deserialize rule options with serde
(#20206) (camc314)
- f85e16c linter/plugins: Fix types for visitor compilation (#20203)
(overlookmotel)
- 44e24e0 linter/exhaustive-deps: Ignore type-only typeof deps (#20201)
(camc314)
- 0b04998 linter/no-fallthrough: Deserialize rule options with serde
(#20192) (camc314)
- a1031cb linter/new-cap: Deserialize rule options with serde (#20161)
(camc314)
- ad27fd6 linter: Add help messages to import plugin diagnostics
(#20158) (John Costa)
- 1340307 linter/plugins: Ensure `after` hooks always run (#20167)
(overlookmotel)
- c4812ec linter/plugins: Reset visitor compilation state if error
during compilation (#20166) (overlookmotel)
- 887eecc linter/plugins: Add license notice to `oxlint-plugin-eslint`
package (#20164) (overlookmotel)
- e1713a4 linter/plugins: Include common chunks in
`oxlint-plugin-eslint` package (#20163) (overlookmotel)
- a9acb2b linter: Check `globals` entry for `no-undef`, only check
es2026 globals for `no-extend-native` and
`no-constant-binary-expression` (#20089) (Sysix)
- 5559f0d linter/no-unused-vars: `reportUsedIgnorePattern` should not
report used rest siblings (#20108) (Don Isaac)
- de7c0e2 linter/plugins: Correct error message for `markVariableAsUsed`
(#20152) (overlookmotel)

### ⚡ Performance

- 3a86427 linter/plugins: Pre-populate cache of `EnterExit` objects at
startup (#20194) (overlookmotel)
- d243391 linter/plugins: Replace arrays with `Uint8Array`s (#20190)
(overlookmotel)
- 8742f8b linter/plugins: Pre-populate cache of `VisitProp` objects
(#20189) (overlookmotel)
- 3061acb linter/plugins: Pre-populate cache of `EnterExit` objects
(#20187) (overlookmotel)
- c73912b linter/plugins: Free visit functions earlier (#20186)
(overlookmotel)
- d9f8ff4 linter/plugins: Faster reset of visitor state (#20185)
(overlookmotel)
- 42aff15 oxlint/lsp: Avoid computing diagnostics for non invoked code
actions requests (#20080) (Sysix)

### 📚 Documentation

- a080650 linter/plugins: Fix documentation of visitor compilation
(#20202) (overlookmotel)
- 542a04a linter: Add a link to the cyclomatic complexity Wikipedia
article in `eslint/complexity` (#20174) (connorshea)
# Oxfmt
### 🚀 Features

- 95943aa oxfmt: Support `vite.config.*` `.fmt` field (#20197)
(leaysgur)
- 172fc07 oxfmt: .js/.ts config file support (#20135) (leaysgur)

### 🐛 Bug Fixes

- e483569 oxfmt: Avoid double-escaping in css-in-js (#20211) (leaysgur)

Co-authored-by: leaysgur <6259812+leaysgur@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants