Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from Nov 21, 2019
50 changes: 50 additions & 0 deletions 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 mistake resulting from unfamiliarity with the language or copy-paste error.
g-plane marked this conversation as resolved.
Show resolved Hide resolved

## 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();
}
}
```
1 change: 1 addition & 0 deletions lib/rules/index.js
Expand Up @@ -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"),
Expand Down
58 changes: 58 additions & 0 deletions lib/rules/no-constructor-return.js
@@ -0,0 +1,58 @@
/**
* @fileoverview Rule to disallow returning value from constructor.
* @author Pig Fang <https://github.com/g-plane>
*/

"use strict";

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

module.exports = {
meta: {
type: "suggestion",

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.type === "MethodDefinition" &&
last.parent.kind === "constructor" &&
(node.parent.parent === last || node.argument)
) {
context.report({
node,
messageId: "unexpected"
});
}
}
};
}
};
49 changes: 49 additions & 0 deletions tests/lib/rules/no-constructor-return.js
@@ -0,0 +1,49 @@
/**
* @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: [
"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
}
]
});
1 change: 1 addition & 0 deletions tools/rule-types.json
Expand Up @@ -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",
Expand Down