feat(linter): implement no-return-assign

crates/oxc_linter/src/rules.rs

@@ -97,6 +97,7 @@ mod eslint {
     pub mod no_redeclare;
     pub mod no_regex_spaces;
     pub mod no_restricted_globals;
+    pub mod no_return_assign;
     pub mod no_script_url;
     pub mod no_self_assign;
     pub mod no_self_compare;
@@ -558,6 +559,7 @@ oxc_macros::declare_all_lint_rules! {
     eslint::no_redeclare,
     eslint::no_regex_spaces,
     eslint::no_restricted_globals,
+    eslint::no_return_assign,
     eslint::no_script_url,
     eslint::no_self_assign,
     eslint::no_self_compare,

crates/oxc_linter/src/rules/eslint/no_return_assign.rs

@@ -0,0 +1,201 @@
+use oxc_ast::AstKind;
+use oxc_diagnostics::OxcDiagnostic;
+use oxc_macros::declare_oxc_lint;
+use oxc_span::{GetSpan, Span};
+use serde_json::Value;
+
+use crate::{context::LintContext, rule::Rule, AstNode};
+
+fn no_return_assign_diagnostic(span: Span, message: &str) -> OxcDiagnostic {
+    OxcDiagnostic::warn(message.to_string()).with_label(span)
+}
+
+#[derive(Debug, Default, Clone)]
+pub struct NoReturnAssign {
+    always_disallow_assignment_in_return: bool,
+}
+
+declare_oxc_lint!(
+    /// ### What it does
+    /// Disallows assignment operators in return statements
+    ///
+    /// ### Why is this bad?
+    /// Assignment is allowed by js in return expression, but usually, an expression with only one equal sign is intended to be a comparison.
+    /// However, because of the missing equal sign, this turns to assignment, which is valid js code
+    /// Because of this ambiguity, it’s considered a best practice to not use assignment in return statements.
+    ///
+    /// ### Examples
+    ///
+    /// Examples of **incorrect** code for this rule:
+    /// ```js
+    /// () => a = b;
+    /// function x() { return a = b; }
+    /// ```
+    ///
+    /// Examples of **correct** code for this rule:
+    /// ```js
+    /// () => (a = b)
+    /// function x() { var result = a = b; return result; }
+    /// ```
+    NoReturnAssign,
+    style, // See <https://oxc.rs/docs/contribute/linter.html#rule-category> for details
+    suggestion  // Options are 'fix', 'fix_dangerous', 'suggestion', and 'conditional_fix_suggestion'
+);
+
+fn is_sentinel_node(ast_kind: AstKind) -> bool {
+    (ast_kind.is_statement() && !matches!(&ast_kind, AstKind::ExpressionStatement(_)))
+        || matches!(
+            &ast_kind,
+            AstKind::ArrowFunctionExpression(_) | AstKind::Function(_) | AstKind::Class(_)
+        )
+}
+
+impl Rule for NoReturnAssign {
+    fn from_configuration(value: Value) -> Self {
+        let always_disallow_assignment_in_return = value
+            .get(0)
+            .and_then(Value::as_str)
+            .map_or_else(|| false, |value| value != "except-parens");
+        Self { always_disallow_assignment_in_return }
+    }
+
+    fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
+        if let AstKind::AssignmentExpression(_) = node.kind() {
+            if !self.always_disallow_assignment_in_return
+                && ctx
+                    .nodes()
+                    .parent_node(node.id())
+                    .is_some_and(|node| node.kind().as_parenthesized_expression().is_some())
+            {
+                return;
+            }
+            let mut parent_node = ctx.nodes().parent_node(node.id());
+            while parent_node.is_some_and(|parent| !is_sentinel_node(parent.kind())) {
+                parent_node = ctx.nodes().parent_node(parent_node.unwrap().id());
+            }
+            if parent_node.is_none() {
+                return;
+            }
+            let parent = parent_node.unwrap();
+            match parent.kind() {
+                AstKind::ReturnStatement(_) => {
+                    ctx.diagnostic(no_return_assign_diagnostic(
+                        parent.span(),
+                        "Return statement should not contain assignment.",
+                    ));
+                }
+                AstKind::ArrowFunctionExpression(_) => {
+                    ctx.diagnostic(no_return_assign_diagnostic(
+                        parent.span(),
+                        "Arrow function should not return assignment.",
+                    ));
+                }
+                _ => (),
+            }
+        }
+    }
+}
+
+#[test]
+fn test() {
+    use crate::tester::Tester;
+
+    let pass = vec![
+        ("module.exports = {'a': 1};", None), // {                "sourceType": "module"            },
+        ("var result = a * b;", None),
+        ("function x() { var result = a * b; return result; }", None),
+        ("function x() { return (result = a * b); }", None),
+        (
+            "function x() { var result = a * b; return result; }",
+            Some(serde_json::json!(["except-parens"])),
+        ),
+        ("function x() { return (result = a * b); }", Some(serde_json::json!(["except-parens"]))),
+        (
+            "function x() { var result = a * b; return result; }",
+            Some(serde_json::json!(["always"])),
+        ),
+        (
+            "function x() { return function y() { result = a * b }; }",
+            Some(serde_json::json!(["always"])),
+        ),
+        ("() => { return (result = a * b); }", Some(serde_json::json!(["except-parens"]))),
+        ("() => (result = a * b)", Some(serde_json::json!(["except-parens"]))),
+        ("const foo = (a,b,c) => ((a = b), c)", None),
+        (
+            "function foo(){
+			            return (a = b)
+			        }",
+            None,
+        ),
+        (
+            "function bar(){
+			            return function foo(){
+			                return (a = b) && c
+			            }
+			        }",
+            None,
+        ),
+        ("const foo = (a) => (b) => (a = b)", None), // { "ecmaVersion": 6 }
+    ];
+
+    let fail = vec![
+        ("function x() { return result = a * b; };", None),
+        ("function x() { return (result) = (a * b); };", None),
+        ("function x() { return result = a * b; };", Some(serde_json::json!(["except-parens"]))),
+        (
+            "function x() { return (result) = (a * b); };",
+            Some(serde_json::json!(["except-parens"])),
+        ),
+        ("() => { return result = a * b; }", None),
+        ("() => result = a * b", None),
+        ("function x() { return result = a * b; };", Some(serde_json::json!(["always"]))),
+        ("function x() { return (result = a * b); };", Some(serde_json::json!(["always"]))),
+        (
+            "function x() { return result || (result = a * b); };",
+            Some(serde_json::json!(["always"])),
+        ),
+        (
+            "function foo(){
+			                return a = b
+			            }",
+            None,
+        ),
+        (
+            "function doSomething() {
+			                return foo = bar && foo > 0;
+			            }",
+            None,
+        ),
+        (
+            "function doSomething() {
+			                return foo = function(){
+			                    return (bar = bar1)
+			                }
+			            }",
+            None,
+        ),
+        (
+            "function doSomething() {
+			                return foo = () => a
+			            }",
+            None,
+        ), // { "ecmaVersion": 6 },
+        (
+            "function doSomething() {
+			                return () => a = () => b
+			            }",
+            None,
+        ), // { "ecmaVersion": 6 },
+        (
+            "function foo(a){
+			                return function bar(b){
+			                    return a = b
+			                }
+			            }",
+            None,
+        ),
+        ("const foo = (a) => (b) => a = b", None), // { "ecmaVersion": 6 }
+    ];
+
+    Tester::new(NoReturnAssign::NAME, pass, fail).test_and_snapshot();
+}

crates/oxc_linter/src/snapshots/no_return_assign.snap

@@ -0,0 +1,111 @@
+---
+source: crates/oxc_linter/src/tester.rs
+---
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:1:16]
+ 1 │ function x() { return result = a * b; };
+   ·                ──────────────────────
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:1:16]
+ 1 │ function x() { return (result) = (a * b); };
+   ·                ──────────────────────────
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:1:16]
+ 1 │ function x() { return result = a * b; };
+   ·                ──────────────────────
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:1:16]
+ 1 │ function x() { return (result) = (a * b); };
+   ·                ──────────────────────────
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:1:9]
+ 1 │ () => { return result = a * b; }
+   ·         ──────────────────────
+   ╰────
+
+  ⚠ eslint(no-return-assign): Arrow function should not return assignment.
+   ╭─[no_return_assign.tsx:1:1]
+ 1 │ () => result = a * b
+   · ────────────────────
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:1:16]
+ 1 │ function x() { return result = a * b; };
+   ·                ──────────────────────
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:1:16]
+ 1 │ function x() { return (result = a * b); };
+   ·                ────────────────────────
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:1:16]
+ 1 │ function x() { return result || (result = a * b); };
+   ·                ──────────────────────────────────
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:2:20]
+ 1 │ function foo(){
+ 2 │                             return a = b
+   ·                             ────────────
+ 3 │                         }
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:2:20]
+ 1 │ function doSomething() {
+ 2 │                             return foo = bar && foo > 0;
+   ·                             ────────────────────────────
+ 3 │                         }
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:2:20]
+ 1 │     function doSomething() {
+ 2 │ ╭─▶                             return foo = function(){
+ 3 │ │                                   return (bar = bar1)
+ 4 │ ╰─▶                             }
+ 5 │                             }
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:2:20]
+ 1 │ function doSomething() {
+ 2 │                             return foo = () => a
+   ·                             ────────────────────
+ 3 │                         }
+   ╰────
+
+  ⚠ eslint(no-return-assign): Arrow function should not return assignment.
+   ╭─[no_return_assign.tsx:2:27]
+ 1 │ function doSomething() {
+ 2 │                             return () => a = () => b
+   ·                                    ─────────────────
+ 3 │                         }
+   ╰────
+
+  ⚠ eslint(no-return-assign): Return statement should not contain assignment.
+   ╭─[no_return_assign.tsx:3:24]
+ 2 │                             return function bar(b){
+ 3 │                                 return a = b
+   ·                                 ────────────
+ 4 │                             }
+   ╰────
+
+  ⚠ eslint(no-return-assign): Arrow function should not return assignment.
+   ╭─[no_return_assign.tsx:1:20]
+ 1 │ const foo = (a) => (b) => a = b
+   ·                    ────────────
+   ╰────