Skip to content

Commit

Permalink
Make SymbolTable's property scope building support class-side inherit…
Browse files Browse the repository at this point in the history
…ance

The only real change is that SymbolTable#createPropertyScopeFor now supports cases where an object's implicit prototype is a class, and can resolve references to statically inherited properties.

Also adds a package-private option to run passes in PhaseOptimizer even if they don't support all features in the current AST.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=213700105
  • Loading branch information
lauraharker authored and tjgq committed Sep 21, 2018
1 parent a80c28f commit b79c9ac
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 23 deletions.
26 changes: 26 additions & 0 deletions src/com/google/javascript/jscomp/CompilerOptions.java
Expand Up @@ -119,6 +119,14 @@ public enum PropertyCollapseLevel {


private Optional<Boolean> languageOutIsDefaultStrict = Optional.absent(); private Optional<Boolean> languageOutIsDefaultStrict = Optional.absent();


/**
* Skips passes (logging a warning) whose PassFactory feature set doesn't include some features
* currently in the AST.
*
* <p>At the moment, this should only ever be set to false for testing.
*/
private boolean skipUnsupportedPasses = true;

/** /**
* The builtin set of externs to be used * The builtin set of externs to be used
*/ */
Expand Down Expand Up @@ -1985,6 +1993,24 @@ public FeatureSet getOutputFeatureSet() {
return languageIn.toFeatureSet(); return languageIn.toFeatureSet();
} }


/**
* Sets the behavior of PhaseOptimizer when given a pass that can't handle features in the current
* AST.
*
* <p>Currently the only options are either to run the pass anyway, and see what happens, or to
* skip the pass and log a warning. Only test code should do the former.
*
* <p>In the future we may make this option public. We may also make it into an enum, and add an
* option to throw runtime errors upon seeing unsupported passses.
*/
void setSkipUnsupportedPasses(boolean skipUnsupportedPasses) {
this.skipUnsupportedPasses = skipUnsupportedPasses;
}

boolean shouldSkipUnsupportedPasses() {
return skipUnsupportedPasses;
}

public boolean needsTranspilationFrom(FeatureSet languageLevel) { public boolean needsTranspilationFrom(FeatureSet languageLevel) {
// TODO(johnplaisted): This isn't really accurate. This should instead be the *parsed* language, // TODO(johnplaisted): This isn't really accurate. This should instead be the *parsed* language,
// not the *input* language. // not the *input* language.
Expand Down
3 changes: 2 additions & 1 deletion src/com/google/javascript/jscomp/PhaseOptimizer.java
Expand Up @@ -276,7 +276,8 @@ class NamedPass implements CompilerPass {


@Override @Override
public void process(Node externs, Node root) { public void process(Node externs, Node root) {
if (!factory.featureSet().contains(compiler.getFeatureSet())) { if (compiler.getOptions().shouldSkipUnsupportedPasses()
&& !factory.featureSet().contains(compiler.getFeatureSet())) {
// NOTE: this warning ONLY appears in code using the Google-internal runner. // NOTE: this warning ONLY appears in code using the Google-internal runner.
// Both CommandLineRunner.java and gwt/client/GwtRunner.java explicitly set the logging // Both CommandLineRunner.java and gwt/client/GwtRunner.java explicitly set the logging
// level to Level.OFF to avoid seeing this warning. // level to Level.OFF to avoid seeing this warning.
Expand Down
58 changes: 47 additions & 11 deletions src/com/google/javascript/jscomp/SymbolTable.java
Expand Up @@ -974,22 +974,18 @@ private void createPropertyScopeFor(Symbol s) {
return; return;
} }


SymbolScope parentPropertyScope = null;

ObjectType type = getType(s) == null ? null : getType(s).toObjectType(); ObjectType type = getType(s) == null ? null : getType(s).toObjectType();
if (type == null) { if (type == null) {
return; return;
} }


ObjectType proto = type.getImplicitPrototype(); // Create an empty property scope for the given symbol, maybe with a parent scope if it has
if (proto != null && proto != type && proto.getConstructor() != null) { // an implicit prototype.
Symbol parentSymbol = getSymbolForInstancesOf(proto.getConstructor()); SymbolScope parentPropertyScope = maybeGetParentPropertyScope(type);
if (parentSymbol != null) { s.setPropertyScope(new SymbolScope(null, parentPropertyScope, type, s));
createPropertyScopeFor(parentSymbol);
parentPropertyScope = parentSymbol.getPropertyScope();
}
}


// If this symbol represents some 'a.b.c.prototype', add any instance properties of a.b.c
// into the symbol scope.
ObjectType instanceType = type; ObjectType instanceType = type;
Iterable<String> propNames = type.getOwnPropertyNames(); Iterable<String> propNames = type.getOwnPropertyNames();
if (instanceType.isFunctionPrototypeType()) { if (instanceType.isFunctionPrototypeType()) {
Expand All @@ -1001,7 +997,7 @@ private void createPropertyScopeFor(Symbol s) {
} }
} }


s.setPropertyScope(new SymbolScope(null, parentPropertyScope, type, s)); // Add all declared properties in propNames into the property scope
for (String propName : propNames) { for (String propName : propNames) {
StaticSlot newProp = instanceType.getSlot(propName); StaticSlot newProp = instanceType.getSlot(propName);
if (newProp.getDeclaration() == null) { if (newProp.getDeclaration() == null) {
Expand Down Expand Up @@ -1044,6 +1040,46 @@ private void createPropertyScopeFor(Symbol s) {
} }
} }


/**
* If this type has an implicit prototype set, returns the SymbolScope corresponding to the
* properties of the implicit prototype. Otherwise returns null.
*
* <p>Note that currently we only handle cases where the implicit prototype is a) a class or b) is
* an instance object.
*/
@Nullable
private SymbolScope maybeGetParentPropertyScope(ObjectType symbolObjectType) {
ObjectType proto = symbolObjectType.getImplicitPrototype();
if (proto == null || proto == symbolObjectType) {
return null;
}
final Symbol parentSymbol;
if (isEs6ClassConstructor(proto)) {
// given `class Foo {} class Bar extends Foo {}`, `Foo` is the implicit prototype of `Bar`.
parentSymbol = getSymbolDeclaredBy(proto.toMaybeFunctionType());
} else if (proto.getConstructor() != null) {
// given
// /** @constructor */ function Foo() {}
// /** @constructor */ function Bar() {}
// goog.inherits(Bar, Foo);
// the implicit prototype of Bar.prototype is the instance of Foo.
parentSymbol = getSymbolForInstancesOf(proto.getConstructor());
} else {
return null;
}
if (parentSymbol == null) {
return null;
}
createPropertyScopeFor(parentSymbol);
return parentSymbol.getPropertyScope();
}

private boolean isEs6ClassConstructor(JSType type) {
return type.isFunctionType()
&& type.toMaybeFunctionType().getSource() != null
&& type.toMaybeFunctionType().getSource().isClass();
}

/** Fill in references to "this" variables. */ /** Fill in references to "this" variables. */
void fillThisReferences(Node externs, Node root) { void fillThisReferences(Node externs, Node root) {
(new ThisRefCollector()).process(externs, root); (new ThisRefCollector()).process(externs, root);
Expand Down
28 changes: 27 additions & 1 deletion test/com/google/javascript/jscomp/PhaseOptimizerTest.java
Expand Up @@ -217,6 +217,20 @@ public void testProgress() {
assertEquals(100, Math.round(progressList.get(3))); assertEquals(100, Math.round(progressList.get(3)));
} }


@Test
public void testSetSkipUnsupportedPasses() {
compiler.getOptions().setSkipUnsupportedPasses(true);
addUnsupportedPass("testPassFactory");
assertPasses();
}

@Test
public void testSetDontSkipUnsupportedPasses() {
compiler.getOptions().setSkipUnsupportedPasses(false);
addUnsupportedPass("testPassFactory");
assertPasses("testPassFactory");
}

public void assertPasses(String ... names) { public void assertPasses(String ... names) {
optimizer.process(null, dummyRoot); optimizer.process(null, dummyRoot);
assertEquals(ImmutableList.copyOf(names), passesRun); assertEquals(ImmutableList.copyOf(names), passesRun);
Expand All @@ -232,13 +246,25 @@ private void addLoopedPass(Loop loop, String name, int numChanges) {
createPassFactory(name, numChanges, false)); createPassFactory(name, numChanges, false));
} }


/** Adds a pass with the given name that does not support some of the features used in the AST. */
private void addUnsupportedPass(String name) {
compiler.setFeatureSet(FeatureSet.latest());
optimizer.addOneTimePass(
createPassFactory(name, createPass(name, 0), true, FeatureSet.BARE_MINIMUM));
}

private PassFactory createPassFactory( private PassFactory createPassFactory(
String name, int numChanges, boolean isOneTime) { String name, int numChanges, boolean isOneTime) {
return createPassFactory(name, createPass(name, numChanges), isOneTime); return createPassFactory(name, createPass(name, numChanges), isOneTime);
} }


private PassFactory createPassFactory( private PassFactory createPassFactory(
String name, final CompilerPass pass, boolean isOneTime) { String name, final CompilerPass pass, boolean isOneTime) {
return createPassFactory(name, pass, isOneTime, FeatureSet.latest());
}

private PassFactory createPassFactory(
String name, final CompilerPass pass, boolean isOneTime, FeatureSet featureSet) {
return new PassFactory(name, isOneTime) { return new PassFactory(name, isOneTime) {
@Override @Override
protected CompilerPass create(AbstractCompiler compiler) { protected CompilerPass create(AbstractCompiler compiler) {
Expand All @@ -247,7 +273,7 @@ protected CompilerPass create(AbstractCompiler compiler) {


@Override @Override
public FeatureSet featureSet() { public FeatureSet featureSet() {
return FeatureSet.latest(); return featureSet;
} }
}; };
} }
Expand Down
60 changes: 50 additions & 10 deletions test/com/google/javascript/jscomp/SymbolTableTest.java
Expand Up @@ -15,9 +15,12 @@
*/ */
package com.google.javascript.jscomp; package com.google.javascript.jscomp;


import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.javascript.jscomp.CompilerTestCase.lines; import static com.google.javascript.jscomp.CompilerTestCase.lines;
import static com.google.javascript.jscomp.parsing.Config.JsDocParsing.INCLUDE_DESCRIPTIONS_NO_WHITESPACE; import static com.google.javascript.jscomp.parsing.Config.JsDocParsing.INCLUDE_DESCRIPTIONS_NO_WHITESPACE;
import static com.google.javascript.rhino.testing.NodeSubject.assertNode;
import static com.google.javascript.rhino.testing.TypeSubject.assertType;


import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.Ordering; import com.google.common.collect.Ordering;
Expand All @@ -30,6 +33,7 @@
import com.google.javascript.rhino.JSDocInfo.Visibility; import com.google.javascript.rhino.JSDocInfo.Visibility;
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 com.google.javascript.rhino.jstype.FunctionType;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
Expand All @@ -40,7 +44,11 @@
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
import org.junit.runners.JUnit4; import org.junit.runners.JUnit4;


/** @author nicksantos@google.com (Nick Santos) */ /**
* An integration test for symbol table creation
*
* @author nicksantos@google.com (Nick Santos)
*/


@RunWith(JUnit4.class) @RunWith(JUnit4.class)
public final class SymbolTableTest extends TestCase { public final class SymbolTableTest extends TestCase {
Expand Down Expand Up @@ -419,9 +427,11 @@ public void testSymbolsForType() {
Symbol bar = getGlobalVar(table, "Bar"); Symbol bar = getGlobalVar(table, "Bar");
Symbol fooPrototype = getGlobalVar(table, "Foo.prototype"); Symbol fooPrototype = getGlobalVar(table, "Foo.prototype");
Symbol fn = getGlobalVar(table, "Function"); Symbol fn = getGlobalVar(table, "Function");
assertEquals(ImmutableList.of(foo, bar), table.getAllSymbolsForTypeOf(x)); assertThat(table.getAllSymbolsForTypeOf(x))
assertEquals(ImmutableList.of(fn), table.getAllSymbolsForTypeOf(foo)); .containsExactlyElementsIn(ImmutableList.of(foo, bar));
assertEquals(ImmutableList.of(foo), table.getAllSymbolsForTypeOf(fooPrototype)); assertThat(table.getAllSymbolsForTypeOf(foo)).containsExactlyElementsIn(ImmutableList.of(fn));
assertThat(table.getAllSymbolsForTypeOf(fooPrototype))
.containsExactlyElementsIn(ImmutableList.of(foo));
assertEquals(foo, table.getSymbolDeclaredBy(foo.getType().toMaybeFunctionType())); assertEquals(foo, table.getSymbolDeclaredBy(foo.getType().toMaybeFunctionType()));
} }


Expand Down Expand Up @@ -1320,6 +1330,41 @@ public void testGetEnclosingFunctionScope_FunctionInsideFunction() {
assertEquals(foo, bazFunctionScope.getRootNode()); assertEquals(foo, bazFunctionScope.getRootNode());
} }


@Test
public void testSymbolSuperclassStaticInheritance() {
// set this option so that typechecking sees untranspiled classes.
// TODO(b/76025401): remove this option after class transpilation is always post-typechecking
options.setLanguageOut(LanguageMode.ECMASCRIPT_2015);
options.setSkipUnsupportedPasses(false);

SymbolTable table =
createSymbolTable(
lines(
"class Bar { static staticMethod() {} }",
"class Foo extends Bar {",
" act() { Foo.staticMethod(); }",
"}"));

Symbol foo = getGlobalVar(table, "Foo");
Symbol bar = getGlobalVar(table, "Bar");

FunctionType barType = checkNotNull(bar.getType().toMaybeFunctionType());
FunctionType fooType = checkNotNull(foo.getType().toMaybeFunctionType());

FunctionType superclassCtorType = fooType.getSuperClassConstructor();

assertType(superclassCtorType).isEqualTo(barType);
Symbol superSymbol = table.getSymbolDeclaredBy(superclassCtorType);
assertThat(superSymbol).isEqualTo(bar);

Symbol barStaticMethod = getGlobalVar(table, "Bar.staticMethod");
ImmutableList<Reference> barStaticMethodRefs = table.getReferenceList(barStaticMethod);
assertThat(barStaticMethodRefs).hasSize(2);
assertThat(barStaticMethodRefs.get(0)).isEqualTo(barStaticMethod.getDeclaration());
// We recognize that the call to Foo.staticMethod() is actually a call to Bar.staticMethod().
assertNode(barStaticMethodRefs.get(1).getNode()).matchesQualifiedName("Foo.staticMethod");
}

private void assertSymmetricOrdering(Ordering<Symbol> ordering, Symbol first, Symbol second) { private void assertSymmetricOrdering(Ordering<Symbol> ordering, Symbol first, Symbol second) {
assertEquals(0, ordering.compare(first, first)); assertEquals(0, ordering.compare(first, first));
assertEquals(0, ordering.compare(second, second)); assertEquals(0, ordering.compare(second, second));
Expand Down Expand Up @@ -1363,12 +1408,7 @@ private List<Symbol> getVars(SymbolTable table) {
} }


private SymbolTable createSymbolTable(String input) { private SymbolTable createSymbolTable(String input) {
List<SourceFile> inputs = ImmutableList.of(SourceFile.fromCode("in1", input)); return createSymbolTable(input, EXTERNS);
List<SourceFile> externs = ImmutableList.of(SourceFile.fromCode("externs1", EXTERNS));

Compiler compiler = new Compiler(new BlackHoleErrorManager());
compiler.compile(externs, inputs, options);
return assertSymbolTableValid(compiler.buildKnownSymbolTable());
} }


private SymbolTable createSymbolTable(String input, String externsCode) { private SymbolTable createSymbolTable(String input, String externsCode) {
Expand Down

0 comments on commit b79c9ac

Please sign in to comment.