Merge branch 'bare-ts-no_setter_return'

crates/rome_diagnostics_categories/src/categories.rs

@@ -80,6 +80,7 @@ define_dategories! {
     "lint/nursery/noConditionalAssignment": "https://docs.rome.tools/lint/rules/noConditionalAssignment",
     "lint/nursery/noConstAssign": "https://docs.rome.tools/lint/rules/noConstAssign",
     "lint/nursery/noConstructorReturn": "https://docs.rome.tools/lint/rules/noConstructorReturn",
+    "lint/nursery/noSetterReturn": "https://docs.rome.tools/lint/rules/noSetterReturn",
     "lint/nursery/noDupeKeys":"https://docs.rome.tools/lint/rules/noDupeKeys",
     "lint/nursery/noEmptyInterface": "https://docs.rome.tools/lint/rules/noEmptyInterface",
     "lint/nursery/noExplicitAny": "https://docs.rome.tools/lint/rules/noExplicitAny",

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

@@ -0,0 +1,112 @@
+use rome_analyze::context::RuleContext;
+use rome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{JsReturnStatement, JsSetterClassMember, JsSetterObjectMember};
+use rome_rowan::{declare_node_union, AstNode};
+
+use crate::control_flow::JsAnyControlFlowRoot;
+
+declare_rule! {
+    /// Disallow returning a value from a setter
+    ///
+    /// While returning a value from a setter does not produce an error, the returned value is being ignored. Therefore, returning a value from a setter is either unnecessary or a possible error.
+    ///
+    /// Only returning without a value is allowed, as it’s a control flow statement.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```js,expect_diagnostic
+    /// class A {
+    ///     set foo(x) {
+    ///         return x;
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// const b = {
+    ///     set foo(x) {
+    ///         return x;
+    ///     },
+    /// };
+    /// ```
+    ///
+    /// ```js,expect_diagnostic
+    /// const c = {
+    ///     set foo(x) {
+    ///         if (x) {
+    ///             return x;
+    ///         }
+    ///     },
+    /// };
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```js
+    /// // early-return
+    /// class A {
+    ///     set foo(x) {
+    ///         if (x) {
+    ///             return;
+    ///         }
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// ```js
+    /// // not a setter
+    /// class B {
+    ///   set(x) {
+    ///     return x;
+    ///   }
+    /// }
+    /// ```
+    ///
+    /// ```
+    pub(crate) NoSetterReturn {
+        version: "11.0.0",
+        name: "noSetterReturn",
+        recommended: false,
+    }
+}
+
+declare_node_union! {
+    pub(crate) JsSetterMember = JsSetterClassMember | JsSetterObjectMember
+}
+
+impl Rule for NoSetterReturn {
+    type Query = Ast<JsReturnStatement>;
+    type State = JsSetterMember;
+    type Signals = Option<Self::State>;
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Self::Signals {
+        let ret = ctx.query();
+        // Do not take arg-less returns into account
+        let _arg = ret.argument()?;
+        let setter = ret
+            .syntax()
+            .ancestors()
+            .find(|x| JsAnyControlFlowRoot::can_cast(x.kind()))
+            .and_then(JsSetterMember::cast);
+        setter
+    }
+
+    fn diagnostic(ctx: &RuleContext<Self>, setter: &Self::State) -> Option<RuleDiagnostic> {
+        let ret = ctx.query();
+        Some(
+            RuleDiagnostic::new(
+                rule_category!(),
+                ret.range(),
+                markup! {
+                    "The setter should not "<Emphasis>"return"</Emphasis>" a value."
+                },
+            )
+            .detail(setter.range(), "The setter is here:")
+            .note("Returning a value from a setter is ignored."),
+        )
+    }
+}

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

@@ -0,0 +1,184 @@
+class Basic {
+	set foo(x) {
+		return x;
+	}
+}
+
+const BasicObject = {
+	set foo(x) {
+		return x;
+	},
+};
+
+class BlockReturn {
+	set foo(x) {
+		{
+			return x;
+		}
+	}
+}
+
+class DoWhile {
+	set foo(x) {
+		do {
+			return x;
+		} while (true)
+	}
+}
+
+class Else {
+	set foo(x) {
+		if (x) {
+		} else {
+			return x;
+		}
+	}
+}
+
+class ElseIf {
+	set foo(x) {
+		if (x) {
+		} else if (x) {
+			return x;
+		}
+	}
+}
+
+class If {
+	set foo(x) {
+		if (x) {
+			return x;
+		}
+	}
+}
+
+class Labelled {
+	set foo(x) {
+		label: return x
+	}
+}
+
+class ForOfBlock {
+	set foo(xs) {
+		for (x of xs) {
+			return x;
+		}
+	}
+}
+
+class ForOfSingleStatement {
+	set foo(xs) {
+		for (x of xs) return x;
+	}
+}
+
+class ForInBlock {
+	set foo(xs) {
+		for (x in xs) {
+			return x;
+		}
+	}
+}
+
+class ForInSingleStatement {
+	set foo(xs) {
+		for (x in xs) return x;
+	}
+}
+
+class ForBlock {
+	set foo(x) {
+		for (;;) {
+			return x;
+		}
+	}
+}
+
+class ForSingleStatement {
+	set foo(x) {
+		for (;;) return x;
+	}
+}
+
+class SwitchCaseReturn {
+	set foo(x) {
+		switch (x) {
+			case 1:
+				return x;
+			default:
+				break;
+		}
+	}
+}
+
+class SwitchDefaultReturn {
+	set foo(x) {
+		switch (x) {
+			case 1:
+				break;
+			default:
+				return x;
+		}
+	}
+}
+
+class TryReturnCatch {
+	set foo(x) {
+		try {
+			return x;
+		} catch {}
+	}
+}
+
+class TryCatchReturn {
+	set foo(x) {
+		try {
+		} catch {
+			return x;
+		}
+	}
+}
+
+class TryReturnCatchFinally {
+	set foo(x) {
+		try {
+			return x;
+		} catch {
+		} finally {
+		}
+	}
+}
+
+class TryCatchReturnFinally {
+	set foo(x) {
+		try {
+		} catch {
+			return x;
+		} finally {
+		}
+	}
+}
+
+class TryCatchFinallyReturn {
+	set foo(x) {
+		try {
+		} catch {
+		} finally {
+			return x;
+		}
+	}
+}
+
+class WhileBlock {
+	set foo(x) {
+		while (true) {
+			return x;
+		}
+	}
+}
+
+class WhileSingleStatement {
+	set foo(x) {
+		while (true) return x;
+	}
+}