Skip to content

Commit

Permalink
New: new rule no-constructor-return (fixes #12481) (#12529)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
g-plane authored and aladdin-add committed Nov 21, 2019
1 parent ca3b2a6 commit 1a2eb99
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 0 deletions.
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 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();
}
}
```
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
62 changes: 62 additions & 0 deletions 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"
});
}
}
};
}
};
60 changes: 60 additions & 0 deletions 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
}
]
});
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

0 comments on commit 1a2eb99

Please sign in to comment.