fix(rome_js_analyze): noRedeclare accepts index signatures (#4519)

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
 

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::<String, (TextRange, AnyJsBindingDeclaration)>::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,18 +128,72 @@ 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));
             }
         }
     }
 }
+
+/// 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::<TsIndexSignatureTypeMember>(), second.parent::<TsIndexSignatureTypeMember>()) 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<bool> {
+    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<bool> {
+    let first_parent = first.parent::<TsTypeMemberList>()?;
+    let second_parent = second.parent::<TsTypeMemberList>()?;
+    Some(first_parent == second_parent)
+}

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; }"
 ]

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; }
+      │                ^^^^^
+  
+
+```
+
 

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 };

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 };
+
 ```
 
 

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;
+	};
+};

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;
+	};
+};
+
+```
+
+

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; }"
 ]

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; }
+```
+