Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nzakas committed Sep 5, 2021
1 parent 8c61f5a commit f966fe6
Show file tree
Hide file tree
Showing 3 changed files with 293 additions and 33 deletions.
18 changes: 18 additions & 0 deletions docs/rules/semi.md
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -100,6 +106,10 @@ var name = "ESLint";
object.method = function() {
// ...
};

class Foo {
bar = 1;
}
```

### never
Expand All @@ -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:
Expand Down Expand Up @@ -142,6 +156,10 @@ import b from "b"
;(function() {
// ...
})()

class Foo {
bar = 1
}
```

#### omitLastInOneLineBlock
Expand Down
71 changes: 66 additions & 5 deletions lib/rules/semi.js
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -203,9 +254,6 @@ module.exports = {
if (isEndOfArrowBlock(sourceCode.getLastToken(node, 1))) {
return false;
}
if (t === "PropertyDefinition") {
return Boolean(t.value);
}

return true;
}
Expand Down Expand Up @@ -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))) {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit f966fe6

Please sign in to comment.