Skip to content

fix(noShadow): detect destructured patterns in sibling scopes#9344

Merged
ematipico merged 1 commit intomainfrom
fix/no-shadow-destructuring
Mar 5, 2026
Merged

fix(noShadow): detect destructured patterns in sibling scopes#9344
ematipico merged 1 commit intomainfrom
fix/no-shadow-destructuring

Conversation

@ematipico
Copy link
Member

@ematipico ematipico commented Mar 5, 2026

Summary

I used an AI agent for this fix, and I reviewed the code.

Closes #6921

The rule didn't handle destructuring at all.

Test Plan

Added new tests

Docs

@changeset-bot
Copy link

changeset-bot bot commented Mar 5, 2026

🦋 Changeset detected

Latest commit: ea58a38

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Mar 5, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 5, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0d55a4f1-4153-4394-a25e-685ea41dc579

📥 Commits

Reviewing files that changed from the base of the PR and between 84a9ecf and ea58a38.

⛔ Files ignored due to path filters (2)
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/invalidDestructuring.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/validDestructuring.js.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (5)
  • .changeset/fix-no-shadow-destructuring.md
  • .claude/skills/testing-codegen/SKILL.md
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/invalidDestructuring.js
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/validDestructuring.js
🚧 Files skipped from review as they are similar to previous changes (4)
  • .claude/skills/testing-codegen/SKILL.md
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/validDestructuring.js
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/invalidDestructuring.js

Walkthrough

This PR fixes the nursery noShadow linter to correctly recognise destructured bindings (object, array, nested patterns and rest elements) as declarations in sibling scopes. The rule implementation now uses AnyJsBindingDeclaration and normalises binding patterns via declaration()/parent_binding_pattern_declaration(). New tests (validDestructuring.js, invalidDestructuring.js) exercise the covered destructuring cases. A changeset entry documenting the patch release for @biomejs/biome (references issue #6921) and updates to test naming/conventions guidance were also added.

Suggested reviewers

  • dyc3
  • arendjr
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarises the main change: fixing the noShadow rule to detect destructured patterns in sibling scopes, which directly addresses the core issue.
Description check ✅ Passed The description appropriately explains the fix, discloses AI assistance per guidelines, references the related issue #6921, and confirms test coverage.
Linked Issues check ✅ Passed The PR directly addresses issue #6921 by updating the noShadow rule's handling of destructured declarations in sibling scopes, which was the core objective.
Out of Scope Changes check ✅ Passed All changes are scoped to the noShadow rule fix and testing. The updates to test guidance in SKILL.md support the noShadow testing approach, keeping everything cohesive.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/no-shadow-destructuring

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.claude/skills/testing-codegen/SKILL.md:
- Around line 146-148: The table separator row currently lacks the required
spacing and is tripping MD060; update the table so the separator uses spaced
columns (e.g., change the separator from "|---|---|" to "| --- | --- |") and
ensure there is a blank line immediately before the table; specifically edit the
header/separator for the `biome_js_analyze` row (the line containing the
languages/extensions list) so the pipes have surrounding spaces and the
separator uses at least three dashes per column.
- Around line 172-175: The tree example contradicts the naming rule: update the
example so folder name and expected diagnostics align with the rule referenced
near lines 164-165; either rename validResolutionReact/ to a name that implies
diagnostics (e.g., invalidResolutionReact/) or change the inline comments so
file.js is marked "should not generate diagnostics" and file2.js "should
generate diagnostics" to match the stated naming convention; adjust only the
folder name or the per-file "should generate diagnostics" annotations in the
block containing validResolutionReact/, file.js, and file2.js to restore
consistency.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cf8e61ae-2324-4fe6-a664-cb5a574f9919

📥 Commits

Reviewing files that changed from the base of the PR and between 20a64c4 and 84a9ecf.

⛔ Files ignored due to path filters (2)
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/invalidDestructuring.js.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/validDestructuring.js.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (5)
  • .changeset/fix-no-shadow-destructuring.md
  • .claude/skills/testing-codegen/SKILL.md
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/invalidDestructuring.js
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/validDestructuring.js

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 5, 2026

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 156 skipped benchmarks1


Comparing fix/no-shadow-destructuring (ea58a38) with main (b535f60)

Open in CodSpeed

Footnotes

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

@ematipico ematipico force-pushed the fix/no-shadow-destructuring branch from 84a9ecf to ea58a38 Compare March 5, 2026 14:10
@ematipico ematipico merged commit cb4d7d7 into main Mar 5, 2026
18 checks passed
@ematipico ematipico deleted the fix/no-shadow-destructuring branch March 5, 2026 19:46
@github-actions github-actions bot mentioned this pull request Mar 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

💅 lint/nursery/noShadow incorrectly flags scoped destructured variables

2 participants