feat(rome_js_analyze): noConstructorReturn

crates/rome_diagnostics_categories/src/categories.rs

@@ -79,6 +79,7 @@ define_dategories! {
     "lint/nursery/noBannedTypes":"https://docs.rome.tools/lint/rules/noBannedTypes",
     "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/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_constructor_return.rs

@@ -0,0 +1,83 @@
+use rome_analyze::context::RuleContext;
+use rome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic};
+use rome_console::markup;
+use rome_js_syntax::{JsConstructorClassMember, JsReturnStatement};
+use rome_rowan::AstNode;
+
+use crate::control_flow::JsAnyControlFlowRoot;
+
+declare_rule! {
+    /// Disallow returning a value from a constructor
+    ///
+    /// While returning a value from a constructor does not produce an error, the returned value is being ignored. Therefore, returning a value from a constructor 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 {
+    ///     constructor() {
+    ///         return 0;
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```js
+    /// class A {
+    ///     constructor() {}
+    /// }
+    /// ```
+    ///
+    /// ```js
+    /// class B {
+    ///     constructor(x) {
+    ///         return;
+    ///     }
+    /// }
+    /// ```
+    ///
+    /// ```
+    pub(crate) NoConstructorReturn {
+        version: "11.0.0",
+        name: "noConstructorReturn",
+        recommended: false,
+    }
+}
+
+impl Rule for NoConstructorReturn {
+    type Query = Ast<JsReturnStatement>;
+    type State = JsConstructorClassMember;
+    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 constructor = ret
+            .syntax()
+            .ancestors()
+            .find(|x| JsAnyControlFlowRoot::can_cast(x.kind()))
+            .and_then(JsConstructorClassMember::cast);
+        constructor
+    }
+
+    fn diagnostic(ctx: &RuleContext<Self>, constructor: &Self::State) -> Option<RuleDiagnostic> {
+        let ret = ctx.query();
+        Some(RuleDiagnostic::new(
+            rule_category!(),
+            ret.range(),
+            markup! {
+                "The "<Emphasis>"constructor"</Emphasis>" should not "<Emphasis>"return"</Emphasis>" a value."
+            },
+        ).detail(
+            constructor.range(),
+            "The constructor is here:"
+        ).note("Returning a value from a constructor is ignored."))
+    }
+}

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

@@ -0,0 +1,28 @@
+class A {
+	constructor() {
+		return 0;
+	}
+}
+
+class B {
+	constructor() {
+		return this;
+	}
+}
+
+class C {
+	constructor(x) {
+		this.x = x;
+		return x;
+	}
+}
+
+class D {
+	constructor(x) {
+		if (x > 0) {
+			this.x = x;
+			return x;
+		}
+		this.x = 0;
+	}
+}

crates/rome_js_analyze/tests/specs/nursery/noConstructorReturn/invalid.js.snap

@@ -0,0 +1,155 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 73
+expression: invalid.js
+---
+# Input
+```js
+class A {
+	constructor() {
+		return 0;
+	}
+}
+
+class B {
+	constructor() {
+		return this;
+	}
+}
+
+class C {
+	constructor(x) {
+		this.x = x;
+		return x;
+	}
+}
+
+class D {
+	constructor(x) {
+		if (x > 0) {
+			this.x = x;
+			return x;
+		}
+		this.x = 0;
+	}
+}
+```
+
+# Diagnostics
+```
+invalid.js:3:3 lint/nursery/noConstructorReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! The constructor should not return a value.
+
+    1 │ class A {
+    2 │ 	constructor() {
+  > 3 │ 		return 0;
+      │ 		^^^^^^^^^
+    4 │ 	}
+    5 │ }
+
+  i The constructor is here:
+
+    1 │ class A {
+  > 2 │ 	constructor() {
+      │ 	^^^^^^^^^^^^^^^
+  > 3 │ 		return 0;
+  > 4 │ 	}
+      │ 	^
+    5 │ }
+    6 │ 
+
+  i Returning a value from a constructor is ignored.
+
+
+```
+
+```
+invalid.js:9:3 lint/nursery/noConstructorReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! The constructor should not return a value.
+
+     7 │ class B {
+     8 │ 	constructor() {
+   > 9 │ 		return this;
+       │ 		^^^^^^^^^^^^
+    10 │ 	}
+    11 │ }
+
+  i The constructor is here:
+
+     7 │ class B {
+   > 8 │ 	constructor() {
+       │ 	^^^^^^^^^^^^^^^
+   > 9 │ 		return this;
+  > 10 │ 	}
+       │ 	^
+    11 │ }
+    12 │ 
+
+  i Returning a value from a constructor is ignored.
+
+
+```
+
+```
+invalid.js:16:3 lint/nursery/noConstructorReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! The constructor should not return a value.
+
+    14 │ 	constructor(x) {
+    15 │ 		this.x = x;
+  > 16 │ 		return x;
+       │ 		^^^^^^^^^
+    17 │ 	}
+    18 │ }
+
+  i The constructor is here:
+
+    13 │ class C {
+  > 14 │ 	constructor(x) {
+       │ 	^^^^^^^^^^^^^^^^
+  > 15 │ 		this.x = x;
+  > 16 │ 		return x;
+  > 17 │ 	}
+       │ 	^
+    18 │ }
+    19 │ 
+
+  i Returning a value from a constructor is ignored.
+
+
+```
+
+```
+invalid.js:24:4 lint/nursery/noConstructorReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  ! The constructor should not return a value.
+
+    22 │ 		if (x > 0) {
+    23 │ 			this.x = x;
+  > 24 │ 			return x;
+       │ 			^^^^^^^^^
+    25 │ 		}
+    26 │ 		this.x = 0;
+
+  i The constructor is here:
+
+    20 │ class D {
+  > 21 │ 	constructor(x) {
+       │ 	^^^^^^^^^^^^^^^^
+  > 22 │ 		if (x > 0) {
+  > 23 │ 			this.x = x;
+  > 24 │ 			return x;
+  > 25 │ 		}
+  > 26 │ 		this.x = 0;
+  > 27 │ 	}
+       │ 	^
+    28 │ }
+
+  i Returning a value from a constructor is ignored.
+
+
+```
+
+

crates/rome_js_analyze/tests/specs/nursery/noConstructorReturn/valid.js

@@ -0,0 +1,33 @@
+class A {
+	constructor() {}
+}
+
+class B {
+	constructor() {
+		return;
+	}
+}
+
+class C {
+	constructor(x) {
+		this.x = x;
+	}
+}
+
+class D {
+	constructor(x) {
+		if (x > 0) {
+			this.x = x;
+			return;
+		}
+		this.x = 0;
+	}
+}
+
+class E {
+	constructor(x) {
+		void (() => {
+			return x;
+		})();
+	}
+}

crates/rome_js_analyze/tests/specs/nursery/noConstructorReturn/valid.js.snap

@@ -0,0 +1,43 @@
+---
+source: crates/rome_js_analyze/tests/spec_tests.rs
+assertion_line: 73
+expression: valid.js
+---
+# Input
+```js
+class A {
+	constructor() {}
+}
+
+class B {
+	constructor() {
+		return;
+	}
+}
+
+class C {
+	constructor(x) {
+		this.x = x;
+	}
+}
+
+class D {
+	constructor(x) {
+		if (x > 0) {
+			this.x = x;
+			return;
+		}
+		this.x = 0;
+	}
+}
+
+class E {
+	constructor(x) {
+		void (() => {
+			return x;
+		})();
+	}
+}
+```
+
+