From f966fe6286b6f668812f5155b79d4ee2a8b584b3 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Sun, 5 Sep 2021 13:09:03 -0700 Subject: [PATCH] Fix: Update semi for class-fields (refs #14857) (#14945) * Fix: Update semi for class-fields (refs #14591) * Fixed several bugs * Ensure beforeStatementContinuationChars does not apply to class fields * Fix semi rule bugs for class fields * Fix edge cases --- docs/rules/semi.md | 18 +++ lib/rules/semi.js | 71 +++++++++++- tests/lib/rules/semi.js | 237 +++++++++++++++++++++++++++++++++++----- 3 files changed, 293 insertions(+), 33 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 d79b22bc876..4124a8c508c 100644 --- a/lib/rules/semi.js +++ b/lib/rules/semi.js @@ -77,6 +77,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); @@ -165,6 +167,55 @@ 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; + } + + /* + * 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 (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); + + 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. @@ -203,9 +254,6 @@ module.exports = { if (isEndOfArrowBlock(sourceCode.getLastToken(node, 1))) { return false; } - if (t === "PropertyDefinition") { - return Boolean(t.value); - } return true; } @@ -235,10 +283,19 @@ module.exports = { if (isRedundantSemi(sourceCode.getLastToken(node))) { return true; // `;;` or `;}` } + if (maybeClassFieldAsiHazard(node)) { + return false; + } 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))) { @@ -278,7 +335,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 6c76ad203fa..bc00079d8a0 100644 --- a/tests/lib/rules/semi.js +++ b/tests/lib/rules/semi.js @@ -310,6 +310,133 @@ 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 } + }, + { + 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 } + }, + { + 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 } + }, + { + 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: [ @@ -1710,56 +1837,110 @@ ruleTester.run("semi", rule, { endColumn: 1 }] }, + + // class fields { - code: "class C { foo\n[bar] }", - output: "class C { foo;\n[bar] }", - options: ["never", { beforeStatementContinuationChars: "always" }], + code: "class C { [get];\nfoo\n}", + output: "class C { [get]\nfoo\n}", + options: ["never"], parserOptions: { ecmaVersion: 2022 }, errors: [{ - messageId: "missingSemi", + messageId: "extraSemi", line: 1, - column: 14, - endLine: 2, - endColumn: 1 + column: 16, + endLine: 1, + endColumn: 17 }] }, { - code: "class C { foo\n;[bar] }", - output: "class C { foo\n[bar] }", - options: ["never", { beforeStatementContinuationChars: "never" }], + code: "class C { [set];\nfoo\n}", + output: "class C { [set]\nfoo\n}", + options: ["never"], parserOptions: { ecmaVersion: 2022 }, errors: [{ messageId: "extraSemi", - line: 2, - column: 1, - endLine: 2, - endColumn: 2 + line: 1, + column: 16, + endLine: 1, + endColumn: 17 }] }, { - code: "class C { foo = () => {}\n[bar] }", - output: "class C { foo = () => {};\n[bar] }", - options: ["never", { beforeStatementContinuationChars: "always" }], + code: "class C { #get;\nfoo\n}", + output: "class C { #get\nfoo\n}", + options: ["never"], parserOptions: { ecmaVersion: 2022 }, errors: [{ - messageId: "missingSemi", + messageId: "extraSemi", line: 1, - column: 25, - endLine: 2, - endColumn: 1 + column: 15, + endLine: 1, + endColumn: 16 }] }, { - code: "class C { foo = () => {}\n;[bar] }", - output: "class C { foo = () => {}\n[bar] }", - options: ["never", { beforeStatementContinuationChars: "never" }], + code: "class C { #set;\nfoo\n}", + output: "class C { #set\nfoo\n}", + options: ["never"], parserOptions: { ecmaVersion: 2022 }, errors: [{ messageId: "extraSemi", - line: 2, - column: 1, - endLine: 2, - endColumn: 2 + 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 }] } ]