Skip to content

Commit

Permalink
In SymbolTable deduplicate extern symbols declared on window object b…
Browse files Browse the repository at this point in the history
…y DeclaredGlobalExternsOnWindow.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=218207266
  • Loading branch information
nbeloglazov authored and tjgq committed Oct 23, 2018
1 parent 7143b47 commit 6b31fc1
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 4 deletions.
Expand Up @@ -109,6 +109,7 @@ private static void addExtern(Node node, boolean defineOnWindow) {

newNode.useSourceInfoFromForTree(node);
newNode.setOriginalName(name);
newNode.makeNonIndexable();
node.getGrandparent().addChildToBack(IR.exprResult(newNode));
}

Expand Down
72 changes: 68 additions & 4 deletions src/com/google/javascript/jscomp/SymbolTable.java
Expand Up @@ -633,6 +633,19 @@ private Symbol declareSymbol(
return symbol;
}

/**
* Merges 'from' symbol to 'to' symbol by moving all references to point to the 'to' symbol and
* removing 'from' symbol.
*/
private void mergeSymbol(Symbol from, Symbol to) {
for (Node nodeToMove : from.references.keySet()) {
if (!nodeToMove.equals(from.getDeclarationNode())) {
to.defineReferenceAt(nodeToMove);
}
}
removeSymbol(from);
}

private void removeSymbol(Symbol s) {
SymbolScope scope = getScope(s);
if (!s.equals(scope.ownSymbols.remove(s.getName()))) {
Expand Down Expand Up @@ -1157,6 +1170,59 @@ private boolean isSymbolAQuotedObjectKey(Symbol symbol) {
return node != null && node.isStringKey() && node.isQuotedString();
}

/**
* Heuristic method to check whether symbol was created by DeclaredGlobalExternsOnWindow.java
* pass.
*/
private boolean isSymbolDuplicatedExternOnWindow(Symbol symbol) {
return symbol.getName().startsWith("window.") && !symbol.getDeclarationNode().isIndexable();
}

/**
* DeclaredGLobalExternsOnWindow.java pass duplicates all global variables so that:
*
* <pre>
* var foo;
* </pre>
*
* becomes
*
* <pre>
* var foo;
* window.foo;
* </pre>
*
* This function finds all such cases and merges window.foo symbol back to foo. It changes
* window.foo references to point to foo symbol.
*/
private void mergeExternSymbolsDuplicatedOnWindow() {
// To find duplicated symbols we rely on the fact that duplicated symbol share the same
// source position as original symbol and going to use filename => sourcePosition => symbol
// table.
Table<String, Integer, Symbol> externSymbols = HashBasedTable.create();
for (Symbol symbol : ImmutableList.copyOf(symbols.values())) {
if (symbol.getDeclarationNode() == null
|| !symbol.getDeclarationNode().getStaticSourceFile().isExtern()) {
continue;
}
String sourceFile = symbol.getSourceFileName();
int position = symbol.getDeclarationNode().getSourcePosition();
if (!externSymbols.contains(sourceFile, position)) {
externSymbols.put(sourceFile, position, symbol);
continue;
}
Symbol existingSymbol = externSymbols.get(sourceFile, position);
// Consider 2 possibilies: either symbol or existingSymbol might be the generated symbol we
// are looking for.
if (isSymbolDuplicatedExternOnWindow(existingSymbol)) {
mergeSymbol(existingSymbol, symbol);
externSymbols.put(sourceFile, position, symbol);
} else if (isSymbolDuplicatedExternOnWindow(symbol)) {
mergeSymbol(symbol, existingSymbol);
}
}
}

/**
* Removes various generated symbols that are invisible to users and pollute or mess up index.
* Jscompiler does transpilations that might introduce extra nodes/symbols. Most of these symbols
Expand Down Expand Up @@ -1193,6 +1259,7 @@ void removeGeneratedSymbols() {
}
}
}
mergeExternSymbolsDuplicatedOnWindow();
}

/**
Expand Down Expand Up @@ -1245,10 +1312,7 @@ private void inlineEs6ExportProperty(
if (originalSymbol == null) {
return;
}
for (Node nodeToMove : exportPropertySymbol.references.keySet()) {
originalSymbol.defineReferenceAt(nodeToMove);
}
removeSymbol(exportPropertySymbol);
mergeSymbol(exportPropertySymbol, originalSymbol);
}

/**
Expand Down
18 changes: 18 additions & 0 deletions test/com/google/javascript/jscomp/SymbolTableTest.java
Expand Up @@ -36,8 +36,10 @@
import com.google.javascript.rhino.Token;
import com.google.javascript.rhino.jstype.FunctionType;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import junit.framework.TestCase;
import org.junit.Before;
Expand Down Expand Up @@ -1414,6 +1416,22 @@ public void testSuperKeywords() {
assertThat(numberOfSuperReferences).isEqualTo(1);
}

@Test
public void testDuplicatedWindowExternsMerged() {
// Add window so that it triggers logic that defines all global externs on window.
// See DeclaredGlobalExternsOnWindow.java pass.
String externs = lines("/** @externs */", "var window", "var foo;");
String mainCode = lines("foo;", "window.foo;");
SymbolTable table = createSymbolTable(mainCode, externs);

Map<String, Integer> refsPerFile = new HashMap<>();
for (Reference reference : table.getReferenceList(getGlobalVar(table, "foo"))) {
String file = reference.getSourceFile().getName();
refsPerFile.put(file, refsPerFile.getOrDefault(file, 0) + 1);
}
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 6b31fc1

Please sign in to comment.