Skip to content

Commit

Permalink
Fix: no-extra-parens autofix with in in a for-loop init (fixes esli…
Browse files Browse the repository at this point in the history 
…nt#11706)
  • Loading branch information
@mdjermanovic
mdjermanovic committed Jun 16, 2019
1 parent 4f48f5a commit b248cc4
Show file tree
Hide file tree
Showing 2 changed files with 313 additions and 19 deletions.
147 changes: 130 additions & 17 deletions lib/rules/no-extra-parens.js
View file
Expand Up @@ -81,6 +81,8 @@ module.exports = {
const PRECEDENCE_OF_ASSIGNMENT_EXPR = precedence({ type: "AssignmentExpression" });
const PRECEDENCE_OF_UPDATE_EXPR = precedence({ type: "UpdateExpression" });

let forStatementInitInfo;

/**
* Determines if this rule should be enforced for a node given the current configuration.
* @param {ASTNode} node - The node to be checked.
Expand Down Expand Up @@ -316,19 +318,35 @@ module.exports = {
}
}

context.report({
node,
loc: leftParenToken.loc.start,
messageId: "unexpected",
fix(fixer) {
const parenthesizedSource = sourceCode.text.slice(leftParenToken.range[1], rightParenToken.range[0]);
/**
* Finishes reporting
* @returns {void}
* @private
*/
function finishReport() {
context.report({
node,
loc: leftParenToken.loc.start,
messageId: "unexpected",
fix(fixer) {
const parenthesizedSource = sourceCode.text.slice(leftParenToken.range[1], rightParenToken.range[0]);

return fixer.replaceTextRange([
leftParenToken.range[0],
rightParenToken.range[1]
], (requiresLeadingSpace(node) ? " " : "") + parenthesizedSource + (requiresTrailingSpace(node) ? " " : ""));
}
});
}

if (forStatementInitInfo) {

return fixer.replaceTextRange([
leftParenToken.range[0],
rightParenToken.range[1]
], (requiresLeadingSpace(node) ? " " : "") + parenthesizedSource + (requiresTrailingSpace(node) ? " " : ""));
}
});
// This node is within ForStatement.init, reports will be flushed on exit
forStatementInitInfo.reports.push({ node, finishReport });
return;
}

finishReport();
}

/**
Expand Down Expand Up @@ -498,6 +516,32 @@ module.exports = {
}
}

/**
* Finds the path from the given node to the specified ancestor.
* @param {ASTNode} node First node in the path.
* @param {ASTNode} ancestor Last node in the path.
* @returns {ASTNode[]} Path, including both nodes.
* @throws {Error} If the given node does not have the specified ancestor.
*/
function pathToAncestor(node, ancestor) {
const path = [node];
let currentNode = node;

while (currentNode !== ancestor) {

currentNode = currentNode.parent;

/* istanbul ignore if */
if (currentNode === null) {
throw new Error("node doesn't have the specified ancestor");
}

path.push(currentNode);
}

return path;
}

return {
ArrayExpression(node) {
node.elements
Expand Down Expand Up @@ -540,7 +584,14 @@ module.exports = {
}
},

BinaryExpression: checkBinaryLogical,
BinaryExpression(node) {
if (forStatementInitInfo && node.operator === "in") {
forStatementInitInfo.inExpressionNodes.push(node);
}

checkBinaryLogical(node);
},

CallExpression: checkCallNew,

ConditionalExpression(node) {
Expand Down Expand Up @@ -602,17 +653,79 @@ module.exports = {
},

ForStatement(node) {
if (node.init && hasExcessParens(node.init)) {
report(node.init);
}

if (node.test && hasExcessParens(node.test) && !isCondAssignException(node)) {
report(node.test);
}

if (node.update && hasExcessParens(node.update)) {
report(node.update);
}

if (node.init) {
forStatementInitInfo = {
upper: forStatementInitInfo,
inExpressionNodes: [],
reports: []
};

if (hasExcessParens(node.init)) {
report(node.init);
}
}
},

"ForStatement > *.init:exit"(node) {

/*
* Removing parentheses around `in` expressions might change semantics and cause errors.
*
* For example, this valid for loop:
* for (let a = (b in c); ;);
* after removing parentheses would be treated as an invalid for-in loop:
* for (let a = b in c; ;);
*/

let { reports } = forStatementInitInfo;

if (reports.length) {

const { inExpressionNodes } = forStatementInitInfo;

inExpressionNodes.forEach(inExpressionNode => {
for (const pathNode of pathToAncestor(inExpressionNode, node).reverse()) {
if (isParenthesised(pathNode)) {

/*
* If this node was supposed to be reported, exclude it from the list
* (i.e. treat parentheses as necessary)
*/
reports = reports.filter(w => w.node !== pathNode);

/*
* In any case, this node is parenthesised and will stay parenthesised.
* All extra parentheses inside can be safely removed.
*/
return;
}

/*
* Other enclosing punctuators such as {} are omitted from this check,
* in order to avoid dependencies with other rules.
*
* For example, these parentheses might be safe for removal:
* for (let a = b => { return (b in c) }; ;);
* However, arrow-body-style could remove braces as well, and produce an invalid for-in loop:
* for (let a = b => b in c; ;);
*/
}
});

// flush remaining reports
reports.forEach(({ finishReport }) => finishReport());
}

// ForStatement can be inside a function/arrow expression in another ForStatement.init
forStatementInitInfo = forStatementInitInfo.upper;
},

IfStatement(node) {
Expand Down
185 changes: 183 additions & 2 deletions tests/lib/rules/no-extra-parens.js
View file
Expand Up @@ -466,7 +466,31 @@ ruleTester.run("no-extra-parens", rule, {
"for ((let) in foo);",
"for ((let[foo]) in bar);",
"for ((let)[foo] in bar);",
"for ((let[foo].bar) in baz);"
"for ((let[foo].bar) in baz);",

// https://github.com/eslint/eslint/issues/11706 (also in invalid[])
"for (let a = (b in c); ;);",
"for (let a = (b && c in d); ;);",
"for (let a = (b in c && d); ;);",
"for (let a = (b => b in c); ;);",
"for (let a = b => (b in c); ;);",
"for (let a = b => { return (b in c) }; ;);",
"for (let a = (b in c in d); ;);",
"for (let a = (b in c), d = (e in f); ;);",
"for (let a = (b => c => b in c); ;);",
"for (let a = (b && c && d in e); ;);",
"for (let a = b && (c in d); ;);",
"for (let a = (b in c) && (d in e); ;);",
"for (let a = (b in c), d = () => { for ((e in f);;); }; ;);",
"for ((a in b); ;);",
"for (a = (b in c); ;);",
"for ((a in b && c in d && e in f); ;);",

// https://github.com/eslint/eslint/issues/11706 regression tests (also in invalid[])
"for (let a = b; a; a); a; a;",
"for (a; a; a); a; a;",
"for (; a; a); a; a;",
"for (let a = (b && c) === d; ;);"
],

invalid: [
Expand Down Expand Up @@ -1105,6 +1129,163 @@ ruleTester.run("no-extra-parens", rule, {
"(let)",
"Identifier",
1
)
),

// https://github.com/eslint/eslint/issues/11706 (also in valid[])
{
code: "for ((a = (b in c)); ;);",
output: "for ((a = b in c); ;);",
errors: [
{
messageId: "unexpected"
}
]
},
{
code: "for (let a = ((b in c) && (d in e)); ;);",
output: "for (let a = (b in c && d in e); ;);",
errors: Array(2).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (let a = ((b in c) in d); ;);",
output: "for (let a = (b in c in d); ;);",
errors: [
{
messageId: "unexpected"
}
]
},
{
code: "for (let a = (b && (c in d)), e = (f in g); ;);",
output: "for (let a = (b && c in d), e = (f in g); ;);",
errors: [
{
messageId: "unexpected"
}
]
},
{
code: "for (let a = (b + c), d = (e in f); ;);",
output: "for (let a = b + c, d = (e in f); ;);",
errors: [
{
messageId: "unexpected"
}
]
},

// https://github.com/eslint/eslint/issues/11706 regression tests (also in valid[])
{
code: "for (let a = (b); a > (b); a = (b)) a = (b); a = (b);",
output: "for (let a = b; a > b; a = b) a = b; a = b;",
errors: Array(5).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for ((a = b); (a > b); (a = b)) (a = b); (a = b);",
output: "for (a = b; a > b; a = b) a = b; a = b;",
errors: Array(5).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (let a = b; a > (b); a = (b)) a = (b); a = (b);",
output: "for (let a = b; a > b; a = b) a = b; a = b;",
errors: Array(4).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (let a = b; (a > b); (a = b)) (a = b); (a = b);",
output: "for (let a = b; a > b; a = b) a = b; a = b;",
errors: Array(4).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (; a > (b); a = (b)) a = (b); a = (b);",
output: "for (; a > b; a = b) a = b; a = b;",
errors: Array(4).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (; (a > b); (a = b)) (a = b); (a = b);",
output: "for (; a > b; a = b) a = b; a = b;",
errors: Array(4).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (let a = (b); a = (b in c); a = (b in c)) a = (b in c); a = (b in c);",
output: "for (let a = b; a = b in c; a = b in c) a = b in c; a = b in c;",
errors: Array(5).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (let a = (b); (a in b); (a in b)) (a in b); (a in b);",
output: "for (let a = b; a in b; a in b) a in b; a in b;",
errors: Array(5).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (let a = b; a = (b in c); a = (b in c)) a = (b in c); a = (b in c);",
output: "for (let a = b; a = b in c; a = b in c) a = b in c; a = b in c;",
errors: Array(4).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (let a = b; (a in b); (a in b)) (a in b); (a in b);",
output: "for (let a = b; a in b; a in b) a in b; a in b;",
errors: Array(4).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (; a = (b in c); a = (b in c)) a = (b in c); a = (b in c);",
output: "for (; a = b in c; a = b in c) a = b in c; a = b in c;",
errors: Array(4).fill(
{
messageId: "unexpected"
}
)
},
{
code: "for (; (a in b); (a in b)) (a in b); (a in b);",
output: "for (; a in b; a in b) a in b; a in b;",
errors: Array(4).fill(
{
messageId: "unexpected"
}
)
}
]
});

0 comments on commit b248cc4

Please sign in to comment.