Skip to content

Commit

Permalink
Fix: padded-blocks autofix problems with comments (eslint#8149)
Browse files Browse the repository at this point in the history 
* Fix: padded-blocks autofix problems with comments

* More edge cases
  • Loading branch information
@alberto
alberto authored Feb 27, 2017
1 parent 9994889 commit d02bd11
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 32 deletions.
68 changes: 37 additions & 31 deletions lib/rules/padded-blocks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,42 +90,49 @@ module.exports = {
return node.type === "Line" || node.type === "Block";
}

/**
* 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
*/
function isPaddingBetweenTokens(first, second) {
return second.loc.start.line - first.loc.end.line >= 2;
}


/**
* Checks if the given token has a blank line after it.
* @param {Token} token The token to check.
* @returns {boolean} Whether or not the token is followed by a blank line.
*/
function isTokenTopPadded(token) {
const tokenStartLine = token.loc.start.line,
expectedFirstLine = tokenStartLine + 2;
let first = token;
function getFirstBlockToken(token) {
let prev = token,
first = token;

do {
prev = first;
first = sourceCode.getTokenAfter(first, { includeComments: true });
} while (isComment(first) && first.loc.start.line === tokenStartLine);

const firstLine = first.loc.start.line;
} while (isComment(first) && first.loc.start.line === prev.loc.end.line);

return expectedFirstLine <= firstLine;
return first;
}

/**
* Checks if the given token is preceeded by a blank line.
* @param {Token} token The token to check
* @returns {boolean} Whether or not the token is preceeded by a blank line
*/
function isTokenBottomPadded(token) {
const blockEnd = token.loc.end.line,
expectedLastLine = blockEnd - 2;
let last = token;
function getLastBlockToken(token) {
let last = token,
next = token;

do {
next = last;
last = sourceCode.getTokenBefore(last, { includeComments: true });
} while (isComment(last) && last.loc.end.line === blockEnd);
} while (isComment(last) && last.loc.end.line === next.loc.start.line);

const lastLine = last.loc.end.line;

return lastLine <= expectedLastLine;
return last;
}

/**
Expand Down Expand Up @@ -155,57 +162,56 @@ module.exports = {
*/
function checkPadding(node) {
const openBrace = getOpenBrace(node),
firstBlockToken = getFirstBlockToken(openBrace),
tokenBeforeFirst = sourceCode.getTokenBefore(firstBlockToken, { includeComments: true }),
closeBrace = sourceCode.getLastToken(node),
blockHasTopPadding = isTokenTopPadded(openBrace),
blockHasBottomPadding = isTokenBottomPadded(closeBrace);
lastBlockToken = getLastBlockToken(closeBrace),
tokenAfterLast = sourceCode.getTokenAfter(lastBlockToken, { includeComments: true }),
blockHasTopPadding = isPaddingBetweenTokens(tokenBeforeFirst, firstBlockToken),
blockHasBottomPadding = isPaddingBetweenTokens(lastBlockToken, tokenAfterLast);

if (requirePaddingFor(node)) {
if (!blockHasTopPadding) {
context.report({
node,
loc: { line: openBrace.loc.start.line, column: openBrace.loc.start.column },
loc: { line: tokenBeforeFirst.loc.start.line, column: tokenBeforeFirst.loc.start.column },
fix(fixer) {
return fixer.insertTextAfter(openBrace, "\n");
return fixer.insertTextAfter(tokenBeforeFirst, "\n");
},
message: ALWAYS_MESSAGE
});
}
if (!blockHasBottomPadding) {
context.report({
node,
loc: { line: closeBrace.loc.end.line, column: closeBrace.loc.end.column - 1 },
loc: { line: tokenAfterLast.loc.end.line, column: tokenAfterLast.loc.end.column - 1 },
fix(fixer) {
return fixer.insertTextBefore(closeBrace, "\n");
return fixer.insertTextBefore(tokenAfterLast, "\n");
},
message: ALWAYS_MESSAGE
});
}
} else {
if (blockHasTopPadding) {
const nextToken = sourceCode.getTokenAfter(openBrace, { includeComments: true });

context.report({
node,
loc: { line: openBrace.loc.start.line, column: openBrace.loc.start.column },
loc: { line: tokenBeforeFirst.loc.start.line, column: tokenBeforeFirst.loc.start.column },
fix(fixer) {

// FIXME: The start of this range is sometimes larger than the end.
// https://github.com/eslint/eslint/issues/8116
return fixer.replaceTextRange([openBrace.end, nextToken.start - nextToken.loc.start.column], "\n");
return fixer.replaceTextRange([tokenBeforeFirst.end, firstBlockToken.start - firstBlockToken.loc.start.column], "\n");
},
message: NEVER_MESSAGE
});
}

if (blockHasBottomPadding) {
const previousToken = sourceCode.getTokenBefore(closeBrace, { includeComments: true });

context.report({
node,
loc: { line: closeBrace.loc.end.line, column: closeBrace.loc.end.column - 1 },
loc: { line: tokenAfterLast.loc.end.line, column: tokenAfterLast.loc.end.column - 1 },
message: NEVER_MESSAGE,
fix(fixer) {
return fixer.replaceTextRange([previousToken.end, closeBrace.start - closeBrace.loc.start.column], "\n");
return fixer.replaceTextRange([lastBlockToken.end, tokenAfterLast.start - tokenAfterLast.loc.start.column], "\n");
}
});
}
Expand Down
48 changes: 47 additions & 1 deletion tests/lib/rules/padded-blocks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ ruleTester.run("padded-blocks", rule, {
{ code: "{\n\na()\n//comment\n\n}" },
{ code: "{\n\na = 1\n\n}" },
{ code: "{//comment\n\na();\n\n}" },
{ code: "{ /* comment */\n\na();\n\n}" },
{ code: "{ /* comment \n */\n\na();\n\n}" },
{ code: "{ /* comment \n */ /* another comment \n */\n\na();\n\n}" },
{ code: "{ /* comment \n */ /* another comment \n */\n\na();\n\n/* comment \n */ /* another comment \n */}" },

{ code: "{\n\na();\n\n/* comment */ }" },
{ code: "{\n\na();\n\n/* comment */ }", options: ["always"] },
{ code: "{\n\na();\n\n/* comment */ }", options: [{ blocks: "always" }] },
Expand Down Expand Up @@ -85,6 +90,17 @@ ruleTester.run("padded-blocks", rule, {
}
]
},
{
code: "{ //comment\na();\n\n}",
output: "{ //comment\n\na();\n\n}",
errors: [
{
message: ALWAYS_MESSAGE,
line: 1,
column: 3
}
]
},
{
code: "{\n\na();\n//comment\n}",
output: "{\n\na();\n//comment\n\n}",
Expand Down Expand Up @@ -466,9 +482,39 @@ ruleTester.run("padded-blocks", rule, {
},
{
code: "function foo() { // a\n\n b;\n}",
output: "function foo() { // a\n\n b;\n}",
output: "function foo() { // a\n b;\n}",
options: ["never"],
errors: [NEVER_MESSAGE]
},
{
code: "function foo() { /* a\n */\n\n bar;\n}",
output: "function foo() { /* a\n */\n bar;\n}",
options: ["never"],
errors: [NEVER_MESSAGE]
},
{
code: "function foo() {\n\n bar;\n/* a\n */}",
output: "function foo() {\n\n bar;\n\n/* a\n */}",
options: ["always"],
errors: [ALWAYS_MESSAGE]
},
{
code: "function foo() { /* a\n */\n/* b\n */\n bar;\n}",
output: "function foo() { /* a\n */\n\n/* b\n */\n bar;\n\n}",
options: ["always"],
errors: [ALWAYS_MESSAGE, ALWAYS_MESSAGE]
},
{
code: "function foo() { /* a\n */ /* b\n */\n bar;\n}",
output: "function foo() { /* a\n */ /* b\n */\n\n bar;\n\n}",
options: ["always"],
errors: [ALWAYS_MESSAGE, ALWAYS_MESSAGE]
},
{
code: "function foo() { /* a\n */ /* b\n */\n bar;\n/* c\n *//* d\n */}",
output: "function foo() { /* a\n */ /* b\n */\n\n bar;\n\n/* c\n *//* d\n */}",
options: ["always"],
errors: [ALWAYS_MESSAGE, ALWAYS_MESSAGE]
}
]
});

0 comments on commit d02bd11

Please sign in to comment.