Skip to content

Commit 5c25a26

Browse files
yeonjuanKai Cataldo
yeonjuan
authored and
Kai Cataldo
committed
Update: autofix bug in lines-between-class-members (fixes #12391) (#12632)
* Fix: delete comment bug in lines-between-class-members (fixes #12391) * edit comments * keep padded comments * fix to check for semicolon token. * check token in padding, check end loc of before padding. * add jsdoc param type, fix typo * fix typo in test
1 parent 4b3cc5c commit 5c25a26

2 files changed

Lines changed: 110 additions & 53 deletions

File tree

‎lib/rules/lines-between-class-members.js‎

Lines changed: 42 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -54,62 +54,45 @@ module.exports = {
5454
const sourceCode = context.getSourceCode();
5555

5656
/**
57-
* Checks if there is padding between two tokens
58-
* @param {Token} first The first token
59-
* @param {Token} second The second token
60-
* @returns {boolean} True if there is at least a line between the tokens
57+
* 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.
58+
* @param {Token} prevLastToken The last token in the previous member node.
59+
* @param {Token} nextFirstToken The first token in the next member node.
60+
* @param {number} maxLine The maximum number of allowed line difference between consecutive tokens.
61+
* @returns {Token} The last token among the consecutive tokens.
6162
*/
62-
function isPaddingBetweenTokens(first, second) {
63-
const comments = sourceCode.getCommentsBefore(second);
64-
const len = comments.length;
63+
function findLastConsecutiveTokenAfter(prevLastToken, nextFirstToken, maxLine) {
64+
const after = sourceCode.getTokenAfter(prevLastToken, { includeComments: true });
6565

66-
// If there is no comments
67-
if (len === 0) {
68-
const linesBetweenFstAndSnd = second.loc.start.line - first.loc.end.line - 1;
69-
70-
return linesBetweenFstAndSnd >= 1;
71-
}
72-
73-
74-
// If there are comments
75-
let sumOfCommentLines = 0; // the numbers of lines of comments
76-
let prevCommentLineNum = -1; // line number of the end of the previous comment
77-
78-
for (let i = 0; i < len; i++) {
79-
const commentLinesOfThisComment = comments[i].loc.end.line - comments[i].loc.start.line + 1;
80-
81-
sumOfCommentLines += commentLinesOfThisComment;
82-
83-
/*
84-
* If this comment and the previous comment are in the same line,
85-
* the count of comment lines is duplicated. So decrement sumOfCommentLines.
86-
*/
87-
if (prevCommentLineNum === comments[i].loc.start.line) {
88-
sumOfCommentLines -= 1;
89-
}
90-
91-
prevCommentLineNum = comments[i].loc.end.line;
66+
if (after !== nextFirstToken && after.loc.start.line - prevLastToken.loc.end.line <= maxLine) {
67+
return findLastConsecutiveTokenAfter(after, nextFirstToken, maxLine);
9268
}
69+
return prevLastToken;
70+
}
9371

94-
/*
95-
* If the first block and the first comment are in the same line,
96-
* the count of comment lines is duplicated. So decrement sumOfCommentLines.
97-
*/
98-
if (first.loc.end.line === comments[0].loc.start.line) {
99-
sumOfCommentLines -= 1;
100-
}
72+
/**
73+
* Return the first token among the consecutive tokens that have no exceed max line difference in between, after the last token in the previous member.
74+
* @param {Token} nextFirstToken The first token in the next member node.
75+
* @param {Token} prevLastToken The last token in the previous member node.
76+
* @param {number} maxLine The maximum number of allowed line difference between consecutive tokens.
77+
* @returns {Token} The first token among the consecutive tokens.
78+
*/
79+
function findFirstConsecutiveTokenBefore(nextFirstToken, prevLastToken, maxLine) {
80+
const before = sourceCode.getTokenBefore(nextFirstToken, { includeComments: true });
10181

102-
/*
103-
* If the last comment and the second block are in the same line,
104-
* the count of comment lines is duplicated. So decrement sumOfCommentLines.
105-
*/
106-
if (comments[len - 1].loc.end.line === second.loc.start.line) {
107-
sumOfCommentLines -= 1;
82+
if (before !== prevLastToken && nextFirstToken.loc.start.line - before.loc.end.line <= maxLine) {
83+
return findFirstConsecutiveTokenBefore(before, prevLastToken, maxLine);
10884
}
85+
return nextFirstToken;
86+
}
10987

110-
const linesBetweenFstAndSnd = second.loc.start.line - first.loc.end.line - 1;
111-
112-
return linesBetweenFstAndSnd - sumOfCommentLines >= 1;
88+
/**
89+
* Checks if there is a token or comment between two tokens.
90+
* @param {Token} before The token before.
91+
* @param {Token} after The token after.
92+
* @returns {boolean} True if there is a token or comment between two tokens.
93+
*/
94+
function hasTokenOrCommentBetween(before, after) {
95+
return sourceCode.getTokensBetween(before, after, { includeComments: true }).length !== 0;
11396
}
11497

11598
return {
@@ -120,20 +103,26 @@ module.exports = {
120103
const curFirst = sourceCode.getFirstToken(body[i]);
121104
const curLast = sourceCode.getLastToken(body[i]);
122105
const nextFirst = sourceCode.getFirstToken(body[i + 1]);
123-
const isPadded = isPaddingBetweenTokens(curLast, nextFirst);
124106
const isMulti = !astUtils.isTokenOnSameLine(curFirst, curLast);
125107
const skip = !isMulti && options[1].exceptAfterSingleLine;
126-
108+
const beforePadding = findLastConsecutiveTokenAfter(curLast, nextFirst, 1);
109+
const afterPadding = findFirstConsecutiveTokenBefore(nextFirst, curLast, 1);
110+
const isPadded = afterPadding.loc.start.line - beforePadding.loc.end.line > 1;
111+
const hasTokenInPadding = hasTokenOrCommentBetween(beforePadding, afterPadding);
112+
const curLineLastToken = findLastConsecutiveTokenAfter(curLast, nextFirst, 0);
127113

128114
if ((options[0] === "always" && !skip && !isPadded) ||
129115
(options[0] === "never" && isPadded)) {
130116
context.report({
131117
node: body[i + 1],
132118
messageId: isPadded ? "never" : "always",
133119
fix(fixer) {
120+
if (hasTokenInPadding) {
121+
return null;
122+
}
134123
return isPadded
135-
? fixer.replaceTextRange([curLast.range[1], nextFirst.range[0]], "\n")
136-
: fixer.insertTextAfter(curLast, "\n");
124+
? fixer.replaceTextRange([beforePadding.range[1], afterPadding.range[0]], "\n")
125+
: fixer.insertTextAfter(curLineLastToken, "\n");
137126
}
138127
});
139128
}

‎tests/lib/rules/lines-between-class-members.js‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ ruleTester.run("lines-between-class-members", rule, {
4040
"class A{ foo() {}\n/* a */ /* b */\n\nbar() {}}",
4141
"class A{ foo() {}/* a */ \n\n /* b */bar() {}}",
4242

43+
"class A {\nfoo() {}\n/* comment */;\n;\n\nbar() {}\n}",
44+
"class A {\nfoo() {}\n// comment\n\n;\n;\nbar() {}\n}",
45+
4346
"class foo{ bar(){}\n\n;;baz(){}}",
4447
"class foo{ bar(){};\n\nbaz(){}}",
4548

@@ -73,6 +76,71 @@ ruleTester.run("lines-between-class-members", rule, {
7376
output: "class foo{ bar(){\n}\n\nbaz(){}}",
7477
options: ["always", { exceptAfterSingleLine: true }],
7578
errors: [alwaysError]
79+
}, {
80+
code: "class foo{ bar(){\n}\n/* comment */\nbaz(){}}",
81+
output: "class foo{ bar(){\n}\n\n/* comment */\nbaz(){}}",
82+
options: ["always", { exceptAfterSingleLine: true }],
83+
errors: [alwaysError]
84+
}, {
85+
code: "class foo{ bar(){}\n\n// comment\nbaz(){}}",
86+
output: "class foo{ bar(){}\n// comment\nbaz(){}}",
87+
options: ["never"],
88+
errors: [neverError]
89+
}, {
90+
code: "class foo{ bar(){}\n\n/* comment */\nbaz(){}}",
91+
output: "class foo{ bar(){}\n/* comment */\nbaz(){}}",
92+
options: ["never"],
93+
errors: [neverError]
94+
}, {
95+
code: "class foo{ bar(){}\n/* comment-1 */\n\n/* comment-2 */\nbaz(){}}",
96+
output: "class foo{ bar(){}\n/* comment-1 */\n/* comment-2 */\nbaz(){}}",
97+
options: ["never"],
98+
errors: [neverError]
99+
}, {
100+
code: "class foo{ bar(){}\n\n/* comment */\n\nbaz(){}}",
101+
output: null,
102+
options: ["never"],
103+
errors: [neverError]
104+
}, {
105+
code: "class foo{ bar(){}\n\n// comment\n\nbaz(){}}",
106+
output: null,
107+
options: ["never"],
108+
errors: [neverError]
109+
}, {
110+
code: "class foo{ bar(){}\n/* comment-1 */\n\n/* comment-2 */\n\n/* comment-3 */\nbaz(){}}",
111+
output: null,
112+
options: ["never"],
113+
errors: [neverError]
114+
}, {
115+
code: "class foo{ bar(){}\n/* comment-1 */\n\n;\n\n/* comment-3 */\nbaz(){}}",
116+
output: null,
117+
options: ["never"],
118+
errors: [neverError]
119+
}, {
120+
code: "class A {\nfoo() {}// comment\n;\n/* comment */\nbar() {}\n}",
121+
output: "class A {\nfoo() {}// comment\n\n;\n/* comment */\nbar() {}\n}",
122+
options: ["always"],
123+
errors: [alwaysError]
124+
}, {
125+
code: "class A {\nfoo() {}\n/* comment */;\n;\n/* comment */\nbar() {}\n}",
126+
output: "class A {\nfoo() {}\n\n/* comment */;\n;\n/* comment */\nbar() {}\n}",
127+
options: ["always"],
128+
errors: [alwaysError]
129+
}, {
130+
code: "class foo{ bar(){};\nbaz(){}}",
131+
output: "class foo{ bar(){};\n\nbaz(){}}",
132+
options: ["always"],
133+
errors: [alwaysError]
134+
}, {
135+
code: "class foo{ bar(){} // comment \nbaz(){}}",
136+
output: "class foo{ bar(){} // comment \n\nbaz(){}}",
137+
options: ["always"],
138+
errors: [alwaysError]
139+
}, {
140+
code: "class A {\nfoo() {}\n/* comment */;\n;\nbar() {}\n}",
141+
output: "class A {\nfoo() {}\n\n/* comment */;\n;\nbar() {}\n}",
142+
options: ["always"],
143+
errors: [alwaysError]
76144
}
77145
]
78146
});

0 commit comments

Comments
 (0)