Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 5 commits into from Sep 5, 2021
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c67c7c0
Fix: Update semi for class-fields (refs #14591)
nzakas Aug 18, 2021
39c8cd1
Fixed several bugs
nzakas Aug 23, 2021
d0d46a8
Ensure beforeStatementContinuationChars does not apply to class fields
nzakas Aug 25, 2021
33ee457
Fix semi rule bugs for class fields
nzakas Aug 31, 2021
7628e44
Fix edge cases
nzakas Sep 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions docs/rules/semi.md
View file
Open in desktop
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
67 changes: 65 additions & 2 deletions lib/rules/semi.js
View file
Open in desktop
Expand Up @@ -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);
Expand Down Expand Up @@ -166,6 +168,54 @@ 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 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;
}
nzakas marked this conversation as resolved.
Show resolved Hide resolved

/*
* 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;
}
nzakas marked this conversation as resolved.
Show resolved Hide resolved

const followingToken = sourceCode.getTokenAfter(node);

return unsafeClassFieldFollowers.has(followingToken.value);
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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 @@ -236,10 +286,19 @@ module.exports = {
if (isRedundantSemi(sourceCode.getLastToken(node))) {
return true; // `;;` or `;}`
}
if (never && maybeClassFieldAsiHazard(node)) {
nzakas marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -279,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 {
Expand Down
192 changes: 164 additions & 28 deletions tests/lib/rules/semi.js
View file
Open in desktop
Expand Up @@ -306,6 +306,88 @@ 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 }
}
],
invalid: [
Expand Down Expand Up @@ -1706,56 +1788,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
}]
}
]
Expand Down