diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 265ca4fea7e..e0ab32b4edd 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -169,6 +169,28 @@ module.exports = { return ruleApplies(node) && isParenthesisedTwice(node); } + /** + * Determines if a node that is expected to be parenthesised is surrounded by + * (potentially) invalid extra parentheses with considering precedence level of the node. + * If the preference level of the node is not higher or equal to precedence lower limit, it also checks + * whether the node is surrounded by parentheses twice or not. + * @param {ASTNode} node The node to be checked. + * @param {number} precedenceLowerLimit The lower limit of precedence. + * @returns {boolean} True if the node is has an unexpected extra pair of parentheses. + * @private + */ + function hasExcessParensWithPrecedence(node, precedenceLowerLimit) { + if (ruleApplies(node) && isParenthesised(node)) { + if ( + precedence(node) >= precedenceLowerLimit || + isParenthesisedTwice(node) + ) { + return true; + } + } + return false; + } + /** * Determines if a node test expression is allowed to have a parenthesised assignment * @param {ASTNode} node The node to be checked. @@ -370,17 +392,13 @@ module.exports = { } /** - * Evaluate Unary update + * Evaluate a argument of the node. * @param {ASTNode} node node to evaluate * @returns {void} * @private */ - function checkUnaryUpdate(node) { - if (node.type === "UnaryExpression" && node.argument.type === "BinaryExpression" && node.argument.operator === "**") { - return; - } - - if (hasExcessParens(node.argument) && precedence(node.argument) >= precedence(node)) { + function checkArgumentWithPrecedence(node) { + if (hasExcessParensWithPrecedence(node.argument, precedence(node))) { report(node.argument); } } @@ -411,7 +429,7 @@ module.exports = { function checkCallNew(node) { const callee = node.callee; - if (hasExcessParens(callee) && precedence(callee) >= precedence(node)) { + if (hasExcessParensWithPrecedence(callee, precedence(node))) { const hasNewParensException = callee.type === "NewExpression" && !isNewExpressionWithParens(callee); if ( @@ -428,7 +446,7 @@ module.exports = { } } node.arguments - .filter(arg => hasExcessParens(arg) && precedence(arg) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) + .filter(arg => hasExcessParensWithPrecedence(arg, PRECEDENCE_OF_ASSIGNMENT_EXPR)) .forEach(report); } @@ -443,15 +461,26 @@ module.exports = { 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 shouldSkipLeft = NESTED_BINARY && (node.left.type === "BinaryExpression" || node.left.type === "LogicalExpression"); const shouldSkipRight = NESTED_BINARY && (node.right.type === "BinaryExpression" || node.right.type === "LogicalExpression"); - if (!shouldSkipLeft && hasExcessParens(node.left) && (leftPrecedence > prec || (leftPrecedence === prec && !isExponentiation))) { - report(node.left); + if (!shouldSkipLeft && hasExcessParens(node.left)) { + if ( + !(node.left.type === "UnaryExpression" && isExponentiation) && + (leftPrecedence > prec || (leftPrecedence === prec && !isExponentiation)) || + isParenthesisedTwice(node.left) + ) { + report(node.left); + } } - if (!shouldSkipRight && hasExcessParens(node.right) && (rightPrecedence > prec || (rightPrecedence === prec && isExponentiation))) { - report(node.right); + + if (!shouldSkipRight && hasExcessParens(node.right)) { + if ( + (rightPrecedence > prec || (rightPrecedence === prec && isExponentiation)) || + isParenthesisedTwice(node.right) + ) { + report(node.right); + } } } @@ -484,11 +513,7 @@ module.exports = { * @returns {void} */ function checkSpreadOperator(node) { - const hasExtraParens = precedence(node.argument) >= PRECEDENCE_OF_ASSIGNMENT_EXPR - ? hasExcessParens(node.argument) - : hasDoubleExcessParens(node.argument); - - if (hasExtraParens) { + if (hasExcessParensWithPrecedence(node.argument, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { report(node.argument); } } @@ -650,7 +675,7 @@ module.exports = { return { ArrayExpression(node) { node.elements - .filter(e => e && hasExcessParens(e) && precedence(e) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) + .filter(e => e && hasExcessParensWithPrecedence(e, PRECEDENCE_OF_ASSIGNMENT_EXPR)) .forEach(report); }, @@ -673,18 +698,14 @@ module.exports = { if (astUtils.isOpeningParenToken(tokenBeforeFirst) && astUtils.isOpeningBraceToken(firstBodyToken)) { tokensToIgnore.add(firstBodyToken); } - if (hasExcessParens(node.body) && precedence(node.body) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) { + if (hasExcessParensWithPrecedence(node.body, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { report(node.body); } } }, AssignmentExpression(node) { - if (isReturnAssignException(node)) { - return; - } - - if (hasExcessParens(node.right) && precedence(node.right) >= precedence(node)) { + if (!isReturnAssignException(node) && hasExcessParensWithPrecedence(node.right, precedence(node))) { report(node.right); } }, @@ -701,8 +722,8 @@ module.exports = { ClassBody(node) { node.body - .filter(member => member.type === "MethodDefinition" && member.computed && - member.key && hasExcessParens(member.key) && precedence(member.key) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) + .filter(member => member.type === "MethodDefinition" && member.computed && member.key) + .filter(member => hasExcessParensWithPrecedence(member.key, PRECEDENCE_OF_ASSIGNMENT_EXPR)) .forEach(member => report(member.key)); }, @@ -710,16 +731,18 @@ module.exports = { if (isReturnAssignException(node)) { return; } - - if (hasExcessParens(node.test) && precedence(node.test) >= precedence({ type: "LogicalExpression", operator: "||" })) { + if ( + !isCondAssignException(node) && + hasExcessParensWithPrecedence(node.test, precedence({ type: "LogicalExpression", operator: "||" })) + ) { report(node.test); } - if (hasExcessParens(node.consequent) && precedence(node.consequent) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) { + if (hasExcessParensWithPrecedence(node.consequent, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { report(node.consequent); } - if (hasExcessParens(node.alternate) && precedence(node.alternate) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) { + if (hasExcessParensWithPrecedence(node.alternate, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { report(node.alternate); } }, @@ -756,9 +779,19 @@ module.exports = { tokensToIgnore.add(firstLeftToken); } } - if (!(node.type === "ForOfStatement" && node.right.type === "SequenceExpression") && hasExcessParens(node.right)) { + + if (node.type === "ForOfStatement") { + const hasExtraParens = node.right.type === "SequenceExpression" + ? hasDoubleExcessParens(node.right) + : hasExcessParens(node.right); + + if (hasExtraParens) { + report(node.right); + } + } else if (hasExcessParens(node.right)) { report(node.right); } + if (hasExcessParens(node.left)) { report(node.left); } @@ -910,18 +943,15 @@ module.exports = { ObjectExpression(node) { node.properties - .filter(property => { - const value = property.value; - - return value && hasExcessParens(value) && precedence(value) >= PRECEDENCE_OF_ASSIGNMENT_EXPR; - }).forEach(property => report(property.value)); + .filter(property => property.value && hasExcessParensWithPrecedence(property.value, PRECEDENCE_OF_ASSIGNMENT_EXPR)) + .forEach(property => report(property.value)); }, Property(node) { if (node.computed) { const { key } = node; - if (key && hasExcessParens(key) && precedence(key) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) { + if (key && hasExcessParensWithPrecedence(key, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { report(key); } } @@ -944,8 +974,10 @@ module.exports = { }, SequenceExpression(node) { + const precedenceOfNode = precedence(node); + node.expressions - .filter(e => hasExcessParens(e) && precedence(e) >= precedence(node)) + .filter(e => hasExcessParensWithPrecedence(e, precedenceOfNode)) .forEach(report); }, @@ -969,16 +1001,17 @@ module.exports = { } }, - UnaryExpression: checkUnaryUpdate, - UpdateExpression: checkUnaryUpdate, - AwaitExpression: checkUnaryUpdate, + UnaryExpression: checkArgumentWithPrecedence, + UpdateExpression: checkArgumentWithPrecedence, + AwaitExpression: checkArgumentWithPrecedence, VariableDeclarator(node) { - if (node.init && hasExcessParens(node.init) && - precedence(node.init) >= PRECEDENCE_OF_ASSIGNMENT_EXPR && + if ( + node.init && hasExcessParensWithPrecedence(node.init, PRECEDENCE_OF_ASSIGNMENT_EXPR) && - // RegExp literal is allowed to have parens (#1589) - !(node.init.type === "Literal" && node.init.regex)) { + // RegExp literal is allowed to have parens (#1589) + !(node.init.type === "Literal" && node.init.regex) + ) { report(node.init); } }, @@ -1023,7 +1056,7 @@ module.exports = { AssignmentPattern(node) { const { right } = node; - if (right && hasExcessParens(right) && precedence(right) >= PRECEDENCE_OF_ASSIGNMENT_EXPR) { + if (right && hasExcessParensWithPrecedence(right, PRECEDENCE_OF_ASSIGNMENT_EXPR)) { report(right); } } diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index 707992f80a7..26f682bae08 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -199,6 +199,7 @@ ruleTester.run("no-extra-parens", rule, { "var regex = (/^a$/);", "function a(){ return (/^a$/); }", "function a(){ return (/^a$/).test('a'); }", + "var isA = ((/^a$/)).test('a');", // IIFE is allowed to have parens in any position (#655) "var foo = (function() { return bar(); }())", @@ -250,17 +251,22 @@ ruleTester.run("no-extra-parens", rule, { { code: "var a = (b = c)", options: ["functions"] }, { code: "_ => (a = 0)", options: ["functions"] }, - // ["all", {conditionalAssign: false}] enables extra parens around conditional assignments + // ["all", { conditionalAssign: false }] enables extra parens around conditional assignments { code: "while ((foo = bar())) {}", options: ["all", { conditionalAssign: false }] }, { code: "if ((foo = bar())) {}", options: ["all", { conditionalAssign: false }] }, { code: "do; while ((foo = bar()))", options: ["all", { conditionalAssign: false }] }, { code: "for (;(a = b););", options: ["all", { conditionalAssign: false }] }, + { code: "var a = ((b = c)) ? foo : bar;", options: ["all", { conditionalAssign: false }] }, // ["all", { nestedBinaryExpressions: false }] enables extra parens around conditional assignments { code: "a + (b * c)", options: ["all", { nestedBinaryExpressions: false }] }, { code: "(a * b) + c", options: ["all", { nestedBinaryExpressions: false }] }, { code: "(a * b) / c", options: ["all", { nestedBinaryExpressions: false }] }, { code: "a || (b && c)", options: ["all", { nestedBinaryExpressions: false }] }, + { code: "a + ((b * c))", options: ["all", { nestedBinaryExpressions: false }] }, + { code: "((a * b)) + c", options: ["all", { nestedBinaryExpressions: false }] }, + { code: "((a * b)) / c", options: ["all", { nestedBinaryExpressions: false }] }, + { code: "a || ((b && c))", options: ["all", { nestedBinaryExpressions: false }] }, // ["all", { returnAssign: false }] enables extra parens around expressions returned by return statements { code: "function a(b) { return b || c; }", options: ["all", { returnAssign: false }] }, @@ -276,6 +282,8 @@ ruleTester.run("no-extra-parens", rule, { { code: "b => { return (b = 1) };", options: ["all", { returnAssign: false }] }, { code: "b => { return (b = c) || (b = d) };", options: ["all", { returnAssign: false }] }, { code: "b => { return c ? (d = b) : (e = b) };", options: ["all", { returnAssign: false }] }, + { code: "function a(b) { return ((b = 1)); }", options: ["all", { returnAssign: false }] }, + { code: "b => ((b = 1));", options: ["all", { returnAssign: false }] }, // https://github.com/eslint/eslint/issues/3653 "(function(){}).foo(), 1, 2;", @@ -352,6 +360,8 @@ ruleTester.run("no-extra-parens", rule, { { code: "foo in (bar in baz)", options: ["all", { nestedBinaryExpressions: false }] }, { code: "foo + (bar + baz)", options: ["all", { nestedBinaryExpressions: false }] }, { code: "foo && (bar && baz)", options: ["all", { nestedBinaryExpressions: false }] }, + { code: "((foo instanceof bar)) instanceof baz", options: ["all", { nestedBinaryExpressions: false }] }, + { code: "((foo in bar)) in baz", options: ["all", { nestedBinaryExpressions: false }] }, // https://github.com/eslint/eslint/issues/9019 "(async function() {});", @@ -452,7 +462,9 @@ ruleTester.run("no-extra-parens", rule, { // ["all", { enforceForSequenceExpressions: false }] { code: "(a, b)", options: ["all", { enforceForSequenceExpressions: false }] }, + { code: "((a, b))", options: ["all", { enforceForSequenceExpressions: false }] }, { code: "(foo(), bar());", options: ["all", { enforceForSequenceExpressions: false }] }, + { code: "((foo(), bar()));", options: ["all", { enforceForSequenceExpressions: false }] }, { code: "if((a, b)){}", options: ["all", { enforceForSequenceExpressions: false }] }, { code: "while ((val = foo(), val < 10));", options: ["all", { enforceForSequenceExpressions: false }] }, @@ -463,6 +475,7 @@ ruleTester.run("no-extra-parens", rule, { { code: "(new foo(bar)).baz", options: ["all", { enforceForNewInMemberExpressions: false }] }, { code: "(new foo.bar()).baz", options: ["all", { enforceForNewInMemberExpressions: false }] }, { code: "(new foo.bar()).baz()", options: ["all", { enforceForNewInMemberExpressions: false }] }, + { code: "((new foo.bar())).baz()", options: ["all", { enforceForNewInMemberExpressions: false }] }, "let a = [ ...b ]", "let a = { ...b }", @@ -481,6 +494,7 @@ ruleTester.run("no-extra-parens", rule, { "const A = class extends B {}", "class A extends (B=C) {}", "const A = class extends (B=C) {}", + "class A extends (++foo) {}", "() => ({ foo: 1 })", "() => ({ foo: 1 }).foo", "() => ({ foo: 1 }.foo().bar).baz.qux()", @@ -631,11 +645,15 @@ ruleTester.run("no-extra-parens", rule, { invalid("-(-foo)", "- -foo", "UnaryExpression"), invalid("+(-foo)", "+-foo", "UnaryExpression"), invalid("-(+foo)", "-+foo", "UnaryExpression"), + invalid("-((bar+foo))", "-(bar+foo)", "BinaryExpression"), + invalid("+((bar-foo))", "+(bar-foo)", "BinaryExpression"), invalid("++(foo)", "++foo", "Identifier"), invalid("--(foo)", "--foo", "Identifier"), invalid("(a || b) ? c : d", "a || b ? c : d", "LogicalExpression"), invalid("a ? (b = c) : d", "a ? b = c : d", "AssignmentExpression"), invalid("a ? b : (c = d)", "a ? b : c = d", "AssignmentExpression"), + invalid("(c = d) ? (b) : c", "(c = d) ? b : c", "Identifier", null, { options: ["all", { conditionalAssign: false }] }), + invalid("(c = d) ? b : (c)", "(c = d) ? b : c", "Identifier", null, { options: ["all", { conditionalAssign: false }] }), invalid("f((a = b))", "f(a = b)", "AssignmentExpression"), invalid("a, (b = c)", "a, b = c", "AssignmentExpression"), invalid("a = (b * c)", "a = b * c", "BinaryExpression"), @@ -647,6 +665,9 @@ ruleTester.run("no-extra-parens", rule, { invalid("(2 ** 3)", "2 ** 3", "BinaryExpression", null), invalid("(2 ** 3) + 1", "2 ** 3 + 1", "BinaryExpression", null), invalid("1 - (2 ** 3)", "1 - 2 ** 3", "BinaryExpression", null), + invalid("-((2 ** 3))", "-(2 ** 3)", "BinaryExpression", null), + invalid("typeof ((a ** b));", "typeof (a ** b);", "BinaryExpression", null), + invalid("((-2)) ** 3", "(-2) ** 3", "UnaryExpression", null), invalid("a = (b * c)", "a = b * c", "BinaryExpression", null, { options: ["all", { nestedBinaryExpressions: false }] }), invalid("(b * c)", "b * c", "BinaryExpression", null, { options: ["all", { nestedBinaryExpressions: false }] }), @@ -735,6 +756,7 @@ ruleTester.run("no-extra-parens", rule, { invalid("bar((class{}).foo(), 0);", "bar(class{}.foo(), 0);", "ClassExpression", null), invalid("bar[(class{}).foo()];", "bar[class{}.foo()];", "ClassExpression", null), invalid("var bar = (class{}).foo();", "var bar = class{}.foo();", "ClassExpression", null), + invalid("var foo = ((bar, baz));", "var foo = (bar, baz);", "SequenceExpression", null), // https://github.com/eslint/eslint/issues/4608 invalid("function *a() { yield (b); }", "function *a() { yield b; }", "Identifier", null), @@ -1023,6 +1045,7 @@ ruleTester.run("no-extra-parens", rule, { invalid("async function a() { await (a()); }", "async function a() { await a(); }", "CallExpression", null), invalid("async function a() { await (+a); }", "async function a() { await +a; }", "UnaryExpression", null), invalid("async function a() { +(await a); }", "async function a() { +await a; }", "AwaitExpression", null), + invalid("async function a() { await ((a,b)); }", "async function a() { await (a,b); }", "SequenceExpression", null), invalid("(foo) instanceof bar", "foo instanceof bar", "Identifier", 1, { options: ["all", { nestedBinaryExpressions: false }] }), invalid("(foo) in bar", "foo in bar", "Identifier", 1, { options: ["all", { nestedBinaryExpressions: false }] }), invalid("(foo) + bar", "foo + bar", "Identifier", 1, { options: ["all", { nestedBinaryExpressions: false }] }), @@ -1300,6 +1323,12 @@ ruleTester.run("no-extra-parens", rule, { "AssignmentExpression", 1 ), + invalid( + "class A extends ((++foo)) {}", + "class A extends (++foo) {}", + "UpdateExpression", + 1 + ), invalid( "for (foo of(bar));", "for (foo of bar);", @@ -1312,6 +1341,12 @@ ruleTester.run("no-extra-parens", rule, { "Identifier", 1 ), + invalid( + "for (foo of ((bar, baz)));", + "for (foo of (bar, baz));", + "SequenceExpression", + 1 + ), invalid( "for ((foo)in bar);", "for (foo in bar);", @@ -2041,6 +2076,98 @@ ruleTester.run("no-extra-parens", rule, { "SequenceExpression", 1, { parserOptions: { ecmaVersion: 2020 } } - ) + ), + + // https://github.com/eslint/eslint/issues/12127 + { + code: "[1, ((2, 3))];", + output: "[1, (2, 3)];", + errors: [{ messageId: "unexpected" }] + }, + { + code: "const foo = () => ((bar, baz));", + output: "const foo = () => (bar, baz);", + errors: [{ messageId: "unexpected" }] + }, + { + code: "foo = ((bar, baz));", + output: "foo = (bar, baz);", + errors: [{ messageId: "unexpected" }] + }, + { + code: "foo + ((bar + baz));", + output: "foo + (bar + baz);", + errors: [{ messageId: "unexpected" }] + }, + { + code: "((foo + bar)) + baz;", + output: "(foo + bar) + baz;", + errors: [{ messageId: "unexpected" }] + }, + { + code: "foo * ((bar + baz));", + output: "foo * (bar + baz);", + errors: [{ messageId: "unexpected" }] + }, + { + code: "((foo + bar)) * baz;", + output: "(foo + bar) * baz;", + errors: [{ messageId: "unexpected" }] + }, + { + code: "new A(((foo, bar)))", + output: "new A((foo, bar))", + errors: [{ messageId: "unexpected" }] + }, + { + code: "class A{ [((foo, bar))]() {} }", + output: "class A{ [(foo, bar)]() {} }", + errors: [{ messageId: "unexpected" }] + }, + { + code: "new ((A, B))()", + output: "new (A, B)()", + errors: [{ messageId: "unexpected" }] + }, + { + code: "((foo, bar)) ? bar : baz;", + output: "(foo, bar) ? bar : baz;", + errors: [{ messageId: "unexpected" }] + }, + { + code: "((f ? o : o)) ? bar : baz;", + output: "(f ? o : o) ? bar : baz;", + errors: [{ messageId: "unexpected" }] + }, + { + code: "((f = oo)) ? bar : baz;", + output: "(f = oo) ? bar : baz;", + errors: [{ messageId: "unexpected" }] + }, + { + code: "foo ? ((bar, baz)) : baz;", + output: "foo ? (bar, baz) : baz;", + errors: [{ messageId: "unexpected" }] + }, + { + code: "foo ? bar : ((bar, baz));", + output: "foo ? bar : (bar, baz);", + errors: [{ messageId: "unexpected" }] + }, + { + code: "function foo(bar = ((baz1, baz2))) {}", + output: "function foo(bar = (baz1, baz2)) {}", + errors: [{ messageId: "unexpected" }] + }, + { + code: "var foo = { bar: ((baz1, baz2)) };", + output: "var foo = { bar: (baz1, baz2) };", + errors: [{ messageId: "unexpected" }] + }, + { + code: "var foo = { [((bar1, bar2))]: baz };", + output: "var foo = { [(bar1, bar2)]: baz };", + errors: [{ messageId: "unexpected" }] + } ] });