diff --git a/.changeset/fix-no-shadow-overloads.md b/.changeset/fix-no-shadow-overloads.md new file mode 100644 index 000000000000..4837220e0c1b --- /dev/null +++ b/.changeset/fix-no-shadow-overloads.md @@ -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. diff --git a/crates/biome_js_analyze/src/lint/nursery/no_shadow.rs b/crates/biome_js_analyze/src/lint/nursery/no_shadow.rs index c62ce0e2ccd8..8006fe16dcf9 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_shadow.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_shadow.rs @@ -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; @@ -131,6 +132,13 @@ fn check_shadowing(model: &SemanticModel, binding: Binding) -> Option bool { .skip(1) .any(|ancestor| ancestor.cast::().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::().and_then(|id| { + id.parent::() + .and_then(|p| p.parent_function()) + .or_else(|| { + id.parent::() + .and_then(|p| p.parent_function()) + }) + }); + matches!( + parent_function, + Some( + AnyJsParameterParentFunction::TsConstructorSignatureClassMember(_) + | AnyJsParameterParentFunction::TsMethodSignatureClassMember(_) + | AnyJsParameterParentFunction::TsSetterSignatureClassMember(_) + | AnyJsParameterParentFunction::TsDeclareFunctionDeclaration(_) + | AnyJsParameterParentFunction::TsDeclareFunctionExportDefaultDeclaration(_) + ) + ) +} diff --git a/crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts b/crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts new file mode 100644 index 000000000000..8c05c1229110 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts @@ -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)[]) {} diff --git a/crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts.snap new file mode 100644 index 000000000000..ada9a0b9d970 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noShadow/invalid-overloads.ts.snap @@ -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. + + +``` diff --git a/crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts b/crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts new file mode 100644 index 000000000000..54a9afcd075c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts @@ -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; diff --git a/crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts.snap b/crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts.snap new file mode 100644 index 000000000000..418fe75a89ef --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/nursery/noShadow/valid-overloads.ts.snap @@ -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; + +``` diff --git a/crates/biome_js_semantic/src/events.rs b/crates/biome_js_semantic/src/events.rs index 4696e32375e2..07a9fd5643e2 100644 --- a/crates/biome_js_semantic/src/events.rs +++ b/crates/biome_js_semantic/src/events.rs @@ -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; @@ -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