Fix: Update semi for class-fields (refs #14857) (#14945)

nzakas · web-flow · commit f966fe6286b6 · 2021-09-05T22:09:03.000+02:00
* 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
diff --git 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
@@ -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
