From d69b208f76addc485e671d10c921fcde0ef5049e Mon Sep 17 00:00:00 2001 From: johnplaisted Date: Fri, 25 Jan 2019 18:00:48 -0800 Subject: [PATCH] Make ModuleMapCreator tolerant to missing module references. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=231002820 --- .../javascript/jscomp/modules/Binding.java | 3 + .../javascript/jscomp/modules/Export.java | 3 +- .../jscomp/modules/ModuleMapCreator.java | 121 ++++++++++++++++-- .../jscomp/modules/NonEsModuleProcessor.java | 2 +- 4 files changed, 116 insertions(+), 13 deletions(-) diff --git a/src/com/google/javascript/jscomp/modules/Binding.java b/src/com/google/javascript/jscomp/modules/Binding.java index ada02ddaa32..d2e758b99f5 100644 --- a/src/com/google/javascript/jscomp/modules/Binding.java +++ b/src/com/google/javascript/jscomp/modules/Binding.java @@ -78,7 +78,10 @@ Binding withSource(Node sourceNode) { *

This is generally a NAME node inside an import or export statement that represents where the * name was bound. However as {@code export * from} has no NAME nodes the source node in that * instance should be the entire export node. + * + *

Null for missing ES modules and non-ES modules as they are currently not scanned. */ + @Nullable public abstract Node sourceNode(); /** diff --git a/src/com/google/javascript/jscomp/modules/Export.java b/src/com/google/javascript/jscomp/modules/Export.java index 733b315c956..3f7b4fa35bc 100644 --- a/src/com/google/javascript/jscomp/modules/Export.java +++ b/src/com/google/javascript/jscomp/modules/Export.java @@ -82,7 +82,6 @@ final Export build() { /** Export from an ES module. */ private void validateEsModule(Export e) { - checkNotNull(e.exportNode()); checkState(e.closureNamespace() == null); checkState( @@ -157,7 +156,7 @@ final Export mutatedCopy() { /** * Node that this export originates from. Used for its source location. * - *

Null only if from non-ES module. + *

Null only if from non-ES module or from a missing ES module. */ @Nullable public abstract Node exportNode(); diff --git a/src/com/google/javascript/jscomp/modules/ModuleMapCreator.java b/src/com/google/javascript/jscomp/modules/ModuleMapCreator.java index 3fe08b4e02e..680ffa94482 100644 --- a/src/com/google/javascript/jscomp/modules/ModuleMapCreator.java +++ b/src/com/google/javascript/jscomp/modules/ModuleMapCreator.java @@ -21,6 +21,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Sets; import com.google.javascript.jscomp.AbstractCompiler; import com.google.javascript.jscomp.CompilerPass; import com.google.javascript.jscomp.DiagnosticType; @@ -28,6 +29,7 @@ import com.google.javascript.jscomp.deps.ModuleLoader; import com.google.javascript.jscomp.deps.ModuleLoader.ModulePath; import com.google.javascript.jscomp.modules.ModuleMetadataMap.ModuleMetadata; +import com.google.javascript.jscomp.modules.ModuleMetadataMap.ModuleType; import com.google.javascript.rhino.Node; import java.util.HashMap; import java.util.HashSet; @@ -232,6 +234,77 @@ public final int hashCode() { final class ModuleRequestResolver { private ModuleRequestResolver() {} + private UnresolvedModule getFallbackForMissingNonClosureModule(ModuleLoader.ModulePath path) { + ModuleMetadata metadata = + ModuleMetadata.builder() + .rootNode(null) + .path(path) + .moduleType(ModuleType.ES6_MODULE) + .isTestOnly(false) + .usesClosure(false) + .build(); + + return new UnresolvedModule() { + @Override + Module resolve(@Nullable String moduleSpecifier) { + return Module.builder() + .boundNames(ImmutableMap.of()) + .namespace(ImmutableMap.of()) + .localNameToLocalExport(ImmutableMap.of()) + .path(path) + .metadata(metadata) + .build(); + } + + @Override + boolean isEsModule() { + return true; + } + + @Override + ImmutableSet getExportedNames() { + return ImmutableSet.of(); + } + + @Override + protected ImmutableSet getExportedNames(Set visited) { + return ImmutableSet.of(); + } + + @Override + ResolveExportResult resolveExport( + @Nullable String moduleSpecifier, + String exportName, + Set resolveSet, + Set exportStarSet) { + return ResolveExportResult.of( + Binding.from( + Export.builder() + .localName(exportName) + .moduleMetadata(metadata) + .modulePath(path) + .closureNamespace(null) + .build(), + /* sourceNode= */ null)); + } + }; + } + + private UnresolvedModule getFallbackForMissingClosureModule(String namespace) { + return nonEsModuleProcessor.process( + this, + ModuleMetadata.builder() + .addGoogNamespace(namespace) + .isTestOnly(false) + .moduleType(ModuleType.GOOG_PROVIDE) + .path(null) + .rootNode(null) + .usesClosure(true) + .build(), + /* path= */ null, + /* script= */ null); + } + @Nullable UnresolvedModule resolve(Import i) { return resolve(i.moduleRequest(), i.modulePath(), i.importNode()); @@ -250,6 +323,8 @@ private UnresolvedModule resolve( UnresolvedModule module = unresolvedModulesByClosureNamespace.get(namespace); if (module == null) { compiler.report(JSError.make(forLineInfo, MISSING_NAMESPACE_IMPORT, namespace)); + module = getFallbackForMissingClosureModule(namespace); + unresolvedModulesByClosureNamespace.put(namespace, module); } return module; } @@ -262,7 +337,13 @@ private UnresolvedModule resolve( forLineInfo.getCharno()); if (requestedPath == null) { - return null; + requestedPath = modulePath.resolveModuleAsPath(moduleRequest); + + if (!unresolvedModules.containsKey(requestedPath.toModuleName())) { + UnresolvedModule module = getFallbackForMissingNonClosureModule(requestedPath); + unresolvedModules.put(requestedPath.toModuleName(), module); + return module; + } } return unresolvedModules.get(requestedPath.toModuleName()); @@ -340,18 +421,38 @@ private ModuleMap create() { } } - for (Map.Entry e : unresolvedModules.entrySet()) { - Module resolved = e.getValue().resolve(); - resolvedModules.put(e.getKey(), resolved); + // We need to resolve in a loop as any missing reference will add a fake to the + // unresolvedModules map (see getFallback* methods above). This would cause a concurrent + // modification exception if we just iterated over unresolvedModules. So the first loop through + // should resolve any "known" modules, and the second any "unrecognized" modules. + do { + Set toResolve = + Sets.difference(unresolvedModules.keySet(), resolvedModules.keySet()).immutableCopy(); + + for (String key : toResolve) { + Module resolved = unresolvedModules.get(key).resolve(); + resolvedModules.put(key, resolved); - for (String namespace : resolved.metadata().googNamespaces()) { - resolvedClosureModules.put(namespace, resolved); + for (String namespace : resolved.metadata().googNamespaces()) { + resolvedClosureModules.put(namespace, resolved); + } } - } - for (Map.Entry e : unresolvedModulesByClosureNamespace.entrySet()) { - resolvedClosureModules.put(e.getKey(), e.getValue().resolve()); - } + } while (!resolvedModules.keySet().containsAll(unresolvedModules.keySet())); + + do { + Set toResolve = + Sets.difference( + unresolvedModulesByClosureNamespace.keySet(), resolvedClosureModules.keySet()) + .immutableCopy(); + + for (String namespace : toResolve) { + resolvedClosureModules.put( + namespace, unresolvedModulesByClosureNamespace.get(namespace).resolve()); + } + } while (!resolvedClosureModules + .keySet() + .containsAll(unresolvedModulesByClosureNamespace.keySet())); unresolvedModules.clear(); unresolvedModulesByClosureNamespace.clear(); diff --git a/src/com/google/javascript/jscomp/modules/NonEsModuleProcessor.java b/src/com/google/javascript/jscomp/modules/NonEsModuleProcessor.java index cae9aa4936c..63d22573786 100644 --- a/src/com/google/javascript/jscomp/modules/NonEsModuleProcessor.java +++ b/src/com/google/javascript/jscomp/modules/NonEsModuleProcessor.java @@ -62,7 +62,7 @@ public ResolveExportResult resolveExport( Set resolveSet, Set exportStarSet) { String namespace = null; - if (GoogEsImports.isGoogImportSpecifier(moduleSpecifier)) { + if (moduleSpecifier != null && GoogEsImports.isGoogImportSpecifier(moduleSpecifier)) { namespace = GoogEsImports.getClosureIdFromGoogImportSpecifier(moduleSpecifier); } return ResolveExportResult.of(