Skip to content

Commit

Permalink
New: new rule no-constructor-return (fixes #12481)
Browse files Browse the repository at this point in the history
  • Loading branch information
g-plane committed Nov 5, 2019
1 parent 5868550 commit 3aa5fbf
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 0 deletions.
49 changes: 49 additions & 0 deletions docs/rules/no-constructor-return.md
@@ -0,0 +1,49 @@
# Disallow returning value in constructor (no-constructor-return)

In JavaScript returning value in constructor is allowed, however this looks meaningless. Forbidding this pattern prevents mistake resulting from unfamiliarity with the language or copy-paste error.

Note that returning nothing with flow control is allowed.

## Rule Details

This rule disallows return statement in constructor of a class.

Examples of **incorrect** code for this rule:

```js
/*eslint no-constructor-return: "error"*/

class A {
constructor() {

This comment has been minimized.

Copy link
@kaicataldo

kaicataldo Nov 6, 2019

Member

I think this example would be clearer if the constructor had statements in the body. Something like:

constructor(a) {
    this.a = a;
    return 'a';
}

This comment has been minimized.

Copy link
@g-plane

g-plane Nov 6, 2019

Author Member

Updated at later commit.

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() { }

This comment has been minimized.

Copy link
@kaicataldo

kaicataldo Nov 6, 2019

Member

I think this example would be clearer if the constructor had statements in the body. Something like:

constructor(a) {
    this.a = a;
}
}

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
52 changes: 52 additions & 0 deletions lib/rules/no-constructor-return.js
@@ -0,0 +1,52 @@
/**
* @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: "code",

messages: {
unexpected: "Unexpected return statement in constructor."
}
},

create(context) {
return {
"MethodDefinition[kind='constructor'] ReturnStatement"(node) {
const ancestors = context.getAncestors();
const ancestor = ancestors[ancestors.length - 3];

if (
ancestor &&
ancestor.type === "MethodDefinition" &&
ancestor.kind === "constructor" ||
node.argument
) {
context.report({
node,
messageId: "unexpected"
});
}
}
};
}
};
46 changes: 46 additions & 0 deletions tests/lib/rules/no-constructor-return.js
@@ -0,0 +1,46 @@
/**
* @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() } } }"
],
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": "suggestion",
"no-continue": "suggestion",
"no-control-regex": "problem",
"no-debugger": "problem",
Expand Down

0 comments on commit 3aa5fbf

Please sign in to comment.