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

Fix: Account for comments in implicit-arrow-linebreak #10545

Merged
merged 27 commits into from Dec 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4495482
Add and pass for a test case for when a comment exists between an arr…
Jun 29, 2018
7b9bdf4
Fix formatting
Jun 29, 2018
fd31e18
Add and pass test case for when arrow expression is set to variable
Jun 29, 2018
1d216a5
Add test cases for when comments exist within parens enwrapped arrow …
Jun 29, 2018
f1d6e09
Add test cases for block comments, multiple comments within an arrow …
Jun 30, 2018
41f3ec4
Add a test case to ensure filtering of comments between start of arro…
Jun 30, 2018
b418604
Change condition to check for length of comment text length
Jun 30, 2018
1f03b18
Condense logic for comments filtering
Jun 30, 2018
d1f1a43
Reword documentation for helper function
Jun 30, 2018
e6098d1
Account for multi arrow function test case with comments for implicit…
Jul 3, 2018
7ddaa0c
Add test case for when some arrow function bodies are wrapped with pa…
Jul 7, 2018
5d63f71
Export fixer logic for besides to autoFixBesides function, account fo…
Jul 9, 2018
706a48e
REmove extra semicolon from top
Jul 9, 2018
089310a
Add test cases for async arrow functions, method chains, object liter…
Jul 10, 2018
d50c95f
Fix lines for cherry-pick
peanutenthusiast Jul 11, 2018
2daf6c9
Remove line
peanutenthusiast Jul 11, 2018
f52d5d5
Rename fixerTarget to arrowBody for autoFixBesides and addParentheses…
peanutenthusiast Jul 11, 2018
4ee9ac5
Add valid test cases
Aug 22, 2018
37bd7a0
Account for promise constructor and callback test cases
Aug 22, 2018
157fdec
Add test cases for multiline comments
Aug 22, 2018
9452331
Remove dangling commas, add comments and extra details to jsdocs for …
Aug 22, 2018
b247f58
Merge branch 'master' into fix-implicit-arrow-linebreak
peanutenthusiast Aug 27, 2018
6a2039a
Update branch, fix import statement for ast-utils
peanutenthusiast Aug 27, 2018
474c39f
Merge branch 'fix-implicit-arrow-linebreak' of https://github.com/pea…
peanutenthusiast Aug 27, 2018
388db16
Apply requested change to string template for tests on lines 258 and 269
Dec 6, 2018
26635d6
Merge branch 'master' into updated
Dec 18, 2018
65a7b61
Add and pass test cases for variable declarations with multiple decla…
Dec 18, 2018
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
144 changes: 143 additions & 1 deletion lib/rules/implicit-arrow-linebreak.js
Expand Up @@ -4,6 +4,12 @@
*/
"use strict";

const {
isArrowToken,
isParenthesised,
isOpeningParenToken
} = require("../util/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -41,6 +47,142 @@ module.exports = {
return context.options[0] || "beside";
}

/**
* Formats the comments depending on whether it's a line or block comment.
* @param {Comment[]} comments The array of comments between the arrow and body
* @param {Integer} column The column number of the first token
* @returns {string} A string of comment text joined by line breaks
*/
function formatComments(comments, column) {
const whiteSpaces = " ".repeat(column);

return `${comments.map(comment => {

if (comment.type === "Line") {
return `//${comment.value}`;
}

return `/*${comment.value}*/`;
}).join(`\n${whiteSpaces}`)}\n${whiteSpaces}`;
}

/**
* Finds the first token to prepend comments to depending on the parent type
* @param {Node} node The validated node
* @returns {Token|Node} The node to prepend comments to
*/
function findFirstToken(node) {
switch (node.parent.type) {
case "VariableDeclarator":

// If the parent is first or only declarator, return the declaration, else, declarator
return sourceCode.getFirstToken(
node.parent.parent.declarations.length === 1 ||
node.parent.parent.declarations[0].id.name === node.parent.id.name
? node.parent.parent : node.parent
);
case "CallExpression":
case "Property":

// find the object key
return sourceCode.getFirstToken(node.parent);
default:
return node;
}
}

/**
* Helper function for adding parentheses fixes for nodes containing nested arrow functions
* @param {Fixer} fixer Fixer
* @param {Token} arrow - The arrow token
* @param {ASTNode} arrowBody - The arrow function body
* @returns {Function[]} autofixer -- wraps function bodies with parentheses
*/
function addParentheses(fixer, arrow, arrowBody) {
const parenthesesFixes = [];
let closingParentheses = "";

let followingBody = arrowBody;
let currentArrow = arrow;

while (currentArrow) {
if (!isParenthesised(sourceCode, followingBody)) {
parenthesesFixes.push(
fixer.insertTextAfter(currentArrow, " (")
);

const paramsToken = sourceCode.getTokenBefore(currentArrow, token =>
isOpeningParenToken(token) || token.type === "Identifier");

const whiteSpaces = " ".repeat(paramsToken.loc.start.column);

closingParentheses = `\n${whiteSpaces})${closingParentheses}`;
}

currentArrow = sourceCode.getTokenAfter(currentArrow, isArrowToken);

if (currentArrow) {
followingBody = sourceCode.getTokenAfter(currentArrow, token => !isOpeningParenToken(token));
}
}

return [...parenthesesFixes,
fixer.insertTextAfter(arrowBody, closingParentheses)
];
}

/**
* Autofixes the function body to collapse onto the same line as the arrow.
* If comments exist, prepends the comments before the arrow function.
* If the function body contains arrow functions, appends the function bodies with parentheses.
* @param {Token} arrowToken The arrow token.
* @param {ASTNode} arrowBody the function body
* @param {ASTNode} node The evaluated node
* @returns {Function} autofixer -- validates the node to adhere to besides
*/
function autoFixBesides(arrowToken, arrowBody, node) {
return fixer => {
const placeBesides = fixer.replaceTextRange([arrowToken.range[1], arrowBody.range[0]], " ");

const comments = sourceCode.getCommentsInside(node).filter(comment =>
comment.loc.start.line < arrowBody.loc.start.line);

if (comments.length) {

// If the grandparent is not a variable declarator
if (
arrowBody.parent &&
arrowBody.parent.parent &&
arrowBody.parent.parent.type !== "VariableDeclarator"
) {

// If any arrow functions follow, return the necessary parens fixes.
if (sourceCode.getTokenAfter(arrowToken, isArrowToken) && arrowBody.parent.parent.type !== "VariableDeclarator") {
return addParentheses(fixer, arrowToken, arrowBody);
}

// If any arrow functions precede, the necessary fixes have already been returned, so return null.
if (sourceCode.getTokenBefore(arrowToken, isArrowToken) && arrowBody.parent.parent.type !== "VariableDeclarator") {
return null;
}
}

const firstToken = findFirstToken(node);

const commentText = formatComments(comments, firstToken.loc.start.column);

const commentBeforeExpression = fixer.insertTextBeforeRange(
firstToken.range,
commentText
);

return [placeBesides, commentBeforeExpression];
}

return placeBesides;
};
}

/**
* Validates the location of an arrow function body
* @param {ASTNode} node The arrow function body
Expand Down Expand Up @@ -75,7 +217,7 @@ module.exports = {
context.report({
node: fixerTarget,
message: "Expected no linebreak before this expression.",
fix: fixer => fixer.replaceTextRange([tokenBefore.range[1], fixerTarget.range[0]], " ")
fix: autoFixBesides(tokenBefore, fixerTarget, node)
});
}
}
Expand Down