Classes cleanup (#7737)

* Cleanup

* Move verifyConstructorVisitor out of closure

packages/babel-helper-replace-supers/src/index.js

@@ -3,9 +3,6 @@ import traverse from "@babel/traverse";
 import optimiseCall from "@babel/helper-optimise-call-expression";
 import * as t from "@babel/types";
 
-// ✌️
-const HARDCORE_THIS_REF = new WeakSet();
-
 /**
  * Creates an expression which result is the proto of objectRef.
  *
@@ -65,20 +62,13 @@ const visitor = traverse.visitors.merge([
   environmentVisitor,
   {
     ReturnStatement(path, state) {
+      // TODO get this shit out of here
       if (!path.getFunctionParent().isArrowFunctionExpression()) {
         state.returns.push(path);
       }
     },
 
-    ThisExpression(path, state) {
-      if (!HARDCORE_THIS_REF.has(path.node)) {
-        state.thises.push(path);
-      }
-    },
-
     Super(path, state) {
-      state.hasSuper = true;
-
       const { node, parentPath } = path;
       if (parentPath.isCallExpression({ callee: node })) {
         state.bareSupers.add(parentPath);
@@ -90,44 +80,37 @@ const visitor = traverse.visitors.merge([
 ]);
 
 export default class ReplaceSupers {
-  constructor(opts: Object, inClass?: boolean = false) {
+  constructor(opts: Object) {
-    this.forceSuperMemoisation = opts.forceSuperMemoisation;
+    const path = opts.methodPath;
-    this.methodPath = opts.methodPath;
+
-    this.methodNode = opts.methodNode;
+    this.methodPath = path;
-    this.superRef = opts.superRef;
+    this.isStatic =
-    this.isStatic = opts.isStatic;
+      path.isClassMethod({ static: true }) || path.isObjectMethod();
-    this.hasSuper = false;
+    this.inClass = path.isClassMethod();
-    this.inClass = inClass;
+    this.inConstructor = path.isClassMethod({ kind: "constructor" });
-    this.inConstructor = opts.inConstructor;
-    this.isLoose = opts.isLoose;
     this.scope = this.methodPath.scope;
+
     this.file = opts.file;
+    this.superRef = opts.superRef;
+    this.isLoose = opts.isLoose;
     this.opts = opts;
 
     this.bareSupers = new Set();
     this.returns = [];
-    this.thises = [];
   }
 
-  forceSuperMemoisation: boolean;
   methodPath: NodePath;
-  methodNode: Object;
   superRef: Object;
   isStatic: boolean;
-  hasSuper: boolean;
   inClass: boolean;
   inConstructor: boolean;
   isLoose: boolean;
   scope: Scope;
   file;
   opts: {
-    forceSuperMemoisation: boolean,
     getObjetRef: Function,
     methodPath: NodePath,
-    methodNode: Object,
     superRef: Object,
-    inConstructor: boolean,
-    isStatic: boolean,
     isLoose: boolean,
     file: any,
   };
@@ -255,7 +238,9 @@ export default class ReplaceSupers {
     //   true,
     // );
     // TODO this needs cleanup. Should be a single proto lookup
-    const ref = path.scope.generateDeclaredUidIdentifier("ref");
+    const { scope } = path;
+    const ref = scope.generateUidIdentifierBasedOnNode(node);
+    scope.push({ id: ref });
     const setter = this.setSuperProperty(
       property,
       t.binaryExpression(operator.slice(0, -1), t.cloneNode(ref), node.right),
@@ -333,8 +318,6 @@ export default class ReplaceSupers {
   }
 
   optimiseCall(callee, args) {
-    const thisNode = t.thisExpression();
+    return optimiseCall(callee, t.thisExpression(), args);
-    HARDCORE_THIS_REF.add(thisNode);
-    return optimiseCall(callee, thisNode, args);
   }
 }

packages/babel-plugin-transform-classes/src/transformClass.js

@@ -19,6 +19,41 @@ function buildConstructor(classRef, constructorBody, node) {
   return func;
 }
 
+const verifyConstructorVisitor = traverse.visitors.merge([
+  environmentVisitor,
+  {
+    Super(path, state) {
+      if (state.isDerived) return;
+
+      const { node, parentPath } = path;
+      if (parentPath.isCallExpression({ callee: node })) {
+        throw path.buildCodeFrameError(
+          "super() is only allowed in a derived constructor",
+        );
+      }
+    },
+
+    ThisExpression(path, state) {
+      if (!state.isDerived) return;
+
+      const { node, parentPath } = path;
+      if (parentPath.isMemberExpression({ object: node })) {
+        // In cases like this.foo or this[foo], there is no need to add
+        // assertThisInitialized, since they already throw if this is
+        // undefined.
+        return;
+      }
+
+      const assertion = t.callExpression(
+        state.file.addHelper("assertThisInitialized"),
+        [node],
+      );
+      path.replaceWith(assertion);
+      path.skip();
+    },
+  },
+]);
+
 export default function transformClass(
   path: NodePath,
   file: any,
@@ -55,10 +90,7 @@ export default function transformClass(
     pushedInherits: false,
     protoAlias: null,
     isLoose: false,
-    hasBareSuper: false,
 
-    instanceInitializersId: undefined,
-    staticInitializersId: undefined,
     hasInstanceDescriptors: false,
     hasStaticDescriptors: false,
     instanceMutatorMap: {},
@@ -69,43 +101,6 @@ export default function transformClass(
     Object.assign(classState, newState);
   };
 
-  const verifyConstructorVisitor = traverse.visitors.merge([
-    environmentVisitor,
-    {
-      CallExpression: {
-        exit(path) {
-          if (path.get("callee").isSuper()) {
-            setState({ hasBareSuper: true });
-
-            if (!classState.isDerived) {
-              throw path.buildCodeFrameError(
-                "super() is only allowed in a derived constructor",
-              );
-            }
-          }
-        },
-      },
-
-      ThisExpression(path) {
-        if (classState.isDerived) {
-          if (path.parentPath.isMemberExpression({ object: path.node })) {
-            // In cases like this.foo or this[foo], there is no need to add
-            // assertThisInitialized, since they already throw if this is
-            // undefined.
-            return;
-          }
-
-          const assertion = t.callExpression(
-            classState.file.addHelper("assertThisInitialized"),
-            [path.node],
-          );
-          path.replaceWith(assertion);
-          path.skip();
-        }
-      },
-    },
-  ]);
-
   const findThisesVisitor = traverse.visitors.merge([
     environmentVisitor,
     {
@@ -206,24 +201,19 @@ export default function transformClass(
         const isConstructor = node.kind === "constructor";
 
         if (isConstructor) {
-          path.traverse(verifyConstructorVisitor);
+          path.traverse(verifyConstructorVisitor, {
+            isDerived: classState.isDerived,
+            file: classState.file,
+          });
         }
 
-        const replaceSupers = new ReplaceSupers(
+        const replaceSupers = new ReplaceSupers({
-          {
+          methodPath: path,
-            forceSuperMemoisation: isConstructor,
+          objectRef: classState.classRef,
-            methodPath: path,
+          superRef: classState.superName,
-            methodNode: node,
+          isLoose: classState.isLoose,
-            objectRef: classState.classRef,
+          file: classState.file,
-            superRef: classState.superName,
+        });
-            inConstructor: isConstructor,
-            isStatic: node.static,
-            isLoose: classState.isLoose,
-            scope: classState.scope,
-            file: classState.file,
-          },
-          true,
-        );
 
         replaceSupers.replace();
 
@@ -273,23 +263,11 @@ export default function transformClass(
         t.cloneNode(classState.classRef), // Constructor
         t.nullLiteral(), // instanceDescriptors
         t.nullLiteral(), // staticDescriptors
-        t.nullLiteral(), // instanceInitializers
-        t.nullLiteral(), // staticInitializers
       ];
 
       if (instanceProps) args[1] = instanceProps;
       if (staticProps) args[2] = staticProps;
 
-      if (classState.instanceInitializersId) {
-        args[3] = classState.instanceInitializersId;
-        body.unshift(buildObjectAssignment(classState.instanceInitializersId));
-      }
-
-      if (classState.staticInitializersId) {
-        args[4] = classState.staticInitializersId;
-        body.unshift(buildObjectAssignment(classState.staticInitializersId));
-      }
-
       let lastNonNullIndex = 0;
       for (let i = 0; i < args.length; i++) {
         if (!t.isNullLiteral(args[i])) lastNonNullIndex = i;
@@ -306,12 +284,6 @@ export default function transformClass(
     clearDescriptors();
   }
 
-  function buildObjectAssignment(id) {
-    return t.variableDeclaration("var", [
-      t.variableDeclarator(id, t.objectExpression([])),
-    ]);
-  }
-
   function wrapSuperCall(bareSuper, superRef, thisRef, body) {
     let bareSuperNode = bareSuper.node;
     let call;

packages/babel-plugin-transform-exponentiation-operator/test/fixtures/regression/4349/output.js

@@ -14,9 +14,9 @@ function _getPrototypeOf(o) { _getPrototypeOf = Object.getPrototypeOf || functio
 
 foo = _obj = {
   bar() {
-    var _ref;
+    var _super$baz;
 
-    return _ref = _get(_getPrototypeOf(_obj), "baz", this), _set(_getPrototypeOf(_obj), "baz", Math.pow(_ref, 12), this, false);
+    return _super$baz = _get(_getPrototypeOf(_obj), "baz", this), _set(_getPrototypeOf(_obj), "baz", Math.pow(_super$baz, 12), this, false);
   }
 
 };

packages/babel-plugin-transform-object-super/src/index.js

@@ -2,13 +2,10 @@ import { declare } from "@babel/helper-plugin-utils";
 import ReplaceSupers from "@babel/helper-replace-supers";
 import { types as t } from "@babel/core";
 
-function replacePropertySuper(path, node, scope, getObjectRef, file) {
+function replacePropertySuper(path, getObjectRef, file) {
   const replaceSupers = new ReplaceSupers({
     getObjectRef: getObjectRef,
-    methodNode: node,
     methodPath: path,
-    isStatic: true,
-    scope: scope,
     file: file,
   });
 
@@ -28,13 +25,7 @@ export default declare(api => {
         path.get("properties").forEach(propPath => {
           if (!propPath.isMethod()) return;
 
-          replacePropertySuper(
+          replacePropertySuper(propPath, getObjectRef, state);
-            propPath,
-            propPath.node,
-            path.scope,
-            getObjectRef,
-            state,
-          );
         });
 
         if (objectRef) {

packages/babel-plugin-transform-object-super/test/fixtures/object-super/super-exponentiation/output.js

@@ -14,8 +14,8 @@ function _getPrototypeOf(o) { _getPrototypeOf = Object.getPrototypeOf || functio
 
 foo = _obj = {
   bar: function () {
-    var _ref;
+    var _super$baz;
 
-    return _ref = _get(_getPrototypeOf(_obj), "baz", this), _set(_getPrototypeOf(_obj), "baz", _ref ** 12, this, false);
+    return _super$baz = _get(_getPrototypeOf(_obj), "baz", this), _set(_getPrototypeOf(_obj), "baz", _super$baz ** 12, this, false);
   }
 };

packages/babel-plugin-transform-object-super/test/fixtures/object-super/super-increment-postfix/output.js

@@ -5,9 +5,9 @@ var Base = {
 };
 var obj = _obj = {
   bar: function () {
-    var _ref;
+    var _super$test;
 
-    return _ref = Number(babelHelpers.get(babelHelpers.getPrototypeOf(_obj), "test", this)), babelHelpers.set(babelHelpers.getPrototypeOf(_obj), "test", _ref + 1, this, false), _ref;
+    return _super$test = Number(babelHelpers.get(babelHelpers.getPrototypeOf(_obj), "test", this)), babelHelpers.set(babelHelpers.getPrototypeOf(_obj), "test", _super$test + 1, this, false), _super$test;
   }
 };
 Object.setPrototypeOf(obj, Base);

packages/babel-plugin-transform-object-super/test/fixtures/object-super/super-increment-prefix/output.js

@@ -5,9 +5,9 @@ var Base = {
 };
 var obj = _obj = {
   bar: function () {
-    var _ref;
+    var _super$test;
 
-    return _ref = Number(babelHelpers.get(babelHelpers.getPrototypeOf(_obj), "test", this)), babelHelpers.set(babelHelpers.getPrototypeOf(_obj), "test", _ref + 1, this, false);
+    return _super$test = Number(babelHelpers.get(babelHelpers.getPrototypeOf(_obj), "test", this)), babelHelpers.set(babelHelpers.getPrototypeOf(_obj), "test", _super$test + 1, this, false);
   }
 };
 Object.setPrototypeOf(obj, Base);

packages/babel-traverse/src/scope/index.js

@@ -40,6 +40,8 @@ function gatherNodeParts(node: Object, parts: Array) {
     gatherNodeParts(node.id, parts);
   } else if (t.isThisExpression(node)) {
     parts.push("this");
+  } else if (t.isSuper(node)) {
+    parts.push("super");
   }
 }