Skip to content

Commit

Permalink
Remove ES6-style getters and setters in J2clPropertyInlinerPass
Browse files Browse the repository at this point in the history 
This pass currently only inlines getters and setters using Object.defineProperties that are created in transpilation. This adds the same behavior for ES6-style getters and setters from J2CL code.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=183412399
  • Loading branch information
@lauraharker
lauraharker authored and Tyler Breisacher committed Jan 27, 2018
1 parent 1d36721 commit a391c27
Show file tree
Hide file tree
Showing 2 changed files with 229 additions and 36 deletions.
117 changes: 106 additions & 11 deletions src/com/google/javascript/jscomp/J2clPropertyInlinerPass.java
View file
Expand Up @@ -17,6 +17,7 @@
package com.google.javascript.jscomp;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;

import com.google.javascript.jscomp.FunctionInjector.InliningMode;
import com.google.javascript.jscomp.FunctionInjector.Reference;
Expand All @@ -39,9 +40,9 @@
* <li> Avoid inlining if the property is incremented using ++ or --</li>
* </ul>
*
* Since the FunctionInliner class really only works after the CollapseProperties pass has run, we
* Since this pass only really works after the AggressiveInlineAliases pass has run, we
* have to look for Object.defineProperties instead of es6 get and set nodes since es6
* transpilation has already occured.
* transpilation has already occurred if the language out is ES5.
*
*/
public class J2clPropertyInlinerPass implements CompilerPass {
Expand All @@ -62,9 +63,11 @@ public void process(Node externs, Node root) {

class StaticFieldGetterSetterInliner {
Node root;

StaticFieldGetterSetterInliner(Node root) {
this.root = root;
}

private void run() {
GatherJ2CLClassGetterSetters gatherer = new GatherJ2CLClassGetterSetters();
NodeTraversal.traverseEs6(compiler, root, gatherer);
Expand All @@ -73,21 +76,33 @@ private void run() {
new InlinePropertiesPass(result).run();
}

private class J2clProperty {
private final Node getKey;
private final Node setKey;
private boolean isSafeToInline;
private abstract class J2clProperty {
final Node getKey;
final Node setKey;
boolean isSafeToInline;

public J2clProperty(Node getKey, Node setKey) {
this.getKey = getKey;
this.setKey = setKey;
this.isSafeToInline = true;
}

abstract void remove();
}

/** A J2CL property with a getter and setter from an Object.defineProperties call */
private class J2clPropertyEs5 extends J2clProperty {
public J2clPropertyEs5(Node getKey, Node setKey) {
super(getKey, setKey);
checkArgument(getKey.isStringKey() && getKey.getString().equals("get"), getKey);
checkArgument(setKey.isStringKey() && setKey.getString().equals("set"), setKey);
}

@Override
void remove() {
Node nodeToDetach = getKey.getGrandparent();
Node objectLit = nodeToDetach.getParent();
checkArgument(objectLit.isObjectLit());
checkState(objectLit.isObjectLit(), objectLit);
nodeToDetach.detach();
NodeUtil.markFunctionsDeleted(nodeToDetach, compiler);
compiler.reportChangeToEnclosingScope(objectLit);
Expand All @@ -98,6 +113,26 @@ void remove() {
}
}

/** A J2CL property created with a ES6-style static getter and setter */
private class J2clPropertyEs6 extends J2clProperty {
public J2clPropertyEs6(Node getKey, Node setKey) {
super(getKey, setKey);
checkArgument(getKey.isGetterDef(), getKey);
checkArgument(setKey.isSetterDef(), setKey);
}

@Override
void remove() {
Node classMembers = getKey.getParent();
checkState(classMembers.isClassMembers(), classMembers);
classMembers.removeChild(getKey);
classMembers.removeChild(setKey);
NodeUtil.markFunctionsDeleted(getKey, compiler);
NodeUtil.markFunctionsDeleted(setKey, compiler);
compiler.reportChangeToEnclosingScope(classMembers);
}
}

/**
* <li> We match J2CL property getters by looking for the following signature:
* <pre>{@code
Expand Down Expand Up @@ -188,14 +223,19 @@ private Map<String, J2clProperty> getResults() {

@Override
public void visit(NodeTraversal t, Node n, Node parent) {
if (!NodeUtil.isObjectDefinePropertiesDefinition(n)) {
return;
if (n.isClass()) {
visitClass(n);
} else if (NodeUtil.isObjectDefinePropertiesDefinition(n)) {
visitObjectDefineProperties(n);
}
}

void visitObjectDefineProperties(Node n) {
Node className = n.getSecondChild();
if (!className.isName()) {
return;
}
String classNameString = className.getQualifiedName();
String classNameString = className.getString();
for (Node p : NodeUtil.getObjectDefinedPropertiesKeys(n)) {
String name = p.getString();
// J2cl static fields are always synthesized with both a getter and setter.
Expand All @@ -217,11 +257,66 @@ public void visit(NodeTraversal t, Node n, Node parent) {
setKey = innerKey;
}
break;
default: // fall out
}
}
if (getKey != null && setKey != null) {
j2clPropertiesByName.put(
classNameString + "." + name, new J2clProperty(getKey, setKey));
classNameString + "." + name, new J2clPropertyEs5(getKey, setKey));
}
}
}

void visitClass(Node classNode) {
String className = NodeUtil.getName(classNode);
Node classMembers = NodeUtil.getClassMembers(classNode);

Node getKey = null;
Node setterDef = null;
String name = "";
Map<String, Node> nameToGetterOrSetterDef = new HashMap<>();

// These J2CL-created getters and setters always come in pairs. The first time seeing a
// getter/setter for a given name, add it to the nameToGetterOrSetterDef map.
// Upon seeing another getter/setter for that name, create a new J2clProperty for the
// getter/setter pair.
for (Node memberFunction : classMembers.children()) {
if (!memberFunction.isStaticMember()) {
// The only getters and setters we care about are static.
continue;
}
switch (memberFunction.getToken()) {
case GETTER_DEF:
if (matchesJ2clGetKeySignature(className, memberFunction)) {
getKey = memberFunction;
name = memberFunction.getString();
setterDef = nameToGetterOrSetterDef.get(name);

if (setterDef != null) {
j2clPropertiesByName.put(
className + "." + name, new J2clPropertyEs6(getKey, setterDef));
nameToGetterOrSetterDef.remove(name);
} else {
nameToGetterOrSetterDef.put(name, getKey);
}
}
break;
case SETTER_DEF:
if (matchesJ2clSetKeySignature(className, memberFunction)) {
setterDef = memberFunction;
name = memberFunction.getString();
getKey = nameToGetterOrSetterDef.get(name);

if (getKey != null) {
j2clPropertiesByName.put(
className + "." + name, new J2clPropertyEs6(getKey, setterDef));
nameToGetterOrSetterDef.remove(name);
} else {
nameToGetterOrSetterDef.put(name, setterDef);
}
}
break;
default: // fall out
}
}
}
Expand Down
148 changes: 123 additions & 25 deletions test/com/google/javascript/jscomp/J2clPropertyInlinerPassTest.java
View file
Expand Up @@ -24,7 +24,7 @@ public class J2clPropertyInlinerPassTest extends CompilerTestCase {
@Override
protected void setUp() throws Exception {
super.setUp();
enableNormalize(); // Inlining will fail if normailization hasn't happened yet.
enableNormalize(); // Inlining will fail if normalization hasn't happened yet.
}

@Override
Expand Down Expand Up @@ -53,37 +53,39 @@ public void testNoInlineNonJ2clProps() {
Lists.newArrayList(
SourceFile.fromCode(
"someFile.js",
"var A = function() {};"
+ "var A$$0clinit = function() {"
+ " A$$0x = 2;"
+ "};"
+ "Object.defineProperties(A, {x :{"
+ " configurable:true,"
+ " enumerable:true,"
+ " get:function() {"
+ " return A$$0clinit(), A$$0x;"
+ " }"
+ "}});"
+ "var A$$0x = null;"
+ "var x = A.x;")));
lines(
"var A = function() {};",
"var A$$0clinit = function() {",
" A$$0x = 2;",
"};",
"Object.defineProperties(A, {x :{",
" configurable:true,",
" enumerable:true,",
" get:function() {",
" return A$$0clinit(), A$$0x;",
" }",
"}});",
"var A$$0x = null;",
"var x = A.x;"))));
}

public void testNoInlineNonJ2clPropsValue() {
testDoesntChange(
Lists.newArrayList(
SourceFile.fromCode(
"someFile.js",
"var A = function() {};"
+ "var A$$0clinit = function() {"
+ " A$$0x = 2;"
+ "};"
+ "Object.defineProperties(A, {x :{"
+ " configurable:true,"
+ " enumerable:true,"
+ " value: 2"
+ "}});"
+ "var A$$0x = null;"
+ "var x = A.x;")));
lines(
"var A = function() {};",
"var A$$0clinit = function() {",
" A$$0x = 2;",
"};",
"Object.defineProperties(A, {x :{",
" configurable:true,",
" enumerable:true,",
" value: 2",
"}});",
"var A$$0x = null;",
"var x = A.x;"))));
}

// In this test we want to remove the J2CL property but not the entire Object.defineProperties
Expand Down Expand Up @@ -316,4 +318,100 @@ public void testNoInlineIncrementGetter() {
"A.$x = 3;",
"A.x++;"))));
}

public void testInlineEs6Getter() {
test(
lines(
"class A {",
" static $clinit() {",
" A.$clinit = function() {};",
" A.$x = 2;",
" }",
" static get x() {",
" return A.$clinit(), A.$x",
" }",
" static set x(value) {",
" A.$clinit(), A.$x = value;",
" }",
"}",
"A.$x = 3;",
"var xx = A.x"),
lines(
"class A {",
" static $clinit() {",
" A.$clinit = function() {};",
" A.$x = 2;",
" }",
"}",
"A.$x = 3;",
"var xx = (A.$clinit(), A.$x);"));
}

public void testInlineEs6Setter() {
test(
lines(
"class A {",
" static $clinit() {",
" A.$clinit = function() {};",
" A.$x = 2;",
" }",
" static get x() {",
" return A.$clinit(), A.$x",
" }",
" static set x(value) {",
" A.$clinit(), A.$x = value;",
" }",
"}",
"A.$x = 3;",
"A.x = 5;"),
lines(
"class A {",
" static $clinit() {",
" A.$clinit = function() {};",
" A.$x = 2;",
" }",
"}",
"A.$x = 3;",
"{(A.$clinit(), A.$x = 5)}"));
}

public void testInlineEs6GetterSetter_multiple() {
test(
lines(
"class A {",
" static $clinit() {",
" A.$clinit = function() {};",
" A.$x = 2;",
" A.$y = 3;",
" }",
" static get x() {",
" return A.$clinit(), A.$x",
" }",
" static set y(value) {",
" A.$clinit(), A.$y = value;",
" }",
" static get y() {",
" return A.$clinit(), A.$y",
" }",
" static set x(value) {",
" A.$clinit(), A.$x = value;",
" }",
"}",
"var xx = A.x;",
"var yy = A.y",
"A.x = 5;",
"A.y = 5;"),
lines(
"class A {",
" static $clinit() {",
" A.$clinit = function() {};",
" A.$x = 2;",
" A.$y = 3;",
" }",
"}",
"var xx = (A.$clinit(), A.$x);",
"var yy = (A.$clinit(), A.$y)",
"{(A.$clinit(), A.$x = 5)}",
"{(A.$clinit(), A.$y = 5)}"));
}
}

0 comments on commit a391c27

Please sign in to comment.