From 7ed7854a2f8ad8e00e53ea52e9b5e00de418af82 Mon Sep 17 00:00:00 2001 From: johnplaisted Date: Wed, 28 Mar 2018 17:02:39 -0700 Subject: [PATCH] Add an ES6 module's relative path to closure as an implicit provide when parsing deps.js files so that dependency graphs are correct. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=190859615 --- .../jscomp/deps/DepsFileParser.java | 18 +++++++++++---- .../javascript/jscomp/deps/DepsGenerator.java | 14 ++++++++++- .../jscomp/deps/SimpleDependencyInfo.java | 10 ++++++++ .../jscomp/deps/DepsFileParserTest.java | 23 ++++++++++++++++++- 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src/com/google/javascript/jscomp/deps/DepsFileParser.java b/src/com/google/javascript/jscomp/deps/DepsFileParser.java index fb339dd7c3d..3e5f936afea 100644 --- a/src/com/google/javascript/jscomp/deps/DepsFileParser.java +++ b/src/com/google/javascript/jscomp/deps/DepsFileParser.java @@ -48,7 +48,6 @@ */ @GwtIncompatible("java.util.regex") public final class DepsFileParser extends JsFileLineParser { - private static final Logger logger = Logger.getLogger(DepsFileParser.class.getName()); /** @@ -82,7 +81,7 @@ public final class DepsFileParser extends JsFileLineParser { * @param errorManager Handles parse errors. */ public DepsFileParser(ErrorManager errorManager) { - this(Functions.identity(), errorManager); + this(Functions.identity(), errorManager); } /** @@ -166,17 +165,26 @@ protected boolean parseLine(String line) throws ParseException { + addDependencyParams, true); } // Parse the file path. - String path = pathTranslator.apply(parseJsString(depArgsMatch.group(1))); + String relativePath = parseJsString(depArgsMatch.group(1)); + String path = pathTranslator.apply(relativePath); + + List provides = parseJsStringArray(depArgsMatch.group(2)); + Map loadFlags = parseLoadFlags(depArgsMatch.group(4)); + + // ES6 modules are require'd by path but do not provide them in the addDependency call. + if ("es6".equals(loadFlags.get("module"))) { + provides.add(relativePath); + } DependencyInfo depInfo = SimpleDependencyInfo.builder(path, filePath) - .setProvides(parseJsStringArray(depArgsMatch.group(2))) + .setProvides(provides) .setRequires( parseJsStringArray(depArgsMatch.group(3)) .stream() .map(Require::parsedFromDeps) .collect(toImmutableList())) - .setLoadFlags(parseLoadFlags(depArgsMatch.group(4))) + .setLoadFlags(loadFlags) .build(); if (logger.isLoggable(Level.FINE)) { diff --git a/src/com/google/javascript/jscomp/deps/DepsGenerator.java b/src/com/google/javascript/jscomp/deps/DepsGenerator.java index 69af3eff3cc..d1c2570d8fe 100644 --- a/src/com/google/javascript/jscomp/deps/DepsGenerator.java +++ b/src/com/google/javascript/jscomp/deps/DepsGenerator.java @@ -47,6 +47,7 @@ import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.stream.Collectors; /** * Generates deps.js files by scanning JavaScript files for @@ -477,7 +478,18 @@ private Map parseDepsFiles() throws IOException { reportNoDepsInDepsFile(file.getName()); } else { for (DependencyInfo info : depInfos) { - depsFiles.put(info.getPathRelativeToClosureBase(), info); + // DepsFileParser adds an ES6 module's relative path to closure as a provide so that + // the resulting depgraph is valid. But we don't want to write this "fake" provide + // back out, so remove it here. + depsFiles.put( + info.getPathRelativeToClosureBase(), + SimpleDependencyInfo.Builder.from(info) + .setProvides( + info.getProvides() + .stream() + .filter(p -> !p.equals(info.getPathRelativeToClosureBase())) + .collect(Collectors.toList())) + .build()); } } } diff --git a/src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java b/src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java index 6bad0f2e51f..ab69fde3342 100644 --- a/src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java +++ b/src/com/google/javascript/jscomp/deps/SimpleDependencyInfo.java @@ -45,6 +45,16 @@ public static Builder builder(String srcPathRelativeToClosure, String pathOfDefi */ @AutoValue.Builder public abstract static class Builder { + public static Builder from(DependencyInfo copy) { + return new AutoValue_SimpleDependencyInfo.Builder() + .setName(copy.getName()) + .setPathRelativeToClosureBase(copy.getPathRelativeToClosureBase()) + .setProvides(copy.getProvides()) + .setRequires(copy.getRequires()) + .setWeakRequires(copy.getWeakRequires()) + .setLoadFlags(copy.getLoadFlags()); + } + abstract Builder setName(String name); abstract Builder setPathRelativeToClosureBase(String srcPathRelativeToClosure); diff --git a/test/com/google/javascript/jscomp/deps/DepsFileParserTest.java b/test/com/google/javascript/jscomp/deps/DepsFileParserTest.java index ceb69214054..a1e803f5318 100644 --- a/test/com/google/javascript/jscomp/deps/DepsFileParserTest.java +++ b/test/com/google/javascript/jscomp/deps/DepsFileParserTest.java @@ -115,7 +115,7 @@ public void testBadLoadFlagsSyntax() { assertThat(errorManager.getWarningCount()).isEqualTo(0); } - public void testModule() { + public void testGoogModule() { List result = parser.parseFile(SRC_PATH, "goog.addDependency('yes1', [], [], true);\n" + "goog.addDependency('yes2', [], [], false);\n"); @@ -126,6 +126,27 @@ public void testModule() { assertThat(result).isEqualTo(expected); } + public void testEs6Module() { + List result = + parser.parseFile( + SRC_PATH, + "goog.addDependency('path/from/closure.js', [], ['nexttoclosure.js'], " + + "{'module':'es6'});\n" + + "goog.addDependency('nexttoclosure.js', [], [], {'module':'es6'});\n"); + ImmutableList expected = + ImmutableList.of( + SimpleDependencyInfo.builder("path/from/closure.js", SRC_PATH) + .setLoadFlags(ImmutableMap.of("module", "es6")) + .setProvides(ImmutableList.of("path/from/closure.js")) + .setRequires(Require.parsedFromDeps("nexttoclosure.js")) + .build(), + SimpleDependencyInfo.builder("nexttoclosure.js", SRC_PATH) + .setLoadFlags(ImmutableMap.of("module", "es6")) + .setProvides(ImmutableList.of("nexttoclosure.js")) + .build()); + assertThat(result).isEqualTo(expected); + } + public void testLoadFlags() { List result = parser.parseFile(SRC_PATH, "" + "goog.addDependency('yes1', [], [], {'module': 'goog'});\n"