Skip to content
Permalink
Browse files
Update: fix no-unreachable logic for class fields (refs #14857) (#14920)
  • Loading branch information
mdjermanovic committed Aug 20, 2021
1 parent ee1b54f commit 26b0cd924e79a0ab2374c0cd813e92055f9fff7b
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 25 deletions.
@@ -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:

@@ -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 {};
}
}
```
@@ -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.
*/

@@ -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());
@@ -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;
}
}
};
@@ -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: [
@@ -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 }
]
}
]
});

0 comments on commit 26b0cd9

Please sign in to comment.