New: new rule no-constructor-return (fixes #12481) (#12529)

* New: new rule no-constructor-return (fixes #12481) * Docs: polish example * Docs: polish * Fix: fix `fixable` property * Update: update rule type * Fix: inner functions * Chore: refactor * Docs: polish * Docs: polish * Docs: polish * Docs: polish * Fix: rule type * Chore: add more cases
eslint · Nov 21, 2019 · 1a2eb99 · 1a2eb99
1 parent ca3b2a6
commit 1a2eb99
Show file tree

Hide file tree

Showing 5 changed files with 174 additions and 0 deletions.
diff --git a/docs/rules/no-constructor-return.md b/docs/rules/no-constructor-return.md
@@ -0,0 +1,50 @@
+# Disallow returning value in constructor (no-constructor-return)
+
+In JavaScript, returning a value in the constructor of a class may be a mistake. Forbidding this pattern prevents mistakes resulting from unfamiliarity with the language or a copy-paste error.
+
+## Rule Details
+
+This rule disallows return statements in the constructor of a class. Note that returning nothing with flow control is allowed.
+
+Examples of **incorrect** code for this rule:
+
+```js
+/*eslint no-constructor-return: "error"*/
+
+class A {
+    constructor(a) {
+        this.a = a;
+        return a;
+    }
+}
+
+class B {
+    constructor(f) {
+        if (!f) {
+            return 'falsy';
+        }
+    }
+}
+```
+
+Examples of **correct** code for this rule:
+
+```js
+/*eslint no-constructor-return: "error"*/
+
+class C {
+    constructor(c) {
+        this.c = c;
+    }
+}
+
+class D {
+    constructor(f) {
+        if (!f) {
+            return;  // Flow control.
+        }
+
+        f();
+    }
+}
+```
diff --git a/lib/rules/index.js b/lib/rules/index.js
@@ -101,6 +101,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({
     "no-console": () => require("./no-console"),
     "no-const-assign": () => require("./no-const-assign"),
     "no-constant-condition": () => require("./no-constant-condition"),
+    "no-constructor-return": () => require("./no-constructor-return"),
     "no-continue": () => require("./no-continue"),
     "no-control-regex": () => require("./no-control-regex"),
     "no-debugger": () => require("./no-debugger"),

diff --git a/lib/rules/no-constructor-return.js b/lib/rules/no-constructor-return.js
@@ -0,0 +1,62 @@
+/**
+ * @fileoverview Rule to disallow returning value from constructor.
+ * @author Pig Fang <https://github.com/g-plane>
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Rule Definition
+//------------------------------------------------------------------------------
+
+module.exports = {
+    meta: {
+        type: "problem",
+
+        docs: {
+            description: "disallow returning value from constructor",
+            category: "Best Practices",
+            recommended: false,
+            url: "https://eslint.org/docs/rules/no-constructor-return"
+        },
+
+        schema: {},
+
+        fixable: null,
+
+        messages: {
+            unexpected: "Unexpected return statement in constructor."
+        }
+    },
+
+    create(context) {
+        const stack = [];
+
+        return {
+            onCodePathStart(_, node) {
+                stack.push(node);
+            },
+            onCodePathEnd() {
+                stack.pop();
+            },
+            ReturnStatement(node) {
+                const last = stack[stack.length - 1];
+
+                if (!last.parent) {
+                    return;
+                }
+
+                if (
+                    last.parent.type === "MethodDefinition" &&
+                    last.parent.kind === "constructor" &&
+                    (node.parent.parent === last || node.argument)
+                ) {
+                    context.report({
+                        node,
+                        messageId: "unexpected"
+                    });
+                }
+            }
+        };
+    }
+};
diff --git a/tests/lib/rules/no-constructor-return.js b/tests/lib/rules/no-constructor-return.js
@@ -0,0 +1,60 @@
+/**
+ * @fileoverview Tests for no-constructor-return rule.
+ * @author Pig Fang <https://github.com/g-plane>
+ */
+
+"use strict";
+
+//------------------------------------------------------------------------------
+// Requirements
+//------------------------------------------------------------------------------
+
+const rule = require("../../../lib/rules/no-constructor-return"),
+    { RuleTester } = require("../../../lib/rule-tester");
+
+//------------------------------------------------------------------------------
+// Tests
+//------------------------------------------------------------------------------
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } });
+
+const errors = [{ type: "ReturnStatement", messageId: "unexpected" }];
+
+ruleTester.run("no-constructor-return", rule, {
+    valid: [
+        "function fn() { return }",
+        "function fn(kumiko) { if (kumiko) { return kumiko } }",
+        "const fn = function () { return }",
+        "const fn = function () { if (kumiko) { return kumiko } }",
+        "const fn = () => { return }",
+        "const fn = () => { if (kumiko) { return kumiko } }",
+        {
+            code: "return 'Kumiko Oumae'",
+            parserOptions: { ecmaFeatures: { globalReturn: true } }
+        },
+
+        "class C {  }",
+        "class C { constructor() {} }",
+        "class C { constructor() { let v } }",
+        "class C { method() { return '' } }",
+        "class C { get value() { return '' } }",
+        "class C { constructor(a) { if (!a) { return } else { a() } } }",
+        "class C { constructor() { function fn() { return true } } }",
+        "class C { constructor() { this.fn = function () { return true } } }",
+        "class C { constructor() { this.fn = () => { return true } } }"
+    ],
+    invalid: [
+        {
+            code: "class C { constructor() { return } }",
+            errors
+        },
+        {
+            code: "class C { constructor() { return '' } }",
+            errors
+        },
+        {
+            code: "class C { constructor(a) { if (!a) { return '' } else { a() } } }",
+            errors
+        }
+    ]
+});
diff --git a/tools/rule-types.json b/tools/rule-types.json
@@ -88,6 +88,7 @@
     "no-console": "suggestion",
     "no-const-assign": "problem",
     "no-constant-condition": "problem",
+    "no-constructor-return": "problem",
     "no-continue": "suggestion",
     "no-control-regex": "problem",
     "no-debugger": "problem",