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

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

Merged
merged 7 commits into from Dec 20, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
78 changes: 25 additions & 53 deletions lib/rules/lines-between-class-members.js
Expand Up @@ -54,62 +54,33 @@ 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 comment tokens, that have no blank line in between.
* If there is no comment token, return token in the parameter.
* @param {Token} token The last token in the member node.
* @returns {Token} The comment token or token in parameter.
*/
function isPaddingBetweenTokens(first, second) {
const comments = sourceCode.getCommentsBefore(second);
const len = comments.length;
function getLastConsecutiveTokenAfter(token) {
const after = sourceCode.getTokenAfter(token, { 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 (astUtils.isCommentToken(after) && after.loc.start.line - token.loc.end.line <= 1) {
return getLastConsecutiveTokenAfter(after);
}
return token;
}

/*
* 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 comment tokens, that have no blank line in between.
* If there is no comment token, return token in the parameter.
* @param {Token} token The first token in the member node.
* @returns {Token} The comment token or token in parameter.
*/
function getFirstConsecutiveTokenBefore(token) {
const before = sourceCode.getTokenBefore(token, { 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 (astUtils.isCommentToken(before) && token.loc.start.line - before.loc.end.line <= 1) {
return getFirstConsecutiveTokenBefore(before);
}

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

return linesBetweenFstAndSnd - sumOfCommentLines >= 1;
return token;
}

return {
Expand All @@ -120,10 +91,11 @@ 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 = getLastConsecutiveTokenAfter(curLast);
const afterPadding = getFirstConsecutiveTokenBefore(nextFirst);
const isPadded = afterPadding.loc.start.line - beforePadding.loc.start.line > 1;
mdjermanovic marked this conversation as resolved.
Show resolved Hide resolved

if ((options[0] === "always" && !skip && !isPadded) ||
(options[0] === "never" && isPadded)) {
Expand All @@ -132,7 +104,7 @@ module.exports = {
messageId: isPadded ? "never" : "always",
fix(fixer) {
return isPadded
? fixer.replaceTextRange([curLast.range[1], nextFirst.range[0]], "\n")
? fixer.replaceTextRange([beforePadding.range[1], afterPadding.range[0]], "\n")
: fixer.insertTextAfter(curLast, "\n");
}
});
Expand Down
20 changes: 20 additions & 0 deletions tests/lib/rules/lines-between-class-members.js
Expand Up @@ -73,6 +73,26 @@ 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]
}
]
});