From 26b0cd924e79a0ab2374c0cd813e92055f9fff7b Mon Sep 17 00:00:00 2001 From: Milos Djermanovic Date: Fri, 20 Aug 2021 13:55:17 +0200 Subject: [PATCH] Update: fix no-unreachable logic for class fields (refs #14857) (#14920) --- docs/rules/no-unreachable.md | 68 ++++++++++++++++++++++++++++++- lib/rules/no-unreachable.js | 50 ++++++++++++----------- tests/lib/rules/no-unreachable.js | 34 ++++++++++++++++ 3 files changed, 127 insertions(+), 25 deletions(-) diff --git a/docs/rules/no-unreachable.md b/docs/rules/no-unreachable.md index 7bd16d383c9..2fe8a6916e3 100644 --- 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 index 5a2c52ad54d..586be02e3cc 100644 --- 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 index 01589a75cde..03816d7192f 100644 --- 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 } + ] } ] });