From 1a2eb99f11c65813bba11d6576a06cff2b823cc9 Mon Sep 17 00:00:00 2001 From: Pig Fang Date: Thu, 21 Nov 2019 10:19:13 +0800 Subject: [PATCH] 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 --- docs/rules/no-constructor-return.md | 50 +++++++++++++++++++ lib/rules/index.js | 1 + lib/rules/no-constructor-return.js | 62 ++++++++++++++++++++++++ tests/lib/rules/no-constructor-return.js | 60 +++++++++++++++++++++++ tools/rule-types.json | 1 + 5 files changed, 174 insertions(+) create mode 100644 docs/rules/no-constructor-return.md create mode 100644 lib/rules/no-constructor-return.js create mode 100644 tests/lib/rules/no-constructor-return.js diff --git a/docs/rules/no-constructor-return.md b/docs/rules/no-constructor-return.md new file mode 100644 index 00000000000..284b757488e --- /dev/null +++ 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 index c6334918fcb..5e75c2366b7 100644 --- 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 new file mode 100644 index 00000000000..4757770b7cc --- /dev/null +++ b/lib/rules/no-constructor-return.js @@ -0,0 +1,62 @@ +/** + * @fileoverview Rule to disallow returning value from constructor. + * @author Pig Fang + */ + +"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 new file mode 100644 index 00000000000..0a8d3b0aeb3 --- /dev/null +++ b/tests/lib/rules/no-constructor-return.js @@ -0,0 +1,60 @@ +/** + * @fileoverview Tests for no-constructor-return rule. + * @author Pig Fang + */ + +"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 index 9d4a0e7df9e..d4e2da6ddd9 100644 --- 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",