From 3f1bd8ec352abf48b05e75c762a50d61876bbcf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hu=C3=A1ng=20J=C3=B9nli=C3=A0ng?= Date: Sat, 9 Apr 2022 11:48:06 -0400 Subject: [PATCH] Update detection of pure nodes (`Scope#isPure`) (#14424) --- .../output.js | 8 +- .../pipe-body-with-nested-pipe/output.js | 2 +- .../output.js | 8 +- .../pipe-body-with-nested-pipe/output.js | 2 +- .../output.js | 8 +- .../pipe-body-with-nested-pipe/output.js | 2 +- .../output.js | 8 +- .../pipe-body-with-nested-pipe/output.js | 2 +- .../output.js | 8 +- .../pipe-body-with-nested-pipe/output.js | 2 +- .../scripts/generators/validators.js | 2 + .../src/path/generated/validators.ts | 2 +- .../src/path/lib/virtual-types.ts | 4 +- packages/babel-traverse/src/scope/index.ts | 38 +++++- packages/babel-traverse/test/path/isPure.js | 110 ++++++++++++++++++ packages/babel-traverse/test/scope.js | 35 +----- 16 files changed, 163 insertions(+), 78 deletions(-) create mode 100644 packages/babel-traverse/test/path/isPure.js diff --git a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js index de6a062dd636..fced893dbedc 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js +++ b/packages/babel-plugin-proposal-pipeline-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/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-caret/pipe-body-with-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-caret/pipe-body-with-nested-pipe/output.js index c877ba76e3ab..ada1ebb0ab10 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-caret/pipe-body-with-nested-pipe/output.js +++ b/packages/babel-plugin-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/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-at/pipe-body-with-arrow-function-and-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-at/pipe-body-with-arrow-function-and-nested-pipe/output.js index de6a062dd636..fced893dbedc 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-at/pipe-body-with-arrow-function-and-nested-pipe/output.js +++ b/packages/babel-plugin-proposal-pipeline-operator/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/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-at/pipe-body-with-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-at/pipe-body-with-nested-pipe/output.js index c877ba76e3ab..ada1ebb0ab10 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-at/pipe-body-with-nested-pipe/output.js +++ b/packages/babel-plugin-proposal-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/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js index de6a062dd636..fced893dbedc 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-caret/pipe-body-with-arrow-function-and-nested-pipe/output.js +++ b/packages/babel-plugin-proposal-pipeline-operator/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/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-caret/pipe-body-with-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-caret/pipe-body-with-nested-pipe/output.js index c877ba76e3ab..ada1ebb0ab10 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-double-caret/pipe-body-with-nested-pipe/output.js +++ b/packages/babel-plugin-proposal-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/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-hash/pipe-body-with-arrow-function-and-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-hash/pipe-body-with-arrow-function-and-nested-pipe/output.js index de6a062dd636..fced893dbedc 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-hash/pipe-body-with-arrow-function-and-nested-pipe/output.js +++ b/packages/babel-plugin-proposal-pipeline-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/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-hash/pipe-body-with-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-hash/pipe-body-with-nested-pipe/output.js index c877ba76e3ab..ada1ebb0ab10 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-hash/pipe-body-with-nested-pipe/output.js +++ b/packages/babel-plugin-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/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-percent/pipe-body-with-arrow-function-and-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-percent/pipe-body-with-arrow-function-and-nested-pipe/output.js index de6a062dd636..fced893dbedc 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-percent/pipe-body-with-arrow-function-and-nested-pipe/output.js +++ b/packages/babel-plugin-proposal-pipeline-operator/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/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-percent/pipe-body-with-nested-pipe/output.js b/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-percent/pipe-body-with-nested-pipe/output.js index c877ba76e3ab..ada1ebb0ab10 100644 --- a/packages/babel-plugin-proposal-pipeline-operator/test/fixtures/hack-percent/pipe-body-with-nested-pipe/output.js +++ b/packages/babel-plugin-proposal-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 index f806fc76c724..f0e4e239d72c 100644 --- 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;`; } else if (types /* in VirtualTypeAliases */) { output += `is${type}(opts?: object): this is NodePath;`; + } 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 index c552da721622..bc1a246fd319 100644 --- 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; - 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 index 58505278b3c7..d08fd554eeef 100644 --- 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 index 7170b5acacca..d6ec6605fbaa 100644 --- 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 new file mode 100644 index 000000000000..70c67f4f6f0c --- /dev/null +++ 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 index e5c2dfaa9611..375089129e16 100644 --- 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 () {