feat(rome_js_analyzer): implement rule noRedeclaration, no-redeclare (#…

…4053)

crates/rome_analyze/src/context.rs

@@ -56,7 +56,7 @@ where
     /// use rome_analyze::{declare_rule, Rule, RuleCategory, RuleMeta, RuleMetadata};
     /// use rome_analyze::context::RuleContext;
     /// use serde::Deserialize;
-    /// declare_rule! {    
+    /// declare_rule! {
     ///     /// Some doc
     ///     pub(crate) Name {
     ///         version: "0.0.0",

crates/rome_diagnostics_categories/src/categories.rs

@@ -108,6 +108,7 @@ define_dategories! {
     "lint/nursery/noSvgWithoutTitle": "https://docs.rome.tools/lint/rules/noSvgWithoutTitle",
     "lint/nursery/noUselessCatch": "https://docs.rome.tools/lint/rules/noUselessCatch",
     // Insert new nursery rule here
+    "lint/nursery/noRedeclaration": "https://docs.rome.tools/lint/rules/noRedeclaration",
 
     // performance
     "lint/performance/noDelete": "https://docs.rome.tools/lint/rules/noDelete",

crates/rome_js_analyze/src/semantic_analyzers/nursery/no_redeclaration.rs

@@ -0,0 +1,106 @@
+use std::{collections::HashMap, hash::Hash, vec::IntoIter};
+
+use rome_console::markup;
+use rome_js_semantic::{Binding, Scope};
+
+use crate::semantic_services::Semantic;
+use rome_analyze::{context::RuleContext, Rule, RuleDiagnostic};
+use rome_js_syntax::{JsModule, JsScript, TextRange};
+
+use rome_analyze::declare_rule;
+use rome_rowan::{declare_node_union, AstNode};
+
+declare_rule! {
+    /// Eliminate variables that have multiple declarations in the same scope.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// var a = 3;
+    /// var a = 10;
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// class C {
+    ///     static {
+    ///         var c = 3;
+    ///         var c = 10;
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```js
+    /// var a = 3;
+    /// a = 10;
+    /// ```
+    ///
+    pub(crate) NoRedeclaration {
+        version: "12.0.0",
+        name: "noRedeclaration",
+        recommended: true,
+    }
+}
+
+declare_node_union! {
+    pub(crate) AnyJsBlock =  JsScript | JsModule
+}
+
+type Duplicates = HashMap<String, Vec<Binding>>;
+
+type Redeclaration = (String, TextRange, Binding);
+
+impl Rule for NoRedeclaration {
+    type Query = Semantic<AnyJsBlock>;
+    type State = Redeclaration;
+    type Signals = IntoIter<Redeclaration>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let mut redeclarations = Vec::default();
+        for scope in ctx.model().scopes() {
+            check_redeclarations_in_single_scope(&scope, &mut redeclarations);
+        }
+        redeclarations.into_iter()
+    }
+
+    fn diagnostic(_ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
+        let (name, text_range, binding) = state;
+        let diag = RuleDiagnostic::new(
+            rule_category!(),
+            binding.syntax().text_trimmed_range(),
+            markup! {
+               "Shouldn't redeclare '"{ name }"'. Consider to delete it or rename it"
+            },
+        )
+        .detail(
+            text_range,
+            markup! {
+               "'"{ name }"' is defined here."
+            },
+        );
+        Some(diag)
+    }
+}
+
+fn check_redeclarations_in_single_scope(scope: &Scope, redeclarations: &mut Vec<Redeclaration>) {
+    let mut duplicates = Duplicates::default();
+    let bindings = scope.bindings();
+    for binding in bindings {
+        let name = binding.tree().text();
+        duplicates.entry(name).or_default().push(binding)
+    }
+
+    // only keep the actual redeclarations
+    duplicates.retain(|_, list| list.len() > 1);
+
+    for (name, list) in duplicates {
+        let first_binding_range = list[0].syntax().text_trimmed_range();
+        list.into_iter()
+            .skip(1) // skip the first binding
+            .for_each(|binding| redeclarations.push((name.clone(), first_binding_range, binding)))
+    }
+}

crates/rome_js_analyze/src/semantic_analyzers/nursery/use_const.rs

@@ -265,7 +265,7 @@ impl VariableDeclaration {
 /// Visit [JsIdentifierBinding] in the given [JsAnyBindingPattern].
 ///
 /// Traversal stops if the given function returns true.
-fn with_binding_pat_identifiers(
+pub(crate) fn with_binding_pat_identifiers(
     pat: AnyJsBindingPattern,
     f: &mut impl FnMut(JsIdentifierBinding) -> bool,
 ) -> bool {

crates/rome_js_analyze/tests/specs/nursery/noRedeclaration/invalid.jsonc

@@ -0,0 +1,28 @@
+[
+	"var a = 3; var a = 10;",
+	"var c; { var a; var a;} ",
+
+	// in non-strict mode, function a is hoisted
+	"var a; { function a(){} }",
+
+	"switch(foo) { case a: var b = 3;\ncase b: var b = 4}",
+	"var a = 3; var a = 10;",
+	"var a = {}; var a = [];",
+	"var a; function a() {}",
+	"function a() {} function a() {}",
+	"var a = function() { }; var a = function() { }",
+	"var a = function() { }; var a = new Date();",
+	"var a = 3; var a = 10; var a = 15;",
+	"var a; var a;",
+	"export var a; var a;",
+	"class C { static { var a; var a; } }",
+	"class C { static { var a; { var a; } } }",
+	"class C { static { { var a; } var a; } }",
+	"class C { static { { var a; } { var a; } } }",
+	"var a; var {a = 0, b: Object = 0} = {};",
+	"var a; var {a = 0, b: globalThis = 0} = {};",
+	"function f() { var a; var a; }",
+	"function f() { var a; if (test) { var a; } }",
+	"for (var a, a;;);",
+	"for (;;){ var a, a,;}"
+]