diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index dd0a05a09cd..1743c94bfae 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -1807,7 +1807,7 @@ Node parseInputs() { // In this case we must force module rewriting to occur on the imported file Map inputsToRewrite = new HashMap<>(); for (CompilerInput input : inputs) { - for (String require : input.getKnownRequires()) { + for (String require : input.getKnownRequiredSymbols()) { if (inputModuleIdentifiers.containsKey(require) && !inputsToRewrite.containsKey(require)) { inputsToRewrite.put(require, inputModuleIdentifiers.get(require)); @@ -2015,7 +2015,7 @@ private List depthFirstDependenciesFromInput( this.moduleTypesByName.put(input.getPath().toModuleName(), input.getJsModuleType()); ArrayList allDeps = new ArrayList<>(); - allDeps.addAll(input.getRequires()); + allDeps.addAll(input.getRequiredSymbols()); allDeps.addAll(input.getDynamicRequires()); for (String requiredNamespace : allDeps) { CompilerInput requiredInput = null; diff --git a/src/com/google/javascript/jscomp/CompilerInput.java b/src/com/google/javascript/jscomp/CompilerInput.java index c6f162f7f20..87156822bbf 100644 --- a/src/com/google/javascript/jscomp/CompilerInput.java +++ b/src/com/google/javascript/jscomp/CompilerInput.java @@ -40,12 +40,12 @@ import java.util.TreeMap; /** - * A class for the internal representation of an input to the compiler. - * Wraps a {@link SourceAst} and maintain state such as module for the input and - * whether the input is an extern. Also calculates provided and required types. + * A class for the internal representation of an input to the compiler. Wraps a {@link SourceAst} + * and maintain state such as module for the input and whether the input is an extern. Also + * calculates provided and required types. * */ -public class CompilerInput implements SourceAst, DependencyInfo { +public class CompilerInput extends DependencyInfo.Base implements SourceAst { private static final long serialVersionUID = 1L; @@ -58,9 +58,9 @@ public class CompilerInput implements SourceAst, DependencyInfo { // DependencyInfo to delegate to. private DependencyInfo dependencyInfo; - private final List extraRequires = new ArrayList<>(); + private final List extraRequires = new ArrayList<>(); private final List extraProvides = new ArrayList<>(); - private final List orderedRequires = new ArrayList<>(); + private final List orderedRequires = new ArrayList<>(); private final List dynamicRequires = new ArrayList<>(); private boolean hasFullParseDependencyInfo = false; private ModuleType jsModuleType = ModuleType.NONE; @@ -162,7 +162,7 @@ public void setCompiler(AbstractCompiler compiler) { /** Gets a list of types depended on by this input. */ @Override - public ImmutableList getRequires() { + public ImmutableList getRequires() { if (hasFullParseDependencyInfo) { return ImmutableList.copyOf(orderedRequires); } @@ -176,14 +176,16 @@ public ImmutableList getWeakRequires() { } /** - * Gets a list of types depended on by this input, - * but does not attempt to regenerate the dependency information. - * Typically this occurs from module rewriting. + * Gets a list of namespaces and paths depended on by this input, but does not attempt to + * regenerate the dependency information. Typically this occurs from module rewriting. */ - ImmutableCollection getKnownRequires() { + ImmutableCollection getKnownRequires() { return concat( - dependencyInfo != null ? dependencyInfo.getRequires() : ImmutableList.of(), - extraRequires); + dependencyInfo != null ? dependencyInfo.getRequires() : ImmutableList.of(), extraRequires); + } + + ImmutableList getKnownRequiredSymbols() { + return Require.asSymbolList(getKnownRequires()); } /** Gets a list of types provided by this input. */ @@ -212,7 +214,7 @@ public void addProvide(String provide) { } /** Registers a type that this input depends on in the order seen in the file. */ - public boolean addOrderedRequire(String require) { + public boolean addOrderedRequire(Require require) { if (!orderedRequires.contains(require)) { orderedRequires.add(require); return true; @@ -255,7 +257,7 @@ public void setJsModuleType(ModuleType moduleType) { } /** Registers a type that this input depends on. */ - public void addRequire(String require) { + public void addRequire(Require require) { extraRequires.add(require); } @@ -341,7 +343,7 @@ private DependencyInfo generateDependencyInfo() { private static class DepsFinder { private final Map loadFlags = new TreeMap<>(); private final List provides = new ArrayList<>(); - private final List requires = new ArrayList<>(); + private final List requires = new ArrayList<>(); private final ModulePath modulePath; DepsFinder(ModulePath modulePath) { @@ -369,14 +371,13 @@ void visitSubtree(Node n, Node parent) { && n.getFirstChild().isGetProp() && n.getFirstFirstChild().matchesQualifiedName("goog")) { - if (!requires.contains("goog")) { - requires.add("goog"); + if (!requires.contains(Require.BASE)) { + requires.add(Require.BASE); } Node callee = n.getFirstChild(); Node argument = n.getLastChild(); switch (callee.getLastChild().getString()) { - case "module": loadFlags.put("module", "goog"); // Fall-through @@ -391,7 +392,7 @@ void visitSubtree(Node n, Node parent) { if (!argument.isString()) { return; } - requires.add(argument.getString()); + requires.add(Require.googRequireSymbol(argument.getString())); return; case "loadModule": @@ -456,7 +457,8 @@ void visitEs6ModuleName(Node n, Node parent) { // into ModuleLoader. String moduleName = n.getString(); if (moduleName.startsWith("goog:")) { - requires.add(moduleName.substring(5)); // cut off the "goog:" prefix + // cut off the "goog:" prefix + requires.add(Require.googRequireSymbol(moduleName.substring(5))); return; } ModulePath importedModule = @@ -467,7 +469,7 @@ void visitEs6ModuleName(Node n, Node parent) { importedModule = modulePath.resolveModuleAsPath(moduleName); } - requires.add(importedModule.toModuleName()); + requires.add(Require.es6Import(importedModule.toModuleName(), n.getString())); } } @@ -525,11 +527,6 @@ public ImmutableMap getLoadFlags() { return getDependencyInfo().getLoadFlags(); } - @Override - public boolean isModule() { - return "goog".equals(getLoadFlags().get("module")); - } - private static ImmutableSet concat(Iterable first, Iterable second) { return ImmutableSet.builder().addAll(first).addAll(second).build(); } diff --git a/src/com/google/javascript/jscomp/FindModuleDependencies.java b/src/com/google/javascript/jscomp/FindModuleDependencies.java index 4d40ebe06a5..37a1b32ecf5 100644 --- a/src/com/google/javascript/jscomp/FindModuleDependencies.java +++ b/src/com/google/javascript/jscomp/FindModuleDependencies.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.javascript.jscomp.CompilerInput.ModuleType; import com.google.javascript.jscomp.Es6RewriteModules.FindGoogProvideOrGoogModule; +import com.google.javascript.jscomp.deps.DependencyInfo.Require; import com.google.javascript.jscomp.deps.ModuleLoader; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; @@ -73,8 +74,8 @@ public void process(Node root) { // and add "goog" as a dependency. If "goog" is a dependency of the // file we add it here to the ordered requires so that it's always // first. - if (input.getRequires().contains("goog")) { - input.addOrderedRequire("goog"); + if (input.getRequires().contains(Require.BASE)) { + input.addOrderedRequire(Require.BASE); } NodeTraversal.traverseEs6(compiler, root, this); @@ -159,7 +160,7 @@ public void visit(NodeTraversal t, Node n, Node parent) { .matchesQualifiedName("__webpack_require__.e"))) { t.getInput().addDynamicRequire(modulePath.toModuleName()); } else { - t.getInput().addOrderedRequire(modulePath.toModuleName()); + t.getInput().addOrderedRequire(Require.commonJs(modulePath.toModuleName(), path)); } } } @@ -175,9 +176,9 @@ public void visit(NodeTraversal t, Node n, Node parent) { && n.getSecondChild().isString()) { String namespace = n.getSecondChild().getString(); if (namespace.startsWith("goog.")) { - t.getInput().addOrderedRequire("goog"); + t.getInput().addOrderedRequire(Require.BASE); } - t.getInput().addOrderedRequire(namespace); + t.getInput().addOrderedRequire(Require.googRequireSymbol(namespace)); } } @@ -197,9 +198,9 @@ public void exitScope(NodeTraversal t) { private void addEs6ModuleImportToGraph(NodeTraversal t, Node n) { String moduleName = getEs6ModuleNameFromImportNode(t, n); if (moduleName.startsWith("goog.")) { - t.getInput().addOrderedRequire("goog"); + t.getInput().addOrderedRequire(Require.BASE); } - t.getInput().addOrderedRequire(moduleName); + t.getInput().addOrderedRequire(Require.es6Import(moduleName, n.getLastChild().getString())); } /** diff --git a/src/com/google/javascript/jscomp/JSModule.java b/src/com/google/javascript/jscomp/JSModule.java index 9f52d87e1ab..d00b803e3c2 100644 --- a/src/com/google/javascript/jscomp/JSModule.java +++ b/src/com/google/javascript/jscomp/JSModule.java @@ -35,11 +35,11 @@ import java.util.Set; /** - * A JavaScript module has a unique name, consists of a list of compiler inputs, - * and can depend on other modules. + * A JavaScript module has a unique name, consists of a list of compiler inputs, and can depend on + * other modules. * */ -public final class JSModule implements DependencyInfo, Serializable { +public final class JSModule extends DependencyInfo.Base implements Serializable { private static final long serialVersionUID = 1; /** Module name */ @@ -86,10 +86,10 @@ public ImmutableList getProvides() { } @Override - public ImmutableList getRequires() { - ImmutableList.Builder builder = ImmutableList.builder(); + public ImmutableList getRequires() { + ImmutableList.Builder builder = ImmutableList.builder(); for (JSModule m : deps) { - builder.add(m.getName()); + builder.add(Require.compilerModule(m.getName())); } return builder.build(); } diff --git a/src/com/google/javascript/jscomp/JSModuleGraph.java b/src/com/google/javascript/jscomp/JSModuleGraph.java index 3f82318b6f9..283c52984ba 100644 --- a/src/com/google/javascript/jscomp/JSModuleGraph.java +++ b/src/com/google/javascript/jscomp/JSModuleGraph.java @@ -532,7 +532,7 @@ private List getDepthFirstDependenciesOf( return orderedInputs; } - for (String importedNamespace : rootInput.getRequires()) { + for (String importedNamespace : rootInput.getRequiredSymbols()) { CompilerInput dependency = null; if (inputsByProvide.containsKey(importedNamespace) && unreachedInputs.contains(inputsByProvide.get(importedNamespace))) { diff --git a/src/com/google/javascript/jscomp/LazyParsedDependencyInfo.java b/src/com/google/javascript/jscomp/LazyParsedDependencyInfo.java index 7d95a772742..c6b71d0d870 100644 --- a/src/com/google/javascript/jscomp/LazyParsedDependencyInfo.java +++ b/src/com/google/javascript/jscomp/LazyParsedDependencyInfo.java @@ -27,10 +27,8 @@ import java.util.Map; import java.util.TreeMap; -/** - * A DependencyInfo class that determines load flags by parsing the AST just-in-time. - */ -public class LazyParsedDependencyInfo implements DependencyInfo { +/** A DependencyInfo class that determines load flags by parsing the AST just-in-time. */ +public class LazyParsedDependencyInfo extends DependencyInfo.Base { private final DependencyInfo delegate; private final JsAst ast; @@ -77,7 +75,7 @@ public String getPathRelativeToClosureBase() { } @Override - public ImmutableList getRequires() { + public ImmutableList getRequires() { return delegate.getRequires(); } @@ -90,9 +88,4 @@ public ImmutableList getWeakRequires() { public ImmutableList getProvides() { return delegate.getProvides(); } - - @Override - public boolean isModule() { - return "goog".equals(getLoadFlags().get("module")); - } } diff --git a/src/com/google/javascript/jscomp/deps/DefaultDependencyResolver.java b/src/com/google/javascript/jscomp/deps/DefaultDependencyResolver.java index bef70cf04c3..bb4b6e91986 100644 --- a/src/com/google/javascript/jscomp/deps/DefaultDependencyResolver.java +++ b/src/com/google/javascript/jscomp/deps/DefaultDependencyResolver.java @@ -131,7 +131,7 @@ private void addDependency(String symbol, Set seen, List list) } } else if (!seen.containsAll(dependency.getProvides())) { seen.addAll(dependency.getProvides()); - for (String require : dependency.getRequires()) { + for (String require : dependency.getRequiredSymbols()) { addDependency(require, seen, list); } list.add(dependency.getPathRelativeToClosureBase()); @@ -148,7 +148,7 @@ private static Collection parseRequires(String code, boolean addClosureB if (addClosureBase) { requires.add(CLOSURE_BASE_PROVIDE); } - requires.addAll(deps.getRequires()); + requires.addAll(deps.getRequiredSymbols()); errorManager.generateReport(); return requires; } diff --git a/src/com/google/javascript/jscomp/deps/DependencyInfo.java b/src/com/google/javascript/jscomp/deps/DependencyInfo.java index ca4d7731b83..65b700837e5 100644 --- a/src/com/google/javascript/jscomp/deps/DependencyInfo.java +++ b/src/com/google/javascript/jscomp/deps/DependencyInfo.java @@ -16,11 +16,13 @@ package com.google.javascript.jscomp.deps; +import com.google.auto.value.AutoValue; import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.errorprone.annotations.Immutable; import java.io.IOException; import java.io.Serializable; import java.util.ArrayList; @@ -35,6 +37,110 @@ */ public interface DependencyInfo extends Serializable { + /** A dependency link between two files, e.g. goog.require('namespace'), import 'file'; */ + @AutoValue + @Immutable + abstract class Require implements Serializable { + public static final Require BASE = googRequireSymbol("goog"); + + public enum Type { + /** Standard goog.require call for a symbol from a goog.provide or goog.module. */ + GOOG_REQUIRE_SYMBOL, + /** + * goog.require call with a path argument. Currently only supported in goog.modules for ES6 + * modules. + */ + GOOG_REQUIRE_PATH, + /** ES6 import statement. */ + ES6_IMPORT, + /** Parsed from an existing Closure dependency file. */ + PARSED_FROM_DEPS, + /** CommonJS require() call. */ + COMMON_JS, + /** Compiler module dependencies. */ + COMPILER_MODULE + } + + public static ImmutableList asSymbolList(Iterable requires) { + return ImmutableList.copyOf( + Iterables.transform( + requires, + new Function() { + @Override + public String apply(Require input) { + return input.getSymbol(); + } + })); + } + + public static Require googRequireSymbol(String symbol) { + return builder() + .setRawText(symbol) + .setSymbol(symbol) + .setType(Type.GOOG_REQUIRE_SYMBOL) + .build(); + } + + public static Require googRequirePath(String symbol, String rawPath) { + return builder() + .setRawText(rawPath) + .setSymbol(symbol) + .setType(Type.GOOG_REQUIRE_PATH) + .build(); + } + + public static Require es6Import(String symbol, String rawPath) { + return builder().setRawText(rawPath).setSymbol(symbol).setType(Type.ES6_IMPORT).build(); + } + + public static Require commonJs(String symbol, String rawPath) { + return builder().setRawText(rawPath).setSymbol(symbol).setType(Type.COMMON_JS).build(); + } + + public static Require compilerModule(String symbol) { + return builder().setRawText(symbol).setSymbol(symbol).setType(Type.COMPILER_MODULE).build(); + } + + public static Require parsedFromDeps(String symbol) { + return builder().setRawText(symbol).setSymbol(symbol).setType(Type.PARSED_FROM_DEPS).build(); + } + + private static Builder builder() { + return new AutoValue_DependencyInfo_Require.Builder(); + } + + protected abstract Builder toBuilder(); + + public Require withSymbol(String symbol) { + return toBuilder().setSymbol(symbol).build(); + } + + /** + * @return symbol the symbol provided by another {@link DependencyInfo}'s {@link + * DependencyInfo#getProvides()} + */ + public abstract String getSymbol(); + + /** + * @return the raw text of the import string as it appears in the file. Used mostly for error + * reporting. + */ + public abstract String getRawText(); + + public abstract Type getType(); + + @AutoValue.Builder + abstract static class Builder { + public abstract Builder setType(Type value); + + public abstract Builder setRawText(String rawText); + + public abstract Builder setSymbol(String value); + + public abstract Require build(); + } + } + /** Gets the unique name / path of this file. */ String getName(); @@ -45,7 +151,9 @@ public interface DependencyInfo extends Serializable { ImmutableList getProvides(); /** Gets the symbols required by this file. */ - ImmutableList getRequires(); + ImmutableList getRequires(); + + ImmutableList getRequiredSymbols(); /** Gets the symbols weakly required by this file. (i.e. for typechecking only) */ ImmutableList getWeakRequires(); @@ -64,6 +172,11 @@ abstract class Base implements DependencyInfo { @Override public boolean isModule() { return "goog".equals(getLoadFlags().get("module")); } + + @Override + public ImmutableList getRequiredSymbols() { + return Require.asSymbolList(getRequires()); + } } /** Utility methods. */ @@ -79,7 +192,7 @@ public static void writeAddDependency(Appendable out, DependencyInfo info) throw .append("', "); writeJsArray(out, info.getProvides()); out.append(", "); - writeJsArray(out, info.getRequires()); + writeJsArray(out, Require.asSymbolList(info.getRequires())); Map loadFlags = info.getLoadFlags(); if (!loadFlags.isEmpty()) { out.append(", "); diff --git a/src/com/google/javascript/jscomp/deps/DepsFileParser.java b/src/com/google/javascript/jscomp/deps/DepsFileParser.java index 9849c9cb3ee..af4d668d185 100644 --- a/src/com/google/javascript/jscomp/deps/DepsFileParser.java +++ b/src/com/google/javascript/jscomp/deps/DepsFileParser.java @@ -20,9 +20,12 @@ import com.google.common.base.CharMatcher; import com.google.common.base.Function; import com.google.common.base.Functions; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Iterables; import com.google.common.io.Files; import com.google.javascript.jscomp.ErrorManager; +import com.google.javascript.jscomp.deps.DependencyInfo.Require; import java.io.File; import java.io.IOException; import java.io.Reader; @@ -35,6 +38,7 @@ import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.annotation.Nullable; /** * A parser that can extract dependency information from existing deps.js files. @@ -168,7 +172,16 @@ protected boolean parseLine(String line) throws ParseException { DependencyInfo depInfo = SimpleDependencyInfo.builder(path, filePath) .setProvides(parseJsStringArray(depArgsMatch.group(2))) - .setRequires(parseJsStringArray(depArgsMatch.group(3))) + .setRequires( + ImmutableList.copyOf( + Iterables.transform( + parseJsStringArray(depArgsMatch.group(3)), + new Function() { + @Override + public Require apply(@Nullable String input) { + return Require.parsedFromDeps(input); + } + }))) .setLoadFlags(parseLoadFlags(depArgsMatch.group(4))) .build(); diff --git a/src/com/google/javascript/jscomp/deps/DepsGenerator.java b/src/com/google/javascript/jscomp/deps/DepsGenerator.java index 1de5dc28b3b..4ee5d9e7e69 100644 --- a/src/com/google/javascript/jscomp/deps/DepsGenerator.java +++ b/src/com/google/javascript/jscomp/deps/DepsGenerator.java @@ -20,8 +20,11 @@ import com.google.common.base.Function; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMultiset; +import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; +import com.google.common.collect.Multiset; import com.google.javascript.jscomp.CheckLevel; import com.google.javascript.jscomp.Compiler; import com.google.javascript.jscomp.CompilerOptions; @@ -31,6 +34,8 @@ import com.google.javascript.jscomp.JsAst; import com.google.javascript.jscomp.LazyParsedDependencyInfo; import com.google.javascript.jscomp.SourceFile; +import com.google.javascript.jscomp.deps.DependencyInfo.Require; +import com.google.javascript.jscomp.deps.ModuleLoader.ModulePath; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.PrintStream; @@ -69,6 +74,29 @@ public static enum InclusionStrategy { private final ModuleLoader loader; final ErrorManager errorManager; + static final DiagnosticType ES6_IMPORT_FOR_NON_ES6_MODULE = + DiagnosticType.warning( + "DEPS_ES6_IMPORT_FOR_NON_ES6_MODULE", + "Cannot import file \"{0}\" because it is not an ES6 module."); + + static final DiagnosticType GOOG_REQUIRE_PATH_BETWEEN_ES6_MODULES = + DiagnosticType.warning( + "DEPS_GOOG_REQUIRE_PATH_BETWEEN_ES6_MODULES", + "Use ES6 import rather than goog.require to import ES6 module \"{0}\"."); + + static final DiagnosticType GOOG_REQUIRE_PATH_FOR_NON_ES6_MODULE = + DiagnosticType.warning( + "DEPS_GOOG_REQUIRE_PATH_FOR_NON_ES6_MODULE", + "Cannot goog.require \"{0}\" by path. It is not an ES6 module."); + + static final DiagnosticType GOOG_REQUIRE_PATH_FROM_NON_GOOG_MODULE = + DiagnosticType.warning( + "DEPS_GOOG_REQUIRE_PATH_FROM_NON_GOOG_MODULE", + "Cannot goog.require by path outside of a goog.module."); + + static final DiagnosticType UNKNOWN_PATH_IMPORT = + DiagnosticType.warning("DEPS_UNKNOWN_PATH_IMPORT", "Could not find file \"{0}\"."); + static final DiagnosticType SAME_FILE_WARNING = DiagnosticType.warning( "DEPS_SAME_FILE", "Namespace \"{0}\" is both required and provided in the same file."); @@ -133,6 +161,8 @@ public String computeDependencyCalls() throws IOException { cleanUpDuplicatedFiles(depsFiles, jsFiles); + jsFiles = removeMungedSymbols(depsFiles, jsFiles); + // Check for missing provides or other semantic inconsistencies. validateDependencies(depsFiles.values(), jsFiles.values()); @@ -169,6 +199,75 @@ protected void cleanUpDuplicatedFiles(Map depsFiles, } } + /** + * Removes munged symbols in requires and provides. These munged symbols are from ES6 modules + * and are generated by {@link ModulePath#toModuleName()}. We do not wish to write these munged + * symbols to the dependency file. + * + *
    + *
  • Makes any require'd munged symbol the require'd file's relative path to Closure.
  • + *
  • Removes munged symbols from the provides list.
  • + *
+ */ + private Map removeMungedSymbols( + Map depFiles, Map jsFiles) { + Map newJsFiles = new LinkedHashMap<>(); + Map providesMap = new LinkedHashMap<>(); + addToProvideMap(depFiles.values(), providesMap, true); + addToProvideMap(jsFiles.values(), providesMap, false); + + for (DependencyInfo dependencyInfo : jsFiles.values()) { + ArrayList newRequires = new ArrayList<>(); + for (Require require : dependencyInfo.getRequires()) { + if (require.getType() == Require.Type.ES6_IMPORT + || require.getType() == Require.Type.GOOG_REQUIRE_PATH) { + // Symbols are unique per file and have nothing to do with paths so map lookups are safe + // here. + DependencyInfo provider = providesMap.get(require.getSymbol()); + if (provider == null) { + reportMissingFile(dependencyInfo, require.getRawText()); + } else { + // If this is an ES6 module then set the symbol to be its relative path to Closure. + // ES6 modules in a dependency file do not "provide" anything. Requires can match + // a provided symbol or a relative path to Closure. + newRequires.add(require.withSymbol(provider.getPathRelativeToClosureBase())); + } + } else { + // Require is by symbol already so no need to change it. + newRequires.add(require); + } + } + + ImmutableList provides = dependencyInfo.getProvides(); + + if ("es6".equals(dependencyInfo.getLoadFlags().get("module"))) { + String mungedProvide = loader.resolve(dependencyInfo.getName()).toModuleName(); + // Filter out the munged symbol. + // Note that at the moment ES6 modules should not have any other provides! In the future + // we may have additional mechanisms to add goog symbols. But for not nothing is officially + // supported. + ImmutableList.Builder builder = ImmutableList.builder(); + for (String provide : provides) { + if (!provide.equals(mungedProvide)) { + builder.add(provide); + } + } + provides = builder.build(); + } + + newJsFiles.put( + dependencyInfo.getPathRelativeToClosureBase(), + SimpleDependencyInfo.builder( + dependencyInfo.getPathRelativeToClosureBase(), dependencyInfo.getName()) + .setProvides(provides) + .setRequires(newRequires) + .setLoadFlags(dependencyInfo.getLoadFlags()) + .build()); + } + + return newJsFiles; + } + /** * Reports if there are any dependency problems with the given dependency * information. Reported problems include: @@ -176,37 +275,103 @@ protected void cleanUpDuplicatedFiles(Map depsFiles, * - A namespace being required multiple times from within one file * - A namespace being provided and required in the same file * - A namespace being required that is never provided - * @param preparsedFileDepedencies Dependency information from existing + * @param preparsedFileDependencies Dependency information from existing * deps.js files. * @param parsedFileDependencies Dependency information from parsed .js files. */ - private void validateDependencies(Iterable preparsedFileDepedencies, + private void validateDependencies(Iterable preparsedFileDependencies, Iterable parsedFileDependencies) { // Create a map of namespace -> file providing it. // Also report any duplicate provides. Map providesMap = new LinkedHashMap<>(); - addToProvideMap(preparsedFileDepedencies, providesMap); - addToProvideMap(parsedFileDependencies, providesMap); + addToProvideMap(preparsedFileDependencies, providesMap, true); + addToProvideMap(parsedFileDependencies, providesMap, false); // For each require in the parsed sources: for (DependencyInfo depInfo : parsedFileDependencies) { - List requires = new ArrayList<>(depInfo.getRequires()); - for (int i = 0, l = requires.size(); i < l; ++i) { - String namespace = requires.get(i); - // Check for multiple requires. - if (requires.subList(i + 1, l).contains(namespace)) { - reportDuplicateRequire(namespace, depInfo); + Multiset symbols = ImmutableMultiset.copyOf(depInfo.getRequiredSymbols()); + for (String symbol : symbols.elementSet()) { + if (symbols.count(symbol) > 1) { + reportDuplicateRequire(symbol, depInfo); } + } + + for (Require require : depInfo.getRequires()) { + String namespace = require.getSymbol(); // Check for missing provides. DependencyInfo provider = providesMap.get(namespace); if (provider == null) { reportUndefinedNamespace(namespace, depInfo); } else if (provider == depInfo) { reportSameFile(namespace, depInfo); + } else { + boolean isGoogModule = depInfo.isModule(); + boolean isEs6Module = "es6".equals(depInfo.getLoadFlags().get("module")); + boolean providerIsEs6Module = "es6".equals(provider.getLoadFlags().get("module")); + + switch (require.getType()) { + case ES6_IMPORT: + if (!providerIsEs6Module) { + reportEs6ImportForNonEs6Module(provider, depInfo); + } + break; + case GOOG_REQUIRE_PATH: + if (isEs6Module && providerIsEs6Module) { + reportGoogRequirePathBetweenEs6Modules(provider, depInfo); + } else { + if (!isGoogModule) { + reportGoogRequirePathFromNonGoogModule(depInfo); + } + if (!providerIsEs6Module) { + reportGoogRequirePathForNonEs6Module(provider, depInfo); + } + } + break; + case GOOG_REQUIRE_SYMBOL: + case PARSED_FROM_DEPS: + break; + case COMMON_JS: + case COMPILER_MODULE: + default: + throw new IllegalStateException("Unexpected import type: " + require.getType()); + } } } } } + private void reportMissingFile(DependencyInfo depInfo, String path) { + errorManager.report( + CheckLevel.ERROR, JSError.make(depInfo.getName(), -1, -1, UNKNOWN_PATH_IMPORT, path)); + } + + private void reportEs6ImportForNonEs6Module(DependencyInfo provider, DependencyInfo depInfo) { + errorManager.report( + CheckLevel.ERROR, + JSError.make(depInfo.getName(), -1, -1, ES6_IMPORT_FOR_NON_ES6_MODULE, provider.getName())); + } + + private void reportGoogRequirePathBetweenEs6Modules( + DependencyInfo provider, DependencyInfo depInfo) { + errorManager.report( + CheckLevel.ERROR, + JSError.make( + depInfo.getName(), -1, -1, GOOG_REQUIRE_PATH_BETWEEN_ES6_MODULES, provider.getName())); + } + + private void reportGoogRequirePathForNonEs6Module( + DependencyInfo provider, DependencyInfo depInfo) { + errorManager.report( + CheckLevel.ERROR, + JSError.make( + depInfo.getName(), -1, -1, GOOG_REQUIRE_PATH_FOR_NON_ES6_MODULE, provider.getName())); + } + + private void reportGoogRequirePathFromNonGoogModule(DependencyInfo depInfo) { + errorManager.report( + CheckLevel.ERROR, + JSError.make(depInfo.getName(), -1, -1, GOOG_REQUIRE_PATH_FROM_NON_GOOG_MODULE)); + } + private void reportSameFile(String namespace, DependencyInfo depInfo) { errorManager.report(CheckLevel.WARNING, JSError.make(depInfo.getName(), -1, -1, @@ -246,13 +411,36 @@ private void reportNoDepsInDepsFile(String filePath) { } /** - * Adds the given DependencyInfos to the given providesMap. Also checks for - * and reports duplicate provides. + * Adds the given DependencyInfos to the given providesMap. Also checks for and reports duplicate + * provides. */ - private void addToProvideMap(Iterable depInfos, - Map providesMap) { + private void addToProvideMap( + Iterable depInfos, + Map providesMap, + boolean isFromDepsFile) { for (DependencyInfo depInfo : depInfos) { - for (String provide : depInfo.getProvides()) { + List provides = Lists.newArrayList(depInfo.getProvides()); + + // Add a munged symbol to the provides map so that lookups by path requires work as intended. + if (isFromDepsFile) { + // Don't add the dependency file itself but every file it says exists instead. + provides.add( + loader + .resolve( + PathUtil.makeAbsolute(depInfo.getPathRelativeToClosureBase(), closurePathAbs)) + .toModuleName()); + } else { + // ES6 modules already provide these munged symbols. + if (!"es6".equals(depInfo.getLoadFlags().get("module"))) { + provides.add(loader.resolve(depInfo.getName()).toModuleName()); + } + } + + // Also add the relative closure path as a provide. At some point we'll swap out the munged + // symbols for these relative paths. So looks ups by either need to work. + provides.add(depInfo.getPathRelativeToClosureBase()); + + for (String provide : provides) { DependencyInfo prevValue = providesMap.put(provide, depInfo); // Check for duplicate provides. if (prevValue != null) { diff --git a/src/com/google/javascript/jscomp/deps/Es6SortedDependencies.java b/src/com/google/javascript/jscomp/deps/Es6SortedDependencies.java index 2a7296c3f9e..61277e6a9d4 100644 --- a/src/com/google/javascript/jscomp/deps/Es6SortedDependencies.java +++ b/src/com/google/javascript/jscomp/deps/Es6SortedDependencies.java @@ -76,7 +76,7 @@ public ImmutableList getDependenciesOf(List rootInputs, boolean so while (!worklist.isEmpty()) { INPUT input = worklist.pop(); if (includedInputs.add(input)) { - for (String symbolName : input.getRequires()) { + for (String symbolName : input.getRequiredSymbols()) { INPUT importedSymbolName = exportingInputBySymbolName.get(symbolName); if (importedSymbolName != null) { worklist.add(importedSymbolName); @@ -163,7 +163,7 @@ private void processInputs() { } } for (INPUT userOrderedInput : userOrderedInputs) { - for (String symbolName : userOrderedInput.getRequires()) { + for (String symbolName : userOrderedInput.getRequiredSymbols()) { INPUT importedInput = exportingInputBySymbolName.get(symbolName); if (importedInput != null) { importedInputByImportingInput.put(userOrderedInput, importedInput); diff --git a/src/com/google/javascript/jscomp/deps/JsFileParser.java b/src/com/google/javascript/jscomp/deps/JsFileParser.java index f2a883513b7..17a40e47944 100644 --- a/src/com/google/javascript/jscomp/deps/JsFileParser.java +++ b/src/com/google/javascript/jscomp/deps/JsFileParser.java @@ -23,6 +23,7 @@ import com.google.javascript.jscomp.CheckLevel; import com.google.javascript.jscomp.ErrorManager; import com.google.javascript.jscomp.JSError; +import com.google.javascript.jscomp.deps.DependencyInfo.Require; import java.io.Reader; import java.io.StringReader; import java.util.ArrayList; @@ -98,7 +99,8 @@ public final class JsFileParser extends JsFileLineParser { /** The info for the file we are currently parsing. */ private List provides; - private List requires; + + private List requires; private List weakRequires; private boolean fileHasProvidesOrRequires; private ModuleLoader loader = ModuleLoader.EMPTY; @@ -108,6 +110,7 @@ private enum ModuleType { NON_MODULE, UNWRAPPED_GOOG_MODULE, WRAPPED_GOOG_MODULE, + GOOG_PROVIDE, ES6_MODULE, } @@ -246,21 +249,26 @@ protected boolean parseLine(String line) throws ParseException { if (includeGoogBase && !fileHasProvidesOrRequires) { fileHasProvidesOrRequires = true; - requires.add("goog"); + requires.add(Require.BASE); } // See if it's a require or provide. String methodName = googMatcher.group(1); char firstChar = methodName.charAt(0); boolean isModule = firstChar == 'm'; - boolean isProvide = (firstChar == 'p' || isModule); + boolean isProvide = firstChar == 'p'; + boolean providesNamespace = isProvide || isModule; boolean isRequire = firstChar == 'r'; if (isModule && this.moduleType != ModuleType.WRAPPED_GOOG_MODULE) { setModuleType(ModuleType.UNWRAPPED_GOOG_MODULE); } - if (isProvide || isRequire) { + if (isProvide) { + setModuleType(ModuleType.GOOG_PROVIDE); + } + + if (providesNamespace || isRequire) { // Parse the param. String arg = parseJsString(googMatcher.group(2)); // Add the dependency. @@ -269,9 +277,16 @@ protected boolean parseLine(String line) throws ParseException { weakRequires.add(arg); } else if (!"goog".equals(arg)) { // goog is always implicit. - // TODO(nicksantos): I'm pretty sure we don't need this anymore. - // Remove this later. - requires.add(arg); + Require require = Require.googRequireSymbol(arg); + if (ModuleLoader.isRelativeIdentifier(arg)) { + ModuleLoader.ModulePath path = file.resolveJsModule(arg); + if (path == null) { + path = file.resolveModuleAsPath(arg); + } + String symbol = path.toModuleName(); + require = Require.googRequirePath(symbol, arg); + } + requires.add(require); } } else { provides.add(arg); @@ -295,13 +310,14 @@ protected boolean parseLine(String line) throws ParseException { String arg = es6Matcher.group(1); if (arg != null) { if (arg.startsWith("goog:")) { - requires.add(arg.substring(5)); // cut off the "goog:" prefix + // cut off the "goog:" prefix + requires.add(Require.googRequireSymbol(arg.substring(5))); } else { ModuleLoader.ModulePath path = file.resolveJsModule(arg); if (path == null) { path = file.resolveModuleAsPath(arg); } - requires.add(path.toModuleName()); + requires.add(Require.es6Import(path.toModuleName(), arg)); } } } diff --git a/src/com/google/javascript/jscomp/deps/ModuleLoader.java b/src/com/google/javascript/jscomp/deps/ModuleLoader.java index aec4c7cda08..f0ee1a47bb6 100644 --- a/src/com/google/javascript/jscomp/deps/ModuleLoader.java +++ b/src/com/google/javascript/jscomp/deps/ModuleLoader.java @@ -44,8 +44,10 @@ */ public final class ModuleLoader { - public static final DiagnosticType MODULE_CONFLICT = DiagnosticType.warning( - "JSC_MODULE_CONFLICT", "File has both goog.module and ES6 modules: {0}"); + public static final DiagnosticType MODULE_CONFLICT = + DiagnosticType.warning( + "JSC_MODULE_CONFLICT", + "File cannot be a combination of goog.provide, goog.module, and/or ES6 module: {0}"); /** According to the spec, the forward slash should be the delimiter on all platforms. */ public static final String MODULE_SLASH = ModuleNames.MODULE_SLASH; diff --git a/src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java b/src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java index 68b68dccaa4..6bad0f2e51f 100644 --- a/src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java +++ b/src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java @@ -50,8 +50,11 @@ public abstract static class Builder { public abstract Builder setProvides(Collection provides); public abstract Builder setProvides(String... provides); - public abstract Builder setRequires(Collection requires); - public abstract Builder setRequires(String... requires); + + public abstract Builder setRequires(Collection requires); + + public abstract Builder setRequires(Require... requires); + public abstract Builder setWeakRequires(Collection weakRequires); public abstract Builder setWeakRequires(String... weakRequires); public abstract Builder setLoadFlags(Map loadFlags); diff --git a/test/com/google/javascript/jscomp/JSModuleGraphTest.java b/test/com/google/javascript/jscomp/JSModuleGraphTest.java index 7b93c1b5f1d..edc90df5cae 100644 --- a/test/com/google/javascript/jscomp/JSModuleGraphTest.java +++ b/test/com/google/javascript/jscomp/JSModuleGraphTest.java @@ -24,6 +24,7 @@ import com.google.common.collect.Iterables; import com.google.gson.JsonArray; import com.google.gson.JsonObject; +import com.google.javascript.jscomp.deps.DependencyInfo.Require; import java.util.ArrayList; import java.util.Arrays; import java.util.BitSet; @@ -431,7 +432,7 @@ public void testProperEs6ModuleOrdering() throws Exception { for (CompilerInput input : A.getInputs()) { input.setCompiler(compiler); for (String require : orderedRequires.get(input.getSourceFile().getName())) { - input.addOrderedRequire(require); + input.addOrderedRequire(Require.compilerModule(require)); } input.setHasFullParseDependencyInfo(true); } diff --git a/test/com/google/javascript/jscomp/LazyParsedDependencyInfoTest.java b/test/com/google/javascript/jscomp/LazyParsedDependencyInfoTest.java index a61660940b0..3989f2b1848 100644 --- a/test/com/google/javascript/jscomp/LazyParsedDependencyInfoTest.java +++ b/test/com/google/javascript/jscomp/LazyParsedDependencyInfoTest.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableMap; import com.google.javascript.jscomp.deps.DependencyInfo; +import com.google.javascript.jscomp.deps.DependencyInfo.Require; import com.google.javascript.jscomp.deps.ModuleLoader; import com.google.javascript.jscomp.deps.SimpleDependencyInfo; import java.util.Arrays; @@ -31,19 +32,21 @@ public final class LazyParsedDependencyInfoTest extends TestCase { public void testDelegation() { + Require baz = Require.googRequireSymbol("baz"); + Require qux = Require.googRequireSymbol("qux"); Compiler compiler = new Compiler(); JsAst ast = new JsAst(SourceFile.fromCode("file.js", "// nothing here")); SimpleDependencyInfo delegate = SimpleDependencyInfo.builder("path/to/1.js", "path/2.js") .setProvides("foo", "bar") - .setRequires("baz", "qux") + .setRequires(baz, qux) .build(); DependencyInfo info = new LazyParsedDependencyInfo(delegate, ast, compiler); assertThat(info.getName()).isEqualTo("path/2.js"); assertThat(info.getPathRelativeToClosureBase()).isEqualTo("path/to/1.js"); assertThat(info.getProvides()).containsExactly("foo", "bar"); - assertThat(info.getRequires()).containsExactly("baz", "qux"); + assertThat(info.getRequires()).containsExactly(baz, qux); } public void testLoadFlagsParsesEs3() { diff --git a/test/com/google/javascript/jscomp/deps/DependencyInfoTest.java b/test/com/google/javascript/jscomp/deps/DependencyInfoTest.java index bba6127b0ed..0682c89e8e6 100644 --- a/test/com/google/javascript/jscomp/deps/DependencyInfoTest.java +++ b/test/com/google/javascript/jscomp/deps/DependencyInfoTest.java @@ -17,6 +17,7 @@ package com.google.javascript.jscomp.deps; import static com.google.common.truth.Truth.assertThat; +import static com.google.javascript.jscomp.deps.DependencyInfo.Require.googRequireSymbol; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -32,7 +33,7 @@ public void testWriteAddDependency() throws IOException { sb, SimpleDependencyInfo.builder("../some/relative/path.js", "/unused/absolute/path.js") .setProvides(ImmutableList.of("provided.symbol", "other.provide")) - .setRequires(ImmutableList.of("required.symbol", "other.require")) + .setRequires(googRequireSymbol("required.symbol"), googRequireSymbol("other.require")) .setLoadFlags(ImmutableMap.of("module", "goog", "lang", "es6")) .build()); assertThat(sb.toString()) diff --git a/test/com/google/javascript/jscomp/deps/DepsFileParserTest.java b/test/com/google/javascript/jscomp/deps/DepsFileParserTest.java index f2a6a2c5537..ceb69214054 100644 --- a/test/com/google/javascript/jscomp/deps/DepsFileParserTest.java +++ b/test/com/google/javascript/jscomp/deps/DepsFileParserTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableMap; import com.google.javascript.jscomp.ErrorManager; import com.google.javascript.jscomp.PrintStreamErrorManager; +import com.google.javascript.jscomp.deps.DependencyInfo.Require; import java.util.List; import junit.framework.TestCase; @@ -35,7 +36,6 @@ public final class DepsFileParserTest extends TestCase { private DepsFileParser parser; private ErrorManager errorManager; private static final String SRC_PATH = "/path/1.js"; - private static final ImmutableList EMPTY = ImmutableList.of(); @Override public void setUp() { @@ -53,38 +53,32 @@ public void setUp() { * -Correct recording of what was parsed. */ public void testGoodParse() { - final String contents = "/*" - + "goog.addDependency('no1', [], []);*//*\n" - + "goog.addDependency('no2', [ ], [ ]);\n" - + "*/goog.addDependency('yes1', [], []);\n" - + "/* blah */goog.addDependency(\"yes2\", [], [])/* blah*/\n" - + "goog.addDependency('yes3', ['a','b'], ['c']); // goog.addDependency('no3', [], []);\n" - + "// goog.addDependency('no4', [], []);\n" - + "goog.addDependency(\"yes4\", [], [ \"a\",'b' , 'c' ]); //no new line at EOF"; + final String contents = + "/*" + + "goog.addDependency('no1', [], []);*//*\n" + + "goog.addDependency('no2', [ ], [ ]);\n" + + "*/goog.addDependency('yes1', [], []);\n" + + "/* blah */goog.addDependency(\"yes2\", [], [])/* blah*/\n" + + "goog.addDependency('yes3', ['a','b'], ['c']); " + + "// goog.addDependency('no3', [], []);\n" + + "// goog.addDependency('no4', [], []);\n" + + "goog.addDependency(\"yes4\", [], [ \"a\",'b' , 'c' ]); //no new line at EOF"; List result = parser.parseFile(SRC_PATH, contents); - ImmutableList expected = ImmutableList.of( - SimpleDependencyInfo.builder("yes1", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setGoogModule(false) - .build(), - SimpleDependencyInfo.builder("yes2", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setGoogModule(false) - .build(), - SimpleDependencyInfo.builder("yes3", SRC_PATH) - .setProvides(ImmutableList.of("a", "b")) - .setRequires(ImmutableList.of("c")) - .setGoogModule(false) - .build(), - SimpleDependencyInfo.builder("yes4", SRC_PATH) - .setProvides(EMPTY) - .setRequires(ImmutableList.of("a", "b", "c")) - .setGoogModule(false) - .build() - ); + ImmutableList expected = + ImmutableList.of( + SimpleDependencyInfo.builder("yes1", SRC_PATH).build(), + SimpleDependencyInfo.builder("yes2", SRC_PATH).build(), + SimpleDependencyInfo.builder("yes3", SRC_PATH) + .setProvides(ImmutableList.of("a", "b")) + .setRequires(Require.parsedFromDeps("c")) + .build(), + SimpleDependencyInfo.builder("yes4", SRC_PATH) + .setRequires( + Require.parsedFromDeps("a"), + Require.parsedFromDeps("b"), + Require.parsedFromDeps("c")) + .build()); assertThat(result).isEqualTo(expected); assertThat(errorManager.getErrorCount()).isEqualTo(0); @@ -125,18 +119,10 @@ public void testModule() { List result = parser.parseFile(SRC_PATH, "goog.addDependency('yes1', [], [], true);\n" + "goog.addDependency('yes2', [], [], false);\n"); - ImmutableList expected = ImmutableList.of( - SimpleDependencyInfo.builder("yes1", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setGoogModule(true) - .build(), - SimpleDependencyInfo.builder("yes2", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setGoogModule(false) - .build() - ); + ImmutableList expected = + ImmutableList.of( + SimpleDependencyInfo.builder("yes1", SRC_PATH).setGoogModule(true).build(), + SimpleDependencyInfo.builder("yes2", SRC_PATH).build()); assertThat(result).isEqualTo(expected); } @@ -145,22 +131,15 @@ public void testLoadFlags() { + "goog.addDependency('yes1', [], [], {'module': 'goog'});\n" + "goog.addDependency('yes2', [], [], {\"lang\": \"es6\"});\n" + "goog.addDependency('yes3', [], [], {});\n"); - ImmutableList expected = ImmutableList.of( - SimpleDependencyInfo.builder("yes1", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setLoadFlags(ImmutableMap.of("module", "goog")) - .build(), - SimpleDependencyInfo.builder("yes2", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setLoadFlags(ImmutableMap.of("lang", "es6")) - .build(), - SimpleDependencyInfo.builder("yes3", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setGoogModule(false) - .build()); + ImmutableList expected = + ImmutableList.of( + SimpleDependencyInfo.builder("yes1", SRC_PATH) + .setLoadFlags(ImmutableMap.of("module", "goog")) + .build(), + SimpleDependencyInfo.builder("yes2", SRC_PATH) + .setLoadFlags(ImmutableMap.of("lang", "es6")) + .build(), + SimpleDependencyInfo.builder("yes3", SRC_PATH).build()); assertThat(result).isEqualTo(expected); } @@ -171,9 +150,6 @@ public void testShortcutMode() { "goog.addDependency('no1', [], []);"); ImmutableList expected = ImmutableList.of( SimpleDependencyInfo.builder("yes1", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setGoogModule(false) .build()); assertThat(result).isEqualTo(expected); } @@ -184,17 +160,10 @@ public void testNoShortcutMode() { "goog.addDependency('yes1', [], []); \n" + "foo();\n" + "goog.addDependency('yes2', [], []);"); - ImmutableList expected = ImmutableList.of( - SimpleDependencyInfo.builder("yes1", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setGoogModule(false) - .build(), - SimpleDependencyInfo.builder("yes2", SRC_PATH) - .setProvides(EMPTY) - .setRequires(EMPTY) - .setGoogModule(false) - .build()); + ImmutableList expected = + ImmutableList.of( + SimpleDependencyInfo.builder("yes1", SRC_PATH).build(), + SimpleDependencyInfo.builder("yes2", SRC_PATH).build()); assertThat(result).isEqualTo(expected); } } diff --git a/test/com/google/javascript/jscomp/deps/DepsGeneratorTest.java b/test/com/google/javascript/jscomp/deps/DepsGeneratorTest.java index 91e4de72e3d..4c0fe7db405 100644 --- a/test/com/google/javascript/jscomp/deps/DepsGeneratorTest.java +++ b/test/com/google/javascript/jscomp/deps/DepsGeneratorTest.java @@ -40,13 +40,56 @@ protected void setUp() throws Exception { errorManager = new PrintStreamErrorManager(System.err); } + // TODO(johnplaisted): This should eventually be an error. For now people are relying on this + // behavior for interop / ordering. Until we have official channels for these allow this behavior, + // but don't encourage it. + public void testEs6ModuleWithGoogProvide() throws Exception { + List srcs = new ArrayList<>(); + srcs.add( + SourceFile.fromCode( + "/base/javascript/foo/foo.js", + "goog.provide('my.namespace');\nimport '../closure/goog/es6.js';")); + srcs.add(SourceFile.fromCode("/base/javascript/closure/goog/es6.js", "export var es6;")); + DepsGenerator depsGenerator = + new DepsGenerator( + ImmutableList.of(), + srcs, + DepsGenerator.InclusionStrategy.ALWAYS, + "/base/javascript/closure", + errorManager, + new ModuleLoader( + null, + ImmutableList.of("/base/"), + ImmutableList.of(), + ModuleLoader.PathResolver.ABSOLUTE, + ModuleLoader.ResolutionMode.BROWSER)); + String output = depsGenerator.computeDependencyCalls(); + + assertWarnings( + "File cannot be a combination of goog.provide, goog.module, and/or ES6 " + + "module: javascript/foo/foo.js"); + + // Write the output. + assertWithMessage("There should be output").that(output).isNotEmpty(); + + // Write the expected output. + String expected = + LINE_JOINER.join( + "goog.addDependency('../foo/foo.js', ['my.namespace'], " + + "['goog/es6.js'], {'lang': 'es6', 'module': 'es6'});", + "goog.addDependency('goog/es6.js', [], " + "[], {'lang': 'es6', 'module': 'es6'});", + ""); + + assertEquals(expected, output); + } + public void testEs6Modules() throws Exception { List srcs = new ArrayList<>(); - srcs.add(SourceFile.fromCode("/base/javascript/foo/foo.js", "import '../closure/goog/array';")); - srcs.add(SourceFile.fromCode("/base/javascript/closure/goog/array.js", "export var array;")); + srcs.add(SourceFile.fromCode("/base/javascript/foo/foo.js", "import '../closure/goog/es6';")); + srcs.add(SourceFile.fromCode("/base/javascript/closure/goog/es6.js", "export var es6;")); DepsGenerator depsGenerator = new DepsGenerator( - ImmutableList.of(), + ImmutableList.of(), srcs, DepsGenerator.InclusionStrategy.ALWAYS, "/base/javascript/closure", @@ -54,7 +97,89 @@ public void testEs6Modules() throws Exception { new ModuleLoader( null, ImmutableList.of("/base/"), - ImmutableList.of(), + ImmutableList.of(), + ModuleLoader.PathResolver.ABSOLUTE, + ModuleLoader.ResolutionMode.BROWSER)); + String output = depsGenerator.computeDependencyCalls(); + + assertNoWarnings(); + + // Write the output. + assertWithMessage("There should be output").that(output).isNotEmpty(); + + // Write the expected output. + String expected = + LINE_JOINER.join( + "goog.addDependency('../foo/foo.js', [], " + + "['goog/es6.js'], {'lang': 'es6', 'module': 'es6'});", + "goog.addDependency('goog/es6.js', [], " + "[], {'lang': 'es6', 'module': 'es6'});", + ""); + + assertEquals(expected, output); + } + + public void testGoogPathRequireForEs6ModuleFromGoogModule() throws Exception { + List srcs = new ArrayList<>(); + srcs.add( + SourceFile.fromCode( + "/base/javascript/foo/foo.js", + LINE_JOINER.join( + "goog.module('foo');", "const es6 = goog.require('../closure/goog/es6.js');"))); + srcs.add(SourceFile.fromCode("/base/javascript/closure/goog/es6.js", "export var es6;")); + DepsGenerator depsGenerator = + new DepsGenerator( + ImmutableList.of(), + srcs, + DepsGenerator.InclusionStrategy.ALWAYS, + "/base/javascript/closure", + errorManager, + new ModuleLoader( + null, + ImmutableList.of("/base/"), + ImmutableList.of(), + ModuleLoader.PathResolver.ABSOLUTE, + ModuleLoader.ResolutionMode.BROWSER)); + String output = depsGenerator.computeDependencyCalls(); + + assertNoWarnings(); + + // Write the output. + assertWithMessage("There should be output").that(output).isNotEmpty(); + + // Write the expected output. + String expected = + LINE_JOINER.join( + "goog.addDependency('../foo/foo.js', ['foo'], " + + "['goog/es6.js'], {'lang': 'es6', 'module': 'goog'});", + "goog.addDependency('goog/es6.js', [], " + "[], {'lang': 'es6', 'module': 'es6'});", + ""); + + assertEquals(expected, output); + } + + public void testGoogPathRequireForEs6ModuleInDepsFileFromGoogModule() throws Exception { + ImmutableList deps = + ImmutableList.of( + SourceFile.fromCode( + "deps.js", + "goog.addDependency('goog/es6.js', [], [], {'lang': 'es6', 'module': 'es6'})")); + ImmutableList srcs = + ImmutableList.of( + SourceFile.fromCode( + "/base/javascript/foo/foo.js", + LINE_JOINER.join( + "goog.module('foo');", "const es6 = goog.require('../closure/goog/es6.js');"))); + DepsGenerator depsGenerator = + new DepsGenerator( + deps, + srcs, + DepsGenerator.InclusionStrategy.ALWAYS, + "/base/javascript/closure", + errorManager, + new ModuleLoader( + null, + ImmutableList.of("/base/"), + ImmutableList.of(), ModuleLoader.PathResolver.ABSOLUTE, ModuleLoader.ResolutionMode.BROWSER)); String output = depsGenerator.computeDependencyCalls(); @@ -67,15 +192,59 @@ public void testEs6Modules() throws Exception { // Write the expected output. String expected = LINE_JOINER.join( - "goog.addDependency('../foo/foo.js', ['module$javascript$foo$foo'], " - + "['module$javascript$closure$goog$array'], {'lang': 'es6', 'module': 'es6'});", - "goog.addDependency('goog/array.js', ['module$javascript$closure$goog$array'], " - + "[], {'lang': 'es6', 'module': 'es6'});", + "goog.addDependency('../foo/foo.js', ['foo'], " + + "['goog/es6.js'], {'lang': 'es6', 'module': 'goog'});", + "", + "// Included from: deps.js", + "goog.addDependency('goog/es6.js', [], [], {'lang': 'es6', 'module': 'es6'});", ""); assertEquals(expected, output); } + public void testGoogPathRequireForGoogModuleFromGoogModuleIsInvalid() throws Exception { + List srcs = new ArrayList<>(); + srcs.add( + SourceFile.fromCode( + "/base/javascript/foo/foo.js", + LINE_JOINER.join( + "goog.module('foo');", + "const examplemodule = goog.require('../closure/goog/examplemodule.js');"))); + srcs.add( + SourceFile.fromCode( + "/base/javascript/closure/goog/examplemodule.js", + "goog.module('goog.examplemodule');")); + + doErrorMessagesRun( + ImmutableList.of(), + srcs, + true /* fatal */, + "Cannot goog.require \"/base/javascript/closure/goog/examplemodule.js\" by path. It is " + + "not an ES6 module."); + } + + public void testGoogPathRequireForGoogModuleFromEs6ModuleIsInvalid() throws Exception { + List srcs = new ArrayList<>(); + srcs.add( + SourceFile.fromCode( + "/base/javascript/foo/foo.js", + LINE_JOINER.join( + "const examplemodule = goog.require('../closure/goog/examplemodule.js');", + "export default examplemodule;"))); + srcs.add( + SourceFile.fromCode( + "/base/javascript/closure/goog/examplemodule.js", + "goog.module('goog.examplemodule');")); + + doErrorMessagesRun( + ImmutableList.of(), + srcs, + true /* fatal */, + "Cannot goog.require \"/base/javascript/closure/goog/examplemodule.js\" by path. It is " + + "not an ES6 module.", + "Cannot goog.require by path outside of a goog.module."); + } + /** * Ensures that deps files are handled correctly both when listed as deps and when listed as * sources. @@ -294,15 +463,15 @@ private String testMergeStrategyHelper(DepsGenerator.InclusionStrategy mergeStra String output = depsGenerator.computeDependencyCalls(); - assertWithMessage("There should be output files").that(output).isNotEmpty(); assertNoWarnings(); + assertWithMessage("There should be output files").that(output).isNotEmpty(); return output; } private void doErrorMessagesRun( - List deps, List srcs, boolean fatal, - String errorMessage) throws Exception { + List deps, List srcs, boolean fatal, String... errorMessages) + throws Exception { DepsGenerator depsGenerator = new DepsGenerator( @@ -321,10 +490,10 @@ private void doErrorMessagesRun( if (fatal) { assertWithMessage("No output should have been created.").that(output).isNull(); - assertError(errorMessage); + assertErrors(errorMessages); } else { assertWithMessage("Output should have been created.").that(output).isNotNull(); - assertWarning(errorMessage); + assertWarnings(errorMessages); } } @@ -441,6 +610,49 @@ public void testNoDepsInDepsFile() throws Exception { "No dependencies found in file"); } + public void testUnknownEs6Module() throws Exception { + SourceFile src1 = SourceFile.fromCode("src1.js", "import './missing.js';\n"); + + doErrorMessagesRun( + ImmutableList.of(), + ImmutableList.of(src1), + true /* fatal */, + "Could not find file \"./missing.js\"."); + } + + public void testGoogPathRequireFromNonModuleIsInvalid() throws Exception { + List srcs = new ArrayList<>(); + srcs.add( + SourceFile.fromCode( + "/base/javascript/foo/foo.js", + LINE_JOINER.join( + "goog.provide('foo');", + "const array = goog.require('../closure/goog/array.js');"))); + srcs.add(SourceFile.fromCode("/base/javascript/closure/goog/array.js", "export var array;")); + + doErrorMessagesRun( + ImmutableList.of(), + srcs, + true /* fatal */, + "Cannot goog.require by path outside of a goog.module."); + } + + public void testGoogPathRequireBetweenEs6ModulesIsInvalid() throws Exception { + List srcs = new ArrayList<>(); + srcs.add( + SourceFile.fromCode( + "/base/javascript/foo/foo.js", + "export var foo; const array = goog.require('../closure/goog/array.js');")); + srcs.add(SourceFile.fromCode("/base/javascript/closure/goog/array.js", "export var array;")); + + doErrorMessagesRun( + ImmutableList.of(), + srcs, + true /* fatal */, + "Use ES6 import rather than goog.require to import ES6 module " + + "\"/base/javascript/closure/goog/array.js\"."); + } + private void assertErrorWarningCount(int errorCount, int warningCount) { if (errorManager.getErrorCount() != errorCount) { fail(String.format("Expected %d errors but got\n%s", @@ -456,14 +668,18 @@ private void assertNoWarnings() { assertErrorWarningCount(0, 0); } - private void assertWarning(String message) { - assertErrorWarningCount(0, 1); - assertThat(errorManager.getWarnings()[0].description).isEqualTo(message); + private void assertWarnings(String... messages) { + assertErrorWarningCount(0, messages.length); + for (int i = 0; i < messages.length; i++) { + assertThat(errorManager.getWarnings()[i].description).isEqualTo(messages[i]); + } } - private void assertError(String message) { - assertErrorWarningCount(1, 0); - assertThat(errorManager.getErrors()[0].description).isEqualTo(message); + private void assertErrors(String... messages) { + assertErrorWarningCount(messages.length, 0); + for (int i = 0; i < messages.length; i++) { + assertThat(errorManager.getErrors()[i].description).isEqualTo(messages[i]); + } } private static void assertContains(String part, String whole) { diff --git a/test/com/google/javascript/jscomp/deps/Es6SortedDependenciesTest.java b/test/com/google/javascript/jscomp/deps/Es6SortedDependenciesTest.java index b5112968134..927e03b1565 100644 --- a/test/com/google/javascript/jscomp/deps/Es6SortedDependenciesTest.java +++ b/test/com/google/javascript/jscomp/deps/Es6SortedDependenciesTest.java @@ -19,6 +19,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; +import com.google.javascript.jscomp.deps.DependencyInfo.Require; import java.util.List; import junit.framework.TestCase; @@ -34,17 +35,20 @@ private static SortedDependencies createSortedDependencies } public void testSort() throws Exception { - SimpleDependencyInfo a = SimpleDependencyInfo.builder("a", "a") - .setRequires("b", "c") - .build(); - SimpleDependencyInfo b = SimpleDependencyInfo.builder("b", "b") - .setProvides("b") - .setRequires("d") - .build(); - SimpleDependencyInfo c = SimpleDependencyInfo.builder("c", "c") - .setProvides("c") - .setRequires("d") - .build(); + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("a", "a") + .setRequires(Require.googRequireSymbol("b"), Require.googRequireSymbol("c")) + .build(); + SimpleDependencyInfo b = + SimpleDependencyInfo.builder("b", "b") + .setProvides("b") + .setRequires(Require.googRequireSymbol("d")) + .build(); + SimpleDependencyInfo c = + SimpleDependencyInfo.builder("c", "c") + .setProvides("c") + .setRequires(Require.googRequireSymbol("d")) + .build(); SimpleDependencyInfo d = SimpleDependencyInfo.builder("d", "d") .setProvides("d") .build(); @@ -88,22 +92,26 @@ public void testSort() throws Exception { } public void testSort2() throws Exception { - SimpleDependencyInfo ab = SimpleDependencyInfo.builder("ab", "ab") - .setProvides("a", "b") - .setRequires("d", "f") - .build(); - SimpleDependencyInfo c = SimpleDependencyInfo.builder("c", "c") - .setProvides("c") - .setRequires("h") - .build(); - SimpleDependencyInfo d = SimpleDependencyInfo.builder("d", "d") - .setProvides("d") - .setRequires("e", "f") - .build(); - SimpleDependencyInfo ef = SimpleDependencyInfo.builder("ef", "ef") - .setProvides("e", "f") - .setRequires("g", "c") - .build(); + SimpleDependencyInfo ab = + SimpleDependencyInfo.builder("ab", "ab") + .setProvides("a", "b") + .setRequires(Require.googRequireSymbol("d"), Require.googRequireSymbol("f")) + .build(); + SimpleDependencyInfo c = + SimpleDependencyInfo.builder("c", "c") + .setProvides("c") + .setRequires(Require.googRequireSymbol("h")) + .build(); + SimpleDependencyInfo d = + SimpleDependencyInfo.builder("d", "d") + .setProvides("d") + .setRequires(Require.googRequireSymbol("e"), Require.googRequireSymbol("f")) + .build(); + SimpleDependencyInfo ef = + SimpleDependencyInfo.builder("ef", "ef") + .setProvides("e", "f") + .setRequires(Require.googRequireSymbol("g"), Require.googRequireSymbol("c")) + .build(); SimpleDependencyInfo g = SimpleDependencyInfo.builder("g", "g") .setProvides("g") .build(); @@ -126,18 +134,21 @@ public void testSort2() throws Exception { } public void testSort3() { - SimpleDependencyInfo a = SimpleDependencyInfo.builder("a", "a") - .setProvides("a") - .setRequires("c") - .build(); - SimpleDependencyInfo b = SimpleDependencyInfo.builder("b", "b") - .setProvides("b") - .setRequires("a") - .build(); - SimpleDependencyInfo c = SimpleDependencyInfo.builder("c", "c") - .setProvides("c") - .setRequires("b") - .build(); + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("a", "a") + .setProvides("a") + .setRequires(Require.googRequireSymbol("c")) + .build(); + SimpleDependencyInfo b = + SimpleDependencyInfo.builder("b", "b") + .setProvides("b") + .setRequires(Require.googRequireSymbol("a")) + .build(); + SimpleDependencyInfo c = + SimpleDependencyInfo.builder("c", "c") + .setProvides("c") + .setRequires(Require.googRequireSymbol("b")) + .build(); assertOrder( ImmutableList.of(a, b, c), ImmutableList.of(b, c, a)); @@ -145,10 +156,11 @@ public void testSort3() { public void testSort4() throws Exception { // Check the degenerate case. - SimpleDependencyInfo a = SimpleDependencyInfo.builder("a", "a") - .setProvides("a") - .setRequires("a") - .build(); + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("a", "a") + .setProvides("a") + .setRequires(Require.googRequireSymbol("a")) + .build(); assertSortedDeps( ImmutableList.of(a), ImmutableList.of(a), @@ -175,103 +187,123 @@ public void testSort5() throws Exception { } public void testSort6() { - SimpleDependencyInfo a = SimpleDependencyInfo.builder("gin", "gin") - .setProvides("gin") - .setRequires("tonic") - .build(); - SimpleDependencyInfo b = SimpleDependencyInfo.builder("tonic", "tonic") - .setProvides("tonic") - .setRequires("gin2") - .build(); - SimpleDependencyInfo c = SimpleDependencyInfo.builder("gin2", "gin2") - .setProvides("gin2") - .setRequires("gin") - .build(); - SimpleDependencyInfo d = SimpleDependencyInfo.builder("gin3", "gin3") - .setProvides("gin3") - .setRequires("gin") - .build(); + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("gin", "gin") + .setProvides("gin") + .setRequires(Require.googRequireSymbol("tonic")) + .build(); + SimpleDependencyInfo b = + SimpleDependencyInfo.builder("tonic", "tonic") + .setProvides("tonic") + .setRequires(Require.googRequireSymbol("gin2")) + .build(); + SimpleDependencyInfo c = + SimpleDependencyInfo.builder("gin2", "gin2") + .setProvides("gin2") + .setRequires(Require.googRequireSymbol("gin")) + .build(); + SimpleDependencyInfo d = + SimpleDependencyInfo.builder("gin3", "gin3") + .setProvides("gin3") + .setRequires(Require.googRequireSymbol("gin")) + .build(); assertOrder(ImmutableList.of(a, b, c, d), ImmutableList.of(c, b, a, d)); } public void testSort7() { - SimpleDependencyInfo a = SimpleDependencyInfo.builder("gin", "gin") - .setProvides("gin") - .setRequires("tonic") - .build(); - SimpleDependencyInfo b = SimpleDependencyInfo.builder("tonic", "tonic") - .setProvides("tonic") - .setRequires("gin") - .build(); - SimpleDependencyInfo c = SimpleDependencyInfo.builder("gin2", "gin2") - .setProvides("gin2") - .setRequires("gin") - .build(); - SimpleDependencyInfo d = SimpleDependencyInfo.builder("gin3", "gin3") - .setProvides("gin3") - .setRequires("gin") - .build(); + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("gin", "gin") + .setProvides("gin") + .setRequires(Require.googRequireSymbol("tonic")) + .build(); + SimpleDependencyInfo b = + SimpleDependencyInfo.builder("tonic", "tonic") + .setProvides("tonic") + .setRequires(Require.googRequireSymbol("gin")) + .build(); + SimpleDependencyInfo c = + SimpleDependencyInfo.builder("gin2", "gin2") + .setProvides("gin2") + .setRequires(Require.googRequireSymbol("gin")) + .build(); + SimpleDependencyInfo d = + SimpleDependencyInfo.builder("gin3", "gin3") + .setProvides("gin3") + .setRequires(Require.googRequireSymbol("gin")) + .build(); assertOrder( ImmutableList.of(a, b, c, d), ImmutableList.of(b, a, c, d)); } public void testSort8() { - SimpleDependencyInfo a = SimpleDependencyInfo.builder("A", "A") - .setProvides("A") - .setRequires("B") - .build(); - SimpleDependencyInfo b = SimpleDependencyInfo.builder("B", "B") - .setProvides("B") - .setRequires("C") - .build(); - SimpleDependencyInfo c = SimpleDependencyInfo.builder("C", "C") - .setProvides("C") - .setRequires("D") - .build(); - SimpleDependencyInfo d = SimpleDependencyInfo.builder("D", "D") - .setProvides("D") - .setRequires("A") - .build(); + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("A", "A") + .setProvides("A") + .setRequires(Require.googRequireSymbol("B")) + .build(); + SimpleDependencyInfo b = + SimpleDependencyInfo.builder("B", "B") + .setProvides("B") + .setRequires(Require.googRequireSymbol("C")) + .build(); + SimpleDependencyInfo c = + SimpleDependencyInfo.builder("C", "C") + .setProvides("C") + .setRequires(Require.googRequireSymbol("D")) + .build(); + SimpleDependencyInfo d = + SimpleDependencyInfo.builder("D", "D") + .setProvides("D") + .setRequires(Require.googRequireSymbol("A")) + .build(); assertOrder( ImmutableList.of(a, b, c, d), ImmutableList.of(d, c, b, a)); } public void testSort9() { - SimpleDependencyInfo a = SimpleDependencyInfo.builder("A", "A") - .setProvides("A") - .setRequires("B") - .build(); - SimpleDependencyInfo a2 = SimpleDependencyInfo.builder("A", "A") - .setProvides("A") - .setRequires("B1") - .build(); - SimpleDependencyInfo b = SimpleDependencyInfo.builder("B", "B") - .setProvides("B") - .setRequires("C") - .build(); - SimpleDependencyInfo c = SimpleDependencyInfo.builder("C", "C") - .setProvides("C") - .setRequires("E") - .build(); - SimpleDependencyInfo d = SimpleDependencyInfo.builder("D", "D") - .setProvides("D") - .setRequires("A") - .build(); - SimpleDependencyInfo e = SimpleDependencyInfo.builder("B1", "B1") - .setProvides("B1") - .setRequires("C1") - .build(); - SimpleDependencyInfo f = SimpleDependencyInfo.builder("C1", "C1") - .setProvides("C1") - .setRequires("D1") - .build(); - SimpleDependencyInfo g = SimpleDependencyInfo.builder("D1", "D1") - .setProvides("D1") - .setRequires("A") - .build(); + SimpleDependencyInfo a = + SimpleDependencyInfo.builder("A", "A") + .setProvides("A") + .setRequires(Require.googRequireSymbol("B")) + .build(); + SimpleDependencyInfo a2 = + SimpleDependencyInfo.builder("A", "A") + .setProvides("A") + .setRequires(Require.googRequireSymbol("B1")) + .build(); + SimpleDependencyInfo b = + SimpleDependencyInfo.builder("B", "B") + .setProvides("B") + .setRequires(Require.googRequireSymbol("C")) + .build(); + SimpleDependencyInfo c = + SimpleDependencyInfo.builder("C", "C") + .setProvides("C") + .setRequires(Require.googRequireSymbol("E")) + .build(); + SimpleDependencyInfo d = + SimpleDependencyInfo.builder("D", "D") + .setProvides("D") + .setRequires(Require.googRequireSymbol("A")) + .build(); + SimpleDependencyInfo e = + SimpleDependencyInfo.builder("B1", "B1") + .setProvides("B1") + .setRequires(Require.googRequireSymbol("C1")) + .build(); + SimpleDependencyInfo f = + SimpleDependencyInfo.builder("C1", "C1") + .setProvides("C1") + .setRequires(Require.googRequireSymbol("D1")) + .build(); + SimpleDependencyInfo g = + SimpleDependencyInfo.builder("D1", "D1") + .setProvides("D1") + .setRequires(Require.googRequireSymbol("A")) + .build(); assertOrder(ImmutableList.of(a, a2, b, c, d, e, f, g), ImmutableList.of(c, b, a, g, f, e, a2, d)); @@ -281,7 +313,7 @@ public void testSort10() throws Exception { SimpleDependencyInfo a = SimpleDependencyInfo.builder("A", "A") .setProvides("A") - .setRequires("C") + .setRequires(Require.googRequireSymbol("C")) .build(); SimpleDependencyInfo b = SimpleDependencyInfo.builder("B", "B") .setProvides("B") @@ -316,12 +348,4 @@ private static void assertOrder( SortedDependencies sorted = createSortedDependencies(shuffle); assertThat(sorted.getSortedList()).isEqualTo(expected); } - - private static ImmutableList requires(String... strings) { - return ImmutableList.copyOf(strings); - } - - private static ImmutableList provides(String... strings) { - return ImmutableList.copyOf(strings); - } } diff --git a/test/com/google/javascript/jscomp/deps/JsFileParserTest.java b/test/com/google/javascript/jscomp/deps/JsFileParserTest.java index 1bd2eb33a5f..5e732828329 100644 --- a/test/com/google/javascript/jscomp/deps/JsFileParserTest.java +++ b/test/com/google/javascript/jscomp/deps/JsFileParserTest.java @@ -17,12 +17,14 @@ package com.google.javascript.jscomp.deps; import static com.google.common.truth.Truth.assertThat; +import static com.google.javascript.jscomp.deps.DependencyInfo.Require.es6Import; +import static com.google.javascript.jscomp.deps.DependencyInfo.Require.googRequirePath; +import static com.google.javascript.jscomp.deps.DependencyInfo.Require.googRequireSymbol; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.javascript.jscomp.ErrorManager; import com.google.javascript.jscomp.PrintStreamErrorManager; -import java.util.Collections; import junit.framework.TestCase; /** @@ -62,12 +64,13 @@ public void testParseFile() { + "goog.require(\"bar.data.SuperstarAddStarThreadActionRequestDelegate\"); " + "//no new line at EOF"; - DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) - .setProvides(ImmutableList.of("yes1", "yes2")) - .setRequires( - ImmutableList.of("yes3", "bar.data.SuperstarAddStarThreadActionRequestDelegate")) - .setGoogModule(false) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) + .setProvides(ImmutableList.of("yes1", "yes2")) + .setRequires( + googRequireSymbol("yes3"), + googRequireSymbol("bar.data.SuperstarAddStarThreadActionRequestDelegate")) + .build(); DependencyInfo result = parser.parseFile(SRC_PATH, CLOSURE_PATH, contents); @@ -85,11 +88,16 @@ public void testParseFile2() { + "var C = goog.require(\"a.b.C\");\n" + "let {D, E} = goog.require('a.b.d');"; - DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) - .setProvides(ImmutableList.of("yes1")) - .setRequires(ImmutableList.of("yes2", "a.b.C", "a.b.d")) - .setLoadFlags(ImmutableMap.of("module", "goog")) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) + .setProvides(ImmutableList.of("yes1")) + .setRequires( + ImmutableList.of( + googRequireSymbol("yes2"), + googRequireSymbol("a.b.C"), + googRequireSymbol("a.b.d"))) + .setLoadFlags(ImmutableMap.of("module", "goog")) + .build(); DependencyInfo result = parser.parseFile(SRC_PATH, CLOSURE_PATH, contents); @@ -107,11 +115,16 @@ public void testParseFile3() { + "var C=goog.require(\"a.b.C\");\n" + "const {\n D,\n E\n}=goog.require(\"a.b.d\");"; - DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) - .setProvides(ImmutableList.of("yes1")) - .setRequires(ImmutableList.of("yes2", "a.b.C", "a.b.d")) - .setLoadFlags(ImmutableMap.of("module", "goog")) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) + .setProvides(ImmutableList.of("yes1")) + .setRequires( + ImmutableList.of( + googRequireSymbol("yes2"), + googRequireSymbol("a.b.C"), + googRequireSymbol("a.b.d"))) + .setLoadFlags(ImmutableMap.of("module", "goog")) + .build(); DependencyInfo result = parser.parseFile(SRC_PATH, CLOSURE_PATH, contents); @@ -163,11 +176,16 @@ public void testParseWrappedGoogModule() { + "var C=goog.require(\"a.b.C\");\n" + "const {\n D,\n E\n}=goog.require(\"a.b.d\");});"; - DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) - .setProvides(ImmutableList.of("yes1")) - .setRequires(ImmutableList.of("yes2", "a.b.C", "a.b.d")) - .setLoadFlags(ImmutableMap.of()) - .build(); // wrapped modules aren't marked as modules + DependencyInfo expected = + SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) + .setProvides(ImmutableList.of("yes1")) + .setRequires( + ImmutableList.of( + googRequireSymbol("yes2"), + googRequireSymbol("a.b.C"), + googRequireSymbol("a.b.d"))) + .setLoadFlags(ImmutableMap.of()) + .build(); // wrapped modules aren't marked as modules DependencyInfo result = parser.parseFile(SRC_PATH, CLOSURE_PATH, contents); @@ -188,12 +206,17 @@ public void testParseEs6Module() { + "import \"./dquote\";\n" + "export * from './exported';\n"; - DependencyInfo expected = SimpleDependencyInfo.builder("a.js", "b.js") - .setProvides(ImmutableList.of("module$b")) - .setRequires(ImmutableList.of( - "module$yes2", "module$a$b$C", "module$a$b$d", "module$dquote", "module$exported")) - .setLoadFlags(ImmutableMap.of("module", "es6")) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder("a.js", "b.js") + .setProvides(ImmutableList.of("module$b")) + .setRequires( + es6Import("module$yes2", "./yes2"), + es6Import("module$a$b$C", "./a/b/C"), + es6Import("module$a$b$d", "./a/b/d"), + es6Import("module$dquote", "./dquote"), + es6Import("module$exported", "./exported")) + .setLoadFlags(ImmutableMap.of("module", "es6")) + .build(); DependencyInfo result = parser.parseFile("b.js", "a.js", contents); @@ -214,9 +237,11 @@ public void testParseEs6Module2() { DependencyInfo expected = SimpleDependencyInfo.builder("../../a/b.js", "/foo/bar/a/b.js") .setProvides(ImmutableList.of("module$foo$bar$a$b")) - .setRequires(ImmutableList.of( - "module$foo$bar$a$x", "module$foo$bar$y", - "module$foo$bar$a$z", "module$foo$bar$c$w")) + .setRequires( + ImmutableList.of( + es6Import("module$foo$bar$a$x", "./x"), es6Import("module$foo$bar$y", "../y"), + es6Import("module$foo$bar$a$z", "../a/z"), + es6Import("module$foo$bar$c$w", "../c/w"))) .setLoadFlags(ImmutableMap.of("module", "es6")) .build(); @@ -234,11 +259,13 @@ public void testParseEs6Module3() { + "import 'goog:foo.bar.baz';\n" + "goog.require('baz.qux');\n"; - DependencyInfo expected = SimpleDependencyInfo.builder("b.js", "a.js") - .setProvides(ImmutableList.of("module$a")) - .setRequires(ImmutableList.of("foo.bar.baz", "baz.qux")) - .setLoadFlags(ImmutableMap.of("module", "es6")) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder("b.js", "a.js") + .setProvides(ImmutableList.of("module$a")) + .setRequires( + ImmutableList.of(googRequireSymbol("foo.bar.baz"), googRequireSymbol("baz.qux"))) + .setLoadFlags(ImmutableMap.of("module", "es6")) + .build(); DependencyInfo result = parser.parseFile("a.js", "b.js", contents); @@ -264,13 +291,18 @@ public void testParseEs6Module4() { + "import '../closure/d/e';\n" + "import '../../corge/f';\n"; - DependencyInfo expected = SimpleDependencyInfo.builder("../bar/baz.js", "/foo/js/bar/baz.js") - .setProvides(ImmutableList.of("module$js$bar$baz")) - .setRequires(ImmutableList.of( - "module$js$bar$a", "module$js$bar$qux$b", "module$js$closure$c", - "module$js$closure$d$e", "module$corge$f")) - .setLoadFlags(ImmutableMap.of("module", "es6")) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder("../bar/baz.js", "/foo/js/bar/baz.js") + .setProvides(ImmutableList.of("module$js$bar$baz")) + .setRequires( + ImmutableList.of( + es6Import("module$js$bar$a", "./a"), + es6Import("module$js$bar$qux$b", "./qux/b"), + es6Import("module$js$closure$c", "../closure/c"), + es6Import("module$js$closure$d$e", "../closure/d/e"), + es6Import("module$corge$f", "../../corge/f"))) + .setLoadFlags(ImmutableMap.of("module", "es6")) + .build(); DependencyInfo result = parser @@ -280,6 +312,36 @@ public void testParseEs6Module4() { assertDeps(expected, result); } + // TODO(johnplaisted): This should eventually be an error. For now people are relying on this + // behavior for interop / ordering. Until we have official channels for these allow this behavior, + // but don't encourage it. + public void testParseEs6ModuleWithGoogProvide() { + ModuleLoader loader = + new ModuleLoader( + null, + ImmutableList.of("/foo"), + ImmutableList.of(), + ModuleLoader.ResolutionMode.BROWSER); + + String contents = "goog.provide('my.namespace');\nexport {};"; + + DependencyInfo expected = + SimpleDependencyInfo.builder("../bar/baz.js", "/foo/js/bar/baz.js") + .setProvides(ImmutableList.of("my.namespace", "module$js$bar$baz")) + .setLoadFlags(ImmutableMap.of("module", "es6")) + .build(); + + DependencyInfo result = + parser + .setModuleLoader(loader) + .parseFile("/foo/js/bar/baz.js", "../bar/baz.js", contents); + + assertThat(result).isEqualTo(expected); + assertThat(errorManager.getErrorCount()).isEqualTo(0); + assertThat(errorManager.getWarningCount()).isEqualTo(1); + assertThat(errorManager.getWarnings()[0].getType()).isEqualTo(ModuleLoader.MODULE_CONFLICT); + } + /** * Tests: * -Shortcut mode doesn't stop at setTestOnly() or declareLegacyNamespace(). @@ -293,11 +355,13 @@ public void testNoShortcutForCommonModuleModifiers() { + "var C=goog.require(\"a.b.C\");\n" + "const {\n D,\n E\n}=goog.require(\"a.b.d\");"; - DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) - .setProvides(ImmutableList.of("yes1")) - .setRequires(ImmutableList.of("yes2", "a.b.C", "a.b.d")) - .setGoogModule(true) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) + .setProvides(ImmutableList.of("yes1")) + .setRequires( + googRequireSymbol("yes2"), googRequireSymbol("a.b.C"), googRequireSymbol("a.b.d")) + .setGoogModule(true) + .build(); DependencyInfo result = parser.parseFile(SRC_PATH, CLOSURE_PATH, contents); @@ -310,7 +374,6 @@ public void testMultiplePerLine() { DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) .setProvides(ImmutableList.of("yes1", "yes2", "yes3")) - .setRequires(Collections.emptyList()) .setGoogModule(false) .build(); @@ -329,7 +392,6 @@ public void testShortcutMode1() { DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) .setProvides(ImmutableList.of("yes1", "yes2")) - .setRequires(Collections.emptyList()) .setGoogModule(false) .build(); DependencyInfo result = parser.parseFile(SRC_PATH, CLOSURE_PATH, contents); @@ -344,7 +406,6 @@ public void testShortcutMode2() { DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) .setProvides(ImmutableList.of("yes1")) - .setRequires(Collections.emptyList()) .setGoogModule(false) .build(); DependencyInfo result = parser.parseFile(SRC_PATH, CLOSURE_PATH, contents); @@ -359,7 +420,6 @@ public void testShortcutMode3() { DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) .setProvides(ImmutableList.of("yes1")) - .setRequires(Collections.emptyList()) .setGoogModule(false) .build(); DependencyInfo result = parser.parseFile(SRC_PATH, CLOSURE_PATH, contents); @@ -375,7 +435,6 @@ public void testIncludeGoog1() { DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) .setProvides(ImmutableList.of("goog")) - .setRequires(Collections.emptyList()) .setGoogModule(false) .build(); DependencyInfo result = parser.setIncludeGoogBase(true).parseFile( @@ -386,11 +445,11 @@ public void testIncludeGoog1() { public void testIncludeGoog2() { String contents = "goog.require('bar');"; - DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) - .setProvides(ImmutableList.of()) - .setRequires(ImmutableList.of("goog", "bar")) - .setGoogModule(false) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) + .setRequires(ImmutableList.of(googRequireSymbol("goog"), googRequireSymbol("bar"))) + .setGoogModule(false) + .build(); DependencyInfo result = parser.setIncludeGoogBase(true).parseFile( SRC_PATH, CLOSURE_PATH, contents); assertDeps(expected, result); @@ -404,11 +463,12 @@ public void testIncludeGoog3() { " */\n" + "var COMPILED = false;\n"; - DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) - .setProvides(ImmutableList.of("x")) - .setRequires(ImmutableList.of("goog")) - .setGoogModule(false) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) + .setProvides(ImmutableList.of("x")) + .setRequires(ImmutableList.of(googRequireSymbol("goog"))) + .setGoogModule(false) + .build(); DependencyInfo result = parser.setIncludeGoogBase(true).parseFile( SRC_PATH, CLOSURE_PATH, contents); assertDeps(expected, result); @@ -417,16 +477,39 @@ public void testIncludeGoog3() { public void testIncludeGoog4() { String contents = "goog.addDependency('foo', [], []);\n"; - DependencyInfo expected = SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) - .setProvides(ImmutableList.of()) - .setRequires(ImmutableList.of("goog")) - .setGoogModule(false) - .build(); + DependencyInfo expected = + SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) + .setRequires(ImmutableList.of(googRequireSymbol("goog"))) + .setGoogModule(false) + .build(); DependencyInfo result = parser.setIncludeGoogBase(true).parseFile( SRC_PATH, CLOSURE_PATH, contents); assertDeps(expected, result); } + public void testParseGoogRequirePath() { + String contents = + "" + + "goog.module('yes1');\n" + + "var yes2=goog.require('yes2');\n" + + "var C=goog.require('./c.js');\n" + + "var DQuote=goog.require(\"../d/quote.js\");"; + + DependencyInfo expected = + SimpleDependencyInfo.builder(CLOSURE_PATH, SRC_PATH) + .setProvides("yes1") + .setRequires( + googRequireSymbol("yes2"), + googRequirePath("module$c", "./c.js"), + googRequirePath("module$__$d$quote", "../d/quote.js")) + .setLoadFlags(ImmutableMap.of("module", "goog")) + .build(); + + DependencyInfo result = parser.parseFile(SRC_PATH, CLOSURE_PATH, contents); + + assertDeps(expected, result); + } + /** Asserts the deps match without errors */ private void assertDeps(DependencyInfo expected, DependencyInfo actual) { assertThat(actual).isEqualTo(expected);