Skip to content

Commit

Permalink
Support ES6 classes in InlineProperties
Browse files Browse the repository at this point in the history 
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=224367397
  • Loading branch information
@lauraharker @brad4d
lauraharker authored and brad4d committed Dec 7, 2018
1 parent 09eb7af commit 85d9035
Show file tree
Hide file tree
Showing 2 changed files with 227 additions and 20 deletions.
73 changes: 53 additions & 20 deletions src/com/google/javascript/jscomp/InlineProperties.java
View file
Expand Up @@ -20,11 +20,11 @@


import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
import com.google.javascript.rhino.IR; import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.jstype.FunctionType; import com.google.javascript.rhino.jstype.FunctionType;
import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.jstype.JSType;
import com.google.javascript.rhino.jstype.JSTypeNative; import com.google.javascript.rhino.jstype.JSTypeNative;
import com.google.javascript.rhino.jstype.ObjectType;
import java.util.HashMap; import java.util.HashMap;
import java.util.Map; import java.util.Map;


Expand Down Expand Up @@ -103,14 +103,17 @@ class GatherCandidates extends AbstractPostOrderCallback {


@Override @Override
public void visit(NodeTraversal t, Node n, Node parent) { public void visit(NodeTraversal t, Node n, Node parent) {
boolean invalidatingPropRef = false; // These are assigned at most once in the branches below
String propName = null; final boolean invalidatingPropRef;
final String propName;

if (n.isGetProp()) { if (n.isGetProp()) {
propName = n.getLastChild().getString(); propName = n.getLastChild().getString();
if (parent.isAssign()) { if (parent.isAssign()) {
invalidatingPropRef = !isValidCandidateDefinition(t, n, parent); invalidatingPropRef = !maybeRecordCandidateDefinition(t, n, parent);
} else if (NodeUtil.isLValue(n)) { } else if (NodeUtil.isLValue(n)) {
// Other LValue references invalidate // Other LValue references invalidate
// e.g. in an enhanced for loop or a destructuring statement
invalidatingPropRef = true; invalidatingPropRef = true;
} else if (parent.isDelProp()) { } else if (parent.isDelProp()) {
// Deletes invalidate // Deletes invalidate
Expand All @@ -119,13 +122,21 @@ public void visit(NodeTraversal t, Node n, Node parent) {
// A property read doesn't invalidate // A property read doesn't invalidate
invalidatingPropRef = false; invalidatingPropRef = false;
} }
} else if (n.isStringKey()) { } else if ((n.isStringKey() && !n.getParent().isObjectPattern())
|| n.isGetterDef()
|| n.isSetterDef()
|| n.isMemberFunctionDef()) {
propName = n.getString(); propName = n.getString();
// For now, any object literal key invalidates // For now, any object literal key invalidates
// TODO(johnlenz): support prototype properties like: // TODO(johnlenz): support prototype properties like:
// foo.prototype = { a: 1, b: 2 }; // foo.prototype = { a: 1, b: 2 };
// TODO(johnlenz): Object.create(), Object.createProperty // TODO(johnlenz): Object.create(), Object.createProperty
// and getter/setter defs and member functions also invalidate
// since we do not inline functions in this pass
// Note that string keys in destructuring patterns are fine, since they just access the prop
invalidatingPropRef = true; invalidatingPropRef = true;
} else {
return;
} }


if (invalidatingPropRef) { if (invalidatingPropRef) {
Expand All @@ -135,7 +146,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
} }


/** @return Whether this is a valid definition for a candidate property. */ /** @return Whether this is a valid definition for a candidate property. */
private boolean isValidCandidateDefinition(NodeTraversal t, Node n, Node parent) { private boolean maybeRecordCandidateDefinition(NodeTraversal t, Node n, Node parent) {
checkState(n.isGetProp() && parent.isAssign(), n); checkState(n.isGetProp() && parent.isAssign(), n);
Node src = n.getFirstChild(); Node src = n.getFirstChild();
String propName = n.getLastChild().getString(); String propName = n.getLastChild().getString();
Expand All @@ -145,9 +156,10 @@ private boolean isValidCandidateDefinition(NodeTraversal t, Node n, Node parent)
// This is a simple assignment like: // This is a simple assignment like:
// this.foo = 1; // this.foo = 1;
if (inConstructor(t)) { if (inConstructor(t)) {
// This maybe a valid assignment. // This may be a valid assignment.
return maybeStoreCandidateValue(getJSType(src), propName, value); return maybeStoreCandidateValue(getJSType(src), propName, value);
} }
return false;
} else if (t.inGlobalHoistScope() } else if (t.inGlobalHoistScope()
&& src.isGetProp() && src.isGetProp()
&& src.getLastChild().getString().equals("prototype")) { && src.getLastChild().getString().equals("prototype")) {
Expand Down Expand Up @@ -198,13 +210,22 @@ private boolean maybeStoreCandidateValue(JSType type, String propName, Node valu
return false; return false;
} }


/**
* Returns whether the traversal is directly in an ES6 class constructor or an @constructor
* function
*
* <p>This returns false for nested functions inside ctors, including arrow functions (even
* though the `this` is the same). This pass only cares about property definitions executed once
* per ctor invocation, and in general we don't know how many times an arrow fn will be
* executed. In the future, we could special case arrow fn IIFEs in this pass if it becomes
* useful.
*/
private boolean inConstructor(NodeTraversal t) { private boolean inConstructor(NodeTraversal t) {
Node root = t.getEnclosingFunction(); Node root = t.getEnclosingFunction();
if (root == null) { if (root == null) { // we might be in the global scope
return false; return false;
} }
JSDocInfo info = NodeUtil.getBestJSDocInfo(root); return NodeUtil.isConstructor(root);
return info != null && info.isConstructor();
} }
} }


Expand All @@ -231,17 +252,29 @@ && isMatchingType(target, info.type)) {
private boolean isMatchingType(Node n, JSType src) { private boolean isMatchingType(Node n, JSType src) {
src = src.restrictByNotNullOrUndefined(); src = src.restrictByNotNullOrUndefined();
JSType dest = getJSType(n).restrictByNotNullOrUndefined(); JSType dest = getJSType(n).restrictByNotNullOrUndefined();
if (!invalidatingTypes.isInvalidating(dest)) { if (invalidatingTypes.isInvalidating(dest)) {
if (dest.isConstructor() || src.isConstructor()) { return false;
// Don't inline constructor properties referenced from }
// subclass constructor references. This would be appropriate if (dest.isConstructor() || src.isConstructor()) {
// for ES6 class with Class-side inheritence but not // instead of using .isSubtypeOf for functions, check the prototype chain, since the
// traditional Closure classes from which subclass constructor // FunctionType subtyping semantics is not what we want.
// don't inherit the super-classes constructor properties. // This case is for ES6 class-side inheritance
return dest.equals(src); return hasInPrototypeChain(dest.toMaybeFunctionType(), src.toMaybeFunctionType());
} else { }
return dest.isSubtypeOf(src); return dest.isSubtypeOf(src);
}

@SuppressWarnings("ReferenceEquality")
private boolean hasInPrototypeChain(FunctionType subCtor, FunctionType superCtor) {
if (subCtor == null || superCtor == null) {
return false;
}
ObjectType proto = subCtor;
while (proto != null) {
if (proto == superCtor) {
return true;
} }
proto = proto.getImplicitPrototype();
} }
return false; return false;
} }
Expand Down
174 changes: 174 additions & 0 deletions test/com/google/javascript/jscomp/InlinePropertiesTest.java
View file
Expand Up @@ -531,4 +531,178 @@ public void testStructuralInterfacesNoPropInlining2() {
"function f(/** ? */ x) { return x.foo; }", "function f(/** ? */ x) { return x.foo; }",
"f(new C());")); "f(new C());"));
} }

@Test
public void testConstInstanceProp_es6Class() {
// Replace a reference to known constant property.
test(
lines(
"class C {", //
" constructor() {",
" this.foo = 1;",
" }",
"}",
"new C().foo;"),
lines(
"class C {", //
" constructor() {",
" this.foo = 1;",
" }",
"}",
"new C(), 1;"));
}

@Test
public void testMultipleConstInstanceProp_es6Class() {
test(
lines(
"class Foo {",
" constructor() {",
" /** @type {?number} */",
" this.a = 1;",
" /** @type {number} */",
" this.b = 2;",
" }",
"}",
"var x = (new Foo).b;"),
lines(
"class Foo {",
" constructor() {",
" /** @type {?number} */",
" this.a = 1;",
" /** @type {number} */",
" this.b = 2;",
" }",
"}",
"var x = (new Foo, 2);"));
}

@Test
public void testConstInstancePropInArrowFunction_es6Class() {
// Don't replace a reference to known constant property defined in an arrow function.
testSame(
lines(
"/** @unrestricted */", // make this not a struct, so we can define this.foo
"class C {", //
" constructor() {",
" (() => {",
" this.foo = 1;",
" })();",
" }",
"}",
"new C().foo;"));
}

@Test
public void testConstClassProps_es6Class() {
// Inline constant class properties,
test(
lines(
"class C {}", //
"C.bar = 2;",
"C.foo = 1;",
"var z = C.foo;"),
lines(
"class C {}", //
"C.bar = 2;",
"C.foo = 1;",
"var z = 1;"));
}

@Test
public void testConstClassPropsInheritedProp_es6Class() {
test(
lines(
"class C {}", //
"class D extends C {}",
"C.foo = 1;",
"var z = D.foo;\n"),
lines(
"class C {}", //
"class D extends C {}",
"C.foo = 1;",
"var z = 1;\n"));
}

@Test
public void testConstClassPropsInheritedPropChain_es6Class() {
test(
lines(
"class C {}", //
"class D extends C {}",
"class E extends D {}",
"class F extends E {}",
"C.foo = 1;",
"var z = F.foo;"),
lines(
"class C {}", //
"class D extends C {}",
"class E extends D {}",
"class F extends E {}",
"C.foo = 1;",
"var z = 1;"));
}

@Test
public void testConstClassPropsNonInheritedProp_es6Class() {
// Test that we don't accidentally treat the superclass as having a subclass prop
testSame(
lines(
"class C {}", //
"class D extends C {}",
"D.foo = 1;",
"var z = C.foo;"));
}

@Test
public void testNonConstClassProp_es6ClassWithStaticMethod() {
testSame(
lines(
"class C { static foo() {} }", //
"alert(C.foo);",
"C.foo = 1;"));
}

@Test
public void testConstPrototypeProp_es6Class() {
test(
lines(
"class C {}", //
"C.prototype.foo = 1;",
"new C().foo;"),
lines(
"class C {}", //
"C.prototype.foo = 1;",
"new C(), 1;"));
}

@Test
public void testNonConstPrototypePropFromMemberFn() {
testSame(
lines(
"class C {", //
" foo() {}",
"}",
"C.prototype.foo = 4;",
"(new C()).foo"));
}

@Test
public void testObjectPatternStringKeyDoesntInvalidateProp() {
test(
lines(
"/** @constructor */",
"function C() {",
" this.foo = 3;",
"}",
"(new C()).foo",
"const {foo} = new C();"),
lines(
"/** @constructor */",
"function C() {",
" this.foo = 3;",
"}",
"new C(), 3;",
"const {foo} = new C();"));
}
} }

0 comments on commit 85d9035

Please sign in to comment.