Skip to content

Commit

Permalink
Update: add fixer for newline-after-var (fixes #5959) (#7375)
Browse files Browse the repository at this point in the history 
* Update: add fixer for `newline-after-var` (fixes #5959)

* Add tests for lines that only contain semicolons
  • Loading branch information
@not-an-aardvark
not-an-aardvark committed Oct 26, 2016
1 parent 6e9ff08 commit ee3bcea
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 48 deletions.
2 changes: 2 additions & 0 deletions docs/rules/newline-after-var.md
View file
@@ -1,5 +1,7 @@
# require or disallow an empty line after variable declarations (newline-after-var)

(fixable) The `--fix` option on the [command line](../user-guide/command-line-interface#fix) automatically fixes problems reported by this rule.

As of today there is no consistency in separating variable declarations from the rest of the code. Some developers leave an empty line between var statements and the rest of the code like:

```js
Expand Down
61 changes: 49 additions & 12 deletions lib/rules/newline-after-var.js
View file
Expand Up @@ -21,7 +21,9 @@ module.exports = {
{
enum: ["never", "always"]
}
]
],

fixable: "whitespace"
},

create(context) {
Expand Down Expand Up @@ -119,21 +121,25 @@ module.exports = {
return !token || (token.type === "Punctuator" && token.value === "}");
}

/**
* Gets the last line of a group of consecutive comments
* @param {number} commentStartLine The starting line of the group
* @returns {number} The number of the last comment line of the group
*/
function getLastCommentLineOfBlock(commentStartLine) {
const currentCommentEnd = commentEndLine[commentStartLine];

return commentEndLine[currentCommentEnd + 1] ? getLastCommentLineOfBlock(currentCommentEnd + 1) : currentCommentEnd;
}

/**
* Determine if a token starts more than one line after a comment ends
* @param {token} token The token being checked
* @param {integer} commentStartLine The line number on which the comment starts
* @returns {boolean} True if `token` does not start immediately after a comment
*/
function hasBlankLineAfterComment(token, commentStartLine) {
const commentEnd = commentEndLine[commentStartLine];

// If there's another comment, repeat check for blank line
if (commentEndLine[commentEnd + 1]) {
return hasBlankLineAfterComment(token, commentEnd + 1);
}

return (token.loc.start.line > commentEndLine[commentStartLine] + 1);
return token.loc.start.line > getLastCommentLineOfBlock(commentStartLine) + 1;
}

/**
Expand All @@ -145,8 +151,18 @@ module.exports = {
* @returns {void}
*/
function checkForBlankLine(node) {

/*
* lastToken is the last token on the node's line. It will usually also be the last token of the node, but it will
* sometimes be second-last if there is a semicolon on a different line.
*/
const lastToken = getLastToken(node),
nextToken = sourceCode.getTokenAfter(node),

/*
* If lastToken is the last token of the node, nextToken should be the token after the node. Otherwise, nextToken
* is the last token of the node.
*/
nextToken = lastToken === sourceCode.getLastToken(node) ? sourceCode.getTokenAfter(node) : sourceCode.getLastToken(node),
nextLineNum = lastToken.loc.end.line + 1;

// Ignore if there is no following statement
Expand Down Expand Up @@ -180,7 +196,17 @@ module.exports = {
const hasNextLineComment = (typeof commentEndLine[nextLineNum] !== "undefined");

if (mode === "never" && noNextLineToken && !hasNextLineComment) {
context.report(node, NEVER_MESSAGE, { identifier: node.name });
context.report({
node,
message: NEVER_MESSAGE,
data: { identifier: node.name },
fix(fixer) {
const NEWLINE_REGEX = /\r\n|\r|\n|\u2028|\u2029/;
const linesBetween = sourceCode.getText().slice(lastToken.range[1], nextToken.range[0]).split(NEWLINE_REGEX);

return fixer.replaceTextRange([lastToken.range[1], nextToken.range[0]], `${linesBetween.slice(0, -1).join("")}\n${linesBetween[linesBetween.length - 1]}`);
}
});
}

// Token on the next line, or comment without blank line
Expand All @@ -190,7 +216,18 @@ module.exports = {
hasNextLineComment && !hasBlankLineAfterComment(nextToken, nextLineNum)
)
) {
context.report(node, ALWAYS_MESSAGE, { identifier: node.name });
context.report({
node,
message: ALWAYS_MESSAGE,
data: { identifier: node.name },
fix(fixer) {
if ((noNextLineToken ? getLastCommentLineOfBlock(nextLineNum) : lastToken.loc.end.line) === nextToken.loc.start.line) {
return fixer.insertTextBefore(nextToken, "\n\n");
}

return fixer.insertTextBeforeRange([nextToken.range[0] - nextToken.loc.start.column, nextToken.range[1]], "\n");
}
});
}
}

Expand Down
111 changes: 75 additions & 36 deletions tests/lib/rules/newline-after-var.js
View file
Expand Up @@ -45,22 +45,29 @@ const ONE_BLANK = "var greet = 'hello';\n\nconsole.log(greet);",
NEXT_LINE_COMMENT_ONE_BLANK = "var greet = 'hello';\n// next-line comment\n\nconsole.log(greet);",
NEXT_LINE_BLOCK_COMMENT_ONE_BLANK = "var greet = 'hello';\n/* block comment\nblock comment */\n\nconsole.log(greet);",
NEXT_LINE_TWO_COMMENTS_ONE_BLANK = "var greet = 'hello';\n// next-line comment\n// second-line comment\n\nconsole.log(greet);",
NEXT_LINE_COMMENT_BLOCK_COMMENT_ONE_BLANK = "var greet = 'hello';\n// next-line comment\n/* block comment\nblock comment */\n\nconsole.log(greet);",
MIXED_COMMENT_ONE_BLANK = "var greet = 'hello';\n// inline comment\nvar name = 'world';\n\nconsole.log(greet, name);",
MIXED_BLOCK_COMMENT_ONE_BLANK = "var greet = 'hello';\n/* block comment\nblock comment */\nvar name = 'world';\n\nconsole.log(greet, name);",
MULTI_VAR_ONE_BLANK = "var greet = 'hello';\nvar name = 'world';\n\nconsole.log(greet, name);",
MULTI_VAR_NO_BREAK_ONE_BLANK = "var greet = 'hello';var name = 'world';\n\nconsole.log(greet, name);",
MULTI_DEC_ONE_BLANK = "var greet = 'hello', name = 'world';\n\nconsole.log(greet, name);",
MULTI_LINE_ONE_BLANK = "var greet = 'hello',\nname = 'world';\n\nconsole.log(greet, name);",
MULTI_LINE_ONE_BLANK_WITH_COMMENTS = "var greet = 'hello', // inline comment\nname = 'world'; // inline comment\n\nconsole.log(greet, name);",
MULTI_LINE_NEXT_LINE_COMMENT_ONE_BLANK = "var greet = 'hello',\nname = 'world';\n// next-line comment\n\nconsole.log(greet);",
MULTI_LINE_NEXT_LINE_BLOCK_COMMENT_ONE_BLANK = "var greet = 'hello',\nname = 'world';\n/* block comment\nblock comment */\n\nconsole.log(greet);",
LET_ONE_BLANK = "let greet = 'hello';\n\nconsole.log(greet);",
CONST_ONE_BLANK = "const greet = 'hello';\n\nconsole.log(greet);",
MIXED_LET_VAR = "let greet = 'hello';\nvar name = 'world';\n\nconsole.log(greet, name);",
MIXED_CONST_VAR = "const greet = 'hello';\nvar name = 'world';\n\nconsole.log(greet, name);",
MIXED_LET_CONST = "let greet = 'hello';\nconst name = 'world';\n\nconsole.log(greet, name);",
NOT_END_OF_FUNCTION_ONE_BLANK = "function example() {\nvar greet = 'hello';\n\nconsole.log(greet);\n}",
NOT_END_OF_FUNCTION_EXPRESSION_ONE_BLANK = "var f = function() {\nvar greet = 'hello';\n\nconsole.log(greet);\n};",
NOT_END_OF_ARROW_FUNCTION_ONE_BLANK = "() => {\nvar greet = 'hello';\n\nconsole.log(greet);\n}",
ONE_BLANK_BEFORE_CASE = "switch(a) {\ncase 0:\nvar foo;\n\ncase 1:}";


// Valid for "Never"
const NO_BREAK = "var greet = 'hello'; console.log(greet);",
const NO_BREAK = "var greet = 'hello';console.log(greet);",
NO_BLANK = "var greet = 'hello';\nconsole.log(greet);",
NO_BLANK_WITH_TRAILING_WS = "var greet = 'hello'; \nconsole.log(greet);",
NO_BLANK_WITH_INLINE_COMMENT = "var greet = 'hello'; // inline comment\nconsole.log(greet);",
Expand All @@ -70,9 +77,9 @@ const NO_BREAK = "var greet = 'hello'; console.log(greet);",
NEXT_LINE_COMMENT_BLOCK_COMMENT_NO_BLANK = "var greet = 'hello';\n// next-line comment\n/* block comment\nblock comment */\nconsole.log(greet);",
MIXED_COMMENT_NO_BLANK = "var greet = 'hello';\n// inline comment\nvar name = 'world';\nconsole.log(greet, name);",
MIXED_BLOCK_COMMENT_NO_BLANK = "var greet = 'hello';\n/* block comment\nblock comment */\nvar name = 'world';\nconsole.log(greet, name);",
MULTI_VAR_NO_BREAK = "var greet = 'hello'; var name = 'world'; console.log(greet, name);",
MULTI_VAR_NO_BREAK = "var greet = 'hello';var name = 'world';console.log(greet, name);",
MULTI_VAR_NO_BLANK = "var greet = 'hello';\nvar name = 'world';\nconsole.log(greet, name);",
MULTI_DEC_NO_BREAK = "var greet = 'hello', name = 'world'; console.log(greet, name);",
MULTI_DEC_NO_BREAK = "var greet = 'hello', name = 'world';console.log(greet, name);",
MULTI_DEC_NO_BLANK = "var greet = 'hello', name = 'world';\nconsole.log(greet, name);",
MULTI_LINE_NO_BLANK = "var greet = 'hello',\nname = 'world';\nconsole.log(greet, name);",
MULTI_LINE_NO_BLANK_WITH_COMMENTS = "var greet = 'hello', // inline comment\nname = 'world'; // inline comment\nconsole.log(greet, name);",
Expand Down Expand Up @@ -242,57 +249,70 @@ ruleTester.run("newline-after-var", rule, {
`,
options: ["never"]
},
{
code: `
var a = 1
;
(b || c).doSomething();
`,
options: ["never"]
}
],

invalid: [

// should disallow no line break in "always" mode
{ code: NO_BREAK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_VAR_NO_BREAK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_DEC_NO_BREAK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NO_BREAK, output: ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_VAR_NO_BREAK, output: MULTI_VAR_NO_BREAK_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_DEC_NO_BREAK, output: MULTI_DEC_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },

// should disallow no blank line in "always" mode
{ code: NO_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NO_BLANK_WITH_TRAILING_WS, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NO_BLANK_WITH_INLINE_COMMENT, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_VAR_NO_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_DEC_NO_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_LINE_NO_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: LET_NO_BLANK, options: ["always"], parserOptions: { ecmaVersion: 6 }, errors: [ALWAYS_ERROR] },
{ code: CONST_NO_BLANK, options: ["always"], parserOptions: { ecmaVersion: 6 }, errors: [ALWAYS_ERROR] },
{ code: NOT_END_OF_FUNCTION, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NOT_END_OF_FUNCTION_EXPRESSION, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NOT_END_OF_ARROW_FUNCTION, options: ["always"], parserOptions: { ecmaVersion: 6 }, errors: [ALWAYS_ERROR]},
{ code: NO_BLANK_BEFORE_CASE, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NO_BLANK, output: ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NO_BLANK_WITH_TRAILING_WS, output: ONE_BLANK_WITH_TRAILING_WS, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NO_BLANK_WITH_INLINE_COMMENT, output: ONE_BLANK_WITH_INLINE_COMMENT, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_VAR_NO_BLANK, output: MULTI_VAR_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_DEC_NO_BLANK, output: MULTI_DEC_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_LINE_NO_BLANK, output: MULTI_LINE_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: LET_NO_BLANK, output: LET_ONE_BLANK, options: ["always"], parserOptions: { ecmaVersion: 6 }, errors: [ALWAYS_ERROR] },
{ code: CONST_NO_BLANK, output: CONST_ONE_BLANK, options: ["always"], parserOptions: { ecmaVersion: 6 }, errors: [ALWAYS_ERROR] },
{ code: NOT_END_OF_FUNCTION, output: NOT_END_OF_FUNCTION_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NOT_END_OF_FUNCTION_EXPRESSION, output: NOT_END_OF_FUNCTION_EXPRESSION_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NOT_END_OF_ARROW_FUNCTION, output: NOT_END_OF_ARROW_FUNCTION_ONE_BLANK, options: ["always"], parserOptions: { ecmaVersion: 6 }, errors: [ALWAYS_ERROR]},
{ code: NO_BLANK_BEFORE_CASE, output: ONE_BLANK_BEFORE_CASE, options: ["always"], errors: [ALWAYS_ERROR] },

// should disallow blank lines in "never" mode
{ code: ONE_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: TWO_BLANKS, options: ["never"], errors: [NEVER_ERROR] },
{ code: THREE_BLANKS, options: ["never"], errors: [NEVER_ERROR] },
{ code: ONE_BLANK_WITH_TRAILING_WS, options: ["never"], errors: [NEVER_ERROR] },
{ code: ONE_BLANK_WITH_INLINE_COMMENT, options: ["never"], errors: [NEVER_ERROR] },
{ code: MULTI_VAR_ONE_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: MULTI_DEC_ONE_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: MULTI_LINE_ONE_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: MULTI_LINE_ONE_BLANK_WITH_COMMENTS, options: ["never"], errors: [NEVER_ERROR] },
{ code: LET_ONE_BLANK, options: ["never"], parserOptions: { ecmaVersion: 6 }, errors: [NEVER_ERROR] },
{ code: CONST_ONE_BLANK, options: ["never"], parserOptions: { ecmaVersion: 6 }, errors: [NEVER_ERROR] },
{ code: ONE_BLANK_BEFORE_CASE, options: ["never"], errors: [NEVER_ERROR] },
{ code: ONE_BLANK, output: NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: TWO_BLANKS, output: NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: THREE_BLANKS, output: NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: ONE_BLANK_WITH_TRAILING_WS, output: NO_BLANK_WITH_TRAILING_WS, options: ["never"], errors: [NEVER_ERROR] },
{ code: ONE_BLANK_WITH_INLINE_COMMENT, NO_BLANK_WITH_INLINE_COMMENT, options: ["never"], errors: [NEVER_ERROR] },
{ code: MULTI_VAR_ONE_BLANK, output: MULTI_VAR_NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: MULTI_DEC_ONE_BLANK, output: MULTI_DEC_NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: MULTI_LINE_ONE_BLANK, output: MULTI_LINE_NO_BLANK, options: ["never"], errors: [NEVER_ERROR] },
{ code: MULTI_LINE_ONE_BLANK_WITH_COMMENTS, output: MULTI_LINE_NO_BLANK_WITH_COMMENTS, options: ["never"], errors: [NEVER_ERROR] },
{ code: LET_ONE_BLANK, output: LET_NO_BLANK, options: ["never"], parserOptions: { ecmaVersion: 6 }, errors: [NEVER_ERROR] },
{ code: CONST_ONE_BLANK, output: CONST_NO_BLANK, options: ["never"], parserOptions: { ecmaVersion: 6 }, errors: [NEVER_ERROR] },
{ code: ONE_BLANK_BEFORE_CASE, output: NO_BLANK_BEFORE_CASE, options: ["never"], errors: [NEVER_ERROR] },

// should disallow a comment on the next line that's not in turn followed by a blank in "always" mode
{ code: NEXT_LINE_COMMENT, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NEXT_LINE_BLOCK_COMMENT, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_LINE_NEXT_LINE_COMMENT, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_LINE_NEXT_LINE_BLOCK_COMMENT, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NEXT_LINE_TWO_COMMENTS_NO_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NEXT_LINE_COMMENT_BLOCK_COMMENT_NO_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NEXT_LINE_COMMENT, output: NEXT_LINE_COMMENT_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NEXT_LINE_BLOCK_COMMENT, output: NEXT_LINE_BLOCK_COMMENT_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_LINE_NEXT_LINE_COMMENT, output: MULTI_LINE_NEXT_LINE_COMMENT_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: MULTI_LINE_NEXT_LINE_BLOCK_COMMENT, output: MULTI_LINE_NEXT_LINE_BLOCK_COMMENT_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NEXT_LINE_TWO_COMMENTS_NO_BLANK, output: NEXT_LINE_TWO_COMMENTS_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },
{ code: NEXT_LINE_COMMENT_BLOCK_COMMENT_NO_BLANK, output: NEXT_LINE_COMMENT_BLOCK_COMMENT_ONE_BLANK, options: ["always"], errors: [ALWAYS_ERROR] },

// https://github.com/eslint/eslint/issues/6834
{
code: `
var a = 1
;(b || c).doSomething()
`,
output: `
var a = 1
;(b || c).doSomething()
`,
options: ["always"],
errors: [ALWAYS_ERROR]
},
Expand All @@ -302,8 +322,27 @@ ruleTester.run("newline-after-var", rule, {
;(b || c).doSomething()
`,
output: `
var a = 1
;(b || c).doSomething()
`,
options: ["never"],
errors: [NEVER_ERROR]
},
{
code: `
var a = 1
;
(b || c).doSomething();
`,
output: `
var a = 1
;
(b || c).doSomething();
`,
options: ["never"],
errors: [NEVER_ERROR]
}
]
});

0 comments on commit ee3bcea

Please sign in to comment.