Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-no-shadow-overloads.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@biomejs/biome": patch
---

Fixed [#7125](https://github.com/biomejs/biome/issues/7125): The rule `noShadow` no longer incorrectly flags parameters in TypeScript constructor and method overload signatures.
40 changes: 37 additions & 3 deletions crates/biome_js_analyze/src/lint/nursery/no_shadow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ use biome_console::markup;
use biome_diagnostics::Severity;
use biome_js_semantic::{Binding, SemanticModel};
use biome_js_syntax::{
JsClassExpression, JsFunctionExpression, JsIdentifierBinding, JsParameterList,
TsIdentifierBinding, TsPropertySignatureTypeMember, TsTypeParameter, TsTypeParameterName,
binding_ext::AnyJsBindingDeclaration,
JsClassExpression, JsFormalParameter, JsFunctionExpression, JsIdentifierBinding,
JsParameterList, JsRestParameter, TsIdentifierBinding, TsPropertySignatureTypeMember,
TsTypeParameter, TsTypeParameterName, binding_ext::AnyJsBindingDeclaration,
binding_ext::AnyJsParameterParentFunction,
};
use biome_rowan::{AstNode, SyntaxNodeCast, TokenText, declare_node_union};
use biome_rule_options::no_shadow::NoShadowOptions;
Expand Down Expand Up @@ -131,6 +132,13 @@ fn check_shadowing(model: &SemanticModel, binding: Binding) -> Option<ShadowedBi
return None;
}

if is_in_overload_signature(&binding) {
// Parameters in TypeScript overload signatures (constructor, method,
// and function overloads without a body) are type-only and don't exist
// at runtime. They should not be treated as shadowing outer variables.
return None;
}

let name = get_binding_name(&binding)?;
let binding_hoisted_scope = model
.scope_hoisted_to(&binding.syntax())
Expand Down Expand Up @@ -282,3 +290,29 @@ fn is_inside_function_parameters(binding: &Binding) -> bool {
.skip(1)
.any(|ancestor| ancestor.cast::<JsParameterList>().is_some())
}

/// 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.
fn is_in_overload_signature(binding: &Binding) -> bool {
let node = binding.syntax();
let parent_function = node.clone().cast::<JsIdentifierBinding>().and_then(|id| {
id.parent::<JsFormalParameter>()
.and_then(|p| p.parent_function())
.or_else(|| {
id.parent::<JsRestParameter>()
.and_then(|p| p.parent_function())
})
});
matches!(
parent_function,
Some(
AnyJsParameterParentFunction::TsConstructorSignatureClassMember(_)
| AnyJsParameterParentFunction::TsMethodSignatureClassMember(_)
| AnyJsParameterParentFunction::TsSetterSignatureClassMember(_)
| AnyJsParameterParentFunction::TsDeclareFunctionDeclaration(_)
| AnyJsParameterParentFunction::TsDeclareFunctionExportDefaultDeclaration(_)
)
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/* should generate diagnostics */

// Shadowing overload implementation parameter inside body with let/const
class ShadowInBody {
constructor(a: string);
constructor(a: number);
constructor(a: unknown) {
let a = 4;
}
}

// Rest parameter in implementation shadows outer variable
const items = [1, 2, 3];
function restShadow(...items: string[]): void;
function restShadow(...items: number[]): void;
function restShadow(...items: (string | number)[]) {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: invalid-overloads.ts
---
# Input
```ts
/* should generate diagnostics */

// Shadowing overload implementation parameter inside body with let/const
class ShadowInBody {
constructor(a: string);
constructor(a: number);
constructor(a: unknown) {
let a = 4;
}
}

// Rest parameter in implementation shadows outer variable
const items = [1, 2, 3];
function restShadow(...items: string[]): void;
function restShadow(...items: number[]): void;
function restShadow(...items: (string | number)[]) {}

```

# Diagnostics
```
invalid-overloads.ts:8:13 lint/nursery/noShadow ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This variable shadows another variable with the same name in the outer scope.

6 │ constructor(a: number);
7 │ constructor(a: unknown) {
> 8 │ let a = 4;
│ ^
9 │ }
10 │ }

i This is the shadowed variable, which is now inaccessible in the inner scope.

5 │ constructor(a: string);
6 │ constructor(a: number);
> 7 │ constructor(a: unknown) {
│ ^
8 │ let a = 4;
9 │ }

i Consider renaming this variable. It's easy to confuse the origin of variables if they share the same name.

i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information.


```

```
invalid-overloads.ts:16:24 lint/nursery/noShadow ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This variable shadows another variable with the same name in the outer scope.

14 │ function restShadow(...items: string[]): void;
15 │ function restShadow(...items: number[]): void;
> 16 │ function restShadow(...items: (string | number)[]) {}
│ ^^^^^
17 │

i This is the shadowed variable, which is now inaccessible in the inner scope.

12 │ // Rest parameter in implementation shadows outer variable
> 13 │ const items = [1, 2, 3];
│ ^^^^^
14 │ function restShadow(...items: string[]): void;
15 │ function restShadow(...items: number[]): void;

i Consider renaming this variable. It's easy to confuse the origin of variables if they share the same name.

i This rule belongs to the nursery group, which means it is not yet stable and may change in the future. Visit https://biomejs.dev/linter/#nursery for more information.


```
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/* should not generate diagnostics */

// Constructor overloads: same parameter name in overloads and implementation
class Foo {
prop: number | boolean;

constructor(param1: number);
constructor(otherName: boolean);
constructor(dodo: boolean);
constructor(param1: boolean | number) {
this.prop = param1;
const otherName = 5;
}

param1 = 5;

// Method using same names as constructor overload parameters
method(param1: string): void;
method(otherName: boolean): void;
method(param1: string | boolean): void {
let dodo: number;
}
}

// Constructor overloads with different parameter names in implementation
class Bar {
constructor(a: number);
constructor(b: number) {}

// Method using same name as constructor overload parameter
doThing(a: number) {}
}

// Class without overloads: parameter names can be reused in methods
class Baz {
private apple: null;
constructor(apple: null) {}
bakeAPie(apple: null) {}
}

// Regular overloaded functions
function overloaded(prop: string): void;
function overloaded(prop: number): void;
function overloaded(prop: string | number) {}

// Method overloads
class MethodOverloads {
method(x: string): void;
method(x: number): void;
method(x: string | number): void {}
}

// Rest parameters in function overload signatures
function withRest(...args: string[]): void;
function withRest(...args: number[]): void;
function withRest(...args: (string | number)[]) {}

// Rest parameters in constructor overload signatures
class RestConstructor {
constructor(...items: string[]);
constructor(...items: number[]);
constructor(...items: (string | number)[]) {}

doStuff(...items: boolean[]) {}
}

// Rest parameters in method overload signatures
class RestMethod {
method(...vals: string[]): void;
method(...vals: number[]): void;
method(...vals: (string | number)[]): void {}
}

// Overload signature parameters should not shadow outer-scope variables
// (only the overload signatures, the implementation is a real parameter)
const outerVar = 10;
function overloaded2(outerVar: string): void;
function overloaded2(outerVar: number): void;
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: valid-overloads.ts
---
# Input
```ts
/* should not generate diagnostics */

// Constructor overloads: same parameter name in overloads and implementation
class Foo {
prop: number | boolean;

constructor(param1: number);
constructor(otherName: boolean);
constructor(dodo: boolean);
constructor(param1: boolean | number) {
this.prop = param1;
const otherName = 5;
}

param1 = 5;

// Method using same names as constructor overload parameters
method(param1: string): void;
method(otherName: boolean): void;
method(param1: string | boolean): void {
let dodo: number;
}
}

// Constructor overloads with different parameter names in implementation
class Bar {
constructor(a: number);
constructor(b: number) {}

// Method using same name as constructor overload parameter
doThing(a: number) {}
}

// Class without overloads: parameter names can be reused in methods
class Baz {
private apple: null;
constructor(apple: null) {}
bakeAPie(apple: null) {}
}

// Regular overloaded functions
function overloaded(prop: string): void;
function overloaded(prop: number): void;
function overloaded(prop: string | number) {}

// Method overloads
class MethodOverloads {
method(x: string): void;
method(x: number): void;
method(x: string | number): void {}
}

// Rest parameters in function overload signatures
function withRest(...args: string[]): void;
function withRest(...args: number[]): void;
function withRest(...args: (string | number)[]) {}

// Rest parameters in constructor overload signatures
class RestConstructor {
constructor(...items: string[]);
constructor(...items: number[]);
constructor(...items: (string | number)[]) {}

doStuff(...items: boolean[]) {}
}

// Rest parameters in method overload signatures
class RestMethod {
method(...vals: string[]): void;
method(...vals: number[]): void;
method(...vals: (string | number)[]): void {}
}

// Overload signature parameters should not shadow outer-scope variables
// (only the overload signatures, the implementation is a real parameter)
const outerVar = 10;
function overloaded2(outerVar: string): void;
function overloaded2(outerVar: number): void;

```
6 changes: 6 additions & 0 deletions crates/biome_js_semantic/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,11 @@ impl SemanticEventExtractor {
| TS_DECLARE_FUNCTION_EXPORT_DEFAULT_DECLARATION
| TS_CALL_SIGNATURE_TYPE_MEMBER
| TS_CONSTRUCT_SIGNATURE_TYPE_MEMBER
| TS_CONSTRUCTOR_SIGNATURE_CLASS_MEMBER
| TS_METHOD_SIGNATURE_CLASS_MEMBER
| TS_METHOD_SIGNATURE_TYPE_MEMBER
| TS_GETTER_SIGNATURE_CLASS_MEMBER
| TS_SETTER_SIGNATURE_CLASS_MEMBER
| TS_INDEX_SIGNATURE_CLASS_MEMBER
| TS_INDEX_SIGNATURE_TYPE_MEMBER => {
self.is_ambient_context = true;
Expand Down Expand Up @@ -880,8 +883,11 @@ impl SemanticEventExtractor {
| TS_DECLARE_FUNCTION_EXPORT_DEFAULT_DECLARATION
| TS_CALL_SIGNATURE_TYPE_MEMBER
| TS_CONSTRUCT_SIGNATURE_TYPE_MEMBER
| TS_CONSTRUCTOR_SIGNATURE_CLASS_MEMBER
| TS_METHOD_SIGNATURE_CLASS_MEMBER
| TS_METHOD_SIGNATURE_TYPE_MEMBER
| TS_GETTER_SIGNATURE_CLASS_MEMBER
| TS_SETTER_SIGNATURE_CLASS_MEMBER
| TS_INDEX_SIGNATURE_CLASS_MEMBER
| TS_INDEX_SIGNATURE_TYPE_MEMBER
| TS_INTERFACE_DECLARATION
Expand Down
Loading