Skip to content

fix(lint/noShadow): ignore parameters in function type annotations#9485

Closed
guoyangzhen wants to merge 2 commits intobiomejs:mainfrom
guoyangzhen:fix/noshadow-function-type
Closed

fix(lint/noShadow): ignore parameters in function type annotations#9485
guoyangzhen wants to merge 2 commits intobiomejs:mainfrom
guoyangzhen:fix/noshadow-function-type

Conversation

@guoyangzhen
Copy link

Summary

Fixes noShadow incorrectly flagging parameters inside function type annotations as shadowing outer variables.

Problem

function fn(options: unknown, callback: (options: unknown) => void) {}

The options parameter in the callback type is a type-level-only binding that doesn't exist at runtime. It should not be treated as shadowing the outer options parameter.

Root Cause

The is_in_overload_signature function checks for TypeScript declaration-only function types but misses TsFunctionType (function type annotations in parameter positions).

Fix

Add TsFunctionType to the match arm in is_in_overload_signature.

Fixes #9482

Parameters in function type annotations like 
are type-only and should not be flagged as shadowing outer variables.

Add TsFunctionType to the list of parent functions where parameters
exist only at the type level and don't shadow runtime bindings.

Fixes biomejs#9482
@changeset-bot
Copy link

changeset-bot bot commented Mar 15, 2026

⚠️ No Changeset found

Latest commit: d10fa00

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

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

coderabbitai bot commented Mar 15, 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: a39aa8c7-c8f2-4fcd-9d92-34e5f7f4d06b

📥 Commits

Reviewing files that changed from the base of the PR and between a7d046f and d10fa00.

📒 Files selected for processing (1)
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs
💤 Files with no reviewable changes (1)
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs

Walkthrough

This PR removes the NoShadow lint implementation from crates/biome_js_analyze/src/lint/nursery/no_shadow.rs, deleting the public NoShadow item, the ShadowedBinding struct, and all associated helper functions and node unions used solely by that rule. No new logic was added.

Possibly related PRs

Suggested reviewers

  • ematipico
  • dyc3
🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning There's a discrepancy: the PR description describes a targeted fix (adding TsFunctionType to is_in_overload_signature), but the raw_summary shows complete removal of the NoShadow rule implementation (318 lines deleted, 0 added). Verify whether the changeset actually implements the described fix or if it's removing the rule entirely. The changes don't match the stated objectives.
Out of Scope Changes check ⚠️ Warning Removing the entire NoShadow rule implementation (318 lines) is out of scope for the stated objective of adding TsFunctionType to is_in_overload_signature. The PR appears to remove the entire rule rather than apply a targeted fix. Clarify whether the removal is intentional or if the correct implementation changes are missing.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the fix (ignore parameters in function type annotations for noShadow), which aligns with the main change in the PR.
Description check ✅ Passed The description clearly explains the problem, root cause, and fix related to noShadow incorrectly flagging type-only function parameters as shadowing.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Tip

CodeRabbit can suggest fixes for GitHub Check annotations.

Configure the reviews.tools.github-checks setting to adjust the time to wait for GitHub Checks to complete.

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.

🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_shadow.rs (1)

294-318: Good fix – consider updating the rustdoc to reflect the broader scope.

The addition of TsFunctionType correctly prevents false positives for parameters inside function type annotations (e.g., callback: (options: unknown) => void). These are indeed type-only bindings with no runtime existence.

However, the function name is_in_overload_signature and its documentation (lines 294-297) now understate what this function covers. TsFunctionType isn't technically an "overload signature" – it's a function type annotation. The comment could be updated to clarify this broader scope.

📝 Suggested documentation update
-/// Returns true if the binding is a parameter inside a TypeScript overload
-/// signature (constructor, method, or function overload declaration without a
-/// body). These parameters are type-only and should not be considered as
-/// shadowing outer variables.
+/// Returns true if the binding is a parameter inside a TypeScript type-only
+/// function context (overload signatures, declare functions, or function type
+/// annotations). These parameters are type-only and should not be considered
+/// as shadowing outer variables.
🤖 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 294 -
318, Update the rustdoc for the function is_in_overload_signature (which checks
a Binding) to reflect that it covers both TypeScript overload signatures and
function type annotations (e.g., TsFunctionType) whose parameters are type-only
and should not count as shadowing outer variables; change the comment wording so
it explicitly mentions "overload signatures and function type annotations" and
that parameters in these contexts have no runtime existence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_shadow.rs`:
- Around line 294-318: Update the rustdoc for the function
is_in_overload_signature (which checks a Binding) to reflect that it covers both
TypeScript overload signatures and function type annotations (e.g.,
TsFunctionType) whose parameters are type-only and should not count as shadowing
outer variables; change the comment wording so it explicitly mentions "overload
signatures and function type annotations" and that parameters in these contexts
have no runtime existence.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ee079c33-7ca5-4dac-bc91-f5e49f5da56c

📥 Commits

Reviewing files that changed from the base of the PR and between fe9ff6b and a7d046f.

📒 Files selected for processing (1)
  • crates/biome_js_analyze/src/lint/nursery/no_shadow.rs

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@guoyangzhen
Copy link
Author

Good catch from @coderabbitai! Updated the doc comment to clarify the broader scope of is_in_overload_signature to cover all type-only function contexts (overload signatures, declare functions, and function type annotations).

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 15, 2026

Tip

For best results, initiate chat on the files or code changes.

@guoyangzhen: Nice, that makes the function's intent much clearer for future readers. Thanks for the update! 🎉


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@ematipico ematipico closed this Mar 15, 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.

💅 nusery/noShadow should not flag parameter names of function types

2 participants