feat(rome_js_analyze): noAssignInExpressions

crates/rome_cli/tests/snapshots/main_commands_check/nursery_unstable.snap

@@ -1,5 +1,6 @@
 ---
 source: crates/rome_cli/tests/snap_test.rs
+assertion_line: 334
 expression: content
 ---
 ## `check.js`
@@ -17,13 +18,16 @@ some errors were emitted while running checks
 # Emitted Messages
 
 ```block
-check.js:1:4 lint/nursery/noConditionalAssignment  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+check.js:1:4 lint/nursery/noAssignInExpressions  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-  × Expected a conditional expression and instead saw an assignment.
+  × The assignment should not be in an expression.
 
   > 1 │ if(a = b) {}
       │    ^^^^^
 
+  i The use of assignments in expressions is confusing.
+    Expressions are often considered as side-effect free.
+
   i Suggested fix: Did you mean '==='?
 
     1 │ if(a·===·b)·{}

crates/rome_diagnostics_categories/src/categories.rs

@@ -43,8 +43,8 @@ define_dategories! {
 
     // nursery
     "lint/nursery/noAccessKey": "https://docs.rome.tools/lint/rules/noAccessKey",
+    "lint/nursery/noAssignInExpressions": "https://docs.rome.tools/lint/rules/noAssignInExpressions",
     "lint/nursery/noBannedTypes":"https://docs.rome.tools/lint/rules/noBannedTypes",
-    "lint/nursery/noConditionalAssignment": "https://docs.rome.tools/lint/rules/noConditionalAssignment",
     "lint/nursery/noConstEnum": "https://docs.rome.tools/lint/rules/noConstEnum",
     "lint/nursery/noConstructorReturn": "https://docs.rome.tools/lint/rules/noConstructorReturn",
     "lint/nursery/noDistractingElements": "https://docs.rome.tools/lint/rules/noDistractingElements",

crates/rome_js_analyze/src/analyzers/nursery/no_assign_in_expressions.rs

@@ -0,0 +1,125 @@
+use rome_analyze::context::RuleContext;
+use rome_analyze::{declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_diagnostics::Applicability;
+use rome_js_factory::make;
+use rome_js_syntax::{
+    JsAssignmentExpression, JsAssignmentOperator, JsExpressionStatement, JsForStatement,
+    JsParenthesizedExpression, JsSequenceExpression, JsSyntaxKind,
+};
+use rome_rowan::{AstNode, BatchMutationExt};
+
+use crate::JsRuleAction;
+
+declare_rule! {
+    /// Disallow assignments in expressions.
+    ///
+    /// In expressions, it is common to mistype a comparison operator (such as `==`) as an assignment operator (such as `=`).
+    /// Moreover, the use of assignments in expressions is confusing.
+    /// Indeed, expressions are often considered as side-effect free.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```ts,expect_diagnostic
+    /// let a, b;
+    /// a = (b = 1) + 1;
+    /// ```
+    ///
+    /// ```ts,expect_diagnostic
+    /// let a;
+    /// if (a = 1) {
+    /// }
+    /// ```
+    ///
+    /// ```ts,expect_diagnostic
+    /// function f(a) {
+    ///     return a = 1;
+    /// }
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```ts
+    /// let a;
+    /// a = 1;
+    /// ```
+    pub(crate) NoAssignInExpressions {
+        version: "12.0.0",
+        name: "noAssignInExpressions",
+        recommended: true,
+    }
+}
+
+impl Rule for NoAssignInExpressions {
+    type Query = Ast<JsAssignmentExpression>;
+    type State = ();
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let assign = ctx.query();
+        let mut ancestor = assign
+            .syntax()
+            .ancestors()
+            .take_while(|x| {
+                // Allow parens and multiple assign such as `a = b = (c = d)`
+                JsAssignmentExpression::can_cast(x.kind())
+                    || JsParenthesizedExpression::can_cast(x.kind())
+            })
+            .last()?;
+        let mut prev_ancestor = ancestor;
+        ancestor = prev_ancestor.parent()?;
+        while JsSequenceExpression::can_cast(ancestor.kind()) {
+            // Allow statements separated by sequences such as `a = 1, b = 2`
+            prev_ancestor = ancestor;
+            ancestor = prev_ancestor.parent()?;
+        }
+        if JsExpressionStatement::can_cast(ancestor.kind()) {
+            None
+        } else if let Some(for_stmt) = JsForStatement::cast(ancestor) {
+            if let Some(for_test) = for_stmt.test() {
+                // Disallow assignment in test part of a `for`
+                (for_test.syntax() == &prev_ancestor).then_some(())
+            } else {
+                // Allow assignment in initializer and update parts of a `for`
+                None
+            }
+        } else {
+            Some(())
+        }
+    }
+
+    fn diagnostic(ctx: &RuleContext<Self>, _: &Self::State) -> Option<RuleDiagnostic> {
+        let assign = ctx.query();
+        Some(RuleDiagnostic::new(
+            rule_category!(),
+            assign.range(),
+            markup! {
+                "The "<Emphasis>"assignment"</Emphasis>" should not be in an "<Emphasis>"expression"</Emphasis>"."
+            },
+        ).note(
+            "The use of assignments in expressions is confusing.\nExpressions are often considered as side-effect free."
+        ))
+    }
+
+    fn action(ctx: &RuleContext<Self>, _: &Self::State) -> Option<JsRuleAction> {
+        let assign = ctx.query();
+        let op = assign.operator().ok()?;
+        if let JsAssignmentOperator::Assign = op {
+            let operator_token = assign.operator_token().ok()?;
+            let new_operator_token = make::token(JsSyntaxKind::EQ3);
+            let mut mutation = ctx.root().begin();
+            mutation.replace_token(operator_token, new_operator_token);
+            Some(JsRuleAction {
+                mutation,
+                applicability: Applicability::MaybeIncorrect,
+                category: ActionCategory::QuickFix,
+                message: markup!("Did you mean '==='?").to_owned(),
+            })
+        } else {
+            None
+        }
+    }
+}

crates/rome_js_analyze/tests/specs/nursery/noAssignInExpressions/invalid.js

@@ -0,0 +1,81 @@
+{
+	let a;
+	(a += 1) + 2;
+}
+
+{
+	let a, b;
+	a = (b /*before*/ = /*after*/ 1) + 1;
+}
+
+{
+	let a, b;
+	a = ((b = 1), a);
+}
+
+{
+	let a, b;
+	a = (b = 1, b = 2);
+}
+
+{
+	let a, b;
+	a = (class {}, b = 2, function() {});
+}
+
+{
+	let a;
+	const b = (a = 0) ? 1 : 0;
+}
+
+{
+	let a, b;
+	const c = a && (b = 0) ? 1 : 0;
+}
+
+function f(a) {
+	return (a = 5 + 1);
+}
+
+if (a = 0) {
+}
+
+if (a || (a = b)) {
+}
+
+if (a += b) {
+}
+
+while ((a = 0)) {}
+
+while (a *= b) {}
+
+do {} while ((a = a + 1));
+
+do {} while (((a -= 1), a));
+
+do {} while (((a = a + 1), a));
+
+do {} while (a || (a = b));
+
+for (let a = 5; (a = 0); i--) {}
+
+for (let x = 0; (x += 1); ) {}
+
+for (let l; typeof l === "undefined" ? (l = 0) : l; i++) {}
+
+for (; (a = y); ) {}
+
+for (let a = (b = 1); a < 5; ) {}
+
+switch (foo) {
+	case (a = b):
+		bar();
+}
+
+switch (foo) {
+	case baz + (a = b):
+		bar();
+}
+
+((3496.29).bkufyydt = 2e308) ? foo : bar;