Skip to content

Commit

Permalink
Improve SymbolTable handling of inheritance (interfaces, records).
Browse files Browse the repository at this point in the history
When looking up property on a symbol try using  ObjectType.getPropertyNode() if available. That function handles interfaces/records with multiple parents.

1. Fixes inheritance in @records.
2. Supports multiple inheritance for interfaces.

PiperOrigin-RevId: 556971231
  • Loading branch information
nbeloglazov authored and copybara-github committed Aug 15, 2023
1 parent 84e1928 commit f13691d
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 9 deletions.
30 changes: 22 additions & 8 deletions src/com/google/javascript/jscomp/SymbolTable.java
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
import com.google.javascript.rhino.jstype.SimpleSlot;
import com.google.javascript.rhino.jstype.StaticTypedScope;
import com.google.javascript.rhino.jstype.StaticTypedSlot;
import com.google.javascript.rhino.jstype.UnionType;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -361,10 +360,9 @@ public List<Symbol> getAllSymbolsForType(JSType type) {
return ImmutableList.of();
}

UnionType unionType = type.toMaybeUnionType();
if (unionType != null) {
if (type.isUnionType()) {
ImmutableList.Builder<Symbol> result = ImmutableList.builder();
for (JSType alt : unionType.getAlternates()) {
for (JSType alt : type.toMaybeUnionType().getAlternates()) {
// Our type system never has nested unions.
Symbol altSym = getSymbolForTypeHelper(alt, true);
if (altSym != null) {
Expand Down Expand Up @@ -831,7 +829,7 @@ void fillNamespaceReferences() {
if (namespace == null && root.scope.isGlobalScope()) {
// Originally UNKNOWN_TYPE has been always used for namespace symbols even though
// compiler does have type information attached to a node. Unclear why. Changing code to
// propery type mostly works. It only fails on Foo.prototype cases for some reason.
// property type mostly works. It only fails on Foo.prototype cases for some reason.
// It's pretty rare case when Foo.prototype defined in global scope though so for now
// just carve it out.
JSType nodeType = currentNode.getJSType();
Expand Down Expand Up @@ -1890,9 +1888,25 @@ public void process(Node externs, Node root) {
}

private boolean maybeDefineReference(Node n, String propName, Symbol ownerSymbol) {
// getPropertyScope() will be null in some rare cases where there
// are no extern declarations for built-in types (like Function).
if (ownerSymbol != null && ownerSymbol.getPropertyScope() != null) {
if (ownerSymbol == null) {
return false;
}
// If current symbol is ObjectType - try using ObjectType.getPropertyNode to find a node.
// That function searches for property in all extended classes and interfaces, which supports
// multiple extended interfaces.
JSType owner = ownerSymbol.getType();
if (owner != null && owner.isObjectType()) {
Node propNode = owner.toObjectType().getPropertyNode(propName);
Symbol propSymbol = symbols.get(propNode, propName);
if (propSymbol != null) {
propSymbol.defineReferenceAt(n);
return true;
}
}
// Use PropertyScope to find property. PropertyScope doesn't support multiple parents so all
// cases where multiple parents can occur (e.g. class implementing 2+ interfaces) must be
// handled above.
if (ownerSymbol.getPropertyScope() != null) {
Symbol prop = ownerSymbol.getPropertyScope().getSlot(propName);
if (prop != null) {
prop.defineReferenceAt(n);
Expand Down
96 changes: 95 additions & 1 deletion test/com/google/javascript/jscomp/SymbolTableTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,100 @@ public void testMethodReferences() {
assertThat(table.getReferences(method)).hasSize(3);
}

@Test
public void testMethodReferencesNullableClass() {
SymbolTable table =
createSymbolTable(
lines(
"class DomHelper { method() {} }",
"let /** ?DomHelper */ helper;",
"helper.method();"));

Symbol method = getGlobalVar(table, "DomHelper.prototype.method");
assertThat(table.getReferences(method)).hasSize(2);
}

@Test
public void testMethodReferencesNullableInterface() {
SymbolTable table =
createSymbolTable(
lines(
"/** @interface */ class DomHelper { method() {} }",
"let /** ?DomHelper */ helper;",
"helper.method();"));

Symbol method = getGlobalVar(table, "DomHelper.prototype.method");
assertThat(table.getReferences(method)).hasSize(2);
}

@Test
public void testMethodReferencesNullableRecord() {
SymbolTable table =
createSymbolTable(
lines(
"/** @record */ function DomHelper() {}",
"DomHelper.prototype.method = function() {};",
"let /** ?DomHelper */ helper;",
"helper.method();"));

Symbol method = getGlobalVar(table, "DomHelper.prototype.method");
assertThat(table.getReferences(method)).hasSize(2);
}

@Test
public void testRecordInheritance() {
SymbolTable table =
createSymbolTable(
lines(
"/** @record */ function DomHelper() {}",
"DomHelper.prototype.method = function() {};",
"",
"/** @record @extends {DomHelper} */ function DomHelper2() {}",
"DomHelper2.prototype.method2 = function() {};",
"",
"let /** ?DomHelper2 */ helper;",
"helper.method();"));

Symbol method = getGlobalVar(table, "DomHelper.prototype.method");
assertThat(table.getReferences(method)).hasSize(2);
}

@Test
public void testUnionType() {
SymbolTable table =
createSymbolTable(
lines(
"class First { method() {} }",
"class Second { method() {} }",
"let /** !Second|!First */ obj;",
"obj.method();"));

assertThat(table.getReferences(getGlobalVar(table, "First.prototype.method"))).hasSize(2);
assertThat(table.getReferences(getGlobalVar(table, "Second.prototype.method"))).hasSize(2);
}

@Test
public void testMultipleInterfaceInheritance() {
SymbolTable table =
createSymbolTable(
lines(
"/** @interface */",
"class First { doFirst() {} }",
"",
"/** @interface */",
"class Second { doSecond() {} }",
"",
"/** @interface @extends {First} @extends {Second} */",
"class Third {}",
"",
"let /** !Third */ obj;",
"obj.doFirst();",
"obj.doSecond();"));

assertThat(table.getReferences(getGlobalVar(table, "First.prototype.doFirst"))).hasSize(2);
assertThat(table.getReferences(getGlobalVar(table, "Second.prototype.doSecond"))).hasSize(2);
}

@Test
public void testSuperClassMethodReferences() {
SymbolTable table =
Expand Down Expand Up @@ -1737,7 +1831,7 @@ public void testDuplicatedWindowExternsMergedWithWindowPrototype() {
}
assertThat(refsPerFile).containsExactly("in1", 2, "externs1", 1);
}

private void assertSymmetricOrdering(Ordering<Symbol> ordering, Symbol first, Symbol second) {
assertThat(ordering.compare(first, first)).isEqualTo(0);
assertThat(ordering.compare(second, second)).isEqualTo(0);
Expand Down

0 comments on commit f13691d

Please sign in to comment.