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

eslint · Aug 20, 2021 · 26b0cd9 · 26b0cd9
1 parent ee1b54f
commit 26b0cd9
Show file tree

Hide file tree

Showing 3 changed files with 127 additions and 25 deletions.
diff --git a/docs/rules/no-unreachable.md b/docs/rules/no-unreachable.md
@@ -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 {};
+    }
+}
+```
diff --git a/lib/rules/no-unreachable.js b/lib/rules/no-unreachable.js
@@ -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;
                 }
             }
         };

diff --git a/tests/lib/rules/no-unreachable.js b/tests/lib/rules/no-unreachable.js
@@ -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 }
+            ]
         }
     ]
 });