Skip to content

Commit

Permalink
Update: add fixer for newline-before-return (fixes #5958) (#7050)
Browse files Browse the repository at this point in the history 
* Update: add fixer for newline-before-return (fixes #5958)

* Update: improve canFix() logic and add more tests (refs #5958)

* Update: make fixer a bit smarter and add more tests (refs #5958)

* Update: update docs and fix coding style issue (refs #5958)
  • Loading branch information
@vitorbal @ilyavolodin
vitorbal authored and ilyavolodin committed Sep 7, 2016
1 parent 1f995c3 commit 656bb6e
Show file tree
Hide file tree
Showing 3 changed files with 178 additions and 29 deletions.
2 changes: 2 additions & 0 deletions docs/rules/newline-before-return.md
View file
@@ -1,5 +1,7 @@
# require an empty line before `return` statements (newline-before-return)

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

There is no hard and fast rule about whether empty lines should precede `return` statements in JavaScript. However, clearly delineating where a function is returning can greatly increase the readability and clarity of the code. For example:

```js
Expand Down
71 changes: 63 additions & 8 deletions lib/rules/newline-before-return.js
View file
Expand Up @@ -15,7 +15,7 @@ module.exports = {
category: "Stylistic Issues",
recommended: false
},

fixable: "whitespace",
schema: []
},

Expand Down Expand Up @@ -103,14 +103,13 @@ module.exports = {
}

/**
* Checks whether node is preceded by a newline
* @param {ASTNode} node - node to check
* @returns {boolean} Whether or not the node is preceded by a newline
* Returns the line number of the token before the node that is passed in as an argument
* @param {ASTNode} node - The node to use as the start of the calculation
* @returns {number} Line number of the token before `node`
* @private
*/
function hasNewlineBefore(node) {
const tokenBefore = sourceCode.getTokenBefore(node),
lineNumNode = node.loc.start.line;
function getLineNumberOfTokenBefore(node) {
const tokenBefore = sourceCode.getTokenBefore(node);
let lineNumTokenBefore;

/**
Expand All @@ -127,11 +126,58 @@ module.exports = {
lineNumTokenBefore = 0; // global return at beginning of script
}

return lineNumTokenBefore;
}

/**
* Checks whether node is preceded by a newline
* @param {ASTNode} node - node to check
* @returns {boolean} Whether or not the node is preceded by a newline
* @private
*/
function hasNewlineBefore(node) {
const lineNumNode = node.loc.start.line;
const lineNumTokenBefore = getLineNumberOfTokenBefore(node);
const commentLines = calcCommentLines(node, lineNumTokenBefore);

return (lineNumNode - lineNumTokenBefore - commentLines) > 1;
}

/**
* Checks whether it is safe to apply a fix to a given return statement.
*
* The fix is not considered safe if the given return statement has leading comments,
* as we cannot safely determine if the newline should be added before or after the comments.
* For more information, see: https://github.com/eslint/eslint/issues/5958#issuecomment-222767211
*
* @param {ASTNode} node - The return statement node to check.
* @returns {boolean} `true` if it can fix the node.
* @private
*/
function canFix(node) {
const leadingComments = sourceCode.getComments(node).leading;
const lastLeadingComment = leadingComments[leadingComments.length - 1];
const tokenBefore = sourceCode.getTokenBefore(node);

if (leadingComments.length === 0) {
return true;
}

// if the last leading comment ends in the same line as the previous token and
// does not share a line with the `return` node, we can consider it safe to fix.
// Example:
// function a() {
// var b; //comment
// return;
// }
if (lastLeadingComment.loc.end.line === tokenBefore.loc.end.line &&
lastLeadingComment.loc.end.line !== node.loc.start.line) {
return true;
}

return false;
}

//--------------------------------------------------------------------------
// Public
//--------------------------------------------------------------------------
Expand All @@ -141,7 +187,16 @@ module.exports = {
if (!isFirstNode(node) && !hasNewlineBefore(node)) {
context.report({
node,
message: "Expected newline before return statement."
message: "Expected newline before return statement.",
fix(fixer) {
if (canFix(node)) {
const tokenBefore = sourceCode.getTokenBefore(node);
const newlines = node.loc.start.line === tokenBefore.loc.end.line ? "\n\n" : "\n";

return fixer.insertTextBefore(node, newlines);
}
return null;
}
});
}
}
Expand Down
134 changes: 113 additions & 21 deletions tests/lib/rules/newline-before-return.js
View file
Expand Up @@ -110,79 +110,171 @@ ruleTester.run("newline-before-return", rule, {
],

invalid: [
{
code: "function a() {\nvar b; return;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b; \n\nreturn;\n}"
},
{
code: "function a() {\nvar b;\nreturn;\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b;\n\nreturn;\n}"
},
{
code: "function a() {\nif (b) return b;\nelse if (c) return c;\nelse {\ne();\nreturn d;\n}\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nif (b) return b;\nelse if (c) return c;\nelse {\ne();\n\nreturn d;\n}\n}"
},
{
code: "function a() {\nif (b) return b;\nelse if (c) return c;\nelse {\ne(); return d;\n}\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nif (b) return b;\nelse if (c) return c;\nelse {\ne(); \n\nreturn d;\n}\n}"
},
{
code: "function a() {\n while (b) {\nc();\nreturn;\n}\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\n while (b) {\nc();\n\nreturn;\n}\n}"
},
{
code: "function a() {\ndo {\nc();\nreturn;\n} while (b);\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\ndo {\nc();\n\nreturn;\n} while (b);\n}"
},
{
code: "function a() {\nfor (var b; b < c; b++) {\nc();\nreturn;\n}\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nfor (var b; b < c; b++) {\nc();\n\nreturn;\n}\n}"
},
{
code: "function a() {\nfor (b in c) {\nd();\nreturn;\n}\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nfor (b in c) {\nd();\n\nreturn;\n}\n}"
},
{
code: "function a() {\nfor (b of c) {\nd();\nreturn;\n}\n}",
parserOptions: { ecmaVersion: 6 },
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nfor (b of c) {\nd();\n\nreturn;\n}\n}"
},
{
code: "function a() {\nif (b) {\nc();\n}\n//comment\nreturn b;\n}",
errors: ["Expected newline before return statement."]
},
{
code: "function a() {\nif (b) {\nc();\n}\n//comment\nreturn b;\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nif (b) {\nc();\n}\n//comment\nreturn b;\n}"
},
{
code: "function a() {\n/*comment\ncomment*/\nif (b) {\nc();\nreturn b;\n} else {\n//comment\n\nreturn d;\n}\n/*multi-line\ncomment*/\nreturn e;\n}",
errors: ["Expected newline before return statement.", "Expected newline before return statement."]
errors: ["Expected newline before return statement.", "Expected newline before return statement."],
output: "function a() {\n/*comment\ncomment*/\nif (b) {\nc();\n\nreturn b;\n} else {\n//comment\n\nreturn d;\n}\n/*multi-line\ncomment*/\nreturn e;\n}"
},
{
code: "function a() {\nif (b) { return; } //comment\nreturn c;\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nif (b) { return; } //comment\n\nreturn c;\n}"
},
{
code: "function a() {\nif (b) { return; } /*multi-line\ncomment*/\nreturn c;\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nif (b) { return; } /*multi-line\ncomment*/\nreturn c;\n}"
},
{
code: "function a() {\nif (b) { return; }\n/*multi-line\ncomment*/ return c;\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nif (b) { return; }\n/*multi-line\ncomment*/ return c;\n}"
},
{
code: "function a() {\nif (b) { return; } /*multi-line\ncomment*/ return c;\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nif (b) { return; } /*multi-line\ncomment*/ return c;\n}"
},
{
code: "var a;\nreturn;",
parserOptions: { ecmaFeatures: { globalReturn: true } },
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "var a;\n\nreturn;"
},
{
code: "var a; return;",
parserOptions: { ecmaFeatures: { globalReturn: true } },
errors: ["Expected newline before return statement."],
output: "var a; \n\nreturn;"
},
{
code: "function a() {\n{\n//comment\n}\nreturn\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\n{\n//comment\n}\n\nreturn\n}"
},
{
code: "function a() {\n{\n//comment\n} return\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\n{\n//comment\n} \n\nreturn\n}"
},
{
code: "function a() {\nvar c;\nwhile (b) {\n c = d; //comment\n}\nreturn c;\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nvar c;\nwhile (b) {\n c = d; //comment\n}\n\nreturn c;\n}"
},
{
code: "function a() {\nfor (var b; b < c; b++) {\nif (d) {\nbreak; //comment\n}\nreturn;\n}\n}",
errors: ["Expected newline before return statement."]
errors: ["Expected newline before return statement."],
output: "function a() {\nfor (var b; b < c; b++) {\nif (d) {\nbreak; //comment\n}\n\nreturn;\n}\n}"
},

// Testing edge cases of the fixer when the `return` statement has leading comments.
// https://github.com/eslint/eslint/issues/5958
{
code: "function a() {\nvar b; /*multi-line\ncomment*/\nreturn c;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b; /*multi-line\ncomment*/\nreturn c;\n}"
},
{
code: "function a() {\nvar b;\n/*multi-line\ncomment*/ return c;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b;\n/*multi-line\ncomment*/ return c;\n}"
},
{
code: "function a() {\nvar b; /*multi-line\ncomment*/ return c;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b; /*multi-line\ncomment*/ return c;\n}"
},
{
code: "function a() {\nvar b;\n//comment\nreturn;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b;\n//comment\nreturn;\n}"
},
{
code: "function a() {\nvar b; //comment\nreturn;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b; //comment\n\nreturn;\n}"
},
{
code: "function a() {\nvar b;\n/* comment */ return;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b;\n/* comment */ return;\n}"
},
{
code: "function a() {\nvar b;\n//comment\n/* comment */ return;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b;\n//comment\n/* comment */ return;\n}"
},
{
code: "function a() {\nvar b; /* comment */ return;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b; /* comment */ return;\n}"
},
{
code: "function a() {\nvar b; /* comment */\nreturn;\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b; /* comment */\n\nreturn;\n}"
},
{
code: "function a() {\nvar b;\nreturn; //comment\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b;\n\nreturn; //comment\n}"
},
{
code: "function a() {\nvar b; return; //comment\n}",
errors: ["Expected newline before return statement."],
output: "function a() {\nvar b; \n\nreturn; //comment\n}"
}
]
});

0 comments on commit 656bb6e

Please sign in to comment.