feat(biome_js_analyze): implement noReturnAssign rule

.changeset/slick-masks-wear.md

@@ -0,0 +1,19 @@
+---
+"@biomejs/biome": patch
+---
+
+Added new rule
+[noReturnAssign](https://biomejs.dev/linter/rules/no-return-assign):
+
+Disallows assignments inside return statements
+
+
+Based on [no-return-assign](https://eslint.org/docs/latest/rules/no-return-assign)
+
+Disallowed example:
+
+```js
+function f(a) {
+    return a = 1;
+}
+```

crates/biome_diagnostics_categories/src/categories.rs

@@ -168,6 +168,7 @@ define_categories! {
     "lint/nursery/noContinue": "https://biomejs.dev/linter/rules/no-continue",
     "lint/nursery/noDeprecatedImports": "https://biomejs.dev/linter/rules/no-deprecated-imports",
     "lint/nursery/noDuplicateDependencies": "https://biomejs.dev/linter/rules/no-duplicate-dependencies",
+    "lint/nursery/noDuplicatedSpreadProps": "https://biomejs.dev/linter/rules/no-duplicated-spread-props",
     "lint/nursery/noEmptySource": "https://biomejs.dev/linter/rules/no-empty-source",
     "lint/nursery/noEqualsToNull": "https://biomejs.dev/linter/rules/no-equals-to-null",
     "lint/nursery/noFloatingPromises": "https://biomejs.dev/linter/rules/no-floating-promises",
@@ -183,8 +184,8 @@ define_categories! {
     "lint/nursery/noNextAsyncClientComponent": "https://biomejs.dev/linter/rules/no-next-async-client-component",
     "lint/nursery/noParametersOnlyUsedInRecursion": "https://biomejs.dev/linter/rules/no-parameters-only-used-in-recursion",
     "lint/nursery/noReactForwardRef": "https://biomejs.dev/linter/rules/no-react-forward-ref",
+    "lint/nursery/noReturnAssign": "https://biomejs.dev/linter/rules/no-return-assign",
     "lint/nursery/noShadow": "https://biomejs.dev/linter/rules/no-shadow",
-    "lint/nursery/noDuplicatedSpreadProps": "https://biomejs.dev/linter/rules/no-duplicated-spread-props",
     "lint/nursery/noSyncScripts": "https://biomejs.dev/linter/rules/no-sync-scripts",
     "lint/nursery/noTernary": "https://biomejs.dev/linter/rules/no-ternary",
     "lint/nursery/noUnknownAttribute": "https://biomejs.dev/linter/rules/no-unknown-attribute",

crates/biome_js_analyze/src/lint/nursery.rs

@@ -19,6 +19,7 @@ pub mod no_multi_str;
 pub mod no_next_async_client_component;
 pub mod no_parameters_only_used_in_recursion;
 pub mod no_react_forward_ref;
+pub mod no_return_assign;
 pub mod no_shadow;
 pub mod no_sync_scripts;
 pub mod no_ternary;
@@ -44,4 +45,4 @@ pub mod use_sorted_classes;
 pub mod use_spread;
 pub mod use_vue_define_macros_order;
 pub mod use_vue_multi_word_component_names;
-declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_continue :: NoContinue , self :: no_deprecated_imports :: NoDeprecatedImports , self :: no_duplicated_spread_props :: NoDuplicatedSpreadProps , self :: no_empty_source :: NoEmptySource , self :: no_equals_to_null :: NoEqualsToNull , self :: no_floating_promises :: NoFloatingPromises , self :: no_for_in :: NoForIn , self :: no_import_cycles :: NoImportCycles , self :: no_increment_decrement :: NoIncrementDecrement , self :: no_jsx_literals :: NoJsxLiterals , self :: no_leaked_render :: NoLeakedRender , self :: no_misused_promises :: NoMisusedPromises , self :: no_multi_str :: NoMultiStr , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_parameters_only_used_in_recursion :: NoParametersOnlyUsedInRecursion , self :: no_react_forward_ref :: NoReactForwardRef , self :: no_shadow :: NoShadow , self :: no_sync_scripts :: NoSyncScripts , self :: no_ternary :: NoTernary , self :: no_unknown_attribute :: NoUnknownAttribute , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_unused_expressions :: NoUnusedExpressions , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_duplicate_keys :: NoVueDuplicateKeys , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: use_array_sort_compare :: UseArraySortCompare , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_find :: UseFind , self :: use_max_params :: UseMaxParams , self :: use_qwik_method_usage :: UseQwikMethodUsage , self :: use_qwik_valid_lexical_scope :: UseQwikValidLexicalScope , self :: use_sorted_classes :: UseSortedClasses , self :: use_spread :: UseSpread , self :: use_vue_define_macros_order :: UseVueDefineMacrosOrder , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } }
+declare_lint_group! { pub Nursery { name : "nursery" , rules : [self :: no_continue :: NoContinue , self :: no_deprecated_imports :: NoDeprecatedImports , self :: no_duplicated_spread_props :: NoDuplicatedSpreadProps , self :: no_empty_source :: NoEmptySource , self :: no_equals_to_null :: NoEqualsToNull , self :: no_floating_promises :: NoFloatingPromises , self :: no_for_in :: NoForIn , self :: no_import_cycles :: NoImportCycles , self :: no_increment_decrement :: NoIncrementDecrement , self :: no_jsx_literals :: NoJsxLiterals , self :: no_leaked_render :: NoLeakedRender , self :: no_misused_promises :: NoMisusedPromises , self :: no_multi_str :: NoMultiStr , self :: no_next_async_client_component :: NoNextAsyncClientComponent , self :: no_parameters_only_used_in_recursion :: NoParametersOnlyUsedInRecursion , self :: no_react_forward_ref :: NoReactForwardRef , self :: no_return_assign :: NoReturnAssign , self :: no_shadow :: NoShadow , self :: no_sync_scripts :: NoSyncScripts , self :: no_ternary :: NoTernary , self :: no_unknown_attribute :: NoUnknownAttribute , self :: no_unnecessary_conditions :: NoUnnecessaryConditions , self :: no_unresolved_imports :: NoUnresolvedImports , self :: no_unused_expressions :: NoUnusedExpressions , self :: no_useless_catch_binding :: NoUselessCatchBinding , self :: no_useless_undefined :: NoUselessUndefined , self :: no_vue_data_object_declaration :: NoVueDataObjectDeclaration , self :: no_vue_duplicate_keys :: NoVueDuplicateKeys , self :: no_vue_reserved_keys :: NoVueReservedKeys , self :: no_vue_reserved_props :: NoVueReservedProps , self :: use_array_sort_compare :: UseArraySortCompare , self :: use_consistent_arrow_return :: UseConsistentArrowReturn , self :: use_exhaustive_switch_cases :: UseExhaustiveSwitchCases , self :: use_explicit_type :: UseExplicitType , self :: use_find :: UseFind , self :: use_max_params :: UseMaxParams , self :: use_qwik_method_usage :: UseQwikMethodUsage , self :: use_qwik_valid_lexical_scope :: UseQwikValidLexicalScope , self :: use_sorted_classes :: UseSortedClasses , self :: use_spread :: UseSpread , self :: use_vue_define_macros_order :: UseVueDefineMacrosOrder , self :: use_vue_multi_word_component_names :: UseVueMultiWordComponentNames ,] } }

crates/biome_js_analyze/src/lint/nursery/no_return_assign.rs

@@ -0,0 +1,118 @@
+use biome_analyze::{
+    QueryMatch, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule,
+};
+use biome_console::markup;
+use biome_diagnostics::Severity;
+use biome_js_syntax::{
+    AnyJsExpression, JsArrowFunctionExpression, JsAssignmentExpression, JsReturnStatement,
+};
+use biome_rowan::{AstNode, TextRange, WalkEvent, declare_node_union};
+use biome_rule_options::no_return_assign::NoReturnAssignOptions;
+
+use crate::services::semantic::Semantic;
+
+declare_lint_rule! {
+    /// Disallow assignments in return statements.
+    ///
+    /// In return statements, it is common to mistype a comparison operator (such as `==`) as an assignment operator (such as `=`).
+    /// Moreover, the use of assignments in a return statement is confusing.
+    /// Return statements are often considered side-effect free.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// function f(a) {
+    ///     return a = 1;
+    /// }
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```js
+    /// function f(a) {
+    ///     a = 1;
+    ///     return a;
+    /// }
+    /// ```
+    ///
+    /// ```js
+    /// function f(a) {
+    ///     return a == 1;
+    /// }
+    /// ```
+    ///
+    pub NoReturnAssign {
+        version: "next",
+        name: "noReturnAssign",
+        language: "js",
+        sources: &[RuleSource::Eslint("no-return-assign").same()],
+        recommended: false,
+        severity: Severity::Error,
+    }
+}
+
+declare_node_union! {
+    pub AnyReturn = JsReturnStatement | JsArrowFunctionExpression
+}
+
+impl Rule for NoReturnAssign {
+    type Query = Semantic<AnyReturn>;
+    type State = TextRange;
+    type Signals = Vec<Self::State>;
+    type Options = NoReturnAssignOptions;
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        run_options(ctx).unwrap_or_default()
+    }
+
+    fn diagnostic(ctx: &RuleContext<Self>, state: &Self::State) -> Option<RuleDiagnostic> {
+        Some(
+            RuleDiagnostic::new(
+                rule_category!(),
+                state,
+                match ctx.query() {
+                    AnyReturn::JsArrowFunctionExpression(_) => markup! {
+                        <Emphasis>"Arrow function"</Emphasis>" should not return "<Emphasis>"assignment"</Emphasis>"."
+                    },
+                    AnyReturn::JsReturnStatement(_) => markup! {
+                        <Emphasis>"Function"</Emphasis>" should not return "<Emphasis>"assignment"</Emphasis>"."
+                    },
+                }
+            ).note(markup! {
+                "Return statements are often considered side-effect free.\nYou likely want to do a comparison `==`\nOtherwise move the assignment outside of the return statement"
+            }))
+    }
+}
+
+fn run_options(ctx: &RuleContext<NoReturnAssign>) -> Option<Vec<TextRange>> {
+    match ctx.query() {
+        AnyReturn::JsReturnStatement(query) => Some(traverse_expression(&query.argument()?)),
+
+        AnyReturn::JsArrowFunctionExpression(query) => Some(traverse_expression(
+            query.body().ok()?.as_any_js_expression()?,
+        )),
+    }
+}
+
+fn traverse_expression(root: &AnyJsExpression) -> Vec<TextRange> {
+    let mut signal = Vec::new();
+    let mut iter = root.syntax().preorder();
+
+    while let Some(event) = iter.next() {
+        if let WalkEvent::Enter(node) = event {
+            if JsAssignmentExpression::can_cast(node.kind()) {
+                signal.push(node.text_range());
+            }
+
+            let is_expression = AnyJsExpression::can_cast(node.kind());
+
+            if !is_expression {
+                std::println!("{:?}", node.kind());
+                iter.skip_subtree();
+            }
+        }
+    }
+    signal
+}

crates/biome_js_analyze/tests/specs/nursery/noReturnAssign/invalid.js

@@ -0,0 +1,110 @@
+/* should generate diagnostics */
+
+const f = (a) => a = 1;
+
+function f(a) {
+	return a = 1;
+}
+
+function f(a) {
+	return (a = 1);
+}
+
+function f(a, b, c) {
+	return (a, b, c = 1);
+}
+
+function f(a, b, c) {
+	return a == (b = c);
+}
+function f(a, b) {
+	return a && (b = 1);
+}
+
+function f(a, b, c) {
+	return a = (b = c);
+}
+
+function f(a, b, c) {
+	return (a = 1, b = 2, c = 3);
+}
+
+function f(a) {
+	return [a = 1];
+}
+
+async function f(a) {
+	return await (a = 1);
+}
+
+function f(a) {
+	return 5 + (a = 1);
+}
+
+function f(a) {
+	return (a = 1) + 5;
+}
+
+function f(a) {
+	return foo(a = 1);
+}
+
+function f(a, b) {
+	return b[a = 1];
+}
+
+function f(a) {
+	return (a = 1) ? true : false;
+}
+
+function f(a) {
+	return true ? a = 1 : false;
+}
+
+function f(a, b) {
+	return true ? false : a = 1;
+}
+
+function f(a, b) {
+	return (a = 1) in b;
+}
+
+function f(a, Class) {
+	return (a = 1) instanceof Class;
+}
+
+function f(a, b) {
+	return (a = 1) || b;
+}
+
+function f(a, b) {
+	return a || (b = 1);
+}
+
+function f(a, b) {
+	return (a = 1) ?? b;
+}
+
+function f(a, b) {
+	return a ?? (b = 1);
+}
+
+function f(a) {
+	return !(a = 1);
+}
+
+function f(a) {
+	return typeof (a = 1);
+}
+
+function f(a) {
+	return void (a = 1);
+}
+
+function f(a) {
+	return -(a = 1);
+}
+
+function f(a) {
+	return <div prop={a = 1} />;
+}