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

Update: autofix bug in lines-between-class-members (fixes #12391) #12632

Merged
merged 7 commits into from Dec 20, 2019
Merged

Update: autofix bug in lines-between-class-members (fixes #12391) #12632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5a5b598
Fix: delete comment bug in lines-between-class-members (fixes #12391)
yeonjuan Dec 2, 2019
fd80992
edit comments
yeonjuan Dec 5, 2019
b85eaa6
keep padded comments
yeonjuan Dec 8, 2019
7ec30b2
fix to check for semicolon token.
yeonjuan Dec 10, 2019
0cc88f8
check token in padding, check end loc of before padding.
yeonjuan Dec 11, 2019
05a1bfa
add jsdoc param type, fix typo
yeonjuan Dec 11, 2019
6360080
fix typo in test
yeonjuan Dec 17, 2019
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
95 changes: 42 additions & 53 deletions lib/rules/lines-between-class-members.js
View file
Open in desktop
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
Open in desktop
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]
}
]
});