Ensure inherited statics throw in loose mode

Re: tc39/proposal-class-fields#43
CodingItWrong · Mar 13, 2018 · a9a7bd5 · a9a7bd5
1 parent ad661da
commit a9a7bd5
Show file tree

Hide file tree

Showing 15 changed files with 161 additions and 96 deletions.
diff --git a/packages/babel-helpers/src/helpers.js b/packages/babel-helpers/src/helpers.js
@@ -600,25 +600,21 @@ helpers.classPrivateFieldKey = () => template.program.ast`
   }
 `;
 
-helpers.classPrivateFieldGet = () => template.program.ast`
-  export default function _classPrivateFieldGet(receiver, privateMap) {
-    if (!privateMap.has(receiver)) {
-      throw new TypeError("attempted to get private field on non-instance");
+helpers.classPrivateFieldBase = () => template.program.ast`
+  export default function _classPrivateFieldBase(receiver, privateKey) {
+    if (!Object.prototype.hasOwnProperty.call(receiver, privateKey)) {
+      throw new TypeError("attempted to use private field on non-instance");
     }
-    return privateMap.get(receiver);
+    return receiver;
   }
 `;
 
-helpers.classPrivateFieldGetStatic = () => template.program.ast`
+helpers.classPrivateFieldGet = () => template.program.ast`
   export default function _classPrivateFieldGet(receiver, privateMap) {
-    while (receiver && !privateMap.has(receiver)) {
-      receiver = Object.getPrototypeOf(receiver);
-    }
-    if (receiver) {
-      return privateMap.get(receiver);
-    } else {
+    if (!privateMap.has(receiver)) {
       throw new TypeError("attempted to get private field on non-instance");
     }
+    return privateMap.get(receiver);
   }
 `;
 
@@ -632,21 +628,6 @@ helpers.classPrivateFieldPut = () => template.program.ast`
   }
 `;
 
-helpers.classPrivateFieldPutStatic = () => template.program.ast`
-  export default function _classPrivateFieldPutStatic(receiver, privateMap, value) {
-    while (receiver && !privateMap.has(receiver)) {
-      receiver = Object.getPrototypeOf(receiver);
-    }
-    if (receiver) {
-      privateMap.set(receiver, value);
-      return value;
-    } else {
-      throw new TypeError("attempted to set private field on non-instance");
-    }
-  }
-`;
-
-
 helpers.set = () => template.program.ast`
   export default function _set(object, property, value, receiver) {
     var desc = Object.getOwnPropertyDescriptor(object, property);

diff --git a/packages/babel-plugin-proposal-class-properties/src/index.js b/packages/babel-plugin-proposal-class-properties/src/index.js
@@ -203,8 +203,13 @@ export default declare((api, options) => {
       }
       if (!parentPath.isMemberExpression()) return;
 
+      const object = parentPath.get("object");
+
+      object.replaceWith(
+        t.callExpression(this.base, [object.node, this.privateKey]),
+      );
       parentPath.node.computed = true;
-      path.replaceWith(this.privateName);
+      path.replaceWith(this.privateKey);
     },
 
     ClassBody(path) {
@@ -260,13 +265,7 @@ export default declare((api, options) => {
     });
   };
 
-  const buildPrivateClassPropertySpec = (
-    ref,
-    prop,
-    classBody,
-    nodes,
-    isStatic = false,
-  ) => {
+  const buildPrivateClassPropertySpec = (ref, prop, classBody, nodes) => {
     const { node } = prop;
     const { name } = node.key.id;
     const { file } = classBody.hub;
@@ -275,8 +274,8 @@ export default declare((api, options) => {
     classBody.traverse(privateNameRemapper, {
       name,
       privateMap,
-      get: file.addHelper(`classPrivateFieldGet${isStatic ? "Static" : ""}`),
-      put: file.addHelper(`classPrivateFieldPut${isStatic ? "Static" : ""}`),
+      get: file.addHelper("classPrivateFieldGet"),
+      put: file.addHelper("classPrivateFieldPut"),
     });
 
     nodes.push(
@@ -301,15 +300,19 @@ export default declare((api, options) => {
     const { key, value } = prop.node;
     const { name } = key.id;
     const { file } = classBody.hub;
-    const privateName = classBody.scope.generateDeclaredUidIdentifier(name);
+    const privateKey = classBody.scope.generateDeclaredUidIdentifier(name);
 
-    classBody.traverse(privateNameRemapperLoose, { name, privateName });
+    classBody.traverse(privateNameRemapperLoose, {
+      name,
+      privateKey,
+      base: file.addHelper("classPrivateFieldBase"),
+    });
 
     nodes.push(
       t.expressionStatement(
         t.assignmentExpression(
           "=",
-          privateName,
+          privateKey,
           t.callExpression(file.addHelper("classPrivateFieldKey"), [
             t.stringLiteral(name),
           ]),
@@ -326,7 +329,7 @@ export default declare((api, options) => {
       });
     `({
       REF: ref,
-      KEY: privateName,
+      KEY: privateKey,
       VALUE: value || prop.scope.buildUndefinedNode(),
     });
   };
@@ -403,7 +406,7 @@ export default declare((api, options) => {
         for (const prop of staticProps) {
           if (prop.isClassPrivateProperty()) {
             staticNodes.push(
-              buildPrivateClassProperty(ref, prop, body, staticNodes, true),
+              buildPrivateClassProperty(ref, prop, body, staticNodes),
             );
           } else {
             staticNodes.push(buildPublicClassProperty(ref, prop));

diff --git a/...l-plugin-proposal-class-properties/test/fixtures/general-private/static-inherited/exec.js b/...l-plugin-proposal-class-properties/test/fixtures/general-private/static-inherited/exec.js
@@ -1,13 +1,21 @@
 class Base {
   static #foo = 1;
 
-  static m() {
+  static getThis() {
     return this.#foo;
   }
 
-  static update(val) {
+  static updateThis(val) {
     return this.#foo = val;
   }
+
+  static getClass() {
+    return Base.#foo;
+  }
+
+  static updateClass(val) {
+    return Base.#foo = val;
+  }
 }
 
 class Sub1 extends Base {
@@ -21,21 +29,42 @@ class Sub1 extends Base {
 class Sub2 extends Base {
 }
 
-assert.equal(Base.m(), 1);
-assert.equal(Sub1.m(), 1);
-assert.equal(Sub2.m(), 1);
+assert.equal(Base.getThis(), 1);
+assert.equal(Base.getClass(), 1);
+assert.throws(() => Sub1.getThis());
+assert.equal(Sub1.getClass(), 1);
+assert.throws(() => Sub2.getThis());
+assert.equal(Sub2.getClass(), 1);
 
 assert.equal(Sub1.update(3), 3);
-assert.equal(Base.m(), 1);
-assert.equal(Sub1.m(), 1);
-assert.equal(Sub2.m(), 1);
-
-assert.equal(Base.update(4), 4);
-assert.equal(Base.m(), 4);
-assert.equal(Sub1.m(), 4);
-assert.equal(Sub2.m(), 4);
-
-assert.equal(Sub2.update(5), 5);
-assert.equal(Base.m(), 5);
-assert.equal(Sub1.m(), 5);
-assert.equal(Sub2.m(), 5);
+assert.equal(Base.getThis(), 1);
+assert.equal(Base.getClass(), 1);
+assert.throws(() => Sub1.getThis());
+assert.equal(Sub1.getClass(), 1);
+assert.throws(() => Sub2.getThis());
+assert.equal(Sub2.getClass(), 1);
+
+assert.equal(Base.updateThis(4), 4);
+assert.equal(Base.getThis(), 4);
+assert.equal(Base.getClass(), 4);
+assert.throws(() => Sub1.getThis());
+assert.equal(Sub1.getClass(), 4);
+assert.throws(() => Sub2.getThis());
+assert.equal(Sub2.getClass(), 4);
+
+assert.equal(Base.updateClass(5), 5);
+assert.equal(Base.getThis(), 5);
+assert.equal(Base.getClass(), 5);
+assert.throws(() => Sub1.getThis());
+assert.equal(Sub1.getClass(), 5);
+assert.throws(() => Sub2.getThis());
+assert.equal(Sub2.getClass(), 5);
+
+assert.throws(() => Sub2.updateThis(6));
+assert.equal(Sub2.updateClass(7), 7);
+assert.equal(Base.getThis(), 7);
+assert.equal(Base.getClass(), 7);
+assert.throws(() => Sub1.getThis());
+assert.equal(Sub1.getClass(), 7);
+assert.throws(() => Sub2.getThis());
+assert.equal(Sub2.getClass(), 7);
diff --git a/...ugin-proposal-class-properties/test/fixtures/general-private/static-inherited/expected.js b/...ugin-proposal-class-properties/test/fixtures/general-private/static-inherited/expected.js
@@ -8,7 +8,7 @@ var Base = function () {
   babelHelpers.createClass(Base, null, [{
     key: "m",
     value: function m() {
-      return babelHelpers.classPrivateFieldGetStatic(this, _foo);
+      return babelHelpers.classPrivateFieldGet(this, _foo);
     }
   }]);
   return Base;

diff --git a/...s/babel-plugin-proposal-class-properties/test/fixtures/general-private/static/expected.js b/...s/babel-plugin-proposal-class-properties/test/fixtures/general-private/static/expected.js
@@ -8,12 +8,12 @@ var Foo = function () {
   babelHelpers.createClass(Foo, [{
     key: "test",
     value: function test() {
-      return babelHelpers.classPrivateFieldGetStatic(Foo, _bar);
+      return babelHelpers.classPrivateFieldGet(Foo, _bar);
     }
   }], [{
     key: "test",
     value: function test() {
-      return babelHelpers.classPrivateFieldGetStatic(Foo, _bar);
+      return babelHelpers.classPrivateFieldGet(Foo, _bar);
     }
   }]);
   return Foo;

diff --git a/...babel-plugin-proposal-class-properties/test/fixtures/loose-private/assignment/expected.js b/...babel-plugin-proposal-class-properties/test/fixtures/loose-private/assignment/expected.js
@@ -12,10 +12,10 @@ var Foo = function () {
   babelHelpers.createClass(Foo, [{
     key: "test",
     value: function test(other) {
-      this[_foo] += 1;
-      this[_foo] = 2;
-      other.obj[_foo] += 1;
-      other.obj[_foo] = 2;
+      babelHelpers.classPrivateFieldBase(this, _foo)[_foo] += 1;
+      babelHelpers.classPrivateFieldBase(this, _foo)[_foo] = 2;
+      babelHelpers.classPrivateFieldBase(other.obj, _foo)[_foo] += 1;
+      babelHelpers.classPrivateFieldBase(other.obj, _foo)[_foo] = 2;
     }
   }]);
   return Foo;

diff --git a/packages/babel-plugin-proposal-class-properties/test/fixtures/loose-private/call/expected.js b/packages/babel-plugin-proposal-class-properties/test/fixtures/loose-private/call/expected.js
@@ -14,9 +14,9 @@ var Foo = function () {
   babelHelpers.createClass(Foo, [{
     key: "test",
     value: function test(other) {
-      this[_foo]();
+      babelHelpers.classPrivateFieldBase(this, _foo)[_foo]();
 
-      other.obj[_foo]();
+      babelHelpers.classPrivateFieldBase(other.obj, _foo)[_foo]();
     }
   }]);
   return Foo;

diff --git a/.../babel-plugin-proposal-class-properties/test/fixtures/loose-private/canonical/expected.js b/.../babel-plugin-proposal-class-properties/test/fixtures/loose-private/canonical/expected.js
@@ -11,35 +11,35 @@ var Point = function () {
       writable: true,
       value: void 0
     });
-    this[_x] = +x;
-    this[_y] = +y;
+    babelHelpers.classPrivateFieldBase(this, _x)[_x] = +x;
+    babelHelpers.classPrivateFieldBase(this, _y)[_y] = +y;
   }
 
   babelHelpers.createClass(Point, [{
     key: "equals",
     value: function equals(p) {
-      return this[_x] === p[_x] && this[_y] === p[_y];
+      return babelHelpers.classPrivateFieldBase(this, _x)[_x] === babelHelpers.classPrivateFieldBase(p, _x)[_x] && babelHelpers.classPrivateFieldBase(this, _y)[_y] === babelHelpers.classPrivateFieldBase(p, _y)[_y];
     }
   }, {
     key: "toString",
     value: function toString() {
-      return `Point<${this[_x]},${this[_y]}>`;
+      return `Point<${babelHelpers.classPrivateFieldBase(this, _x)[_x]},${babelHelpers.classPrivateFieldBase(this, _y)[_y]}>`;
     }
   }, {
     key: "x",
     get: function () {
-      return this[_x];
+      return babelHelpers.classPrivateFieldBase(this, _x)[_x];
     },
     set: function (value) {
-      this[_x] = +value;
+      babelHelpers.classPrivateFieldBase(this, _x)[_x] = +value;
     }
   }, {
     key: "y",
     get: function () {
-      return this[_y];
+      return babelHelpers.classPrivateFieldBase(this, _y)[_y];
     },
     set: function (value) {
-      this[_y] = +value;
+      babelHelpers.classPrivateFieldBase(this, _y)[_y] = +value;
     }
   }]);
   return Point;

diff --git a/...s/babel-plugin-proposal-class-properties/test/fixtures/loose-private/multiple/expected.js b/...s/babel-plugin-proposal-class-properties/test/fixtures/loose-private/multiple/expected.js
@@ -8,7 +8,7 @@ var Foo = function Foo() {
   });
   Object.defineProperty(this, _y, {
     writable: true,
-    value: this[_x]
+    value: babelHelpers.classPrivateFieldBase(this, _x)[_x]
   });
 };
 

diff --git a/...ugin-proposal-class-properties/test/fixtures/loose-private/private-in-derived/expected.js b/...ugin-proposal-class-properties/test/fixtures/loose-private/private-in-derived/expected.js
@@ -7,16 +7,16 @@ var Outer = function Outer() {
     value: void 0
   });
 
-  var Test = function (_this$_outer) {
-    babelHelpers.inherits(Test, _this$_outer);
+  var Test = function (_babelHelpers$classPr) {
+    babelHelpers.inherits(Test, _babelHelpers$classPr);
 
     function Test() {
       babelHelpers.classCallCheck(this, Test);
       return babelHelpers.possibleConstructorReturn(this, (Test.__proto__ || Object.getPrototypeOf(Test)).apply(this, arguments));
     }
 
     return Test;
-  }(this[_outer]);
+  }(babelHelpers.classPrivateFieldBase(this, _outer)[_outer]);
 };
 
 _outer = babelHelpers.classPrivateFieldKey("outer");
diff --git a/...es/babel-plugin-proposal-class-properties/test/fixtures/loose-private/reevaluated/exec.js b/...es/babel-plugin-proposal-class-properties/test/fixtures/loose-private/reevaluated/exec.js
@@ -37,7 +37,7 @@ assert.equal(Foo1.static(), "bar");
 assert.equal(Foo2.instance(f2), "foo");
 assert.equal(Foo2.static(), "bar");
 
-assert.equal(f1.instance.call(f2), undefined);
-assert.equal(f2.instance.call(f1), undefined);
-assert.equal(Foo1.instance(f2), undefined);
-assert.equal(Foo2.instance(f1), undefined);
+assert.throws(() => f1.instance.call(f2));
+assert.throws(() => f2.instance.call(f1));
+assert.throws(() => Foo1.instance(f2));
+assert.throws(() => Foo2.instance(f1));