Skip to content

Commit

Permalink
Chore: refactor fix of arrow-body-style
Browse files Browse the repository at this point in the history
  • Loading branch information
@mysticatea
mysticatea committed Apr 18, 2017
1 parent 926b00a commit 98e31c3
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 46 deletions.
126 changes: 83 additions & 43 deletions lib/rules/arrow-body-style.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,29 @@ module.exports = {
const requireReturnForObjectLiteral = options[1] && options[1].requireReturnForObjectLiteral;
const sourceCode = context.getSourceCode();

/**
* Checks whether the given node has ASI problem or not.
* @param {Token} token The token to check.
* @returns {boolean} `true` if it changes semantics if `;` or `}` followed by the token are removed.
*/
function hasASIProblem(token) {
return token && token.type === "Punctuator" && /^[([/`+-]/.test(token.value);
}

/**
* Gets the closing parenthesis which is the pair of the given opening parenthesis.
* @param {Token} token The opening parenthesis token to get.
* @returns {Token} The found closing parenthesis token.
*/
function findClosingParen(token) {
let node = sourceCode.getNodeByRangeIndex(token.range[1]);

while (!astUtils.isParenthesised(sourceCode, node)) {
node = node.parent;
}
return sourceCode.getTokenAfter(node);
}

/**
* Determines whether a arrow function body needs braces
* @param {ASTNode} node The arrow function node.
Expand All @@ -91,47 +114,55 @@ module.exports = {
loc: arrowBody.loc.start,
message: "Unexpected block statement surrounding arrow body.",
fix(fixer) {
if (blockBody.length !== 1 || blockBody[0].type !== "ReturnStatement" || !blockBody[0].argument) {
return null;
const fixes = [];

if (blockBody.length !== 1 ||
blockBody[0].type !== "ReturnStatement" ||
!blockBody[0].argument ||
hasASIProblem(sourceCode.getTokenAfter(arrowBody))
) {
return fixes;
}

const sourceText = sourceCode.getText();
const returnKeyword = sourceCode.getFirstToken(blockBody[0]);
const firstValueToken = sourceCode.getTokenAfter(returnKeyword);
let lastValueToken = sourceCode.getLastToken(blockBody[0]);

if (astUtils.isSemicolonToken(lastValueToken)) {

/* The last token of the returned value is the last token of the ReturnExpression (if
* the ReturnExpression has no semicolon), or the second-to-last token (if the ReturnExpression
* has a semicolon).
*/
lastValueToken = sourceCode.getTokenBefore(lastValueToken);
const openingBrace = sourceCode.getFirstToken(arrowBody);
const closingBrace = sourceCode.getLastToken(arrowBody);
const firstValueToken = sourceCode.getFirstToken(blockBody[0], 1);
const lastValueToken = sourceCode.getLastToken(blockBody[0]);
const commentsExist =
sourceCode.commentsExistBetween(openingBrace, firstValueToken) ||
sourceCode.commentsExistBetween(lastValueToken, closingBrace);

// Remove tokens around the return value.
// If comments don't exist, remove extra spaces as well.
if (commentsExist) {
fixes.push(
fixer.remove(openingBrace),
fixer.remove(closingBrace),
fixer.remove(sourceCode.getTokenAfter(openingBrace)) // return keyword
);
} else {
fixes.push(
fixer.removeRange([openingBrace.range[0], firstValueToken.range[0]]),
fixer.removeRange([lastValueToken.range[1], closingBrace.range[1]])
);
}

const tokenAfterArrowBody = sourceCode.getTokenAfter(arrowBody);

if (tokenAfterArrowBody && tokenAfterArrowBody.type === "Punctuator" && /^[([/`+-]/.test(tokenAfterArrowBody.value)) {
// If the first token of the reutrn value is `{`,
// enclose the return value by parentheses to avoid syntax error.
if (astUtils.isOpeningBraceToken(firstValueToken)) {
fixes.push(
fixer.insertTextBefore(firstValueToken, "("),
fixer.insertTextAfter(lastValueToken, ")")
);
}

// Don't do a fix if the next token would cause ASI issues when preceded by the returned value.
return null;
// If the last token of the return statement is semicolon, remove it.
// Non-block arrow body is an expression, not a statement.
if (astUtils.isSemicolonToken(lastValueToken)) {
fixes.push(fixer.remove(lastValueToken));
}

const textBeforeReturn = sourceText.slice(arrowBody.range[0] + 1, returnKeyword.range[0]);
const textBetweenReturnAndValue = sourceText.slice(returnKeyword.range[1], firstValueToken.range[0]);
const rawReturnValueText = sourceText.slice(firstValueToken.range[0], lastValueToken.range[1]);
const returnValueText = astUtils.isOpeningBraceToken(firstValueToken) ? `(${rawReturnValueText})` : rawReturnValueText;
const textAfterValue = sourceText.slice(lastValueToken.range[1], blockBody[0].range[1] - 1);
const textAfterReturnStatement = sourceText.slice(blockBody[0].range[1], arrowBody.range[1] - 1);

/*
* For fixes that only contain spaces around the return value, remove the extra spaces.
* This avoids ugly fixes that end up with extra spaces after the arrow, e.g. `() => 0 ;`
*/
return fixer.replaceText(
arrowBody,
(textBeforeReturn + textBetweenReturnAndValue).replace(/^\s*$/, "") + returnValueText + (textAfterValue + textAfterReturnStatement).replace(/^\s*$/, "")
);
return fixes;
}
});
}
Expand All @@ -141,22 +172,31 @@ module.exports = {
node,
loc: arrowBody.loc.start,
message: "Expected block statement surrounding arrow body.",
fix(fixer) {
const lastTokenBeforeBody = sourceCode.getLastTokenBetween(sourceCode.getFirstToken(node), arrowBody, astUtils.isNotOpeningParenToken);
const firstBodyToken = sourceCode.getTokenAfter(lastTokenBeforeBody);

return fixer.replaceTextRange(
[firstBodyToken.range[0], node.range[1]],
`{return ${sourceCode.getText().slice(firstBodyToken.range[0], node.range[1])}}`
);
*fix(fixer) {
const arrowToken = sourceCode.getTokenBefore(arrowBody, astUtils.isArrowToken);
const firstBodyToken = sourceCode.getTokenAfter(arrowToken);
const lastBodyToken = sourceCode.getLastToken(node);
const isParenthesisedObjectLiteral =
astUtils.isOpeningParenToken(firstBodyToken) &&
astUtils.isOpeningBraceToken(sourceCode.getTokenAfter(firstBodyToken));

// Wrap the value by a block and a return statement.
yield fixer.insertTextBefore(firstBodyToken, "{return ");
yield fixer.insertTextAfter(lastBodyToken, "}");

// If the value is object literal, remove parentheses which were forced by syntax.
if (isParenthesisedObjectLiteral) {
yield fixer.remove(firstBodyToken);
yield fixer.remove(findClosingParen(firstBodyToken));
}
}
});
}
}
}

return {
ArrowFunctionExpression: validate
"ArrowFunctionExpression:exit": validate
};
}
};
21 changes: 21 additions & 0 deletions lib/token-store/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const assert = require("assert");
const cursors = require("./cursors");
const ForwardTokenCursor = require("./forward-token-cursor");
const PaddedTokenCursor = require("./padded-token-cursor");
const utils = require("./utils");

//------------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -540,4 +541,24 @@ module.exports = class TokenStore {
padding
).getAllTokens();
}

//--------------------------------------------------------------------------
// Others.
//--------------------------------------------------------------------------

/**
* Checks whether any comments exist or not between the given 2 nodes.
*
* @param {ASTNode} left - The node to check.
* @param {ASTNode} right - The node to check.
* @returns {boolean} `true` if one or more comments exist.
*/
commentsExistBetween(left, right) {
const index = utils.search(this[COMMENTS], left.range[1]);

return (
index < this[COMMENTS].length &&
this[COMMENTS][index].range[1] <= right.range[0]
);
}
};
18 changes: 15 additions & 3 deletions tests/lib/rules/arrow-body-style.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ ruleTester.run("arrow-body-style", rule, {
},
{
code: "var foo = () => ({});",
output: "var foo = () => {return ({})};",
output: "var foo = () => {return {}};",
options: ["always"],
errors: [
{ line: 1, column: 18, type: "ArrowFunctionExpression", message: "Expected block statement surrounding arrow body." }
Expand Down Expand Up @@ -160,15 +160,15 @@ ruleTester.run("arrow-body-style", rule, {
},
{
code: "var foo = () => ({});",
output: "var foo = () => {return ({})};",
output: "var foo = () => {return {}};",
options: ["as-needed", { requireReturnForObjectLiteral: true }],
errors: [
{ line: 1, column: 18, type: "ArrowFunctionExpression", message: "Expected block statement surrounding arrow body." }
]
},
{
code: "var foo = () => ({ bar: 0 });",
output: "var foo = () => {return ({ bar: 0 })};",
output: "var foo = () => {return { bar: 0 }};",
options: ["as-needed", { requireReturnForObjectLiteral: true }],
errors: [
{ line: 1, column: 18, type: "ArrowFunctionExpression", message: "Expected block statement surrounding arrow body." }
Expand Down Expand Up @@ -290,6 +290,18 @@ ruleTester.run("arrow-body-style", rule, {
errors: [
{ line: 2, column: 31, type: "ArrowFunctionExpression", message: "Unexpected block statement surrounding arrow body." }
]
},
{
code: "var foo = () => ({foo: 1}).foo();",
output: "var foo = () => {return {foo: 1}.foo()};",
options: ["always"],
errors: ["Expected block statement surrounding arrow body."]
},
{
code: "var foo = () => ({foo: 1}.foo());",
output: "var foo = () => {return {foo: 1}.foo()};",
options: ["always"],
errors: ["Expected block statement surrounding arrow body."]
}
]
});
13 changes: 13 additions & 0 deletions tests/lib/token-store.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1272,4 +1272,17 @@ describe("TokenStore", () => {
);
});
});

describe("when calling commentsExistBetween", () => {

it("should retrieve false if comments don't exist", () => {
assert.isFalse(store.commentsExistBetween(AST.tokens[0], AST.tokens[1]));
});

it("should retrieve true if comments exist", () => {
assert.isTrue(store.commentsExistBetween(AST.tokens[1], AST.tokens[2]));
});

});

});

0 comments on commit 98e31c3

Please sign in to comment.