Skip to content

Commit

Permalink
Don't copy static-inherited members that are assigned after the $jsco…
Browse files Browse the repository at this point in the history
…mp.inherits call.

Because the type checker is not yet aware of static inheritance, this ensures that we get concrete type errors whenever the naive $jscomp.inherits implementation used by IE10 and below will cause problems.

Related to #2708

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=175623237
  • Loading branch information
shicks authored and Tyler Breisacher committed Nov 14, 2017
1 parent 93cea5b commit 2c26378
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 7 deletions.
35 changes: 28 additions & 7 deletions src/com/google/javascript/jscomp/Es6ToEs3ClassSideInheritance.java
Expand Up @@ -27,10 +27,12 @@
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token; import com.google.javascript.rhino.Token;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.LinkedHashSet; import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Set; import java.util.Set;


/** /**
Expand Down Expand Up @@ -132,18 +134,18 @@ public void process(Node externs, Node root) {
FindStaticMembers findStaticMembers = new FindStaticMembers(); FindStaticMembers findStaticMembers = new FindStaticMembers();
TranspilationPasses.processTranspile(compiler, externs, findStaticMembers); TranspilationPasses.processTranspile(compiler, externs, findStaticMembers);
TranspilationPasses.processTranspile(compiler, root, findStaticMembers); TranspilationPasses.processTranspile(compiler, root, findStaticMembers);
processInherits(findStaticMembers.inheritsCalls); processInherits(findStaticMembers);
} }


@Override @Override
public void hotSwapScript(Node scriptRoot, Node originalRoot) { public void hotSwapScript(Node scriptRoot, Node originalRoot) {
FindStaticMembers findStaticMembers = new FindStaticMembers(); FindStaticMembers findStaticMembers = new FindStaticMembers();
TranspilationPasses.processTranspile(compiler, scriptRoot, findStaticMembers); TranspilationPasses.processTranspile(compiler, scriptRoot, findStaticMembers);
processInherits(findStaticMembers.inheritsCalls); processInherits(findStaticMembers);
} }


private void processInherits(List<Node> inheritsCalls) { private void processInherits(FindStaticMembers findStaticMembers) {
for (Node inheritsCall : inheritsCalls) { for (Node inheritsCall : findStaticMembers.inheritsCalls) {
Node superclassNameNode = inheritsCall.getLastChild(); Node superclassNameNode = inheritsCall.getLastChild();
String superclassQname = superclassNameNode.getQualifiedName(); String superclassQname = superclassNameNode.getQualifiedName();
Node subclassNameNode = superclassNameNode.getPrevious(); Node subclassNameNode = superclassNameNode.getPrevious();
Expand All @@ -157,7 +159,7 @@ private void processInherits(List<Node> inheritsCalls) {
if (superClass == null || subClass == null) { if (superClass == null || subClass == null) {
continue; continue;
} }
copyStaticMembers(superClass, subClass, inheritsCall); copyStaticMembers(superClass, subClass, inheritsCall, findStaticMembers);
copyDeclarations(superClass, subClass, inheritsCall); copyDeclarations(superClass, subClass, inheritsCall);
} }
} }
Expand Down Expand Up @@ -205,7 +207,8 @@ private void copyDeclarations(
} }


private void copyStaticMembers( private void copyStaticMembers(
JavascriptClass superClass, JavascriptClass subClass, Node inheritsCall) { JavascriptClass superClass, JavascriptClass subClass, Node inheritsCall,
FindStaticMembers findStaticMembers) {
for (Node staticMember : superClass.staticMembers) { for (Node staticMember : superClass.staticMembers) {
checkState(staticMember.isAssign(), staticMember); checkState(staticMember.isAssign(), staticMember);
String memberName = staticMember.getFirstChild().getLastChild().getString(); String memberName = staticMember.getFirstChild().getLastChild().getString();
Expand All @@ -215,6 +218,13 @@ private void copyStaticMembers(
if (isOverriden(subClass, memberName)) { if (isOverriden(subClass, memberName)) {
continue; continue;
} }
if (findStaticMembers.isBefore(inheritsCall, staticMember)) {
// Don't copy members that are defined after the $jscomp.inherits call,
// since they will not work correctly in IE<11, where static inheritance
// is done by copying, rather than prototype manipulation.
continue;
}

JSDocInfoBuilder info = JSDocInfoBuilder.maybeCopyFrom(staticMember.getJSDocInfo()); JSDocInfoBuilder info = JSDocInfoBuilder.maybeCopyFrom(staticMember.getJSDocInfo());
Node function = staticMember.getLastChild(); Node function = staticMember.getLastChild();
Node sourceInfoNode = function; Node sourceInfoNode = function;
Expand Down Expand Up @@ -279,14 +289,18 @@ private boolean isReferenceToClass(NodeTraversal t, Node n) {
} }


private class FindStaticMembers extends AbstractPostOrderCallback { private class FindStaticMembers extends AbstractPostOrderCallback {
private final List<Node> inheritsCalls = new ArrayList<>(); final List<Node> inheritsCalls = new ArrayList<>();
// Store the order we find class definitions and static fields. Copied statics must occur
// after both the namespace and the copied property are defined.
final Map<Node, Integer> nodeOrder = new HashMap<>();


@Override @Override
public void visit(NodeTraversal t, Node n, Node parent) { public void visit(NodeTraversal t, Node n, Node parent) {
switch (n.getToken()) { switch (n.getToken()) {
case CALL: case CALL:
if (n.getFirstChild().matchesQualifiedName(Es6RewriteClass.INHERITS)) { if (n.getFirstChild().matchesQualifiedName(Es6RewriteClass.INHERITS)) {
inheritsCalls.add(n); inheritsCalls.add(n);
nodeOrder.put(n, nodeOrder.size());
} }
if (NodeUtil.isObjectDefinePropertiesDefinition(n)) { if (NodeUtil.isObjectDefinePropertiesDefinition(n)) {
visitDefinedPropertiesCall(t, n); visitDefinedPropertiesCall(t, n);
Expand Down Expand Up @@ -359,6 +373,7 @@ private void visitAssign(NodeTraversal t, Node n) {
Node classNode = getProp.getFirstChild(); Node classNode = getProp.getFirstChild();
if (isReferenceToClass(t, classNode)) { if (isReferenceToClass(t, classNode)) {
classByAlias.get(classNode.getQualifiedName()).staticMembers.add(n); classByAlias.get(classNode.getQualifiedName()).staticMembers.add(n);
nodeOrder.put(n, nodeOrder.size());
} }
} }
} }
Expand All @@ -376,5 +391,11 @@ private void visitVar(Node n) {
} }
} }
} }

boolean isBefore(Node earlier, Node later) {
Integer earlierPosition = nodeOrder.get(earlier);
Integer laterPosition = nodeOrder.get(later);
return earlierPosition != null && laterPosition != null && earlierPosition < laterPosition;
}
} }
} }
Expand Up @@ -44,6 +44,7 @@ protected int getNumRepetitions() {
public void testSimple() { public void testSimple() {
test( test(
lines( lines(
// Note: let statement is necessary so that input language is ES6; else pass is skipped.
"let x = 1;", "let x = 1;",
"/** @constructor */", "/** @constructor */",
"function Example() {}", "function Example() {}",
Expand Down Expand Up @@ -95,6 +96,43 @@ public void testTyped() {
"Subclass.staticMethod = Example.staticMethod;")); "Subclass.staticMethod = Example.staticMethod;"));
} }



public void testNestedSubclass() {
test(
lines(
"let x = 1;",
"/** @constructor */",
"function A() {}",
"A.a = 19;",
"/** @constructor */",
"A.B = function() {};",
"/** @constructor @extends {A} */",
"A.C = function() {};",
"$jscomp.inherits(A.C, A);",
"/** @constructor */",
"A.D = function() {};",
"A.e = 42;"),
lines(
"let x = 1;",
"/** @constructor */",
"function A() {}",
"A.a = 19;",
"/** @constructor */",
"A.B = function() {}",
"/** @constructor @extends {A} */",
"A.C = function() {};",
"$jscomp.inherits(A.C, A);",
"/** @constructor @extends {A} @suppress {visibility} */",
"A.C.C = A.C;",
"/** @constructor @suppress {visibility} */",
"A.C.B = A.B;",
"/** @suppress {visibility} */",
"A.C.a = A.a;",
"/** @constructor */",
"A.D = function() {};",
"A.e = 42;"));
}

public void testOverride() { public void testOverride() {
testSame( testSame(
lines( lines(
Expand Down

0 comments on commit 2c26378

Please sign in to comment.