Skip to content

Commit

Permalink
Fix: no-extra-parens incorrect precedence (fixes #7978) (#7999)
Browse files Browse the repository at this point in the history
  • Loading branch information
@alberto @ilyavolodin
alberto authored and ilyavolodin committed Feb 3, 2017
1 parent d6b6ba1 commit f2a3580
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 9 deletions.
12 changes: 7 additions & 5 deletions lib/ast-utils.js
View file
Expand Up @@ -673,6 +673,8 @@ module.exports = {
case "/":
case "%":
return 13;
case "**":
return 15;

// no default
}
Expand All @@ -681,25 +683,25 @@ module.exports = {

case "UnaryExpression":
case "AwaitExpression":
return 14;
return 16;

case "UpdateExpression":
return 15;
return 17;

case "CallExpression":

// IIFE is allowed to have parens in any position (#655)
if (node.callee.type === "FunctionExpression") {
return -1;
}
return 16;
return 18;

case "NewExpression":
return 17;
return 19;

// no default
}
return 18;
return 20;
},

/**
Expand Down
30 changes: 26 additions & 4 deletions lib/rules/no-extra-parens.js
View file
Expand Up @@ -167,6 +167,19 @@ module.exports = {
return false;
}

/**
* Determines if a constructor function is newed-up with parens
* @param {ASTNode} newExpression - The NewExpression node to be checked.
* @returns {boolean} True if the constructor is called with parens.
* @private
*/
function isNewExpressionWithParens(newExpression) {
const lastToken = sourceCode.getLastToken(newExpression);
const penultimateToken = sourceCode.getTokenBefore(lastToken);

return newExpression.arguments.length > 0 || penultimateToken.value === "(" && lastToken.value === ")";
}

/**
* Determines if a node is or contains an assignment expression
* @param {ASTNode} node - The node to be checked.
Expand Down Expand Up @@ -353,6 +366,10 @@ module.exports = {
* @private
*/
function dryUnaryUpdate(node) {
if (node.type === "UnaryExpression" && node.argument.type === "BinaryExpression" && node.argument.operator === "**") {
return;
}

if (hasExcessParens(node.argument) && precedence(node.argument) >= precedence(node)) {
report(node.argument);
}
Expand All @@ -367,7 +384,8 @@ module.exports = {
function dryCallNew(node) {
if (hasExcessParens(node.callee) && precedence(node.callee) >= precedence(node) && !(
node.type === "CallExpression" &&
node.callee.type === "FunctionExpression" &&
(node.callee.type === "FunctionExpression" ||
node.callee.type === "NewExpression" && !isNewExpressionWithParens(node.callee)) &&

// One set of parentheses are allowed for a function expression
!hasDoubleExcessParens(node.callee)
Expand Down Expand Up @@ -395,13 +413,17 @@ module.exports = {
*/
function dryBinaryLogical(node) {
const prec = precedence(node);
const shouldSkipLeft = NESTED_BINARY && (node.left.type === "BinaryExpression" || node.left.type === "LogicalExpression");
const leftPrecedence = precedence(node.left);
const rightPrecedence = precedence(node.right);
const isExponentiation = node.operator === "**";
const shouldSkipLeft = (NESTED_BINARY && (node.left.type === "BinaryExpression" || node.left.type === "LogicalExpression")) ||
node.left.type === "UnaryExpression" && isExponentiation;
const shouldSkipRight = NESTED_BINARY && (node.right.type === "BinaryExpression" || node.right.type === "LogicalExpression");

if (!shouldSkipLeft && hasExcessParens(node.left) && precedence(node.left) >= prec) {
if (!shouldSkipLeft && hasExcessParens(node.left) && (leftPrecedence > prec || (leftPrecedence === prec && !isExponentiation))) {
report(node.left);
}
if (!shouldSkipRight && hasExcessParens(node.right) && precedence(node.right) > prec) {
if (!shouldSkipRight && hasExcessParens(node.right) && (rightPrecedence > prec || (rightPrecedence === prec && isExponentiation))) {
report(node.right);
}
}
Expand Down
25 changes: 25 additions & 0 deletions tests/lib/rules/no-extra-parens.js
View file
Expand Up @@ -89,6 +89,10 @@ ruleTester.run("no-extra-parens", rule, {
"(++a)(b); (c++)(d);",
"new (A())",
"new A()()",
"(new A)()",
"(new (Foo || Bar))()",
{ code: "(2 + 3) ** 4", parserOptions: { ecmaVersion: 7 } },
{ code: "2 ** (2 + 3)", parserOptions: { ecmaVersion: 7 } },

// same precedence
"a, b, c",
Expand Down Expand Up @@ -120,6 +124,8 @@ ruleTester.run("no-extra-parens", rule, {
"a(b)(c)",
"a((b, c))",
"new new A",
{ code: "2 ** 3 ** 4", parserOptions: { ecmaVersion: 7 } },
{ code: "(2 ** 3) ** 4", parserOptions: { ecmaVersion: 7 } },

// constructs that contain expressions
"if(a);",
Expand Down Expand Up @@ -184,6 +190,15 @@ ruleTester.run("no-extra-parens", rule, {
// Object literals as arrow function bodies need parentheses
{ code: "x => ({foo: 1})", parserOptions: { ecmaVersion: 6 } },


// Exponentiation operator `**`
{ code: "1 + 2 ** 3", parserOptions: { ecmaVersion: 7 } },
{ code: "1 - 2 ** 3", parserOptions: { ecmaVersion: 7 } },
{ code: "2 ** -3", parserOptions: { ecmaVersion: 7 } },
{ code: "(-2) ** 3", parserOptions: { ecmaVersion: 7 } },
{ code: "(+2) ** 3", parserOptions: { ecmaVersion: 7 } },
{ code: "+ (2 ** 3)", parserOptions: { ecmaVersion: 7 } },

// https://github.com/eslint/eslint/issues/5789
{ code: "a => ({b: c}[d])", parserOptions: { ecmaVersion: 6 } },
{ code: "a => ({b: c}.d())", parserOptions: { ecmaVersion: 6 } },
Expand Down Expand Up @@ -409,6 +424,11 @@ ruleTester.run("no-extra-parens", rule, {
invalid("a + (b * c)", "a + b * c", "BinaryExpression"),
invalid("(a * b) + c", "a * b + c", "BinaryExpression"),
invalid("(a * b) / c", "a * b / c", "BinaryExpression"),
invalid("(2) ** 3 ** 4", "2 ** 3 ** 4", "Literal", null, { parserOptions: { ecmaVersion: 7 } }),
invalid("2 ** (3 ** 4)", "2 ** 3 ** 4", "BinaryExpression", null, { parserOptions: { ecmaVersion: 7 } }),
invalid("(2 ** 3)", "2 ** 3", "BinaryExpression", null, { parserOptions: { ecmaVersion: 7 } }),
invalid("(2 ** 3) + 1", "2 ** 3 + 1", "BinaryExpression", null, { parserOptions: { ecmaVersion: 7 } }),
invalid("1 - (2 ** 3)", "1 - 2 ** 3", "BinaryExpression", null, { parserOptions: { ecmaVersion: 7 } }),

invalid("a = (b * c)", "a = b * c", "BinaryExpression", null, { options: ["all", { nestedBinaryExpressions: false }] }),
invalid("(b * c)", "b * c", "BinaryExpression", null, { options: ["all", { nestedBinaryExpressions: false }] }),
Expand All @@ -426,6 +446,11 @@ ruleTester.run("no-extra-parens", rule, {
invalid("((function foo() {return 1;}))()", "(function foo() {return 1;})()", "FunctionExpression"),
invalid("((function(){ return bar(); })())", "(function(){ return bar(); })()", "CallExpression"),

invalid("new (A)", "new A", "Identifier"),
invalid("(new A())()", "new A()()", "NewExpression"),
invalid("(new A(1))()", "new A(1)()", "NewExpression"),
invalid("((new A))()", "(new A)()", "NewExpression"),

invalid("0, (_ => 0)", "0, _ => 0", "ArrowFunctionExpression", 1, { parserOptions: { ecmaVersion: 6 } }),
invalid("(_ => 0), 0", "_ => 0, 0", "ArrowFunctionExpression", 1, { parserOptions: { ecmaVersion: 6 } }),
invalid("a = (_ => 0)", "a = _ => 0", "ArrowFunctionExpression", 1, { parserOptions: { ecmaVersion: 6 } }),
Expand Down

0 comments on commit f2a3580

Please sign in to comment.