Skip to content

Commit

Permalink
Update: autofix bug in lines-between-class-members (fixes #12391) (#1…
Browse files Browse the repository at this point in the history 
…2632)

* 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
  • Loading branch information
@yeonjuan @kaicataldo
yeonjuan authored and kaicataldo committed Dec 20, 2019
1 parent 4b3cc5c commit 5c25a26
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 53 deletions.
95 changes: 42 additions & 53 deletions lib/rules/lines-between-class-members.js
View file
Expand Up @@ -54,62 +54,45 @@ module.exports = {
const sourceCode = context.getSourceCode();

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

// If there is no comments
if (len === 0) {
const linesBetweenFstAndSnd = second.loc.start.line - first.loc.end.line - 1;

return linesBetweenFstAndSnd >= 1;
}


// If there are comments
let sumOfCommentLines = 0; // the numbers of lines of comments
let prevCommentLineNum = -1; // line number of the end of the previous comment

for (let i = 0; i < len; i++) {
const commentLinesOfThisComment = comments[i].loc.end.line - comments[i].loc.start.line + 1;

sumOfCommentLines += commentLinesOfThisComment;

/*
* If this comment and the previous comment are in the same line,
* the count of comment lines is duplicated. So decrement sumOfCommentLines.
*/
if (prevCommentLineNum === comments[i].loc.start.line) {
sumOfCommentLines -= 1;
}

prevCommentLineNum = comments[i].loc.end.line;
if (after !== nextFirstToken && after.loc.start.line - prevLastToken.loc.end.line <= maxLine) {
return findLastConsecutiveTokenAfter(after, nextFirstToken, maxLine);
}
return prevLastToken;
}

/*
* If the first block and the first comment are in the same line,
* the count of comment lines is duplicated. So decrement sumOfCommentLines.
*/
if (first.loc.end.line === comments[0].loc.start.line) {
sumOfCommentLines -= 1;
}
/**
* 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.
* @param {Token} nextFirstToken The first token in the next member node.
* @param {Token} prevLastToken The last token in the previous member node.
* @param {number} maxLine The maximum number of allowed line difference between consecutive tokens.
* @returns {Token} The first token among the consecutive tokens.
*/
function findFirstConsecutiveTokenBefore(nextFirstToken, prevLastToken, maxLine) {
const before = sourceCode.getTokenBefore(nextFirstToken, { includeComments: true });

/*
* If the last comment and the second block are in the same line,
* the count of comment lines is duplicated. So decrement sumOfCommentLines.
*/
if (comments[len - 1].loc.end.line === second.loc.start.line) {
sumOfCommentLines -= 1;
if (before !== prevLastToken && nextFirstToken.loc.start.line - before.loc.end.line <= maxLine) {
return findFirstConsecutiveTokenBefore(before, prevLastToken, maxLine);
}
return nextFirstToken;
}

const linesBetweenFstAndSnd = second.loc.start.line - first.loc.end.line - 1;

return linesBetweenFstAndSnd - sumOfCommentLines >= 1;
/**
* Checks if there is a token or comment between two tokens.
* @param {Token} before The token before.
* @param {Token} after The token after.
* @returns {boolean} True if there is a token or comment between two tokens.
*/
function hasTokenOrCommentBetween(before, after) {
return sourceCode.getTokensBetween(before, after, { includeComments: true }).length !== 0;
}

return {
Expand All @@ -120,20 +103,26 @@ module.exports = {
const curFirst = sourceCode.getFirstToken(body[i]);
const curLast = sourceCode.getLastToken(body[i]);
const nextFirst = sourceCode.getFirstToken(body[i + 1]);
const isPadded = isPaddingBetweenTokens(curLast, nextFirst);
const isMulti = !astUtils.isTokenOnSameLine(curFirst, curLast);
const skip = !isMulti && options[1].exceptAfterSingleLine;

const beforePadding = findLastConsecutiveTokenAfter(curLast, nextFirst, 1);
const afterPadding = findFirstConsecutiveTokenBefore(nextFirst, curLast, 1);
const isPadded = afterPadding.loc.start.line - beforePadding.loc.end.line > 1;
const hasTokenInPadding = hasTokenOrCommentBetween(beforePadding, afterPadding);
const curLineLastToken = findLastConsecutiveTokenAfter(curLast, nextFirst, 0);

if ((options[0] === "always" && !skip && !isPadded) ||
(options[0] === "never" && isPadded)) {
context.report({
node: body[i + 1],
messageId: isPadded ? "never" : "always",
fix(fixer) {
if (hasTokenInPadding) {
return null;
}
return isPadded
? fixer.replaceTextRange([curLast.range[1], nextFirst.range[0]], "\n")
: fixer.insertTextAfter(curLast, "\n");
? fixer.replaceTextRange([beforePadding.range[1], afterPadding.range[0]], "\n")
: fixer.insertTextAfter(curLineLastToken, "\n");
}
});
}
Expand Down
68 changes: 68 additions & 0 deletions tests/lib/rules/lines-between-class-members.js
View file
Expand Up @@ -40,6 +40,9 @@ ruleTester.run("lines-between-class-members", rule, {
"class A{ foo() {}\n/* a */ /* b */\n\nbar() {}}",
"class A{ foo() {}/* a */ \n\n /* b */bar() {}}",

"class A {\nfoo() {}\n/* comment */;\n;\n\nbar() {}\n}",
"class A {\nfoo() {}\n// comment\n\n;\n;\nbar() {}\n}",

"class foo{ bar(){}\n\n;;baz(){}}",
"class foo{ bar(){};\n\nbaz(){}}",

Expand Down Expand Up @@ -73,6 +76,71 @@ ruleTester.run("lines-between-class-members", rule, {
output: "class foo{ bar(){\n}\n\nbaz(){}}",
options: ["always", { exceptAfterSingleLine: true }],
errors: [alwaysError]
}, {
code: "class foo{ bar(){\n}\n/* comment */\nbaz(){}}",
output: "class foo{ bar(){\n}\n\n/* comment */\nbaz(){}}",
options: ["always", { exceptAfterSingleLine: true }],
errors: [alwaysError]
}, {
code: "class foo{ bar(){}\n\n// comment\nbaz(){}}",
output: "class foo{ bar(){}\n// comment\nbaz(){}}",
options: ["never"],
errors: [neverError]
}, {
code: "class foo{ bar(){}\n\n/* comment */\nbaz(){}}",
output: "class foo{ bar(){}\n/* comment */\nbaz(){}}",
options: ["never"],
errors: [neverError]
}, {
code: "class foo{ bar(){}\n/* comment-1 */\n\n/* comment-2 */\nbaz(){}}",
output: "class foo{ bar(){}\n/* comment-1 */\n/* comment-2 */\nbaz(){}}",
options: ["never"],
errors: [neverError]
}, {
code: "class foo{ bar(){}\n\n/* comment */\n\nbaz(){}}",
output: null,
options: ["never"],
errors: [neverError]
}, {
code: "class foo{ bar(){}\n\n// comment\n\nbaz(){}}",
output: null,
options: ["never"],
errors: [neverError]
}, {
code: "class foo{ bar(){}\n/* comment-1 */\n\n/* comment-2 */\n\n/* comment-3 */\nbaz(){}}",
output: null,
options: ["never"],
errors: [neverError]
}, {
code: "class foo{ bar(){}\n/* comment-1 */\n\n;\n\n/* comment-3 */\nbaz(){}}",
output: null,
options: ["never"],
errors: [neverError]
}, {
code: "class A {\nfoo() {}// comment\n;\n/* comment */\nbar() {}\n}",
output: "class A {\nfoo() {}// comment\n\n;\n/* comment */\nbar() {}\n}",
options: ["always"],
errors: [alwaysError]
}, {
code: "class A {\nfoo() {}\n/* comment */;\n;\n/* comment */\nbar() {}\n}",
output: "class A {\nfoo() {}\n\n/* comment */;\n;\n/* comment */\nbar() {}\n}",
options: ["always"],
errors: [alwaysError]
}, {
code: "class foo{ bar(){};\nbaz(){}}",
output: "class foo{ bar(){};\n\nbaz(){}}",
options: ["always"],
errors: [alwaysError]
}, {
code: "class foo{ bar(){} // comment \nbaz(){}}",
output: "class foo{ bar(){} // comment \n\nbaz(){}}",
options: ["always"],
errors: [alwaysError]
}, {
code: "class A {\nfoo() {}\n/* comment */;\n;\nbar() {}\n}",
output: "class A {\nfoo() {}\n\n/* comment */;\n;\nbar() {}\n}",
options: ["always"],
errors: [alwaysError]
}
]
});

0 comments on commit 5c25a26

Please sign in to comment.