Skip to content

Commit

Permalink
Fix bug in indexer caused by aliasing object literals.
Browse files Browse the repository at this point in the history
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=161896989
  • Loading branch information
nbeloglazov authored and blickly committed Jul 18, 2017
1 parent 638aee4 commit 4169bde
Showing 1 changed file with 53 additions and 11 deletions.
64 changes: 53 additions & 11 deletions src/com/google/javascript/jscomp/SymbolTable.java
Expand Up @@ -52,6 +52,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -668,10 +669,10 @@ private Symbol declareSymbol(

private void removeSymbol(Symbol s) {
SymbolScope scope = getScope(s);
if (scope.ownSymbols.remove(s.getName()) != s) {
if (!scope.ownSymbols.remove(s.getName()).equals(s)) {
throw new IllegalStateException("Symbol not found in scope " + s);
}
if (symbols.remove(s.getDeclaration().getNode(), s.getName()) != s) {
if (!symbols.remove(s.getDeclaration().getNode(), s.getName()).equals(s)) {
throw new IllegalStateException("Symbol not found in table " + s);
}
}
Expand Down Expand Up @@ -734,6 +735,7 @@ void fillNamespaceReferences() {
}
}

@SuppressWarnings("ReferenceEquality")
void fillPropertyScopes() {
// Collect all object symbols.
// All symbols that came from goog.module are collected separately because they will have to
Expand Down Expand Up @@ -783,12 +785,50 @@ void fillPropertyScopes() {
//
// If we order them in reverse lexicographical order symbols x.y and x will be processed before
// foo. This is wrong as foo is in fact property of x.y namespace. So we must process all
// module$exports$ symbols first. That's why we collected them in separate list.
// module$exports$ symbols first. That's why we collected them in a separate list.
//
Collections.sort(types, getNaturalSymbolOrdering().reverse());
Collections.sort(googModuleExportTypes, getNaturalSymbolOrdering().reverse());
for (Symbol s : Iterables.concat(googModuleExportTypes, types)) {
createPropertyScopeFor(s);
Iterable<Symbol> allTypes = Iterables.concat(googModuleExportTypes, types);

// If you though we are done with tricky case - you were wrong. There is another one!
// The problem with the same property scope appearing several times. For example when using
// aliases:
//
// const OBJ = {one: 1};
// function() {
// const alias = OBJ;
// console.log(alias.one);
// }
//
// In this case both 'OBJ' and 'alias' are considered property scopes and are candidates for
// processing even though they share the same "type" which is "{one: 1}". As they share the same
// type we need to process only one of them. To do that we build a "type => root symbol" map.
// In this case the map will be {one: 1} => OBJ. Using this map will skip 'alias' when creating
// property scopes.
//
// NOTE: we are using IdentityHashMap to compare types using == because we need to find symbols
// that point to the exact same type instance.
Map<JSType, Symbol> symbolThatDeclaresType = new IdentityHashMap<>();
for (Symbol s : allTypes) {
// Symbols are sorted in reverse order so that those with more outer scope will come later in
// the list, and therefore override those set by aliases in more inner scope. The sorting
// happens few lines above.
symbolThatDeclaresType.put(s.getType(), s);
}

for (Symbol s : allTypes) {
// Create property scopes only based on "root" symbols for each type to handle aliases.
if (s.getType() == null || symbolThatDeclaresType.get(s.getType()).equals(s)) {
createPropertyScopeFor(s);
}
}

// Now we need to set the new property scope symbol to all aliases.
for (Symbol s : allTypes) {
if (s.getType() != null) {
s.propertyScope = symbolThatDeclaresType.get(s.getType()).getPropertyScope();
}
}

pruneOrphanedNames();
Expand Down Expand Up @@ -957,15 +997,16 @@ void fillSymbolVisibility(Node externs, Node root) {
}

/**
* Build a property scope for the given symbol. Any properties of the symbol
* will be added to the property scope.
* Build a property scope for the given symbol. Any properties of the symbol will be added to the
* property scope.
*
* It is important that property scopes are created in order from the leaves
* up to the root, so this should only be called from #fillPropertyScopes.
* If you try to create a property scope for a parent before its leaf,
* then the leaf will get cut and re-added to the parent property scope,
* <p>It is important that property scopes are created in order from the leaves up to the root, so
* this should only be called from #fillPropertyScopes. If you try to create a property scope for
* a parent before its leaf, then the leaf will get cut and re-added to the parent property scope,
* and weird things will happen.
*/
// This function uses == to compare types to be exact same instances.
@SuppressWarnings("ReferenceEquality")
private void createPropertyScopeFor(Symbol s) {
// In order to build a property scope for s, we will need to build
// a property scope for all its implicit prototypes first. This means
Expand All @@ -976,6 +1017,7 @@ private void createPropertyScopeFor(Symbol s) {
}

SymbolScope parentPropertyScope = null;

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

0 comments on commit 4169bde

Please sign in to comment.