Record imported symbols in PreprocessorSymbolTable before they are re…

…moved by Es6RewriteModule pass. Tested: kythe tests ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=186799301
google · Feb 25, 2018 · 1f3052e · 1f3052e
1 parent 4b2f978
commit 1f3052e
Show file tree

Hide file tree

Showing 9 changed files with 114 additions and 52 deletions.
diff --git a/src/com/google/javascript/jscomp/ClosureRewriteModule.java b/src/com/google/javascript/jscomp/ClosureRewriteModule.java
@@ -1785,7 +1785,7 @@ private void maybeAddToSymbolTable(Node n) {
    */
   private void maybeAddAliasToSymbolTable(Node n, String module) {
     if (preprocessorSymbolTable != null) {
-      n.putBooleanProp(Node.GOOG_MODULE_ALIAS, true);
+      n.putBooleanProp(Node.MODULE_ALIAS, true);
       // Alias can be used in js types. Types have node type STRING and not NAME so we have to
       // use their name as string.
       String nodeName =

diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java
@@ -65,6 +65,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import javax.annotation.Nullable;
 
 /**
  * Pass factories and meta-data for native JSCompiler passes.
@@ -99,11 +100,9 @@ public final class DefaultPassConfig extends PassConfig {
    */
   private transient GlobalNamespace namespaceForChecks = null;
 
-  /**
+  /** A symbol table for registering references that get removed during preprocessing. */
-   * A symbol table for registering references that get removed during
+  private final transient PreprocessorSymbolTable.CachedInstanceFactory
-   * preprocessing.
+      preprocessorSymbolTableFactory;
-   */
-  private transient PreprocessorSymbolTable preprocessorSymbolTable = null;
 
   /**
    * Global state necessary for doing hotswap recompilation of files with references to
@@ -124,23 +123,16 @@ public DefaultPassConfig(CompilerOptions options) {
     // wrap them in a function call that is stripped later, this shouldn't
     // be done in IDE mode where AST changes may be unexpected.
     protectHiddenSideEffects = options != null && options.shouldProtectHiddenSideEffects();
+    preprocessorSymbolTableFactory = new PreprocessorSymbolTable.CachedInstanceFactory();
   }
 
   GlobalNamespace getGlobalNamespace() {
     return namespaceForChecks;
   }
 
+  @Nullable
   PreprocessorSymbolTable getPreprocessorSymbolTable() {
-    return preprocessorSymbolTable;
+    return preprocessorSymbolTableFactory.getInstanceOrNull();
-  }
-
-  void maybeInitializePreprocessorSymbolTable(AbstractCompiler compiler) {
-    if (options.preservesDetailedSourceInfo()) {
-      Node root = compiler.getRoot();
-      if (preprocessorSymbolTable == null || preprocessorSymbolTable.getRootNode() != root) {
-        preprocessorSymbolTable = new PreprocessorSymbolTable(root);
-      }
-    }
   }
 
   void maybeInitializeModuleRewriteState() {
@@ -158,7 +150,7 @@ protected List<PassFactory> getTranspileOnlyPasses() {
     }
 
     if (options.getLanguageIn().toFeatureSet().has(FeatureSet.Feature.MODULES)) {
-      TranspilationPasses.addEs6ModulePass(passes);
+      TranspilationPasses.addEs6ModulePass(passes, preprocessorSymbolTableFactory);
     }
 
     passes.add(checkSuper);
@@ -305,7 +297,7 @@ protected List<PassFactory> getChecks() {
     }
 
     if (options.getLanguageIn().toFeatureSet().has(FeatureSet.Feature.MODULES)) {
-      TranspilationPasses.addEs6ModulePass(checks);
+      TranspilationPasses.addEs6ModulePass(checks, preprocessorSymbolTableFactory);
     }
 
     checks.add(checkVariableReferences);
@@ -1363,11 +1355,11 @@ public FeatureSet featureSet() {
       new HotSwapPassFactory("closurePrimitives") {
         @Override
         protected HotSwapCompilerPass create(final AbstractCompiler compiler) {
-          maybeInitializePreprocessorSymbolTable(compiler);
+          preprocessorSymbolTableFactory.maybeInitialize(compiler);
           final ProcessClosurePrimitives pass =
               new ProcessClosurePrimitives(
                   compiler,
-                  preprocessorSymbolTable,
+                  preprocessorSymbolTableFactory.getInstanceOrNull(),
                   options.brokenClosureRequiresLevel,
                   options.shouldPreservesGoogProvidesAndRequires());
 
@@ -1454,9 +1446,11 @@ public FeatureSet featureSet() {
       new HotSwapPassFactory("closureGoogScopeAliases") {
         @Override
         protected HotSwapCompilerPass create(AbstractCompiler compiler) {
-          maybeInitializePreprocessorSymbolTable(compiler);
+          preprocessorSymbolTableFactory.maybeInitialize(compiler);
           return new ScopedAliases(
-              compiler, preprocessorSymbolTable, options.getAliasTransformationHandler());
+              compiler,
+              preprocessorSymbolTableFactory.getInstanceOrNull(),
+              options.getAliasTransformationHandler());
         }
 
         @Override
@@ -1610,9 +1604,10 @@ protected FeatureSet featureSet() {
       new HotSwapPassFactory("closureRewriteModule") {
         @Override
         protected HotSwapCompilerPass create(AbstractCompiler compiler) {
-          maybeInitializePreprocessorSymbolTable(compiler);
+          preprocessorSymbolTableFactory.maybeInitialize(compiler);
           maybeInitializeModuleRewriteState();
-          return new ClosureRewriteModule(compiler, preprocessorSymbolTable, moduleRewriteState);
+          return new ClosureRewriteModule(
+              compiler, preprocessorSymbolTableFactory.getInstanceOrNull(), moduleRewriteState);
         }
 
         @Override

diff --git a/src/com/google/javascript/jscomp/Es6RewriteModules.java b/src/com/google/javascript/jscomp/Es6RewriteModules.java
@@ -38,6 +38,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import javax.annotation.Nullable;
 
 /**
  * Rewrites a ES6 module into a form that can be safely concatenated. Note that we treat a file as
@@ -64,6 +65,8 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback
       DiagnosticType.error("JSC_DUPLICATE_EXPORT", "Duplicate export ''{0}''.");
 
   private final AbstractCompiler compiler;
+
+  @Nullable private final PreprocessorSymbolTable preprocessorSymbolTable;
   private int scriptNodeCount;
 
   /**
@@ -85,11 +88,13 @@ public final class Es6RewriteModules extends AbstractPostOrderCallback
   private Set<String> typedefs;
 
   /**
-   * Creates a new Es6RewriteModules instance which can be used to rewrite
+   * Creates a new Es6RewriteModules instance which can be used to rewrite ES6 modules to a
-   * ES6 modules to a concatenable form.
+   * concatenable form.
    */
-  public Es6RewriteModules(AbstractCompiler compiler) {
+  public Es6RewriteModules(
+      AbstractCompiler compiler, @Nullable PreprocessorSymbolTable preprocessorSymbolTable) {
     this.compiler = compiler;
+    this.preprocessorSymbolTable = preprocessorSymbolTable;
   }
 
   /**
@@ -234,6 +239,7 @@ private void visitImport(NodeTraversal t, Node importDecl, Node parent) {
       } else if (child.isImportSpecs()) {
         for (Node grandChild : child.children()) {
           String origName = grandChild.getFirstChild().getString();
+          maybeAddAliasToSymbolTable(grandChild.getFirstChild(), t.getSourceName());
           checkState(grandChild.hasTwoChildren());
           importMap.put(
               grandChild.getLastChild().getString(),
@@ -249,6 +255,7 @@ private void visitImport(NodeTraversal t, Node importDecl, Node parent) {
               t.makeError(
                   importDecl, NAMESPACE_IMPORT_CANNOT_USE_STAR, child.getString(), moduleName));
         }
+        maybeAddAliasToSymbolTable(child, t.getSourceName());
         importMap.put(
             child.getString(),
             new ModuleOriginalNamePair(moduleName, ""));
@@ -628,6 +635,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
           boolean isImportStar = pair.originalName.isEmpty();
           Node moduleAccess = NodeUtil.newQName(compiler, pair.module);
 
+          maybeAddAliasToSymbolTable(n, t.getSourceName());
           if (isImportStar) {
             n.replaceWith(moduleAccess.useSourceInfoIfMissingFromForTree(n));
           } else {
@@ -689,6 +697,7 @@ private void fixTypeNode(NodeTraversal t, Node typeNode) {
             maybeSetNewName(t, typeNode, name, baseName + "$$" + suffix + rest);
           } else if (var == null && importMap.containsKey(baseName)) {
             ModuleOriginalNamePair pair = importMap.get(baseName);
+            maybeAddAliasToSymbolTable(typeNode, t.getSourceName());
             if (pair.originalName.isEmpty()) {
               maybeSetNewName(t, typeNode, name, pair.module + rest);
             } else {
@@ -713,6 +722,30 @@ private void maybeSetNewName(NodeTraversal t, Node node, String name, String new
     }
   }
 
+  /**
+   * Add alias nodes to the symbol table as they going to be removed by rewriter. Example aliases:
+   *
+   * <pre>
+   *   import * as foo from './foo';
+   *   import {doBar} from './bar';
+   * </pre>
+   */
+  private void maybeAddAliasToSymbolTable(Node n, String module) {
+    if (preprocessorSymbolTable != null) {
+      n.putBooleanProp(Node.MODULE_ALIAS, true);
+      // Alias can be used in js types. Types have node type STRING and not NAME so we have to
+      // use their name as string.
+      String nodeName =
+          n.isString() || n.isImportStar()
+              ? n.getString()
+              : preprocessorSymbolTable.getQualifiedName(n);
+      // We need to include module as part of the name because aliases are local to current module.
+      // Aliases with the same name from different module should be completely different entities.
+      String name = "alias_" + module + "_" + nodeName;
+      preprocessorSymbolTable.addReference(n, name);
+    }
+  }
+
   private static class ModuleOriginalNamePair {
     private final String module;
     private final String originalName;

diff --git a/src/com/google/javascript/jscomp/PreprocessorSymbolTable.java b/src/com/google/javascript/jscomp/PreprocessorSymbolTable.java
@@ -29,6 +29,7 @@
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import javax.annotation.Nullable;
 
 /**
  * A symbol table for references that are removed by preprocessor passes
@@ -131,4 +132,30 @@ static final class Reference extends SimpleReference<SimpleSlot> {
       super(symbol, node);
     }
   }
+
+  /**
+   * Object that maybe contains instance of the table. This object is needed because
+   * PreprocessorSymbolTable is used by multiple passes in different parts of code which initialized
+   * at different times (some even before compiler object is created). Instead instance of factory
+   * is passed around. Each pass that uses PreprocessorSymbolTable has to call maybeInitialize()
+   * before getting instance.
+   */
+  public static class CachedInstanceFactory {
+
+    @Nullable private PreprocessorSymbolTable instance;
+
+    public void maybeInitialize(AbstractCompiler compiler) {
+      if (compiler.getOptions().preservesDetailedSourceInfo()) {
+        Node root = compiler.getRoot();
+        if (instance == null || instance.getRootNode() != root) {
+          instance = new PreprocessorSymbolTable(root);
+        }
+      }
+    }
+
+    @Nullable
+    public PreprocessorSymbolTable getInstanceOrNull() {
+      return instance;
+    }
+  }
 }
diff --git a/src/com/google/javascript/jscomp/TranspilationPasses.java b/src/com/google/javascript/jscomp/TranspilationPasses.java
@@ -32,8 +32,22 @@
 public class TranspilationPasses {
   private TranspilationPasses() {}
 
-  public static void addEs6ModulePass(List<PassFactory> passes) {
+  public static void addEs6ModulePass(
-    passes.add(es6RewriteModule);
+      List<PassFactory> passes,
+      final PreprocessorSymbolTable.CachedInstanceFactory preprocessorTableFactory) {
+    passes.add(
+        new HotSwapPassFactory("es6RewriteModule") {
+          @Override
+          protected HotSwapCompilerPass create(AbstractCompiler compiler) {
+            preprocessorTableFactory.maybeInitialize(compiler);
+            return new Es6RewriteModules(compiler, preprocessorTableFactory.getInstanceOrNull());
+          }
+
+          @Override
+          protected FeatureSet featureSet() {
+            return ES_NEXT;
+          }
+        });
   }
 
   public static void addEs6ModuleToCjsPass(List<PassFactory> passes) {
@@ -110,20 +124,6 @@ public static void addRewritePolyfillPass(List<PassFactory> passes) {
     passes.add(rewritePolyfills);
   }
 
-  /** Rewrites ES6 modules */
-  private static final HotSwapPassFactory es6RewriteModule =
-      new HotSwapPassFactory("es6RewriteModule") {
-        @Override
-        protected HotSwapCompilerPass create(AbstractCompiler compiler) {
-          return new Es6RewriteModules(compiler);
-        }
-
-        @Override
-        protected FeatureSet featureSet() {
-          return ES_NEXT;
-        }
-      };
-
   /** Rewrites ES6 modules */
   private static final PassFactory es6RewriteModuleToCjs =
       new PassFactory("es6RewriteModuleToCjs", true) {

diff --git a/src/com/google/javascript/jscomp/parsing/IRFactory.java b/src/com/google/javascript/jscomp/parsing/IRFactory.java
@@ -2337,10 +2337,16 @@ Node processImportDecl(ImportDeclarationTree tree) {
       maybeWarnForFeature(tree, Feature.MODULES);
 
       Node firstChild = transformOrEmpty(tree.defaultBindingIdentifier, tree);
-      Node secondChild = (tree.nameSpaceImportIdentifier != null)
+      Node secondChild;
-          ? newStringNode(Token.IMPORT_STAR, tree.nameSpaceImportIdentifier.value)
+      if (tree.nameSpaceImportIdentifier == null) {
-          : transformListOrEmpty(Token.IMPORT_SPECS, tree.importSpecifierList);
+        secondChild = transformListOrEmpty(Token.IMPORT_SPECS, tree.importSpecifierList);
-      setSourceInfo(secondChild, tree);
+        // Currently source info is "import {foo} from '...';" expression. If needed this should be
+        // changed to use only "{foo}" part.
+        setSourceInfo(secondChild, tree);
+      } else {
+        secondChild = newStringNode(Token.IMPORT_STAR, tree.nameSpaceImportIdentifier.value);
+        setSourceInfo(secondChild, tree.nameSpaceImportIdentifier);
+      }
       Node thirdChild = processString(tree.moduleSpecifier);
 
       return newNode(Token.IMPORT, firstChild, secondChild, thirdChild);

diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java
@@ -155,8 +155,8 @@ public class Node implements Serializable {
       IS_ES6_CLASS = 92, // Indicates that a FUNCTION node is converted from an ES6 class
       TRANSPILED = 93, // Indicates that a SCRIPT represents a transpiled file
       DELETED = 94, // For passes that work only on deleted funs.
-      GOOG_MODULE_ALIAS = 95, // Indicates that the node is an alias of goog.require'd module.
+      MODULE_ALIAS = 95, // Indicates that the node is an alias or a name from goog.require'd module
-      // Aliases are desugared and inlined by compiler passes but we
+      // or ES6 module. Aliases are desugared and inlined by compiler passes but we
       // need to preserve them for building index.
       IS_UNUSED_PARAMETER = 96, // Mark a parameter as unused. Used to defer work from
       // RemovedUnusedVars to OptimizeParameters.
@@ -223,7 +223,7 @@ private static final String propToString(byte propType) {
         case IS_ES6_CLASS:       return "is_es6_class";
         case TRANSPILED:         return "transpiled";
         case DELETED:            return "DELETED";
-        case GOOG_MODULE_ALIAS:  return "goog_module_alias";
+        case MODULE_ALIAS:       return "module_alias";
         case IS_UNUSED_PARAMETER: return "is_unused_parameter";
         case MODULE_EXPORT:
           return "module_export";

diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java
@@ -1738,7 +1738,8 @@ private void testInternal(
 
   private static void transpileToEs5(AbstractCompiler compiler, Node externsRoot, Node codeRoot) {
     List<PassFactory> factories = new ArrayList<>();
-    TranspilationPasses.addEs6ModulePass(factories);
+    TranspilationPasses.addEs6ModulePass(
+        factories, new PreprocessorSymbolTable.CachedInstanceFactory());
     TranspilationPasses.addEs2017Passes(factories);
     TranspilationPasses.addEs2016Passes(factories);
     TranspilationPasses.addEs6EarlyPasses(factories);

diff --git a/test/com/google/javascript/jscomp/Es6RewriteModulesTest.java b/test/com/google/javascript/jscomp/Es6RewriteModulesTest.java
@@ -53,7 +53,7 @@ protected CompilerOptions getOptions() {
 
   @Override
   protected CompilerPass getProcessor(Compiler compiler) {
-    return new Es6RewriteModules(compiler);
+    return new Es6RewriteModules(compiler, /* preprocessorSymbolTable= */ null);
   }
 
   @Override