From ce6d5c574ceac1e6f7c274d62becec12b14c62cd Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sat, 30 May 2020 19:00:39 +0900 Subject: [PATCH 1/6] Fix: false positive new with member in no-extra-parens (fixes #12740) --- lib/rules/no-extra-parens.js | 35 +++++++++++++++++++++++++++--- tests/lib/rules/no-extra-parens.js | 18 ++++++++++++++- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 7cbb7522ebe..b77a168ffa8 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -687,6 +687,34 @@ module.exports = { reportsBuffer.reports = reportsBuffer.reports.filter(r => r.node !== node); } + /** + * Checks whether a node is a MemberExpression at NewExpression's callee. + * @param {ASTNode} node node to check. + * @returns {boolean} True if the node is a MemberExpression at NewExpression's callee. false otherwise. + */ + function isMemberInNewCallee(node) { + if (node.type === "MemberExpression") { + return node.parent.type === "NewExpression" && node.parent.callee === node + ? true + : isMemberInNewCallee(node.parent); + } + return false; + } + + /** + * Checks whether a MemberExpression node contains a CallExpression in member chaining. + * @param {ASTNode} node node to check + * @returns {boolean} True if the MemberExpression contains a CallExpression in member chaining. false otherwise. + */ + function hasCallExpInMemberExp(node) { + if (node.type === "MemberExpression") { + return node.object.type === "CallExpression" + ? true + : hasCallExpInMemberExp(node.object); + } + return false; + } + return { ArrayExpression(node) { node.elements @@ -927,7 +955,8 @@ module.exports = { LogicalExpression: checkBinaryLogical, MemberExpression(node) { - const nodeObjHasExcessParens = hasExcessParens(node.object); + const shouldAllowWrapOnce = isMemberInNewCallee(node) && hasCallExpInMemberExp(node); + const nodeObjHasExcessParens = shouldAllowWrapOnce ? hasDoubleExcessParens(node.object) : hasExcessParens(node.object); if ( nodeObjHasExcessParens && @@ -946,8 +975,8 @@ module.exports = { } if (nodeObjHasExcessParens && - node.object.type === "CallExpression" && - node.parent.type !== "NewExpression") { + node.object.type === "CallExpression" + ) { report(node.object); } diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index 5062857e699..e838b15bae7 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -589,7 +589,15 @@ ruleTester.run("no-extra-parens", rule, { "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; ;);" + "for (let a = (b && c) === d; ;);", + + "new (a()).b.c;", + "new (a().b).c;", + "new (a().b.c);", + "new (a().b().d);", + "new a().b().d;", + "new (a(b()).c)", + "new (a.b()).c" ], invalid: [ @@ -741,6 +749,14 @@ ruleTester.run("no-extra-parens", rule, { invalid("((new A))()", "(new A)()", "NewExpression"), invalid("new (foo\n.baz\n.bar\n.foo.baz)", "new foo\n.baz\n.bar\n.foo.baz", "MemberExpression"), invalid("new (foo.baz.bar.baz)", "new foo.baz.bar.baz", "MemberExpression"), + invalid("new ((a.b())).c", "new (a.b()).c", "CallExpression"), + invalid("new ((a().b)).c", "new (a().b).c", "MemberExpression"), + invalid("new ((a().b().d))", "new (a().b().d)", "MemberExpression"), + invalid("new ((a())).b.d", "new (a()).b.d", "CallExpression"), + invalid("new (a.b).d;", "new a.b.d;", "MemberExpression"), + invalid("(a().b).d;", "a().b.d;", "MemberExpression"), + invalid("(a.b()).d;", "a.b().d;", "CallExpression"), + invalid("(a.b).d;", "a.b.d;", "MemberExpression"), invalid("0, (_ => 0)", "0, _ => 0", "ArrowFunctionExpression", 1), invalid("(_ => 0), 0", "_ => 0, 0", "ArrowFunctionExpression", 1), From 0511b82316137bb163b99222d8f5fbbeddd24bd8 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sat, 30 May 2020 19:06:17 +0900 Subject: [PATCH 2/6] rename func --- lib/rules/no-extra-parens.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index b77a168ffa8..7fb4ca14135 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -692,11 +692,11 @@ module.exports = { * @param {ASTNode} node node to check. * @returns {boolean} True if the node is a MemberExpression at NewExpression's callee. false otherwise. */ - function isMemberInNewCallee(node) { + function isMemberExpInNewCallee(node) { if (node.type === "MemberExpression") { return node.parent.type === "NewExpression" && node.parent.callee === node ? true - : isMemberInNewCallee(node.parent); + : isMemberExpInNewCallee(node.parent); } return false; } @@ -955,7 +955,7 @@ module.exports = { LogicalExpression: checkBinaryLogical, MemberExpression(node) { - const shouldAllowWrapOnce = isMemberInNewCallee(node) && hasCallExpInMemberExp(node); + const shouldAllowWrapOnce = isMemberExpInNewCallee(node) && hasCallExpInMemberExp(node); const nodeObjHasExcessParens = shouldAllowWrapOnce ? hasDoubleExcessParens(node.object) : hasExcessParens(node.object); if ( From e64726a3af7563948d391ddfdba6e281d8b3bcbb Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 18 Jun 2020 22:35:23 +0900 Subject: [PATCH 3/6] remove duplicate function --- lib/rules/no-extra-parens.js | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 9c92d6529d2..2bba7fda38f 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -724,20 +724,6 @@ module.exports = { return false; } - /** - * Checks whether a MemberExpression node contains a CallExpression in member chaining. - * @param {ASTNode} node node to check - * @returns {boolean} True if the MemberExpression contains a CallExpression in member chaining. false otherwise. - */ - function hasCallExpInMemberExp(node) { - if (node.type === "MemberExpression") { - return node.object.type === "CallExpression" - ? true - : hasCallExpInMemberExp(node.object); - } - return false; - } - return { ArrayExpression(node) { node.elements @@ -978,7 +964,7 @@ module.exports = { LogicalExpression: checkBinaryLogical, MemberExpression(node) { - const shouldAllowWrapOnce = isMemberExpInNewCallee(node) && hasCallExpInMemberExp(node); + const shouldAllowWrapOnce = isMemberExpInNewCallee(node) && doesMemberExpressionContainCallExpression(node); const nodeObjHasExcessParens = shouldAllowWrapOnce ? hasDoubleExcessParens(node.object) : hasExcessParens(node.object) && From e19df656aa64ec78055f272f1ee2178e8d7ebd4d Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 18 Jun 2020 22:55:24 +0900 Subject: [PATCH 4/6] check whole node is wrapped or not --- lib/rules/no-extra-parens.js | 4 +++- tests/lib/rules/no-extra-parens.js | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 2bba7fda38f..1474504b483 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -964,7 +964,9 @@ module.exports = { LogicalExpression: checkBinaryLogical, MemberExpression(node) { - const shouldAllowWrapOnce = isMemberExpInNewCallee(node) && doesMemberExpressionContainCallExpression(node); + const shouldAllowWrapOnce = isMemberExpInNewCallee(node) && + doesMemberExpressionContainCallExpression(node) && + !hasExcessParens(node); const nodeObjHasExcessParens = shouldAllowWrapOnce ? hasDoubleExcessParens(node.object) : hasExcessParens(node.object) && diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index 0d51f824e47..dbf58a7541f 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -785,6 +785,7 @@ ruleTester.run("no-extra-parens", rule, { invalid("new ((a().b().d))", "new (a().b().d)", "MemberExpression"), invalid("new ((a())).b.d", "new (a()).b.d", "CallExpression"), invalid("new (a.b).d;", "new a.b.d;", "MemberExpression"), + invalid("new ((a()).b).c", "new (a().b).c", "CallExpression"), invalid("(a().b).d;", "a().b.d;", "MemberExpression"), invalid("(a.b()).d;", "a.b().d;", "CallExpression"), invalid("(a.b).d;", "a.b.d;", "MemberExpression"), From 69a4d61c0525839b2bfb1889a87146a8a982df67 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 18 Jun 2020 23:13:39 +0900 Subject: [PATCH 5/6] remove checking whole node --- lib/rules/no-extra-parens.js | 3 +-- tests/lib/rules/no-extra-parens.js | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 1474504b483..63a40284c9b 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -965,8 +965,7 @@ module.exports = { MemberExpression(node) { const shouldAllowWrapOnce = isMemberExpInNewCallee(node) && - doesMemberExpressionContainCallExpression(node) && - !hasExcessParens(node); + doesMemberExpressionContainCallExpression(node); const nodeObjHasExcessParens = shouldAllowWrapOnce ? hasDoubleExcessParens(node.object) : hasExcessParens(node.object) && diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index dbf58a7541f..0d51f824e47 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -785,7 +785,6 @@ ruleTester.run("no-extra-parens", rule, { invalid("new ((a().b().d))", "new (a().b().d)", "MemberExpression"), invalid("new ((a())).b.d", "new (a()).b.d", "CallExpression"), invalid("new (a.b).d;", "new a.b.d;", "MemberExpression"), - invalid("new ((a()).b).c", "new (a().b).c", "CallExpression"), invalid("(a().b).d;", "a().b.d;", "MemberExpression"), invalid("(a.b()).d;", "a.b().d;", "CallExpression"), invalid("(a.b).d;", "a.b.d;", "MemberExpression"), From 482656ef0ad285328359e1c85825c9f05e74b2d7 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sun, 21 Jun 2020 20:57:22 +0900 Subject: [PATCH 6/6] check member object --- lib/rules/no-extra-parens.js | 2 +- tests/lib/rules/no-extra-parens.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-extra-parens.js b/lib/rules/no-extra-parens.js index 63a40284c9b..1ece81eee81 100644 --- a/lib/rules/no-extra-parens.js +++ b/lib/rules/no-extra-parens.js @@ -719,7 +719,7 @@ module.exports = { if (node.type === "MemberExpression") { return node.parent.type === "NewExpression" && node.parent.callee === node ? true - : isMemberExpInNewCallee(node.parent); + : node.parent.object === node && isMemberExpInNewCallee(node.parent); } return false; } diff --git a/tests/lib/rules/no-extra-parens.js b/tests/lib/rules/no-extra-parens.js index 0d51f824e47..e2501bb45a0 100644 --- a/tests/lib/rules/no-extra-parens.js +++ b/tests/lib/rules/no-extra-parens.js @@ -767,6 +767,7 @@ ruleTester.run("no-extra-parens", rule, { invalid("(new foo(bar)).baz", "new foo(bar).baz", "NewExpression"), invalid("(new foo.bar()).baz", "new foo.bar().baz", "NewExpression"), invalid("(new foo.bar()).baz()", "new foo.bar().baz()", "NewExpression"), + invalid("new a[(b()).c]", "new a[b().c]", "CallExpression"), invalid("(a)()", "a()", "Identifier"), invalid("(a.b)()", "a.b()", "MemberExpression"),