Update detection of pure nodes (Scope#isPure) (#14424)

babel · Apr 9, 2022 · 3f1bd8e · 3f1bd8e
1 parent 99842ac
commit 3f1bd8e
Show file tree

Hide file tree

Showing 16 changed files with 163 additions and 78 deletions.
diff --git a/...operator/test/fixtures/hack-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js b/...operator/test/fixtures/hack-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js
@@ -1,8 +1,4 @@
-var _ref2;
+var _ref;
 
-const result = (_ref2 = Math.pow(5, 2), [1, 2, 3].map(n => {
-  var _ref;
-
-  return _ref = (n + _ref2) * 2, `${_ref} apples`.toUpperCase();
-}).join());
+const result = (_ref = Math.pow(5, 2), [1, 2, 3].map(n => `${(n + _ref) * 2} apples`.toUpperCase()).join());
 expect(result).toEqual('52 APPLES,54 APPLES,56 APPLES');
diff --git a/...-proposal-pipeline-operator/test/fixtures/hack-caret/pipe-body-with-nested-pipe/output.js b/...-proposal-pipeline-operator/test/fixtures/hack-caret/pipe-body-with-nested-pipe/output.js
@@ -1,4 +1,4 @@
 var _ref;
 
-const result = (_ref = Math.pow(5, 2) + 1, `${_ref} apples`.toUpperCase());
+const result = (_ref = Math.pow(5, 2), `${_ref + 1} apples`.toUpperCase());
 expect(result).toEqual('26 APPLES');
diff --git a/...ator/test/fixtures/hack-double-at/pipe-body-with-arrow-function-and-nested-pipe/output.js b/...ator/test/fixtures/hack-double-at/pipe-body-with-arrow-function-and-nested-pipe/output.js
@@ -1,8 +1,4 @@
-var _ref2;
+var _ref;
 
-const result = (_ref2 = Math.pow(5, 2), [1, 2, 3].map(n => {
-  var _ref;
-
-  return _ref = (n + _ref2) * 2, `${_ref} apples`.toUpperCase();
-}).join());
+const result = (_ref = Math.pow(5, 2), [1, 2, 3].map(n => `${(n + _ref) * 2} apples`.toUpperCase()).join());
 expect(result).toEqual('52 APPLES,54 APPLES,56 APPLES');
diff --git a/...posal-pipeline-operator/test/fixtures/hack-double-at/pipe-body-with-nested-pipe/output.js b/...posal-pipeline-operator/test/fixtures/hack-double-at/pipe-body-with-nested-pipe/output.js
@@ -1,4 +1,4 @@
 var _ref;
 
-const result = (_ref = Math.pow(5, 2) + 1, `${_ref} apples`.toUpperCase());
+const result = (_ref = Math.pow(5, 2), `${_ref + 1} apples`.toUpperCase());
 expect(result).toEqual('26 APPLES');
diff --git a/...r/test/fixtures/hack-double-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js b/...r/test/fixtures/hack-double-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js
@@ -1,8 +1,4 @@
-var _ref2;
+var _ref;
 
-const result = (_ref2 = Math.pow(5, 2), [1, 2, 3].map(n => {
-  var _ref;
-
-  return _ref = (n + _ref2) * 2, `${_ref} apples`.toUpperCase();
-}).join());
+const result = (_ref = Math.pow(5, 2), [1, 2, 3].map(n => `${(n + _ref) * 2} apples`.toUpperCase()).join());
 expect(result).toEqual('52 APPLES,54 APPLES,56 APPLES');
diff --git a/...al-pipeline-operator/test/fixtures/hack-double-caret/pipe-body-with-nested-pipe/output.js b/...al-pipeline-operator/test/fixtures/hack-double-caret/pipe-body-with-nested-pipe/output.js
@@ -1,4 +1,4 @@
 var _ref;
 
-const result = (_ref = Math.pow(5, 2) + 1, `${_ref} apples`.toUpperCase());
+const result = (_ref = Math.pow(5, 2), `${_ref + 1} apples`.toUpperCase());
 expect(result).toEqual('26 APPLES');
diff --git a/...-operator/test/fixtures/hack-hash/pipe-body-with-arrow-function-and-nested-pipe/output.js b/...-operator/test/fixtures/hack-hash/pipe-body-with-arrow-function-and-nested-pipe/output.js
@@ -1,8 +1,4 @@
-var _ref2;
+var _ref;
 
-const result = (_ref2 = Math.pow(5, 2), [1, 2, 3].map(n => {
-  var _ref;
-
-  return _ref = (n + _ref2) * 2, `${_ref} apples`.toUpperCase();
-}).join());
+const result = (_ref = Math.pow(5, 2), [1, 2, 3].map(n => `${(n + _ref) * 2} apples`.toUpperCase()).join());
 expect(result).toEqual('52 APPLES,54 APPLES,56 APPLES');
diff --git a/...n-proposal-pipeline-operator/test/fixtures/hack-hash/pipe-body-with-nested-pipe/output.js b/...n-proposal-pipeline-operator/test/fixtures/hack-hash/pipe-body-with-nested-pipe/output.js
@@ -1,4 +1,4 @@
 var _ref;
 
-const result = (_ref = Math.pow(5, 2) + 1, `${_ref} apples`.toUpperCase());
+const result = (_ref = Math.pow(5, 2), `${_ref + 1} apples`.toUpperCase());
 expect(result).toEqual('26 APPLES');
diff --git a/...erator/test/fixtures/hack-percent/pipe-body-with-arrow-function-and-nested-pipe/output.js b/...erator/test/fixtures/hack-percent/pipe-body-with-arrow-function-and-nested-pipe/output.js
@@ -1,8 +1,4 @@
-var _ref2;
+var _ref;
 
-const result = (_ref2 = Math.pow(5, 2), [1, 2, 3].map(n => {
-  var _ref;
-
-  return _ref = (n + _ref2) * 2, `${_ref} apples`.toUpperCase();
-}).join());
+const result = (_ref = Math.pow(5, 2), [1, 2, 3].map(n => `${(n + _ref) * 2} apples`.toUpperCase()).join());
 expect(result).toEqual('52 APPLES,54 APPLES,56 APPLES');
diff --git a/...roposal-pipeline-operator/test/fixtures/hack-percent/pipe-body-with-nested-pipe/output.js b/...roposal-pipeline-operator/test/fixtures/hack-percent/pipe-body-with-nested-pipe/output.js
@@ -1,4 +1,4 @@
 var _ref;
 
-const result = (_ref = Math.pow(5, 2) + 1, `${_ref} apples`.toUpperCase());
+const result = (_ref = Math.pow(5, 2), `${_ref + 1} apples`.toUpperCase());
 expect(result).toEqual('26 APPLES');
diff --git a/packages/babel-traverse/scripts/generators/validators.js b/packages/babel-traverse/scripts/generators/validators.js
@@ -24,6 +24,8 @@ export interface NodePathValidators {
       output += `is${type}(opts?: object): this is NodePath<t.${type}>;`;
     } else if (types /* in VirtualTypeAliases */) {
       output += `is${type}(opts?: object): this is NodePath<VirtualTypeAliases["${type}"]>;`;
+    } else if (type === "Pure") {
+      output += `isPure(constantsOnly?: boolean): boolean;`;
     } else {
       // if it don't have types, then VirtualTypeAliases[type] is t.Node
       // which TS marked as always true

diff --git a/packages/babel-traverse/src/path/generated/validators.ts b/packages/babel-traverse/src/path/generated/validators.ts
@@ -441,7 +441,7 @@ export interface NodePathValidators {
   isNumericLiteralTypeAnnotation(
     opts?: object,
   ): this is NodePath<VirtualTypeAliases["NumericLiteralTypeAnnotation"]>;
-  isPure(opts?: object): boolean;
+  isPure(constantsOnly?: boolean): boolean;
   isReferenced(opts?: object): boolean;
   isReferencedIdentifier(
     opts?: object,

diff --git a/packages/babel-traverse/src/path/lib/virtual-types.ts b/packages/babel-traverse/src/path/lib/virtual-types.ts
@@ -124,8 +124,8 @@ export const Generated = {
 };
 
 export const Pure = {
-  checkPath(path: NodePath, opts?): boolean {
-    return path.scope.isPure(path.node, opts);
+  checkPath(path: NodePath, constantsOnly?: boolean): boolean {
+    return path.scope.isPure(path.node, constantsOnly);
   },
 };
 

diff --git a/packages/babel-traverse/src/scope/index.ts b/packages/babel-traverse/src/scope/index.ts
@@ -42,6 +42,12 @@ import {
   unaryExpression,
   variableDeclaration,
   variableDeclarator,
+  isRecordExpression,
+  isTupleExpression,
+  isObjectProperty,
+  isTopicReference,
+  isMetaProperty,
+  isPrivateName,
 } from "@babel/types";
 import type * as t from "@babel/types";
 import { scope as scopeCache } from "../cache";
@@ -535,7 +541,7 @@ export default class Scope {
    */
 
   isStatic(node: t.Node): boolean {
-    if (isThisExpression(node) || isSuper(node)) {
+    if (isThisExpression(node) || isSuper(node) || isTopicReference(node)) {
       return true;
     }
 
@@ -823,10 +829,20 @@ export default class Scope {
       if (!binding) return false;
       if (constantsOnly) return binding.constant;
       return true;
+    } else if (
+      isThisExpression(node) ||
+      isMetaProperty(node) ||
+      isTopicReference(node) ||
+      isPrivateName(node)
+    ) {
+      return true;
     } else if (isClass(node)) {
       if (node.superClass && !this.isPure(node.superClass, constantsOnly)) {
         return false;
       }
+      if (node.decorators?.length > 0) {
+        return false;
+      }
       return this.isPure(node.body, constantsOnly);
     } else if (isClassBody(node)) {
       for (const method of node.body) {
@@ -838,24 +854,34 @@ export default class Scope {
         this.isPure(node.left, constantsOnly) &&
         this.isPure(node.right, constantsOnly)
       );
-    } else if (isArrayExpression(node)) {
+    } else if (isArrayExpression(node) || isTupleExpression(node)) {
       for (const elem of node.elements) {
-        if (!this.isPure(elem, constantsOnly)) return false;
+        if (elem !== null && !this.isPure(elem, constantsOnly)) return false;
       }
       return true;
-    } else if (isObjectExpression(node)) {
+    } else if (isObjectExpression(node) || isRecordExpression(node)) {
       for (const prop of node.properties) {
         if (!this.isPure(prop, constantsOnly)) return false;
       }
       return true;
     } else if (isMethod(node)) {
       if (node.computed && !this.isPure(node.key, constantsOnly)) return false;
-      if (node.kind === "get" || node.kind === "set") return false;
+      if (node.decorators?.length > 0) {
+        return false;
+      }
       return true;
     } else if (isProperty(node)) {
       // @ts-expect-error todo(flow->ts): computed in not present on private properties
       if (node.computed && !this.isPure(node.key, constantsOnly)) return false;
-      return this.isPure(node.value, constantsOnly);
+      if (node.decorators?.length > 0) {
+        return false;
+      }
+      if (isObjectProperty(node) || node.static) {
+        if (node.value !== null && !this.isPure(node.value, constantsOnly)) {
+          return false;
+        }
+      }
+      return true;
     } else if (isUnaryExpression(node)) {
       return this.isPure(node.argument, constantsOnly);
     } else if (isTaggedTemplateExpression(node)) {

diff --git a/packages/babel-traverse/test/path/isPure.js b/packages/babel-traverse/test/path/isPure.js
@@ -0,0 +1,110 @@
+import { parse } from "@babel/parser";
+
+import _traverse from "../../lib/index.js";
+const traverse = _traverse.default;
+
+function getPath(code) {
+  const ast = parse(code, {
+    plugins: [
+      ["decorators", { version: "2021-12", decoratorsBeforeExport: true }],
+      ["recordAndTuple", { syntaxType: "hash" }],
+      "decoratorAutoAccessors",
+      ["pipelineOperator", { proposal: "hack", topicToken: "%" }],
+    ],
+  });
+  let path;
+  traverse(ast, {
+    Program: function (_path) {
+      path = _path;
+      _path.stop();
+    },
+  });
+  return path;
+}
+
+describe("isPure() returns true", () => {
+  it.each([
+    "class C { [0]() {} }",
+    "class C extends class {} {}",
+    "class C { static accessor x = 1; accessor y = f() }",
+    "class C { #x = f(); static #y }",
+    "class C { static target = new.target }",
+    "class X { get foo() { return 1 } set foo(v) {} }",
+    "class C { static #p = #p in C }",
+  ])(`NodePath(%p).get("body.0").isPure() should be true`, input => {
+    const path = getPath(input).get("body.0");
+    expect(path.node).toBeTruthy();
+    expect(path.isPure()).toBe(true);
+  });
+
+  it.each([
+    "({ x: 1, foo() { return 1 } })",
+    "String.raw`foo`",
+    `"a" + "b"`,
+    `[function () {}]`,
+    `#{ 0: 0, 1n: 1, two: "two"}`,
+    `#[0, 1n, "2", \`3\`]`,
+    `[,]`,
+    `-1 || void 0`,
+    `null ?? (true && false)`,
+    `this`,
+  ])(`NodePath(%p).get("body.0.expression").isPure() should be true`, input => {
+    const path = getPath(input).get("body.0.expression");
+    expect(path.node).toBeTruthy();
+    expect(path.isPure()).toBe(true);
+  });
+
+  it.each(["let a = 1; `${a}`", `let a = 1; a |> % + %`])(
+    `NodePath(%p).get("body.1.expression").isPure() should be true`,
+    input => {
+      const path = getPath(input).get("body.1.expression");
+      expect(path.node).toBeTruthy();
+      expect(path.isPure()).toBe(true);
+    },
+  );
+});
+
+describe("isPure() returns false", () => {
+  it.each([
+    "@dec() class X {}",
+    "@dec class C {}; function dec () {}",
+    "class C { @dec foo() {} }",
+    "class C { @dec foo }",
+    "class C { @dec accessor foo = 1 }",
+    "class C { static {} }",
+    "class C extends class { [f()] } {}",
+  ])(`NodePath(%p).get("body.0").isPure() should be false`, input => {
+    const path = getPath(input).get("body.0");
+    expect(path.node).toBeTruthy();
+    expect(path.isPure()).toBe(false);
+  });
+
+  it.each(["`${a}`", "tagged`foo`"])(
+    `NodePath(%p).get("body.0.expression").isPure() should be false`,
+    input => {
+      const path = getPath(input).get("body.0.expression");
+      expect(path.node).toBeTruthy();
+      expect(path.isPure()).toBe(false);
+    },
+  );
+
+  it.each(["let a = 1; `${a++}`"])(
+    `NodePath(%p).get("body.1.expression").isPure() should be false`,
+    input => {
+      const path = getPath(input).get("body.1.expression");
+      expect(path.node).toBeTruthy();
+      expect(path.isPure()).toBe(false);
+    },
+  );
+});
+
+describe("isPure(constantsOnly: true) returns false", () => {
+  it.each(["x", "1 + x", "({ [x]: 0 })", "(class { static x = x })"])(
+    `NodePath(%p).get("body.0.expression").isPure(/* constantsOnly */true) should be false`,
+    input => {
+      const path = getPath(input).get("body.0.expression");
+      expect(path.node).toBeTruthy();
+      expect(path.isPure(true)).toBe(false);
+    },
+  );
+});
diff --git a/packages/babel-traverse/test/scope.js b/packages/babel-traverse/test/scope.js
@@ -365,39 +365,6 @@ describe("scope", () => {
       ).toBe(false);
     });
 
-    it("purity", function () {
-      expect(
-        getPath("({ x: 1, foo() { return 1 } })")
-          .get("body")[0]
-          .get("expression")
-          .isPure(),
-      ).toBeTruthy();
-      expect(
-        getPath("class X { get foo() { return 1 } }")
-          .get("body")[0]
-          .get("expression")
-          .isPure(),
-      ).toBeFalsy();
-      expect(
-        getPath("`${a}`").get("body")[0].get("expression").isPure(),
-      ).toBeFalsy();
-      expect(
-        getPath("let a = 1; `${a}`").get("body")[1].get("expression").isPure(),
-      ).toBeTruthy();
-      expect(
-        getPath("let a = 1; `${a++}`")
-          .get("body")[1]
-          .get("expression")
-          .isPure(),
-      ).toBeFalsy();
-      expect(
-        getPath("tagged`foo`").get("body")[0].get("expression").isPure(),
-      ).toBeFalsy();
-      expect(
-        getPath("String.raw`foo`").get("body")[0].get("expression").isPure(),
-      ).toBeTruthy();
-    });
-
     test("label", function () {
       expect(getPath("foo: { }").scope.getBinding("foo")).toBeUndefined();
       expect(getPath("foo: { }").scope.getLabel("foo").type).toBe(
@@ -542,7 +509,7 @@ describe("scope", () => {
       path.scope.crawl();
       path.scope.crawl();
 
-      expect(path.scope.references._jsx).toBeTruthy();
+      expect(path.scope.references._jsx).toBe(true);
     });
 
     test("generateUid collision check after re-crawling", function () {