feat(rome_js_analyze): noVar (#3765)

crates/rome_diagnostics_categories/src/categories.rs

@@ -87,6 +87,7 @@ define_dategories! {
     "lint/nursery/noPrecisionLoss": "https://docs.rome.tools/lint/rules/noPrecisionLoss",
     "lint/nursery/noStringCaseMismatch": "https://docs.rome.tools/lint/rules/noStringCaseMismatch",
     "lint/nursery/noUnsafeFinally": "https://docs.rome.tools/lint/rules/noUnsafeFinally",
+    "lint/nursery/noVar": "https://docs.rome.tools/lint/rules/noVar",
     "lint/nursery/useCamelCase": "https://docs.rome.tools/lint/rules/useCamelCase",
     "lint/nursery/useConst":"https://docs.rome.tools/lint/rules/useConst",
     "lint/nursery/useExhaustiveDependencies": "https://docs.rome.tools/lint/rules/useExhaustiveDependencies",

crates/rome_js_analyze/src/control_flow.rs

@@ -7,3 +7,4 @@ mod nodes;
 mod visitor;
 
 pub(crate) use self::visitor::make_visitor;
+pub(crate) use self::visitor::JsAnyControlFlowRoot;

crates/rome_js_analyze/src/control_flow/visitor.rs

@@ -156,7 +156,7 @@ pub(super) struct FunctionVisitor {
 }
 
 declare_node_union! {
-    JsAnyControlFlowRoot = JsModule
+    pub(crate) JsAnyControlFlowRoot = JsModule
         | JsScript
         | JsAnyFunction
         | JsGetterObjectMember

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

@@ -0,0 +1,101 @@
+use crate::{control_flow::JsAnyControlFlowRoot, semantic_services::Semantic, JsRuleAction};
+use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_diagnostics::Applicability;
+use rome_js_factory::make;
+use rome_js_syntax::{JsModule, JsScript, JsSyntaxKind};
+
+use rome_rowan::{AstNode, BatchMutationExt};
+
+use super::use_const::{ConstBindings, VariableDeclaration};
+
+declare_rule! {
+    /// Disallow the use of `var`
+    ///
+    /// ECMAScript 6 allows programmers to create variables with block scope instead of function scope using the let and const keywords. Block scope is common in many other programming languages and helps programmers avoid mistakes.
+    ///
+    /// Source: https://eslint.org/docs/latest/rules/no-var
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// var foo = 1;
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```js
+    /// const foo = 1;
+    /// let bar = 1;
+    ///```
+    pub(crate) NoVar {
+        version: "11.0.0",
+        name: "noVar",
+        recommended: false,
+    }
+}
+
+impl Rule for NoVar {
+    type Query = Semantic<VariableDeclaration>;
+    type State = ();
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let declaration = ctx.query();
+        declaration.is_var().then_some(())
+    }
+
+    fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
+        let declaration = ctx.query();
+        let var_scope = declaration
+            .syntax()
+            .ancestors()
+            .find(|x| JsAnyControlFlowRoot::can_cast(x.kind()))?;
+        let contextual_note = if JsScript::can_cast(var_scope.kind()) {
+            markup! {
+                "A variable declared with "<Emphasis>"var"</Emphasis>" in the global scope pollutes the global object."
+            }
+        } else if JsModule::can_cast(var_scope.kind()) {
+            markup! {
+                "A variable declared with "<Emphasis>"var"</Emphasis>" is accessible in the whole module. Thus, the variable can be accessed before its initialization and outside the block where it is declared."
+            }
+        } else {
+            markup! {
+                "A variable declared with "<Emphasis>"var"</Emphasis>" is accessible in the whole body of the function. Thus, the variable can be accessed before its initialization and outside the block where it is declared."
+            }
+        };
+        Some(RuleDiagnostic::new(
+            rule_category!(),
+            declaration.range(),
+            markup! {
+                "Use "<Emphasis>"let"</Emphasis>" or "<Emphasis>"const"</Emphasis>" instead of "<Emphasis>"var"</Emphasis>"."
+            },
+        ).note(contextual_note).note(
+            markup! {
+                "See "<Hyperlink href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/var">"MDN web docs"</Hyperlink>" for more details."
+            }
+        ))
+    }
+
+    fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
+        let declaration = ctx.query();
+        let model = ctx.model();
+        let maybe_const = ConstBindings::new(declaration, model)?;
+        let replacing_token_kind = if maybe_const.can_fix {
+            JsSyntaxKind::CONST_KW
+        } else {
+            JsSyntaxKind::LET_KW
+        };
+        let mut mutation = ctx.root().begin();
+        mutation.replace_token(declaration.kind_token()?, make::token(replacing_token_kind));
+        Some(JsRuleAction {
+            category: ActionCategory::QuickFix,
+            applicability: Applicability::MaybeIncorrect,
+            message: markup! { "Use '"<Emphasis>{replacing_token_kind.to_string()?}</Emphasis>"' instead." }.to_owned(),
+            mutation,
+        })
+    }
+}

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

@@ -70,8 +70,8 @@ declare_node_union! {
 }
 
 pub(crate) struct ConstBindings {
-    can_be_const: Vec<JsIdentifierBinding>,
-    can_fix: bool,
+    pub can_be_const: Vec<JsIdentifierBinding>,
+    pub can_fix: bool,
 }
 
 enum ConstCheckResult {
@@ -141,7 +141,7 @@ impl Rule for UseConst {
 }
 
 impl ConstBindings {
-    fn new(declaration: &VariableDeclaration, model: &SemanticModel) -> Option<Self> {
+    pub fn new(declaration: &VariableDeclaration, model: &SemanticModel) -> Option<Self> {
         let mut state = Self {
             can_be_const: Vec::new(),
             can_fix: true,
@@ -208,7 +208,7 @@ fn check_binding_can_be_const(
 }
 
 impl VariableDeclaration {
-    fn for_each_declarator(&self, f: impl FnMut(JsVariableDeclarator)) {
+    pub fn for_each_declarator(&self, f: impl FnMut(JsVariableDeclarator)) {
         match self {
             VariableDeclaration::JsVariableDeclaration(x) => x
                 .declarators()
@@ -221,7 +221,7 @@ impl VariableDeclaration {
         }
     }
 
-    fn for_each_binding(&self, mut f: impl FnMut(JsIdentifierBinding, &JsVariableDeclarator)) {
+    pub fn for_each_binding(&self, mut f: impl FnMut(JsIdentifierBinding, &JsVariableDeclarator)) {
         self.for_each_declarator(|declarator| {
             if let Ok(pattern) = declarator.id() {
                 with_binding_pat_identifiers(pattern, &mut |binding| {
@@ -232,19 +232,26 @@ impl VariableDeclaration {
         });
     }
 
-    fn kind_token(&self) -> Option<JsSyntaxToken> {
+    pub fn kind_token(&self) -> Option<JsSyntaxToken> {
         match self {
             Self::JsVariableDeclaration(x) => x.kind().ok(),
             Self::JsForVariableDeclaration(x) => x.kind_token().ok(),
         }
     }
 
-    fn is_let(&self) -> bool {
+    pub fn is_let(&self) -> bool {
         match self {
             Self::JsVariableDeclaration(it) => it.is_let(),
             Self::JsForVariableDeclaration(it) => it.is_let(),
         }
     }
+
+    pub fn is_var(&self) -> bool {
+        match self {
+            Self::JsVariableDeclaration(it) => it.is_var(),
+            Self::JsForVariableDeclaration(it) => it.is_var(),
+        }
+    }
 }
 
 /// Visit [JsIdentifierBinding] in the given [JsAnyBindingPattern].

crates/rome_js_analyze/tests/specs/nursery/noVar/invalidFunctions.js

@@ -0,0 +1,9 @@
+export function f(x) {
+    if(x) {
+        // assign 'y'
+        var /* @type number */ y /*: number */ = 2*x;
+        // assign 'y' to 'x'
+        x = y;
+    }
+    return x;
+}

crates/rome_js_analyze/tests/specs/nursery/noVar/invalidFunctions.js.snap

@@ -0,0 +1,48 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 73
+expression: invalidFunctions.js
+---
+# Input
+```js
+export function f(x) {
+    if(x) {
+        // assign 'y'
+        var /* @type number */ y /*: number */ = 2*x;
+        // assign 'y' to 'x'
+        x = y;
+    }
+    return x;
+}
+```
+
+# Diagnostics
+```
+invalidFunctions.js:4:9 lint/nursery/noVar  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Use let or const instead of var.
+  
+    2 │     if(x) {
+    3 │         // assign 'y'
+  > 4 │         var /* @type number */ y /*: number */ = 2*x;
+      │         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    5 │         // assign 'y' to 'x'
+    6 │         x = y;
+  
+  i A variable declared with var is accessible in the whole body of the function. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
+  
+  i See MDN web docs for more details.
+  
+  i Suggested fix: Use 'const' instead.
+  
+    2 2 │       if(x) {
+    3 3 │           // assign 'y'
+    4   │ - ········var·/*·@type·number·*/·y·/*:·number·*/·=·2*x;
+      4 │ + ········const·/*·@type·number·*/·y·/*:·number·*/·=·2*x;
+    5 5 │           // assign 'y' to 'x'
+    6 6 │           x = y;
+  
+
+```
+
+

crates/rome_js_analyze/tests/specs/nursery/noVar/invalidModule.js

@@ -0,0 +1,2 @@
+var x = 1;
+export const y = x;

crates/rome_js_analyze/tests/specs/nursery/noVar/invalidModule.js.snap

@@ -0,0 +1,35 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 73
+expression: invalidModule.js
+---
+# Input
+```js
+var x = 1;
+export const y = x;
+```
+
+# Diagnostics
+```
+invalidModule.js:1:1 lint/nursery/noVar  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! Use let or const instead of var.
+  
+  > 1 │ var x = 1;
+      │ ^^^^^^^^^
+    2 │ export const y = x;
+  
+  i A variable declared with var is accessible in the whole module. Thus, the variable can be accessed before its initialization and outside the block where it is declared.
+  
+  i See MDN web docs for more details.
+  
+  i Suggested fix: Use 'const' instead.
+  
+    1   │ - var·x·=·1;
+      1 │ + const·x·=·1;
+    2 2 │   export const y = x;
+  
+
+```
+
+

crates/rome_js_analyze/tests/specs/nursery/noVar/invalidScript.jsonc

@@ -0,0 +1,7 @@
+[
+	"var x = 1; foo(x);",
+	"for (var i in [1,2,3]) { foo(i); }",
+	"for (var x of [1,2,3]) { foo(x); }",
+	"var [x = -1, y] = [1,2]; y = 0;",
+	"var {a: x = -1, b: y} = {a:1,b:2}; y = 0;"
+]