diff --git a/CHANGELOG.md b/CHANGELOG.md index 71bdc134da7..dfb89695235 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,7 @@ #### Other changes +- `noRedeclare`: allow redeclare of index signatures are in different type members [#4478](https://github.com/rome/tools/issues/4478) - The rules [`useExhaustiveDependencies`](https://docs.rome.tools/lint/rules/useexhaustivedependencies/) and [`useHookAtTopLevel`](https://docs.rome.tools/lint/rules/usehookattoplevel/) accept a different shape of options diff --git a/crates/rome_js_analyze/src/semantic_analyzers/suspicious/no_redeclare.rs b/crates/rome_js_analyze/src/semantic_analyzers/suspicious/no_redeclare.rs index 451e88bcc21..fc117aa1812 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/suspicious/no_redeclare.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/suspicious/no_redeclare.rs @@ -4,7 +4,9 @@ use rome_analyze::{context::RuleContext, Rule, RuleDiagnostic}; use rome_console::markup; use rome_js_semantic::Scope; use rome_js_syntax::binding_ext::AnyJsBindingDeclaration; -use rome_js_syntax::TextRange; +use rome_js_syntax::{ + AnyTsType, TextRange, TsIndexSignatureParameter, TsIndexSignatureTypeMember, TsTypeMemberList, +}; use rome_rowan::AstNode; use std::collections::HashMap; @@ -115,6 +117,7 @@ fn check_redeclarations_in_single_scope(scope: &Scope, redeclarations: &mut Vec< let mut declarations = HashMap::::default(); for binding in scope.bindings() { let id_binding = binding.tree(); + // We consider only binding of a declaration // This allows to skip function parameters, methods, ... if let Some(decl) = id_binding.declaration() { @@ -125,14 +128,30 @@ fn check_redeclarations_in_single_scope(scope: &Scope, redeclarations: &mut Vec< // e.g. a `function` and a `namespace` // - when both are parameter-like. // A parameter can override a previous parameter. + // - when index signature parameters have the different type annotation or are not in the same type member + if !(first_decl.is_mergeable(&decl) || first_decl.is_parameter_like() && decl.is_parameter_like()) { - redeclarations.push(Redeclaration { - name, - declaration: *first_text_range, - redeclaration: id_binding.syntax().text_trimmed_range(), - }) + match (first_decl, &decl) { + ( + AnyJsBindingDeclaration::TsIndexSignatureParameter(first), + AnyJsBindingDeclaration::TsIndexSignatureParameter(second), + ) => { + if are_index_signature_params_same_type_and_member(first, second) { + redeclarations.push(Redeclaration { + name, + declaration: *first_text_range, + redeclaration: id_binding.syntax().text_trimmed_range(), + }) + } + } + _ => redeclarations.push(Redeclaration { + name, + declaration: *first_text_range, + redeclaration: id_binding.syntax().text_trimmed_range(), + }), + } } } else { declarations.insert(name, (id_binding.syntax().text_trimmed_range(), decl)); @@ -140,3 +159,41 @@ fn check_redeclarations_in_single_scope(scope: &Scope, redeclarations: &mut Vec< } } } + +/// Checks if the both `TsIndexSignatureParameter` have the same type annotation and are in the same type member +fn are_index_signature_params_same_type_and_member( + first: &TsIndexSignatureParameter, + second: &TsIndexSignatureParameter, +) -> bool { + let are_same_index_signature_type_annotations = + are_same_index_signature_type_annotations(first, second); + let (Some(first), Some(second)) = (first.parent::(), second.parent::()) else { + return false + }; + are_same_index_signature_type_annotations.unwrap_or(false) + && are_same_type_members(&first, &second).unwrap_or(false) +} + +/// Checks if the both `TsIndexSignatureParameter` have the same type annotation +fn are_same_index_signature_type_annotations( + first: &TsIndexSignatureParameter, + second: &TsIndexSignatureParameter, +) -> Option { + let first_ts_type = first.type_annotation().ok()?.ty().ok()?; + let second_ts_type = second.type_annotation().ok()?.ty().ok()?; + match (first_ts_type, second_ts_type) { + (AnyTsType::TsStringType(_), AnyTsType::TsStringType(_)) => Some(true), + (AnyTsType::TsNumberType(_), AnyTsType::TsNumberType(_)) => Some(true), + (AnyTsType::TsSymbolType(_), AnyTsType::TsSymbolType(_)) => Some(true), + _ => None, + } +} + +fn are_same_type_members( + first: &TsIndexSignatureTypeMember, + second: &TsIndexSignatureTypeMember, +) -> Option { + let first_parent = first.parent::()?; + let second_parent = second.parent::()?; + Some(first_parent == second_parent) +} diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/invalid.jsonc b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/invalid.jsonc index 122d5a8b66b..af97075bdb7 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/invalid.jsonc +++ b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/invalid.jsonc @@ -25,5 +25,6 @@ "function f() { var a; if (test) { var a; } }", "for (var a, a;;);", "for (;;){ var a, a,;}", - "function f(x) { var x = 5; }" + "function f(x) { var x = 5; }", + "interface A { [index: number]: string; [index: number]: string; }" ] diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/invalid.jsonc.snap b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/invalid.jsonc.snap index b6896691e73..95284338d56 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/invalid.jsonc.snap +++ b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/invalid.jsonc.snap @@ -1,6 +1,5 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs -assertion_line: 96 expression: invalid.jsonc --- # Input @@ -533,4 +532,26 @@ invalid.jsonc:1:18 lint/suspicious/noRedeclare ━━━━━━━━━━━ function f(x) { var x = 5; } ``` +# Input +```js +interface A { [index: number]: string; [index: number]: string; } +``` + +# Diagnostics +``` +invalid.jsonc:1:41 lint/suspicious/noRedeclare ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Shouldn't redeclare 'index'. Consider to delete it or rename it. + + > 1 │ interface A { [index: number]: string; [index: number]: string; } + │ ^^^^^ + + i 'index' is defined here: + + > 1 │ interface A { [index: number]: string; [index: number]: string; } + │ ^^^^^ + + +``` + diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-duplicate.ts b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-duplicate.ts index cd6968b3935..0c6d155198f 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-duplicate.ts +++ b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-duplicate.ts @@ -15,3 +15,6 @@ class A { f(): void {} f(): void {} } + +let a: { [key: string]: string }; +let b: { [key: string]: string }; diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-duplicate.ts.snap b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-duplicate.ts.snap index 00e78f88acf..0163bb347c0 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-duplicate.ts.snap +++ b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-duplicate.ts.snap @@ -1,6 +1,5 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs -assertion_line: 91 expression: valid-duplicate.ts --- # Input @@ -23,6 +22,9 @@ class A { f(): void {} } +let a: { [key: string]: string }; +let b: { [key: string]: string }; + ``` diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-index-signatures.ts b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-index-signatures.ts new file mode 100644 index 00000000000..753c5161b3b --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-index-signatures.ts @@ -0,0 +1,8 @@ +type ValidIndexSignatures = { + a: { + [index: string]: string; + }; + b: { + [index: string]: string; + }; +}; diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-index-signatures.ts.snap b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-index-signatures.ts.snap new file mode 100644 index 00000000000..b5c3a630f86 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid-index-signatures.ts.snap @@ -0,0 +1,18 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: valid-index-signatures.ts +--- +# Input +```js +type ValidIndexSignatures = { + a: { + [index: string]: string; + }; + b: { + [index: string]: string; + }; +}; + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid.jsonc b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid.jsonc index e54adfc1595..454f798dba7 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid.jsonc +++ b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid.jsonc @@ -10,5 +10,6 @@ "class C { static { function a(){} } static { function a(){} } }", "class C { static { var a; { let a; } } }", "class C { static { let a; { let a; } } }", - "class C { static { { let a; } { let a; } } }" + "class C { static { { let a; } { let a; } } }", + "interface A { [index: string]: string; [index: number]: string; }" ] diff --git a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid.jsonc.snap b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid.jsonc.snap index 1b81c6ec73f..cab6d125b81 100644 --- a/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid.jsonc.snap +++ b/crates/rome_js_analyze/tests/specs/suspicious/noRedeclare/valid.jsonc.snap @@ -66,4 +66,9 @@ class C { static { let a; { let a; } } } class C { static { { let a; } { let a; } } } ``` +# Input +```js +interface A { [index: string]: string; [index: number]: string; } +``` +