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

Update: fix no-unreachable logic for class fields (refs #14857) #14920

Merged
merged 1 commit into from Aug 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
68 changes: 67 additions & 1 deletion docs/rules/no-unreachable.md
Expand Up @@ -10,9 +10,21 @@ function fn() {
}
```

Another kind of mistake is defining instance fields in a subclass whose constructor doesn't call `super()`. Instance fields of a subclass are only added to the instance after `super()`. If there are no `super()` calls, their definitions are never applied and therefore are unreachable code.

```js
class C extends B {
#x; // this will never be added to instances

constructor() {
return {};
}
}
```

## Rule Details

This rule disallows unreachable code after `return`, `throw`, `continue`, and `break` statements.
This rule disallows unreachable code after `return`, `throw`, `continue`, and `break` statements. This rule also flags definitions of instance fields in subclasses whose constructors don't have `super()` calls.

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

Expand Down Expand Up @@ -73,3 +85,57 @@ switch (foo) {
var x;
}
```

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

```js
/*eslint no-unreachable: "error"*/

class C extends B {
#x; // unreachable
#y = 1; // unreachable
a; // unreachable
b = 1; // unreachable

constructor() {
return {};
}
}
```

Examples of additional **correct** code for this rule:

```js
/*eslint no-unreachable: "error"*/

class D extends B {
#x;
#y = 1;
a;
b = 1;

constructor() {
super();
}
}

class E extends B {
#x;
#y = 1;
a;
b = 1;

// implicit constructor always calls `super()`
}

class F extends B {
static #x;
static #y = 1;
static a;
static b = 1;

constructor() {
return {};
}
}
```
50 changes: 26 additions & 24 deletions lib/rules/no-unreachable.js
Expand Up @@ -9,9 +9,8 @@
//------------------------------------------------------------------------------

/**
* @typedef {Object} ClassInfo
* @property {ClassInfo | null} upper The class info that encloses this class.
* @property {boolean} hasConstructor The flag about having user-defined constructor.
* @typedef {Object} ConstructorInfo
* @property {ConstructorInfo | null} upper Info about the constructor that encloses this constructor.
* @property {boolean} hasSuperCall The flag about having `super()` expressions.
*/

Expand Down Expand Up @@ -127,8 +126,8 @@ module.exports = {
create(context) {
let currentCodePath = null;

/** @type {ClassInfo | null} */
let classInfo = null;
/** @type {ConstructorInfo | null} */
let constructorInfo = null;

/** @type {ConsecutiveRange} */
const range = new ConsecutiveRange(context.getSourceCode());
Expand Down Expand Up @@ -225,36 +224,39 @@ module.exports = {
reportIfUnreachable();
},

// Address class fields.
"ClassDeclaration, ClassExpression"() {
classInfo = {
upper: classInfo,
hasConstructor: false,
/*
* Instance fields defined in a subclass are never created if the constructor of the subclass
* doesn't call `super()`, so their definitions are unreachable code.
*/
"MethodDefinition[kind='constructor']"() {
constructorInfo = {
upper: constructorInfo,
hasSuperCall: false
};
},
"ClassDeclaration, ClassExpression:exit"(node) {
const { hasConstructor, hasSuperCall } = classInfo;
const hasExtends = Boolean(node.superClass);
"MethodDefinition[kind='constructor']:exit"(node) {
const { hasSuperCall } = constructorInfo;

constructorInfo = constructorInfo.upper;

// skip typescript constructors without the body
if (!node.value.body) {
return;
}

classInfo = classInfo.upper;
const classDefinition = node.parent.parent;

if (hasConstructor && hasExtends && !hasSuperCall) {
for (const element of node.body.body) {
if (element.type === "PropertyDefinition") {
if (classDefinition.superClass && !hasSuperCall) {
for (const element of classDefinition.body.body) {
if (element.type === "PropertyDefinition" && !element.static) {
reportIfUnreachable(element);
}
}
}
},
"MethodDefinition[kind='constructor']"() {
if (classInfo) {
classInfo.hasConstructor = true;
}
},
"CallExpression > Super.callee"() {
if (classInfo) {
classInfo.hasSuperCall = true;
if (constructorInfo) {
constructorInfo.hasSuperCall = true;
}
}
};
Expand Down
34 changes: 34 additions & 0 deletions tests/lib/rules/no-unreachable.js
Expand Up @@ -81,6 +81,10 @@ ruleTester.run("no-unreachable", rule, {
{
code: "class C extends B { foo = reachable; constructor() { super(); } }",
parserOptions: { ecmaVersion: 2022 }
},
{
code: "class C extends B { static foo = reachable; constructor() {} }",
parserOptions: { ecmaVersion: 2022 }
}
],
invalid: [
Expand Down Expand Up @@ -349,6 +353,36 @@ ruleTester.run("no-unreachable", rule, {
{ messageId: "unreachableCode", column: 21, endColumn: 25 },
{ messageId: "unreachableCode", column: 43, endColumn: 47 }
]
},
{
code: "(class extends B { foo; constructor() {} bar; })",
parserOptions: { ecmaVersion: 2022 },
errors: [
{ messageId: "unreachableCode", column: 20, endColumn: 24 },
{ messageId: "unreachableCode", column: 42, endColumn: 46 }
]
},
{
code: "class B extends A { x; constructor() { class C extends D { [super().x]; constructor() {} } } }",
parserOptions: { ecmaVersion: 2022 },
errors: [
{ messageId: "unreachableCode", column: 60, endColumn: 72 }
]
},
{
code: "class B extends A { x; constructor() { class C extends super().x { y; constructor() {} } } }",
parserOptions: { ecmaVersion: 2022 },
errors: [
{ messageId: "unreachableCode", column: 68, endColumn: 70 }
]
},
{
code: "class B extends A { x; static y; z; static q; constructor() {} }",
parserOptions: { ecmaVersion: 2022 },
errors: [
{ messageId: "unreachableCode", column: 21, endColumn: 23 },
{ messageId: "unreachableCode", column: 34, endColumn: 36 }
]
}
]
});