From f4f53c591b110e6604dacdeaf8fe0078d3d00b47 Mon Sep 17 00:00:00 2001 From: Jacob Williams Date: Sun, 2 Feb 2020 16:28:05 -0800 Subject: [PATCH 1/5] Change isReferenced to return false for object/class method parameter names. --- .../src/validators/isReferenced.js | 8 +++ packages/babel-types/test/validators.js | 68 +++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/packages/babel-types/src/validators/isReferenced.js b/packages/babel-types/src/validators/isReferenced.js index 699b291a01d6..a7b0a478fb2b 100644 --- a/packages/babel-types/src/validators/isReferenced.js +++ b/packages/babel-types/src/validators/isReferenced.js @@ -57,6 +57,7 @@ export default function isReferenced( case "ClassPrivateProperty": // no: class { NODE() {} } // yes: class { [NODE]() {} } + // no: class { foo(NODE) {} } case "ClassMethod": case "ClassPrivateMethod": case "ObjectMethod": @@ -66,6 +67,13 @@ export default function isReferenced( if (parent.value === node) { return !grandparent || grandparent.type !== "ObjectPattern"; } + if (parent.params) { + for (const param of parent.params) { + if (param === node) { + return false; + } + } + } return true; // no: class NODE {} diff --git a/packages/babel-types/test/validators.js b/packages/babel-types/test/validators.js index f08dedab1a87..b8ff2e7d8b04 100644 --- a/packages/babel-types/test/validators.js +++ b/packages/babel-types/test/validators.js @@ -179,6 +179,74 @@ describe("validators", function() { expect(t.isReferenced(node, parent)).toBe(true); }); }); + + describe("ObjectMethod", function() { + it("returns false if node is method key", function() { + const node = t.identifier("A"); + const parent = t.objectMethod("method", node, [], t.blockStatement([])); + + expect(t.isReferenced(node, parent)).toBe(false); + }); + + it("returns true if node is computed method key", function() { + const node = t.identifier("A"); + const parent = t.objectMethod( + "method", + node, + [], + t.blockStatement([]), + true, + ); + + expect(t.isReferenced(node, parent)).toBe(true); + }); + + it("returns false if node is method param", function() { + const node = t.identifier("A"); + const parent = t.objectMethod( + "method", + t.identifier("foo"), + [node], + t.blockStatement([]), + ); + + expect(t.isReferenced(node, parent)).toBe(false); + }); + }); + + describe("ClassMethod", function() { + it("returns false if node is method key", function() { + const node = t.identifier("A"); + const parent = t.classMethod("method", node, [], t.blockStatement([])); + + expect(t.isReferenced(node, parent)).toBe(false); + }); + + it("returns true if node is computed method key", function() { + const node = t.identifier("A"); + const parent = t.classMethod( + "method", + node, + [], + t.blockStatement([]), + true, + ); + + expect(t.isReferenced(node, parent)).toBe(true); + }); + + it("returns false if node is method param", function() { + const node = t.identifier("A"); + const parent = t.classMethod( + "method", + t.identifier("foo"), + [node], + t.blockStatement([]), + ); + + expect(t.isReferenced(node, parent)).toBe(false); + }); + }); }); describe("isBinding", function() { From 97765975636a2d00b952a7f3bdfded1fdd351f70 Mon Sep 17 00:00:00 2001 From: Jacob Williams Date: Mon, 3 Feb 2020 11:36:25 -0800 Subject: [PATCH 2/5] use indexOf instead of for-of loop --- packages/babel-types/src/validators/isReferenced.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/packages/babel-types/src/validators/isReferenced.js b/packages/babel-types/src/validators/isReferenced.js index a7b0a478fb2b..79e5c8a6765e 100644 --- a/packages/babel-types/src/validators/isReferenced.js +++ b/packages/babel-types/src/validators/isReferenced.js @@ -67,12 +67,8 @@ export default function isReferenced( if (parent.value === node) { return !grandparent || grandparent.type !== "ObjectPattern"; } - if (parent.params) { - for (const param of parent.params) { - if (param === node) { - return false; - } - } + if (parent.params && parent.params.indexOf(node) !== -1) { + return false; } return true; From f81b3cb791311eb0e695d1e850b602eb7ea55c1a Mon Sep 17 00:00:00 2001 From: Jacob Williams Date: Mon, 3 Feb 2020 12:17:11 -0800 Subject: [PATCH 3/5] replace `.indexOf` check with `.includes` and assume `parent.params` exists Co-Authored-By: Justin Ridgewell --- packages/babel-types/src/validators/isReferenced.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/babel-types/src/validators/isReferenced.js b/packages/babel-types/src/validators/isReferenced.js index 79e5c8a6765e..f148f99d6879 100644 --- a/packages/babel-types/src/validators/isReferenced.js +++ b/packages/babel-types/src/validators/isReferenced.js @@ -67,7 +67,7 @@ export default function isReferenced( if (parent.value === node) { return !grandparent || grandparent.type !== "ObjectPattern"; } - if (parent.params && parent.params.indexOf(node) !== -1) { + if (parent.params.includes(node)) { return false; } return true; From 94addd0a95902a5ebef0debb2ff452bbfbf41796 Mon Sep 17 00:00:00 2001 From: Jacob Williams Date: Mon, 3 Feb 2020 21:12:24 -0800 Subject: [PATCH 4/5] check .params within case block for ClassMethod/ClassPrivateMethod/ObjectMethod only --- .../babel-types/src/validators/isReferenced.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/babel-types/src/validators/isReferenced.js b/packages/babel-types/src/validators/isReferenced.js index f148f99d6879..1f91a0d634dc 100644 --- a/packages/babel-types/src/validators/isReferenced.js +++ b/packages/babel-types/src/validators/isReferenced.js @@ -45,6 +45,15 @@ export default function isReferenced( case "PrivateName": return false; + // no: class { NODE() {} } + // yes: class { [NODE]() {} } + // no: class { foo(NODE) {} } + case "ClassMethod": + case "ClassPrivateMethod": + case "ObjectMethod": + if (parent.params.includes(node)) { + return false; + } // yes: { [NODE]: "" } // no: { NODE: "" } // depends: { NODE } @@ -55,21 +64,12 @@ export default function isReferenced( // yes: class { key = NODE; } case "ClassProperty": case "ClassPrivateProperty": - // no: class { NODE() {} } - // yes: class { [NODE]() {} } - // no: class { foo(NODE) {} } - case "ClassMethod": - case "ClassPrivateMethod": - case "ObjectMethod": if (parent.key === node) { return !!parent.computed; } if (parent.value === node) { return !grandparent || grandparent.type !== "ObjectPattern"; } - if (parent.params.includes(node)) { - return false; - } return true; // no: class NODE {} From ef3c0e515267087c3c850f9572740a09160b900c Mon Sep 17 00:00:00 2001 From: Jacob Williams Date: Tue, 4 Feb 2020 08:11:43 -0800 Subject: [PATCH 5/5] add comment clarifying that case clause fall-through is intentional --- packages/babel-types/src/validators/isReferenced.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/babel-types/src/validators/isReferenced.js b/packages/babel-types/src/validators/isReferenced.js index 1f91a0d634dc..847738fa4c43 100644 --- a/packages/babel-types/src/validators/isReferenced.js +++ b/packages/babel-types/src/validators/isReferenced.js @@ -54,6 +54,8 @@ export default function isReferenced( if (parent.params.includes(node)) { return false; } + // Fall-through to next case clause to check whether the node is the method's name. + // yes: { [NODE]: "" } // no: { NODE: "" } // depends: { NODE }