Skip to content

fix(noShadow): ts overloads#9360

Merged
ematipico merged 4 commits intomainfrom
fix/no-shadow-overloads
Mar 6, 2026
Merged

fix(noShadow): ts overloads#9360
ematipico merged 4 commits intomainfrom
fix/no-shadow-overloads

Conversation

@ematipico
Copy link
Member

Summary

Closes #7125

PR developed with the help of claude code.

Test Plan

Added various test cases

Docs

@changeset-bot
Copy link

changeset-bot bot commented Mar 6, 2026

🦋 Changeset detected

Latest commit: 9e8da4a

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

@ematipico ematipico requested review from a team March 6, 2026 08:55
@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Mar 6, 2026
@ematipico ematipico force-pushed the fix/no-shadow-overloads branch from 68a109e to 8140719 Compare March 6, 2026 09:01
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 6, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

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: d66e724f-4946-4697-8a27-b2801cc10473

📥 Commits

Reviewing files that changed from the base of the PR and between 6855013 and 9e8da4a.

📒 Files selected for processing (1)
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs

Walkthrough

Adds overload-aware handling to the nursery/noShadow lint: new helper is_in_overload_signature detects parameters that belong only to TypeScript overload signatures (constructor, method, function, or rest-parameter overloads) and check_shadowing skips shadow checks for those. Updates AST imports to include parameter-related nodes, adjusts semantic event kinds for TS constructor/getter/setter signature members, and adds tests (valid and invalid overload scenarios) plus a changeset entry. No public API changes.

Possibly related PRs

Suggested reviewers

  • dyc3
  • Conaclos
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(noShadow): ts overloads' clearly identifies the main change: fixing the noShadow rule to correctly handle TypeScript overloads, aligning with the primary objective.
Description check ✅ Passed The description references issue #7125 and mentions added test cases, relating to the changeset's purpose of fixing noShadow rule for TypeScript overloads.
Linked Issues check ✅ Passed The changes fully address issue #7125 by modifying no_shadow.rs to skip shadow checks for overload-signature parameters, adding test cases for valid and invalid overload scenarios, and updating semantic events handling for TypeScript signatures.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the noShadow rule for TypeScript overloads: rule implementation, test files, semantic events, and changeset documentation.

✏️ 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-overloads

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: 1

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

Inline comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_shadow.rs`:
- Around line 294-301: The is_in_overload_signature check currently treats
certain parent kinds as overload signatures but omits setters, causing false
positives; update the matches against parent_function in
is_in_overload_signature to include
AnyJsParameterParentFunction::TsSetterSignatureClassMember(_) alongside
TsConstructorSignatureClassMember, TsMethodSignatureClassMember,
TsDeclareFunctionDeclaration, and TsDeclareFunctionExportDefaultDeclaration so
setter signature parameters are skipped the same way.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 52b82292-f9a2-40a0-b03f-b7d205914abe

📥 Commits

Reviewing files that changed from the base of the PR and between 64ce544 and 68a109e.

⛔ Files ignored due to path filters (2)
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (5)
  • .changeset/fix-no-shadow-overloads.md
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts
  • crates/biome_js_semantic/src/events.rs

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2026

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 53006 53006 0
Passed 51786 51786 0
Failed 1178 1178 0
Panics 42 42 0
Coverage 97.70% 97.70% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 38 38 0
Passed 37 37 0
Failed 1 1 0
Panics 0 0 0
Coverage 97.37% 97.37% 0.00%

markdown/commonmark

Test result main count This PR count Difference
Total 652 652 0
Passed 652 652 0
Failed 0 0 0
Panics 0 0 0
Coverage 100.00% 100.00% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5466 5466 0
Passed 1915 1915 0
Failed 3551 3551 0
Panics 0 0 0
Coverage 35.03% 35.03% 0.00%

ts/babel

Test result main count This PR count Difference
Total 636 636 0
Passed 568 568 0
Failed 68 68 0
Panics 0 0 0
Coverage 89.31% 89.31% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 18875 18875 0
Passed 13014 13014 0
Failed 5860 5860 0
Panics 1 1 0
Coverage 68.95% 68.95% 0.00%

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.

♻️ Duplicate comments (1)
crates/biome_js_analyze/src/lint/nursery/no_shadow.rs (1)

299-317: ⚠️ Potential issue | 🟠 Major

Include setter signatures in overload skip logic.

is_in_overload_signature still misses TsSetterSignatureClassMember, so setter-signature parameters can still be falsely reported as shadowing.

Suggested patch
     matches!(
         parent_function,
         Some(
             AnyJsParameterParentFunction::TsConstructorSignatureClassMember(_)
                 | AnyJsParameterParentFunction::TsMethodSignatureClassMember(_)
+                | AnyJsParameterParentFunction::TsSetterSignatureClassMember(_)
                 | AnyJsParameterParentFunction::TsDeclareFunctionDeclaration(_)
                 | AnyJsParameterParentFunction::TsDeclareFunctionExportDefaultDeclaration(_)
         )
     )
#!/bin/bash
set -euo pipefail

binding_ext_file="$(fd 'binding_ext.rs' | head -n1)"
echo "Using binding_ext file: ${binding_ext_file}"

echo "=== Parent-function enum variants ==="
rg -n "AnyJsParameterParentFunction|TsSetterSignatureClassMember|TsConstructorSignatureClassMember|TsMethodSignatureClassMember" -C2 "${binding_ext_file}"

echo "=== Current overload skip helper ==="
rg -n "fn is_in_overload_signature|AnyJsParameterParentFunction::" -C4 crates/biome_js_analyze/src/lint/nursery/no_shadow.rs

Expected result: enum contains TsSetterSignatureClassMember, but helper match currently does not.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_js_analyze/src/lint/nursery/no_shadow.rs` around lines 299 -
317, The helper is_in_overload_signature is missing the
TsSetterSignatureClassMember variant so parameters in setter signatures are not
skipped; update the matches! in is_in_overload_signature to include
AnyJsParameterParentFunction::TsSetterSignatureClassMember(_) alongside the
existing TsConstructorSignatureClassMember and TsMethodSignatureClassMember
variants so setter-signature parameters are treated as overload signatures and
not reported as shadowing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_shadow.rs`:
- Around line 299-317: The helper is_in_overload_signature is missing the
TsSetterSignatureClassMember variant so parameters in setter signatures are not
skipped; update the matches! in is_in_overload_signature to include
AnyJsParameterParentFunction::TsSetterSignatureClassMember(_) alongside the
existing TsConstructorSignatureClassMember and TsMethodSignatureClassMember
variants so setter-signature parameters are treated as overload signatures and
not reported as shadowing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1a51b303-86bc-46d8-8ec9-2614fe3b7897

📥 Commits

Reviewing files that changed from the base of the PR and between 68a109e and 8140719.

⛔ Files ignored due to path filters (2)
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts.snap is excluded by !**/*.snap and included by **
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts.snap is excluded by !**/*.snap and included by **
📒 Files selected for processing (5)
  • .changeset/fix-no-shadow-overloads.md
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts
  • crates/biome_js_semantic/src/events.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 6, 2026

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 156 skipped benchmarks1


Comparing fix/no-shadow-overloads (9e8da4a) with main (64ce544)

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 and others added 2 commits March 6, 2026 17:41
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@ematipico ematipico merged commit bc5dd99 into main Mar 6, 2026
18 checks passed
@ematipico ematipico deleted the fix/no-shadow-overloads branch March 6, 2026 17:50
@github-actions github-actions bot mentioned this pull request Mar 6, 2026
ematipico added a commit that referenced this pull request Mar 7, 2026
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Co-authored-by: dyc3 <1808807+dyc3@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 L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

💅 nursery/noShadow incorrectly flags Typescript constructor overload parameters in constructor implementation, class methods

2 participants