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.

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

Fix: semicolon-less style in lines-between-class-members (refs #14857) #15045

Merged
merged 1 commit into from Sep 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
21 changes: 21 additions & 0 deletions docs/rules/lines-between-class-members.md
Expand Up @@ -9,6 +9,7 @@ Examples of **incorrect** code for this rule:
```js
/* eslint lines-between-class-members: ["error", "always"]*/
class MyClass {
x;
foo() {
//...
}
Expand All @@ -23,6 +24,8 @@ Examples of **correct** code for this rule:
```js
/* eslint lines-between-class-members: ["error", "always"]*/
class MyClass {
x;

foo() {
//...
}
Expand All @@ -33,6 +36,17 @@ class MyClass {
}
```

Examples of additional **correct** code for this rule:

```js
/* eslint lines-between-class-members: ["error", "always"]*/
class MyClass {
x = 1

;in = 2
}
```

### Options

This rule has a string option and an object option.
Expand All @@ -52,12 +66,15 @@ Examples of **incorrect** code for this rule with the string option:
```js
/* eslint lines-between-class-members: ["error", "always"]*/
class Foo{
x;
bar(){}
baz(){}
}

/* eslint lines-between-class-members: ["error", "never"]*/
class Foo{
x;

bar(){}

baz(){}
Expand All @@ -69,13 +86,16 @@ Examples of **correct** code for this rule with the string option:
```js
/* eslint lines-between-class-members: ["error", "always"]*/
class Foo{
x;

bar(){}

baz(){}
}

/* eslint lines-between-class-members: ["error", "never"]*/
class Foo{
x;
bar(){}
baz(){}
}
Expand All @@ -86,6 +106,7 @@ Examples of **correct** code for this rule with the object option:
```js
/* eslint lines-between-class-members: ["error", "always", { "exceptAfterSingleLine": true }]*/
class Foo{
x; // single line class member
bar(){} // single line class member
baz(){
// multi line class member
Expand Down
52 changes: 50 additions & 2 deletions lib/rules/lines-between-class-members.js
Expand Up @@ -4,6 +4,10 @@
*/
"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -52,6 +56,51 @@ module.exports = {

const sourceCode = context.getSourceCode();

/**
* Gets a pair of tokens that should be used to check lines between two class member nodes.
*
* In most cases, this returns the very last token of the current node and
* the very first token of the next node.
* For example:
*
* class C {
* x = 1; // curLast: `;` nextFirst: `in`
* in = 2
* }
*
* There is only one exception. If the given node ends with a semicolon, and it looks like
* a semicolon-less style's semicolon - one that is not on the same line as the preceding
* token, but is on the line where the next class member starts - this returns the preceding
* token and the semicolon as boundary tokens.
* For example:
*
* class C {
* x = 1 // curLast: `1` nextFirst: `;`
* ;in = 2
* }
* When determining the desired layout of the code, we should treat this semicolon as
* a part of the next class member node instead of the one it technically belongs to.
* @param {ASTNode} curNode Current class member node.
* @param {ASTNode} nextNode Next class member node.
* @returns {Token} The actual last token of `node`.
* @private
*/
function getBoundaryTokens(curNode, nextNode) {
const lastToken = sourceCode.getLastToken(curNode);
const prevToken = sourceCode.getTokenBefore(lastToken);
const nextToken = sourceCode.getFirstToken(nextNode); // skip possible lone `;` between nodes

const isSemicolonLessStyle = (
astUtils.isSemicolonToken(lastToken) &&
!astUtils.isTokenOnSameLine(prevToken, lastToken) &&
astUtils.isTokenOnSameLine(lastToken, nextToken)
);

return isSemicolonLessStyle
? { curLast: prevToken, nextFirst: lastToken }
: { curLast: lastToken, nextFirst: nextToken };
}

/**
* Return the last token among the consecutive tokens that have no exceed max line difference in between, before the first token in the next member.
* @param {Token} prevLastToken The last token in the previous member node.
Expand Down Expand Up @@ -100,8 +149,7 @@ module.exports = {

for (let i = 0; i < body.length - 1; i++) {
const curFirst = sourceCode.getFirstToken(body[i]);
const curLast = sourceCode.getLastToken(body[i]);
const nextFirst = sourceCode.getFirstToken(body[i + 1]);
const { curLast, nextFirst } = getBoundaryTokens(body[i], body[i + 1]);
const isMulti = !astUtils.isTokenOnSameLine(curFirst, curLast);
const skip = !isMulti && options[1].exceptAfterSingleLine;
const beforePadding = findLastConsecutiveTokenAfter(curLast, nextFirst, 1);
Expand Down
45 changes: 44 additions & 1 deletion tests/lib/rules/lines-between-class-members.js
Expand Up @@ -62,7 +62,12 @@ ruleTester.run("lines-between-class-members", rule, {

{ code: "class foo{ bar(){}\nbaz(){}}", options: ["always", { exceptAfterSingleLine: true }] },
{ code: "class foo{ bar(){\n}\n\nbaz(){}}", options: ["always", { exceptAfterSingleLine: true }] },
{ code: "class foo{\naaa;\n#bbb;\nccc(){\n}\n\n#ddd(){\n}\n}", options: ["always", { exceptAfterSingleLine: true }] }
{ code: "class foo{\naaa;\n#bbb;\nccc(){\n}\n\n#ddd(){\n}\n}", options: ["always", { exceptAfterSingleLine: true }] },

// semicolon-less style (semicolons are at the beginning of lines)
{ code: "class C { foo\n\n;bar }", options: ["always"] },
{ code: "class C { foo\n;bar }", options: ["always", { exceptAfterSingleLine: true }] },
{ code: "class C { foo\n;bar }", options: ["never"] }
],
invalid: [
{
Expand Down Expand Up @@ -165,6 +170,44 @@ ruleTester.run("lines-between-class-members", rule, {
output: "class C {\nfield1 = () => {\n}\n\nfield2\nfield3\n}",
options: ["always", { exceptAfterSingleLine: true }],
errors: [alwaysError]
},
{
code: "class C { foo;bar }",
output: "class C { foo;\nbar }",
options: ["always"],
errors: [alwaysError]
},
{
code: "class C { foo;\nbar; }",
output: "class C { foo;\n\nbar; }",
options: ["always"],
errors: [alwaysError]
},
{
code: "class C { foo;\n;bar }",
output: "class C { foo;\n\n;bar }",
options: ["always"],
errors: [alwaysError]
},

// semicolon-less style (semicolons are at the beginning of lines)
{
code: "class C { foo\n;bar }",
output: "class C { foo\n\n;bar }",
options: ["always"],
errors: [alwaysError]
},
{
code: "class C { foo\n\n;bar }",
output: "class C { foo\n;bar }",
options: ["never"],
errors: [neverError]
},
{
code: "class C { foo\n;;bar }",
output: "class C { foo\n\n;;bar }",
options: ["always"],
errors: [alwaysError]
}
]
});