From c67c7c0504e1c5d2dc762a9218e4a92933920610 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Wed, 18 Aug 2021 10:34:12 -0700 Subject: [PATCH 1/5] Fix: Update semi for class-fields (refs #14591) --- lib/rules/semi.js | 29 +++++++++++++++++++++++++++++ tests/lib/rules/semi.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/lib/rules/semi.js b/lib/rules/semi.js index 87086e981b0..8ec650f0678 100644 --- a/lib/rules/semi.js +++ b/lib/rules/semi.js @@ -78,6 +78,8 @@ module.exports = { create(context) { const OPT_OUT_PATTERN = /^[-[(/+`]/u; // One of [(/+-` + const unsafeClassFieldNames = new Set(["get", "set", "static"]); + const unsafeClassFieldFollowers = new Set(["*", "in", "instanceof"]); const options = context.options[1]; const never = context.options[0] === "never"; const exceptOneLine = Boolean(options && options.omitLastInOneLineBlock); @@ -166,6 +168,30 @@ module.exports = { ); } + /** + * Checks if a given PropertyDefinition node followed by a semicolon + * can safely remove that semicolon. It is not to safe to remove if + * the class field name is "get", "set", or "static", or if + * followed by a generator method. + * @param {ASTNode} node The node to check. + * @returns {boolean} `true` if the node cannot have the semicolon + * removed. + */ + function maybeClassFieldAsiHazard(node) { + + if (node.type !== "PropertyDefinition") { + return false; + } + + if (unsafeClassFieldNames.has(node.key.name)) { + return true; + } + + const followingToken = sourceCode.getTokenAfter(node); + + return unsafeClassFieldFollowers.has(followingToken.value); + } + /** * Check whether a given node is on the same line with the next token. * @param {Node} node A statement node to check. @@ -233,6 +259,9 @@ module.exports = { * @returns {boolean} whether the semicolon is unnecessary. */ function canRemoveSemicolon(node) { + if (never && maybeClassFieldAsiHazard(node)) { + return false; + } if (isRedundantSemi(sourceCode.getLastToken(node))) { return true; // `;;` or `;}` } diff --git a/tests/lib/rules/semi.js b/tests/lib/rules/semi.js index 092dae15336..3f18e11bdd5 100644 --- a/tests/lib/rules/semi.js +++ b/tests/lib/rules/semi.js @@ -306,6 +306,36 @@ ruleTester.run("semi", rule, { code: "class C { foo() {}; }", // no-extra-semi reports it options: ["never"], parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { a=b;\n*foo() {} }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { get;\nfoo() {} }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { set;\nfoo() {} }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { static;\nfoo() {} }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { a=b;\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { a=b;\ninstanceof }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } } ], invalid: [ From 39c8cd12738f0286931060a9db0d24eb5e67b4b7 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Mon, 23 Aug 2021 11:04:57 -0700 Subject: [PATCH 2/5] Fixed several bugs --- lib/rules/semi.js | 32 ++++++++++-- tests/lib/rules/semi.js | 106 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 4 deletions(-) diff --git a/lib/rules/semi.js b/lib/rules/semi.js index 8ec650f0678..3d7aa2dd5e5 100644 --- a/lib/rules/semi.js +++ b/lib/rules/semi.js @@ -183,7 +183,31 @@ module.exports = { return false; } - if (unsafeClassFieldNames.has(node.key.name)) { + // computed keys are always valid + if (node.computed) { + return false; + } + + // private identifiers don't have any constraints + if (node.key.type === "PrivateIdentifier") { + return false; + } + + /* + * If there is a static property that is named "static", + * that is considered safe because a third static would + * be an obvious syntax error. + */ + if (node.static && node.key.name === "static") { + return false; + } + + /* + * Certain names are problematic unless they also have an + * initializer to distinguish between keywords and property + * names. + */ + if (unsafeClassFieldNames.has(node.key.name) && !node.value) { return true; } @@ -259,12 +283,12 @@ module.exports = { * @returns {boolean} whether the semicolon is unnecessary. */ function canRemoveSemicolon(node) { - if (never && maybeClassFieldAsiHazard(node)) { - return false; - } if (isRedundantSemi(sourceCode.getLastToken(node))) { return true; // `;;` or `;}` } + if (never && maybeClassFieldAsiHazard(node)) { + return false; + } if (isOnSameLineWithNextToken(node)) { return false; // One liner. } diff --git a/tests/lib/rules/semi.js b/tests/lib/rules/semi.js index 3f18e11bdd5..2a049ab99c6 100644 --- a/tests/lib/rules/semi.js +++ b/tests/lib/rules/semi.js @@ -1787,6 +1787,112 @@ ruleTester.run("semi", rule, { endLine: 2, endColumn: 2 }] + }, + + // class fields + { + code: "class C { [get];\nfoo\n}", + output: "class C { [get]\nfoo\n}", + options: ["never"], + parserOptions: { ecmaVersion: 2022 }, + errors: [{ + messageId: "extraSemi", + line: 1, + column: 16, + endLine: 1, + endColumn: 17 + }] + }, + { + code: "class C { [set];\nfoo\n}", + output: "class C { [set]\nfoo\n}", + options: ["never"], + parserOptions: { ecmaVersion: 2022 }, + errors: [{ + messageId: "extraSemi", + line: 1, + column: 16, + endLine: 1, + endColumn: 17 + }] + }, + { + code: "class C { #get;\nfoo\n}", + output: "class C { #get\nfoo\n}", + options: ["never"], + parserOptions: { ecmaVersion: 2022 }, + errors: [{ + messageId: "extraSemi", + line: 1, + column: 15, + endLine: 1, + endColumn: 16 + }] + }, + { + code: "class C { #set;\nfoo\n}", + output: "class C { #set\nfoo\n}", + options: ["never"], + parserOptions: { ecmaVersion: 2022 }, + errors: [{ + messageId: "extraSemi", + line: 1, + column: 15, + endLine: 1, + endColumn: 16 + }] + }, + { + code: "class C { #static;\nfoo\n}", + output: "class C { #static\nfoo\n}", + options: ["never"], + parserOptions: { ecmaVersion: 2022 }, + errors: [{ + messageId: "extraSemi", + line: 1, + column: 18, + endLine: 1, + endColumn: 19 + }] + }, + { + code: "class C { get=1;\nfoo\n}", + output: "class C { get=1\nfoo\n}", + options: ["never"], + parserOptions: { ecmaVersion: 2022 }, + errors: [{ + messageId: "extraSemi", + line: 1, + column: 16, + endLine: 1, + endColumn: 17 + }] + }, + { + code: "class C { static static;\nfoo\n}", + output: "class C { static static\nfoo\n}", + options: ["never"], + parserOptions: { ecmaVersion: 2022 }, + errors: [{ + messageId: "extraSemi", + line: 1, + column: 24, + endLine: 1, + endColumn: 25 + }] + }, + { + code: "class C { static;\n}", + output: "class C { static\n}", + options: ["never"], + parserOptions: { ecmaVersion: 2022 }, + errors: [{ + messageId: "extraSemi", + line: 1, + column: 17, + endLine: 1, + endColumn: 18 + }] } ] }); From d0d46a8b123d3f56e896558d4fd17708a6493cfa Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Wed, 25 Aug 2021 10:07:23 -0700 Subject: [PATCH 3/5] Ensure beforeStatementContinuationChars does not apply to class fields --- docs/rules/semi.md | 18 +++++++ lib/rules/semi.js | 14 +++++- tests/lib/rules/semi.js | 104 ++++++++++++++++++++-------------------- 3 files changed, 82 insertions(+), 54 deletions(-) diff --git a/docs/rules/semi.md b/docs/rules/semi.md index 049ae41cf16..158cd7f53e2 100644 --- a/docs/rules/semi.md +++ b/docs/rules/semi.md @@ -76,6 +76,8 @@ Object option (when `"never"`): * `"beforeStatementContinuationChars": "always"` requires semicolons at the end of statements if the next line starts with `[`, `(`, `/`, `+`, or `-`. * `"beforeStatementContinuationChars": "never"` disallows semicolons as the end of statements if it doesn't make ASI hazard even if the next line starts with `[`, `(`, `/`, `+`, or `-`. +**Note:** `beforeStatementContinuationChars` does not apply to class fields because class fields are not statements. + ### always Examples of **incorrect** code for this rule with the default `"always"` option: @@ -88,6 +90,10 @@ var name = "ESLint" object.method = function() { // ... } + +class Foo { + bar = 1 +} ``` Examples of **correct** code for this rule with the default `"always"` option: @@ -100,6 +106,10 @@ var name = "ESLint"; object.method = function() { // ... }; + +class Foo { + bar = 1; +} ``` ### never @@ -114,6 +124,10 @@ var name = "ESLint"; object.method = function() { // ... }; + +class Foo { + bar = 1; +} ``` Examples of **correct** code for this rule with the `"never"` option: @@ -142,6 +156,10 @@ import b from "b" ;(function() { // ... })() + +class Foo { + bar = 1 +} ``` #### omitLastInOneLineBlock diff --git a/lib/rules/semi.js b/lib/rules/semi.js index 3d7aa2dd5e5..ac488c86c30 100644 --- a/lib/rules/semi.js +++ b/lib/rules/semi.js @@ -292,7 +292,13 @@ module.exports = { if (isOnSameLineWithNextToken(node)) { return false; // One liner. } - if (beforeStatementContinuationChars === "never" && !maybeAsiHazardAfter(node)) { + + // continuation characters should not apply to class fields + if ( + node.type !== "PropertyDefinition" && + beforeStatementContinuationChars === "never" && + !maybeAsiHazardAfter(node) + ) { return true; // ASI works. This statement doesn't connect to the next. } if (!maybeAsiHazardBefore(sourceCode.getTokenAfter(node))) { @@ -332,7 +338,11 @@ module.exports = { if (never) { if (isSemi && canRemoveSemicolon(node)) { report(node, true); - } else if (!isSemi && beforeStatementContinuationChars === "always" && maybeAsiHazardBefore(sourceCode.getTokenAfter(node))) { + } else if ( + !isSemi && beforeStatementContinuationChars === "always" && + node.type !== "PropertyDefinition" && + maybeAsiHazardBefore(sourceCode.getTokenAfter(node)) + ) { report(node); } } else { diff --git a/tests/lib/rules/semi.js b/tests/lib/rules/semi.js index 2a049ab99c6..3cc82000961 100644 --- a/tests/lib/rules/semi.js +++ b/tests/lib/rules/semi.js @@ -336,6 +336,58 @@ ruleTester.run("semi", rule, { code: "class C { a=b;\ninstanceof }", options: ["never"], parserOptions: { ecmaVersion: 2022 } + }, + { + code: ` + class C { + x + [foo] + + x; + [foo] + + x = "a"; + [foo] + } + `, + options: ["never", { beforeStatementContinuationChars: "never" }], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: ` + class C { + x + [foo] + + x; + [foo] + + x = 1; + [foo] + } + `, + options: ["never", { beforeStatementContinuationChars: "always" }], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { foo\n[bar] }", + options: ["never", { beforeStatementContinuationChars: "always" }], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { foo = () => {}\n[bar] }", + options: ["never", { beforeStatementContinuationChars: "always" }], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { foo\n;[bar] }", + options: ["never", { beforeStatementContinuationChars: "never" }], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { foo = () => {}\n;[bar] }", + options: ["never", { beforeStatementContinuationChars: "never" }], + parserOptions: { ecmaVersion: 2022 } } ], invalid: [ @@ -1736,58 +1788,6 @@ ruleTester.run("semi", rule, { endColumn: 1 }] }, - { - code: "class C { foo\n[bar] }", - output: "class C { foo;\n[bar] }", - options: ["never", { beforeStatementContinuationChars: "always" }], - parserOptions: { ecmaVersion: 2022 }, - errors: [{ - messageId: "missingSemi", - line: 1, - column: 14, - endLine: 2, - endColumn: 1 - }] - }, - { - code: "class C { foo\n;[bar] }", - output: "class C { foo\n[bar] }", - options: ["never", { beforeStatementContinuationChars: "never" }], - parserOptions: { ecmaVersion: 2022 }, - errors: [{ - messageId: "extraSemi", - line: 2, - column: 1, - endLine: 2, - endColumn: 2 - }] - }, - { - code: "class C { foo = () => {}\n[bar] }", - output: "class C { foo = () => {};\n[bar] }", - options: ["never", { beforeStatementContinuationChars: "always" }], - parserOptions: { ecmaVersion: 2022 }, - errors: [{ - messageId: "missingSemi", - line: 1, - column: 25, - endLine: 2, - endColumn: 1 - }] - }, - { - code: "class C { foo = () => {}\n;[bar] }", - output: "class C { foo = () => {}\n[bar] }", - options: ["never", { beforeStatementContinuationChars: "never" }], - parserOptions: { ecmaVersion: 2022 }, - errors: [{ - messageId: "extraSemi", - line: 2, - column: 1, - endLine: 2, - endColumn: 2 - }] - }, // class fields { From 33ee4575d6d1c52191737beb794ce31524d776c3 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Tue, 31 Aug 2021 13:59:25 -0700 Subject: [PATCH 4/5] Fix semi rule bugs for class fields --- lib/rules/semi.js | 57 ++++++++++++++++++++++------------------- tests/lib/rules/semi.js | 20 +++++++++++++++ 2 files changed, 50 insertions(+), 27 deletions(-) diff --git a/lib/rules/semi.js b/lib/rules/semi.js index ac488c86c30..0b77d16c837 100644 --- a/lib/rules/semi.js +++ b/lib/rules/semi.js @@ -183,32 +183,38 @@ module.exports = { return false; } - // computed keys are always valid - if (node.computed) { - return false; - } - - // private identifiers don't have any constraints - if (node.key.type === "PrivateIdentifier") { - return false; - } - /* - * If there is a static property that is named "static", - * that is considered safe because a third static would - * be an obvious syntax error. - */ - if (node.static && node.key.name === "static") { - return false; - } - - /* - * Certain names are problematic unless they also have an - * initializer to distinguish between keywords and property + * Certain names are problematic unless they also have a + * a way to distinguish between keywords and property * names. */ - if (unsafeClassFieldNames.has(node.key.name) && !node.value) { - return true; + if (unsafeClassFieldNames.has(node.key.name)) { + + // computed keys are always valid + if (node.computed) { + return false; + } + + // private identifiers are always valid + if (node.key.type === "PrivateIdentifier") { + return false; + } + + // generally, fields without initializers are valid + if (!node.value) { + + /* + * Special case: If the field name is `static`, + * it is only valid if the field is marked as static, + * so "static static" is okay but "static" is not. + */ + if (node.static && node.key.name === "static") { + return false; + } + + return true; + } + } const followingToken = sourceCode.getTokenAfter(node); @@ -254,9 +260,6 @@ module.exports = { if (isEndOfArrowBlock(sourceCode.getLastToken(node, 1))) { return false; } - if (t === "PropertyDefinition") { - return Boolean(t.value); - } return true; } @@ -286,7 +289,7 @@ module.exports = { if (isRedundantSemi(sourceCode.getLastToken(node))) { return true; // `;;` or `;}` } - if (never && maybeClassFieldAsiHazard(node)) { + if (maybeClassFieldAsiHazard(node)) { return false; } if (isOnSameLineWithNextToken(node)) { diff --git a/tests/lib/rules/semi.js b/tests/lib/rules/semi.js index 3cc82000961..71999c6b310 100644 --- a/tests/lib/rules/semi.js +++ b/tests/lib/rules/semi.js @@ -388,6 +388,26 @@ ruleTester.run("semi", rule, { code: "class C { foo = () => {}\n;[bar] }", options: ["never", { beforeStatementContinuationChars: "never" }], parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { [foo] = bar;\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { #foo = bar;\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { static static = bar;\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { [foo];\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } } ], invalid: [ From 7628e44fc8d635b76a26e0e887bd86a16774ab26 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Fri, 3 Sep 2021 10:12:45 -0700 Subject: [PATCH 5/5] Fix edge cases --- lib/rules/semi.js | 45 ++++++++++++++++++----------------------- tests/lib/rules/semi.js | 25 +++++++++++++++++++++++ 2 files changed, 45 insertions(+), 25 deletions(-) diff --git a/lib/rules/semi.js b/lib/rules/semi.js index 0b77d16c837..425304e0f5e 100644 --- a/lib/rules/semi.js +++ b/lib/rules/semi.js @@ -183,38 +183,33 @@ module.exports = { return false; } + /* + * Computed property names and non-identifiers are always safe + * as they can be distinguished from keywords easily. + */ + const needsNameCheck = !node.computed && node.key.type === "Identifier"; + /* * Certain names are problematic unless they also have a * a way to distinguish between keywords and property * names. */ - if (unsafeClassFieldNames.has(node.key.name)) { - - // computed keys are always valid - if (node.computed) { - return false; - } - - // private identifiers are always valid - if (node.key.type === "PrivateIdentifier") { - return false; - } - - // generally, fields without initializers are valid - if (!node.value) { - - /* - * Special case: If the field name is `static`, - * it is only valid if the field is marked as static, - * so "static static" is okay but "static" is not. - */ - if (node.static && node.key.name === "static") { - return false; - } - + if (needsNameCheck && unsafeClassFieldNames.has(node.key.name)) { + + /* + * Special case: If the field name is `static`, + * it is only valid if the field is marked as static, + * so "static static" is okay but "static" is not. + */ + const isStaticStatic = node.static && node.key.name === "static"; + + /* + * For other unsafe names, we only care if there is no + * initializer. No initializer = hazard. + */ + if (!isStaticStatic && !node.value) { return true; } - } const followingToken = sourceCode.getTokenAfter(node); diff --git a/tests/lib/rules/semi.js b/tests/lib/rules/semi.js index 71999c6b310..ba4fb307eef 100644 --- a/tests/lib/rules/semi.js +++ b/tests/lib/rules/semi.js @@ -408,6 +408,31 @@ ruleTester.run("semi", rule, { code: "class C { [foo];\nin }", options: ["never"], parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { [get];\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { [get] = 5;\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { #get;\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { #set = 5;\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } + }, + { + code: "class C { static static;\nin }", + options: ["never"], + parserOptions: { ecmaVersion: 2022 } } ], invalid: [