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

feat: add new omitLastInOneLineClassBody option to the semi rule #17105

Merged
merged 6 commits into from Apr 27, 2023
Merged

feat: add new omitLastInOneLineClassBody option to the semi rule #17105

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8154a4a
feat: add new `omitLastInOneLineClassBody` option to the `semi` rule
snitin315 Apr 22, 2023
db5dd74
test: add cases for `omitLastInOneLineClassBody` option for `semi`
snitin315 Apr 22, 2023
b6dc3b9
docs: add examples for `omitLastInOneLineClassBody` option in `semi`
snitin315 Apr 22, 2023
d5808e7
docs: update options order in semi rule
snitin315 Apr 22, 2023
de430d3
fix: remove false positive for omitLastInOneLineClassBody
snitin315 Apr 23, 2023
9fe8d3a
tests: add more cases
snitin315 Apr 27, 2023
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
70 changes: 46 additions & 24 deletions docs/src/rules/semi.md
View file
Open in desktop
Expand Up @@ -81,6 +81,7 @@ String option:
Object option (when `"always"`):

* `"omitLastInOneLineBlock": true` ignores the last semicolon in a block in which its braces (and therefore the content of the block) are in the same line
* `"omitLastInOneLineClassBody": true` ignores the last semicolon in a class body in which its braces (and therefore the content of the class body) are in the same line
snitin315 marked this conversation as resolved.
Show resolved Hide resolved

Object option (when `"never"`):

Expand Down Expand Up @@ -132,6 +133,51 @@ class Foo {

:::

#### omitLastInOneLineBlock

Examples of additional **correct** code for this rule with the `"always", { "omitLastInOneLineBlock": true }` options:

::: correct

```js
/*eslint semi: ["error", "always", { "omitLastInOneLineBlock": true}] */

if (foo) { bar() }

if (foo) { bar(); baz() }

function f() { bar(); baz() }

class C {
foo() { bar(); baz() }

static { bar(); baz() }
}
```

:::

#### omitLastInOneLineClassBody

Examples of additional **correct** code for this rule with the `"always", { "omitLastInOneLineClassBody": true }` options:

::: correct

```js
/*eslint semi: ["error", "always", { "omitLastInOneLineClassBody": true}] */

export class SomeClass{
logType(){
console.log(this.type);
}
}

export class Variant1 extends SomeClass{type=1}
export class Variant2 extends SomeClass{type=2}
```

:::

### never

Examples of **incorrect** code for this rule with the `"never"` option:
Expand Down Expand Up @@ -190,30 +236,6 @@ class Foo {

:::

#### omitLastInOneLineBlock

Examples of additional **correct** code for this rule with the `"always", { "omitLastInOneLineBlock": true }` options:

::: correct

```js
/*eslint semi: ["error", "always", { "omitLastInOneLineBlock": true}] */

if (foo) { bar() }

if (foo) { bar(); baz() }

function f() { bar(); baz() }

class C {
foo() { bar(); baz() }

static { bar(); baz() }
}
```

:::

#### beforeStatementContinuationChars

Examples of additional **incorrect** code for this rule with the `"never", { "beforeStatementContinuationChars": "always" }` options:
Expand Down
8 changes: 5 additions & 3 deletions lib/rules/semi.js
View file
Open in desktop
Expand Up @@ -58,7 +58,8 @@ module.exports = {
{
type: "object",
properties: {
omitLastInOneLineBlock: { type: "boolean" }
omitLastInOneLineBlock: { type: "boolean" },
omitLastInOneLineClassBody: { type: "boolean" }
},
additionalProperties: false
}
Expand All @@ -83,6 +84,7 @@ module.exports = {
const options = context.options[1];
const never = context.options[0] === "never";
const exceptOneLine = Boolean(options && options.omitLastInOneLineBlock);
const exceptOneLineClassBody = Boolean(options && options.omitLastInOneLineClassBody);
const beforeStatementContinuationChars = options && options.beforeStatementContinuationChars || "any";
const sourceCode = context.getSourceCode();

Expand Down Expand Up @@ -321,7 +323,7 @@ module.exports = {
return false;
}

if (parent.type === "BlockStatement") {
if (parent.type === "BlockStatement" || (exceptOneLineClassBody && parent.type === "ClassBody")) {
return parent.loc.start.line === parent.loc.end.line;
}

Expand Down Expand Up @@ -353,7 +355,7 @@ module.exports = {
report(node);
}
} else {
const oneLinerBlock = (exceptOneLine && isLastInOneLinerBlock(node));
const oneLinerBlock = ((exceptOneLine || exceptOneLineClassBody) && isLastInOneLinerBlock(node));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per this condition, "omitLastInOneLineClassBody": true would also apply to blocks:

/* eslint semi: ["error", "always", { "omitLastInOneLineClassBody": true}] */

{ foo; } // false positive

It might be clearer to add a new function isLastInOneLinerClassBody.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdjermanovic Good catch. Fixed.


if (isSemi && oneLinerBlock) {
report(node, true);
Expand Down
131 changes: 131 additions & 0 deletions tests/lib/rules/semi.js
View file
Open in desktop
Expand Up @@ -111,6 +111,42 @@ ruleTester.run("semi", rule, {
{ code: "class C {\n static {\n bar(); baz(); } \n}", options: ["always", { omitLastInOneLineBlock: true }], parserOptions: { ecmaVersion: 2022 } },
{ code: "class C {\n static { bar(); baz(); \n} \n}", options: ["always", { omitLastInOneLineBlock: true }], parserOptions: { ecmaVersion: 2022 } },

// omitLastInOneLineClassBody: true
{
code: `
export class SomeClass{
logType(){
console.log(this.type);
}
}

export class Variant1 extends SomeClass{type=1}
export class Variant2 extends SomeClass{type=2}
export class Variant3 extends SomeClass{type=3}
export class Variant4 extends SomeClass{type=4}
export class Variant5 extends SomeClass{type=5}
`,
options: ["always", { omitLastInOneLineClassBody: true }],
parserOptions: { ecmaVersion: 2022, sourceType: "module" }
},
{
code: `
export class SomeClass{
logType(){
console.log(this.type);
}
}

export class Variant1 extends SomeClass{type=1;}
export class Variant2 extends SomeClass{type=2;}
export class Variant3 extends SomeClass{type=3;}
export class Variant4 extends SomeClass{type=4;}
export class Variant5 extends SomeClass{type=5;}
`,
options: ["always", { omitLastInOneLineClassBody: false }],
parserOptions: { ecmaVersion: 2022, sourceType: "module" }
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a few tests to verify that this option applies only to one-line class bodies, for example:

{
    code: "class C {\nfoo;}",
    options: ["always", { omitLastInOneLineClassBody: true }],
    parserOptions: { ecmaVersion: 2022 }
},
{
    code: "class C {foo;\n}",
    options: ["always", { omitLastInOneLineClassBody: true }],
    parserOptions: { ecmaVersion: 2022 }
},
{
    code: "class C {foo;\nbar;}",
    options: ["always", { omitLastInOneLineClassBody: true }],
    parserOptions: { ecmaVersion: 2022 }
}

and a test to verify that the false positive from #17105 (comment) has been fixed:

{
    code: "{ foo; }",
    options: ["always", { omitLastInOneLineClassBody: true }],
    parserOptions: { ecmaVersion: 2022 }
}

and a test to verify that the option applies to one-line class bodies even if the class definition is multiline:

{
    code: "class C\n{ foo }",
    options: ["always", { omitLastInOneLineClassBody: true }],
    parserOptions: { ecmaVersion: 2022 }
}


// method definitions and static blocks don't have a semicolon.
{ code: "class A { a() {} b() {} }", parserOptions: { ecmaVersion: 6 } },
{ code: "var A = class { a() {} b() {} };", parserOptions: { ecmaVersion: 6 } },
Expand Down Expand Up @@ -2309,6 +2345,101 @@ ruleTester.run("semi", rule, {
endLine: 1,
endColumn: 18
}]
},

// omitLastInOneLineClassBody
{
code: `
export class SomeClass{
logType(){
console.log(this.type);
}
}

export class Variant1 extends SomeClass{type=1}
`,
output: `
export class SomeClass{
logType(){
console.log(this.type);
}
}

export class Variant1 extends SomeClass{type=1;}
`,
options: ["always", { omitLastInOneLineClassBody: false }],
parserOptions: { ecmaVersion: 2022, sourceType: "module" },
errors: [
{
messageId: "missingSemi",
line: 8,
column: 63,
endLine: 8,
endColumn: 64
}
]
},
{
code: `
export class SomeClass{
logType(){
console.log(this.type);
}
}

export class Variant1 extends SomeClass{type=1}
`,
output: `
export class SomeClass{
logType(){
console.log(this.type);
}
}

export class Variant1 extends SomeClass{type=1;}
`,
options: ["always", { omitLastInOneLineClassBody: false, omitLastInOneLineBlock: true }],
parserOptions: { ecmaVersion: 2022, sourceType: "module" },
errors: [
{
messageId: "missingSemi",
line: 8,
column: 63,
endLine: 8,
endColumn: 64
}
]
},
{
code: `
export class SomeClass{
logType(){
console.log(this.type);
}
}

export class Variant1 extends SomeClass{type=1;}
`,
output: `
export class SomeClass{
logType(){
console.log(this.type);
}
}

export class Variant1 extends SomeClass{type=1}
`,
options: ["always", { omitLastInOneLineClassBody: true, omitLastInOneLineBlock: false }],
parserOptions: { ecmaVersion: 2022, sourceType: "module" },
errors: [
{
messageId: "extraSemi",
line: 8,
column: 63,
endLine: 8,
endColumn: 64
}
]
}
]
});