Skip to content

Commit

Permalink
Support transpiling arbitrary computed properties in classes
Browse files Browse the repository at this point in the history 
Adds support for
 - non-qualified name computed getters and setters
 - static computed getters and setters

Also deletes tests in Es6TranspilationIntegrationTest that are now completely covered by Es6RewriteClassTest.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=288613380
  • Loading branch information
@lauraharker @brad4d
lauraharker authored and brad4d committed Jan 8, 2020
1 parent 7d2ea86 commit bd0832c
Show file tree
Hide file tree
Showing 6 changed files with 291 additions and 139 deletions.
80 changes: 49 additions & 31 deletions src/com/google/javascript/jscomp/Es6RewriteClass.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ private void visitClass(final NodeTraversal t, final Node classNode, final Node
&& (member.getBooleanProp(Node.COMPUTED_PROP_GETTER)
|| member.getBooleanProp(Node.COMPUTED_PROP_SETTER)))
|| (member.isGetterDef() || member.isSetterDef())) {
visitNonMethodMember(member, metadata);
visitNonMethodMember(member, metadata, t.getScope());
} else if (NodeUtil.isEs6ConstructorMemberFunctionDef(member)) {
constructor = member.getFirstChild().detach().setJSType(classNode.getJSType());
constructor.setJSTypeBeforeCast(classNode.getJSTypeBeforeCast());
Expand Down Expand Up @@ -313,57 +313,75 @@ private Node createObjectDotDefineProperties(Scope scope) {
return astFactory.createQName(scope, "$jscomp.global.Object.defineProperties");
}

/**
* @param member A getter or setter, or a computed property that is a getter/setter.
*/
private Node createObjectDotDefineProperty(Scope scope) {
return astFactory.createQName(scope, "$jscomp.global.Object.defineProperty");
}

/** @param member A getter or setter */
private void addToDefinePropertiesObject(ClassDeclarationMetadata metadata, Node member) {
Preconditions.checkArgument(!member.isComputedProp());
Node obj =
member.isStaticMember()
? metadata.getDefinePropertiesObjForClass()
: metadata.getDefinePropertiesObjForPrototype();
Node prop =
member.isComputedProp()
? NodeUtil.getFirstComputedPropMatchingKey(obj, member.getFirstChild())
: NodeUtil.getFirstPropMatchingKey(obj, member.getString());
Node prop = NodeUtil.getFirstPropMatchingKey(obj, member.getString());
if (prop == null) {
prop = createPropertyDescriptor();
if (member.isComputedProp()) {
obj.addChildToBack(
astFactory.createComputedProperty(member.getFirstChild().cloneTree(), prop));
} else {
Node stringKey = astFactory.createStringKey(member.getString(), prop);
if (member.isQuotedString()) {
stringKey.putBooleanProp(Node.QUOTED_PROP, true);
}
obj.addChildToBack(stringKey);
Node stringKey = astFactory.createStringKey(member.getString(), prop);
if (member.isQuotedString()) {
stringKey.putBooleanProp(Node.QUOTED_PROP, true);
}
obj.addChildToBack(stringKey);
}

Node function = member.getLastChild();
JSDocInfo info = NodeUtil.getBestJSDocInfo(function);

Node stringKey =
astFactory.createStringKey(
(member.isGetterDef() || member.getBooleanProp(Node.COMPUTED_PROP_GETTER))
? "get"
: "set",
function.detach());
astFactory.createStringKey(member.isGetterDef() ? "get" : "set", function.detach());
stringKey.setJSDocInfo(info);
prop.addChildToBack(stringKey);
prop.useSourceInfoIfMissingFromForTree(member);
}

/** Appends an Object.defineProperty call defining the given computed getter or setter */
private void extractComputedProperty(
Node computedMember, ClassDeclarationMetadata metadata, Scope scope) {
Node owner =
computedMember.isStaticMember()
? metadata.getFullClassNameNode()
: metadata.getClassPrototypeNode();
Node property = computedMember.getFirstChild().detach();
Node propertyValue = computedMember.getFirstChild().detach();

Node propertyDescriptor = createPropertyDescriptor();
Node stringKey =
astFactory.createStringKey(
computedMember.getBooleanProp(Node.COMPUTED_PROP_GETTER) ? "get" : "set",
propertyValue);
propertyDescriptor.addChildToBack(stringKey);

Node objectDefinePropertyCall =
astFactory.createCall(
createObjectDotDefineProperty(scope), owner.cloneTree(), property, propertyDescriptor);

metadata.insertNodeAndAdvance(
IR.exprResult(objectDefinePropertyCall).useSourceInfoIfMissingFromForTree(computedMember));
}

/**
* Visits class members other than simple methods: Getters, setters, and computed properties.
* Visits getters and setters, including both static and instance properties, and computed and
* non-computed properties.
*
* <p>Non-computed getters and setters are aggregated into two Object.defineProperties calls. One
* defines static members and the other instance members. This is just an optimization; it's just
* as sound to append a single Object.defineProperty definition for each here.
*
* <p>Computed getters and setters are defined in individual Object.defineProperty calls
*/
private void visitNonMethodMember(Node member, ClassDeclarationMetadata metadata) {
if (member.isComputedProp() && member.isStaticMember()) {
cannotConvertYet(compiler, member, "Static computed property");
return;
}
if (member.isComputedProp() && !member.getFirstChild().isQualifiedName()) {
cannotConvert(
compiler, member.getFirstChild(), "Computed property with non-qualified-name key");
private void visitNonMethodMember(Node member, ClassDeclarationMetadata metadata, Scope scope) {
if (member.isComputedProp()) {
extractComputedProperty(member.detach(), metadata, scope);
return;
}

Expand Down
130 changes: 112 additions & 18 deletions test/com/google/javascript/jscomp/Es6RewriteClassTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2296,13 +2296,56 @@ public void testStaticSetter() {
@Test
public void testClassStaticComputedProps_withSetter() {
setLanguageOut(LanguageMode.ECMASCRIPT5);
testError("/** @unrestricted */ class C { static set [foo](val) {}}", CANNOT_CONVERT_YET);
test(
"/** @unrestricted */ class C { static set [se()](val) {}}",
lines(
"/** @constructor @unrestricted */",
"let C = function() {};",
"$jscomp.global.Object.defineProperty(C, se(), {",
" configurable: true,",
" enumerable: true,",
" set: function(val) {},",
"});"));
}

@Test
public void testClassStaticComputedProps_withGetter() {
setLanguageOut(LanguageMode.ECMASCRIPT5);
testError("/** @unrestricted */ class C { static get [foo]() {}}", CANNOT_CONVERT_YET);
test(
"/** @unrestricted */ class C { static get [se()]() {}}",
lines(
"/** @constructor @unrestricted */",
"let C = function() {};",
"$jscomp.global.Object.defineProperty(C, se(), {",
" configurable: true,",
" enumerable: true,",
" get: function() {},",
"});"));
}

@Test
public void testClassStaticComputedProps_withGetterSetterPair() {
setLanguageOut(LanguageMode.ECMASCRIPT5);
test(
"/** @unrestricted */ class C { static get [se()]() {} static set [se()](val) {}}",
lines(
"/** @constructor @unrestricted */",
"let C = function() {};",
"",
"$jscomp.global.Object.defineProperty(",
" C,",
" se(), {",
" configurable: true,",
" enumerable: true,",
" get: function() {},",
" }",
");",
"",
"$jscomp.global.Object.defineProperty(C, se(), {",
" configurable: true,",
" enumerable: true,",
" set: function(val) {},",
"});"));
}

@Test
Expand Down Expand Up @@ -2331,22 +2374,18 @@ public void testClassComputedPropGetterAndSetter() {
lines(
"/** @constructor @unrestricted */",
"let C = function() {};",
"$jscomp.global.Object.defineProperties(",
" C.prototype,",
" {",
" [foo]: {",
" configurable:true,",
" enumerable:true,",
" /**",
" * @return {boolean}",
" */",
" get: function() {},",
" /**",
" * @param {boolean} val",
" */",
" set: function(val) {},",
" },",
" });"));
"",
"$jscomp.global.Object.defineProperty(C.prototype, foo, {",
" configurable:true,",
" enumerable:true,",
" get: function() {},",
"});",
"",
"$jscomp.global.Object.defineProperty(C.prototype, foo, {",
" configurable:true,",
" enumerable:true,",
" set: function(val) {},",
"});"));
}

@Test
Expand All @@ -2363,6 +2402,61 @@ public void testClassComputedPropGetterAndSetter_withConflictingGetterSetterType
"}"));
}

@Test
public void testClassComputedPropGetterAndSetter_mixedWithOtherMembers() {
setLanguageOut(LanguageMode.ECMASCRIPT5);
test(
externs(
lines(
"function one() {}", //
"function two() {}",
"function three() {}")),
srcs(
lines(
"/** @unrestricted */",
"class C {",
" get [one()]() { return true; }",
" get a() {}",
" [two()]() {}",
" get b() {}",
" c() {}",
" set [three()](val) { return true; }",
"}")),
expected(
lines(
"/** @unrestricted @constructor */",
"let C = function() {};",
"$jscomp.global.Object.defineProperty(C.prototype, one(), {",
" configurable: true,",
" enumerable: true,",
" get: function() {",
" return true",
" },",
"});",
"C.prototype[two()] = function() {};",
"C.prototype.c = function() {};",
"$jscomp.global.Object.defineProperty(C.prototype, three(), {",
" configurable: true,",
" enumerable: true,",
" set: function(val) {",
" return true",
" },",
"});",
"$jscomp.global.Object.defineProperties(C.prototype, {",
" a: {",
" configurable: true,",
" enumerable: true,",
" get: function() {},",
" },",
" b: {",
" configurable: true,",
" enumerable: true,",
" get: function() {},",
" }",
"});",
"")));
}

@Test
public void testComputedPropClass() {
test(
Expand Down
90 changes: 0 additions & 90 deletions test/com/google/javascript/jscomp/Es6TranspilationIntegrationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1883,96 +1883,6 @@ public void testInitSymbolIterator() {
" $jscomp$compprop0)"));
}

@Test
public void testClassComputedPropGetter() {
setLanguageOut(LanguageMode.ECMASCRIPT5);

test(
"/** @unrestricted */ class C { /** @return {number} */ get [foo]() { return 4; }}",
lines(
"/** @constructor @unrestricted */",
"var C = function() {};",
"var $jscomp$compprop0 = {};",
"$jscomp.global.Object.defineProperties(",
" C.prototype,",
" ($jscomp$compprop0[foo] = {",
" configurable:true,",
" enumerable:true,",
" /**",
" * @return {number}",
" */",
" get: function() { return 4; }",
" }, $jscomp$compprop0));"));
assertThat(getLastCompiler().injected).containsExactly("util/global");

testError("class C { get [add + expr]() {} }", CANNOT_CONVERT);
}

@Test
public void testClassComputedPropSetter() {
setLanguageOut(LanguageMode.ECMASCRIPT5);

test(
"/** @unrestricted */ class C { /** @param {string} val */ set [foo](val) {}}",
lines(
"/** @constructor @unrestricted */",
"var C = function() {};",
"var $jscomp$compprop0={};",
"$jscomp.global.Object.defineProperties(",
" C.prototype,",
" ($jscomp$compprop0[foo] = {",
" configurable:true,",
" enumerable:true,",
" /**",
" * @param {string} val",
" */",
" set: function(val) {}",
" }, $jscomp$compprop0));"));

testError("class C { get [sub - expr]() {} }", CANNOT_CONVERT);
}

@Test
public void testClassStaticComputedProps() {
setLanguageOut(LanguageMode.ECMASCRIPT5);

testError("/** @unrestricted */ class C { static set [foo](val) {}}", CANNOT_CONVERT_YET);
testError("/** @unrestricted */ class C { static get [foo]() {}}", CANNOT_CONVERT_YET);
}

@Test
public void testClassComputedPropGetterAndSetter() {
setLanguageOut(LanguageMode.ECMASCRIPT5);

test(
lines(
"/** @unrestricted */",
"class C {",
" /** @return {boolean} */",
" get [foo]() {}",
" /** @param {boolean} val */",
" set [foo](val) {}",
"}"),
lines(
"/** @constructor @unrestricted */",
"var C = function() {};",
"var $jscomp$compprop0={};",
"$jscomp.global.Object.defineProperties(",
" C.prototype,",
" ($jscomp$compprop0[foo] = {",
" configurable:true,",
" enumerable:true,",
" /**",
" * @return {boolean}",
" */",
" get: function() {},",
" /**",
" * @param {boolean} val",
" */",
" set: function(val) {},",
" }, $jscomp$compprop0));"));
}

/** ES5 getters and setters should report an error if the languageOut is ES3. */
@Test
public void testEs5GettersAndSetters_es3() {
Expand Down
Loading

0 comments on commit bd0832c

Please sign in to comment.