Skip to content

Commit

Permalink
Use the ModuleMap in RewriteGoogJsImports.
Browse files Browse the repository at this point in the history 
This pass must also modify the map as it edits import statements before other module rewriting occurs.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=235816111
  • Loading branch information
@brad4d
johnplaisted authored and brad4d committed Feb 27, 2019
1 parent e1f9b6a commit 40f6700
Show file tree
Hide file tree
Showing 6 changed files with 181 additions and 68 deletions.
3 changes: 2 additions & 1 deletion src/com/google/javascript/jscomp/DefaultPassConfig.java
View file
Expand Up @@ -1586,7 +1586,8 @@ protected FeatureSet featureSet() {
new HotSwapPassFactory("rewriteGoogJsImports") { new HotSwapPassFactory("rewriteGoogJsImports") {
@Override @Override
protected HotSwapCompilerPass create(AbstractCompiler compiler) { protected HotSwapCompilerPass create(AbstractCompiler compiler) {
return new RewriteGoogJsImports(compiler, RewriteGoogJsImports.Mode.LINT_AND_REWRITE); return new RewriteGoogJsImports(
compiler, RewriteGoogJsImports.Mode.LINT_AND_REWRITE, compiler.getModuleMap());
} }


@Override @Override
Expand Down
124 changes: 69 additions & 55 deletions src/com/google/javascript/jscomp/RewriteGoogJsImports.java
View file
Expand Up @@ -15,16 +15,23 @@
*/ */
package com.google.javascript.jscomp; package com.google.javascript.jscomp;


import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;


import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback; import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
import com.google.javascript.jscomp.NodeTraversal.AbstractPreOrderCallback; import com.google.javascript.jscomp.NodeTraversal.AbstractPreOrderCallback;
import com.google.javascript.jscomp.deps.ModuleLoader.ModulePath; import com.google.javascript.jscomp.deps.ModuleLoader.ModulePath;
import com.google.javascript.jscomp.modules.Binding;
import com.google.javascript.jscomp.modules.Module;
import com.google.javascript.jscomp.modules.ModuleMap;
import com.google.javascript.rhino.IR; import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Node;
import java.util.HashSet; import java.util.HashMap;
import java.util.Set; import java.util.LinkedHashMap;
import java.util.Map;
import javax.annotation.Nullable; import javax.annotation.Nullable;


/** /**
Expand Down Expand Up @@ -91,61 +98,34 @@ public enum Mode {
private static final String EXPECTED_BASE_PROVIDE = "goog"; private static final String EXPECTED_BASE_PROVIDE = "goog";


private final Mode mode; private final Mode mode;
private final ModuleMap moduleMap;
private final AbstractCompiler compiler; private final AbstractCompiler compiler;
private FindExports exportsFinder; private Module googModule;
private final Map<Module, Module> moduleReplacements = new HashMap<>();


public RewriteGoogJsImports(AbstractCompiler compiler, Mode mode) { public RewriteGoogJsImports(AbstractCompiler compiler, Mode mode, ModuleMap moduleMap) {
checkNotNull(moduleMap);
this.compiler = compiler; this.compiler = compiler;
this.mode = mode; this.mode = mode;
this.moduleMap = moduleMap;
} }


/** Finds all exports in the goog.js file. */ private void changeModules() {
private class FindExports extends AbstractPreOrderCallback { ImmutableMap.Builder<String, Module> resolvedModules = ImmutableMap.builder();
final Set<String> exportedNames; ImmutableMap.Builder<String, Module> closureModules = ImmutableMap.builder();


public FindExports(Node googRoot) { for (Map.Entry<String, Module> m : moduleMap.getModulesByPath().entrySet()) {
checkState(Es6RewriteModules.isEs6ModuleRoot(googRoot)); Module newModule = moduleReplacements.getOrDefault(m.getValue(), m.getValue());
exportedNames = new HashSet<>(); resolvedModules.put(m.getKey(), newModule);
NodeTraversal.traverse(compiler, googRoot, this);
} }


private void visitExport(Node export) { for (Map.Entry<String, Module> m : moduleMap.getModulesByClosureNamespace().entrySet()) {
if (export.getBooleanProp(Node.EXPORT_DEFAULT)) { Module newModule = moduleReplacements.getOrDefault(m.getValue(), m.getValue());
throw new IllegalStateException("goog.js should never have a default export."); closureModules.put(m.getKey(), newModule);
} else if (export.hasTwoChildren()) {
throw new IllegalStateException("goog.js should never export from anything.");
} else if (export.getFirstChild().isExportSpecs()) {
Node spec = export.getFirstFirstChild();
while (spec != null) {
exportedNames.add(spec.getSecondChild().getString());
spec = spec.getNext();
}
} else {
Node maybeName = export.getFirstFirstChild();
while (maybeName != null) {
if (maybeName.isName()) {
exportedNames.add(maybeName.getString());
}
maybeName = maybeName.getNext();
}
}
} }


@Override compiler.setModuleMap(new ModuleMap(resolvedModules.build(), closureModules.build()));
public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) { moduleReplacements.clear();
switch (n.getToken()) {
case EXPORT:
visitExport(n);
return false;
case IMPORT:
throw new IllegalStateException("goog.js should never import anything.");
case SCRIPT:
case MODULE_BODY:
return true;
default:
return false;
}
}
} }


/** /**
Expand All @@ -157,18 +137,28 @@ private class ReferenceReplacer extends AbstractPostOrderCallback {
private final Node googImportNode; private final Node googImportNode;
private final boolean globalizeAllReferences; private final boolean globalizeAllReferences;


public ReferenceReplacer(Node script, Node googImportNode, boolean globalizeAllReferences) { ReferenceReplacer(
Node script, Node googImportNode, Module module, boolean globalizeAllReferences) {
this.googImportNode = googImportNode; this.googImportNode = googImportNode;
this.globalizeAllReferences = globalizeAllReferences; this.globalizeAllReferences = globalizeAllReferences;
NodeTraversal.traverse(compiler, script, this); NodeTraversal.traverse(compiler, script, this);


if (googImportNode.getSecondChild().isImportStar()) { if (googImportNode.getSecondChild().isImportStar()) {
Map<String, Binding> newBindings = new LinkedHashMap<>(module.boundNames());
newBindings.remove("goog");
if (hasBadExport) { if (hasBadExport) {
// We might be able to get rid of this case now. Originally this was to cause a type error
// later in compilation. But now the ModuleMapCreator should log an error for the bad
// export. However if we're still running we might be in some tool that expects us to be
// fault tolerant and still rewrite modules. So assume this is needed in those tools.
googImportNode.getSecondChild().setOriginalName("goog"); googImportNode.getSecondChild().setOriginalName("goog");
googImportNode.getSecondChild().setString("$goog"); googImportNode.getSecondChild().setString("$goog");
newBindings.put("$goog", module.boundNames().get("goog"));
} else { } else {
googImportNode.getSecondChild().replaceWith(IR.empty()); googImportNode.getSecondChild().replaceWith(IR.empty());
} }
Module newModule = module.toBuilder().boundNames(ImmutableMap.copyOf(newBindings)).build();
moduleReplacements.put(module, newModule);
compiler.reportChangeToEnclosingScope(googImportNode); compiler.reportChangeToEnclosingScope(googImportNode);
} }
} }
Expand All @@ -187,7 +177,7 @@ private void maybeRewriteBadGoogJsImportRef(NodeTraversal t, Node nameNode, Node
} }


if (globalizeAllReferences if (globalizeAllReferences
|| exportsFinder.exportedNames.contains(parent.getSecondChild().getString())) { || googModule.namespace().containsKey(parent.getSecondChild().getString())) {
return; return;
} }


Expand Down Expand Up @@ -264,6 +254,8 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {


@Nullable @Nullable
private Node findGoogImportNode(Node scriptRoot) { private Node findGoogImportNode(Node scriptRoot) {
// Cannot use the module map here - information is lost about the imports. The "bound names"
// could be from transitive imports, but we lose the original import.
boolean valid = true; boolean valid = true;
Node googImportNode = null; Node googImportNode = null;


Expand Down Expand Up @@ -308,21 +300,28 @@ private Node findGoogImportNode(Node scriptRoot) {
return null; return null;
} }


@Override private void rewriteImports(Node scriptRoot) {
public void hotSwapScript(Node scriptRoot, Node originalRoot) {
if (Es6RewriteModules.isEs6ModuleRoot(scriptRoot)) { if (Es6RewriteModules.isEs6ModuleRoot(scriptRoot)) {
Node googImportNode = findGoogImportNode(scriptRoot); Node googImportNode = findGoogImportNode(scriptRoot);
NodeTraversal.traverse(compiler, scriptRoot, new FindReexports(googImportNode != null)); NodeTraversal.traverse(compiler, scriptRoot, new FindReexports(googImportNode != null));
Module module = moduleMap.getModule(compiler.getInput(scriptRoot.getInputId()).getPath());
checkNotNull(module);


if (googImportNode != null && mode == Mode.LINT_AND_REWRITE) { if (googImportNode != null && mode == Mode.LINT_AND_REWRITE) {
// If exportsFinder is null then goog.js was not part of the input. Try to be fault tolerant // If googModule is null then goog.js was not part of the input. Try to be fault tolerant
// and just assume that everything exported is on the global goog. // and just assume that everything exported is on the global goog.
new ReferenceReplacer( new ReferenceReplacer(
scriptRoot, googImportNode, /* globalizeAllReferences= */ exportsFinder == null); scriptRoot, googImportNode, module, /* globalizeAllReferences= */ googModule == null);
} }
} }
} }


@Override
public void hotSwapScript(Node scriptRoot, Node originalRoot) {
rewriteImports(scriptRoot);
changeModules();
}

@Nullable @Nullable
private Node findGoogJsScriptNode(Node root) { private Node findGoogJsScriptNode(Node root) {
ModulePath expectedGoogPath = null; ModulePath expectedGoogPath = null;
Expand Down Expand Up @@ -365,7 +364,7 @@ private Node findGoogJsScriptNode(Node root) {


@Override @Override
public void process(Node externs, Node root) { public void process(Node externs, Node root) {
exportsFinder = null; googModule = null;


Node googJsScriptNode = findGoogJsScriptNode(root); Node googJsScriptNode = findGoogJsScriptNode(root);


Expand All @@ -376,14 +375,29 @@ public void process(Node externs, Node root) {


if (mode == Mode.LINT_AND_REWRITE) { if (mode == Mode.LINT_AND_REWRITE) {
if (googJsScriptNode != null) { if (googJsScriptNode != null) {
exportsFinder = new FindExports(googJsScriptNode); ModulePath googJsPath = compiler.getInput(googJsScriptNode.getInputId()).getPath();
googModule = moduleMap.getModule(googJsPath);
checkNotNull(googModule);
Predicate<Binding> isFromGoog =
(Binding b) -> b.originatingExport().modulePath() == googJsPath;
checkState(
googModule.boundNames().values().stream().allMatch(isFromGoog),
"goog.js should never import anything");
checkState(
!googModule.namespace().containsKey("default"),
"goog.js should never have a default export.");
checkState(
googModule.namespace().values().stream().allMatch(isFromGoog),
"goog.js should never export from anything.");
} }
} else { } else {
checkState(mode == Mode.LINT_ONLY); checkState(mode == Mode.LINT_ONLY);
} }


for (Node script : root.children()) { for (Node script : root.children()) {
hotSwapScript(script, null); rewriteImports(script);
} }

changeModules();
} }
} }
25 changes: 15 additions & 10 deletions src/com/google/javascript/jscomp/modules/Module.java
View file
Expand Up @@ -98,26 +98,31 @@ public abstract class Module {
*/ */
abstract UnresolvedModule unresolvedModule(); abstract UnresolvedModule unresolvedModule();


static Builder builder() { /** Creates a new builder. */
public static Builder builder() {
return new AutoValue_Module.Builder(); return new AutoValue_Module.Builder();
} }


/** Returns this module in builder form. */
public abstract Builder toBuilder();

/** Builder for {@link Module}. */
@AutoValue.Builder @AutoValue.Builder
abstract static class Builder { public abstract static class Builder {
abstract Builder metadata(ModuleMetadata value); public abstract Builder metadata(ModuleMetadata value);


abstract Builder path(@Nullable ModuleLoader.ModulePath value); public abstract Builder path(@Nullable ModuleLoader.ModulePath value);


abstract Builder namespace(ImmutableMap<String, Binding> value); public abstract Builder namespace(ImmutableMap<String, Binding> value);


abstract Builder boundNames(ImmutableMap<String, Binding> value); public abstract Builder boundNames(ImmutableMap<String, Binding> value);


abstract Builder localNameToLocalExport(ImmutableMap<String, Export> value); public abstract Builder localNameToLocalExport(ImmutableMap<String, Export> value);


abstract Builder closureNamespace(@Nullable String value); public abstract Builder closureNamespace(@Nullable String value);


abstract Builder unresolvedModule(UnresolvedModule value); public abstract Builder unresolvedModule(UnresolvedModule value);


abstract Module build(); public abstract Module build();
} }
} }
8 changes: 8 additions & 0 deletions src/com/google/javascript/jscomp/modules/ModuleMap.java
View file
Expand Up @@ -46,6 +46,14 @@ public Module getModule(ModuleLoader.ModulePath path) {
return getModule(path.toModuleName()); return getModule(path.toModuleName());
} }


public ImmutableMap<String, Module> getModulesByPath() {
return resolvedModules;
}

public ImmutableMap<String, Module> getModulesByClosureNamespace() {
return resolvedClosureModules;
}

@Nullable @Nullable
public Module getClosureModule(String namespace) { public Module getClosureModule(String namespace) {
return resolvedClosureModules.get(namespace); return resolvedClosureModules.get(namespace);
Expand Down
16 changes: 15 additions & 1 deletion test/com/google/javascript/jscomp/CheckGoogJsImportTest.java
View file
Expand Up @@ -18,8 +18,12 @@
import static com.google.javascript.jscomp.RewriteGoogJsImports.CANNOT_HAVE_MODULE_VAR_NAMED_GOOG; import static com.google.javascript.jscomp.RewriteGoogJsImports.CANNOT_HAVE_MODULE_VAR_NAMED_GOOG;
import static com.google.javascript.jscomp.RewriteGoogJsImports.GOOG_JS_IMPORT_MUST_BE_GOOG_STAR; import static com.google.javascript.jscomp.RewriteGoogJsImports.GOOG_JS_IMPORT_MUST_BE_GOOG_STAR;
import static com.google.javascript.jscomp.RewriteGoogJsImports.GOOG_JS_REEXPORTED; import static com.google.javascript.jscomp.RewriteGoogJsImports.GOOG_JS_REEXPORTED;
import static com.google.javascript.jscomp.deps.ModuleLoader.LOAD_WARNING;


import com.google.javascript.jscomp.RewriteGoogJsImports.Mode; import com.google.javascript.jscomp.RewriteGoogJsImports.Mode;
import com.google.javascript.jscomp.deps.ModuleLoader.ResolutionMode;
import com.google.javascript.jscomp.modules.ModuleMapCreator;
import com.google.javascript.rhino.Node;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
Expand All @@ -33,11 +37,21 @@ public final class CheckGoogJsImportTest extends CompilerTestCase {
@Before @Before
public void setUp() throws Exception { public void setUp() throws Exception {
super.setUp(); super.setUp();
ignoreWarnings(LOAD_WARNING);
} }


@Override @Override
protected CompilerPass getProcessor(Compiler compiler) { protected CompilerPass getProcessor(Compiler compiler) {
return new RewriteGoogJsImports(compiler, Mode.LINT_ONLY); return (Node externs, Node root) -> {
GatherModuleMetadata gmm =
new GatherModuleMetadata(
compiler, /* processCommonJsModules= */ false, ResolutionMode.BROWSER);
gmm.process(externs, root);
ModuleMapCreator mmc = new ModuleMapCreator(compiler, compiler.getModuleMetadataMap());
mmc.process(externs, root);
new RewriteGoogJsImports(compiler, Mode.LINT_ONLY, compiler.getModuleMap())
.process(externs, root);
};
} }


@Override @Override
Expand Down

0 comments on commit 40f6700

Please sign in to comment.