Roll back "Allow the transpiler to be configured with a feature set o…

…utput rather than to ES5." caused some timeouts in internal projects *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=201236603
google · Jun 20, 2018 · 8061863 · 8061863
1 parent 5406b19
commit 8061863
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 122 deletions.
diff --git a/src/com/google/javascript/jscomp/CommandLineRunner.java b/src/com/google/javascript/jscomp/CommandLineRunner.java
@@ -30,14 +30,11 @@
 import com.google.common.io.Files;
 import com.google.javascript.jscomp.AbstractCommandLineRunner.CommandLineConfig.ErrorFormatOption;
 import com.google.javascript.jscomp.CompilerOptions.IsolationMode;
-import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
 import com.google.javascript.jscomp.SourceMap.LocationMapping;
 import com.google.javascript.jscomp.deps.ClosureBundler;
 import com.google.javascript.jscomp.deps.ModuleLoader;
 import com.google.javascript.jscomp.parsing.parser.FeatureSet;
-import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
 import com.google.javascript.jscomp.transpile.BaseTranspiler;
-import com.google.javascript.jscomp.transpile.BaseTranspiler.CompilerSupplier;
 import com.google.javascript.jscomp.transpile.Transpiler;
 import com.google.javascript.rhino.TokenStream;
 import com.google.protobuf.TextFormat;
@@ -1846,9 +1843,13 @@ protected Compiler createCompiler() {
     return new Compiler(getErrorPrintStream());
   }
 
-  @Override
-  protected void prepForBundleAndAppendTo(Appendable out, CompilerInput input, String content)
-      throws IOException {
+  private ClosureBundler bundler;
+
+  private ClosureBundler getBundler() {
+    if (bundler != null) {
+      return bundler;
+    }
+
     ImmutableList<String> moduleRoots;
     if (!flags.moduleRoot.isEmpty()) {
       moduleRoots = ImmutableList.copyOf(flags.moduleRoot);
@@ -1857,29 +1858,25 @@ protected void prepForBundleAndAppendTo(Appendable out, CompilerInput input, Str
     }
 
     CompilerOptions options = createOptions();
+    return bundler = new ClosureBundler(
+        Transpiler.NULL,
+        new BaseTranspiler(
+            new BaseTranspiler.EsmToCjsCompilerSupplier(
+                options.getModuleResolutionMode(),
+                moduleRoots,
+                options.getBrowserResolverPrefixReplacements()),
+            /* runtimeLibraryName= */ ""));
+  }
 
-    new ClosureBundler(
-            "es6".equals(input.getLoadFlags().get("module"))
-                ? new BaseTranspiler(
-                    new CompilerSupplier(
-                        LanguageMode.ECMASCRIPT_NEXT.toFeatureSet().without(Feature.MODULES),
-                        options.getModuleResolutionMode(),
-                        moduleRoots,
-                        options.getBrowserResolverPrefixReplacements()),
-                    /* runtimeLibraryName= */ "")
-                : Transpiler.NULL)
-        .withPath(input.getName())
-        .appendTo(out, input, content);
+  @Override
+  protected void prepForBundleAndAppendTo(Appendable out, CompilerInput input, String content)
+      throws IOException {
+    getBundler().withPath(input.getName()).appendTo(out, input, content);
   }
 
   @Override
   protected void appendRuntimeTo(Appendable out) throws IOException {
-    new ClosureBundler(
-            new BaseTranspiler(
-                new CompilerSupplier(),
-                // Default runtime is the module runtime.
-                /* runtimeLibraryName= */ ""))
-        .appendRuntimeTo(out);
+    getBundler().appendRuntimeTo(out);
   }
 
   @Override

diff --git a/src/com/google/javascript/jscomp/deps/ClosureBundler.java b/src/com/google/javascript/jscomp/deps/ClosureBundler.java
@@ -51,7 +51,7 @@ public ClosureBundler() {
   }
 
   public ClosureBundler(Transpiler transpiler) {
-    this(transpiler, BaseTranspiler.LATEST_TRANSPILER);
+    this(transpiler, BaseTranspiler.ES_MODULE_TO_CJS_TRANSPILER);
   }
 
   public ClosureBundler(Transpiler transpiler, Transpiler es6ModuleTranspiler) {
@@ -79,20 +79,6 @@ private ClosureBundler(
     this.es6ModuleTranspiler = es6ModuleTranspiler;
   }
 
-  public ClosureBundler withTranspilers(
-      Transpiler newTranspiler, Transpiler newEs6ModuleTranspiler) {
-    return new ClosureBundler(
-        newTranspiler, newEs6ModuleTranspiler, mode, sourceUrl, path, sourceMapCache);
-  }
-
-  public ClosureBundler withTranspiler(Transpiler newTranspiler) {
-    return withTranspilers(newTranspiler, es6ModuleTranspiler);
-  }
-
-  public ClosureBundler withEs6ModuleTranspiler(Transpiler newEs6ModuleTranspiler) {
-    return withTranspilers(transpiler, newEs6ModuleTranspiler);
-  }
-
   public final ClosureBundler useEval(boolean useEval) {
     EvalMode newMode = useEval ? EvalMode.EVAL : EvalMode.NORMAL;
     return new ClosureBundler(

diff --git a/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java b/src/com/google/javascript/jscomp/transpile/BaseTranspiler.java
@@ -28,6 +28,7 @@
 import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
 import com.google.javascript.jscomp.DiagnosticGroup;
 import com.google.javascript.jscomp.DiagnosticType;
+import com.google.javascript.jscomp.Es6RewriteModulesToCommonJsModules;
 import com.google.javascript.jscomp.PropertyRenamingPolicy;
 import com.google.javascript.jscomp.Result;
 import com.google.javascript.jscomp.SourceFile;
@@ -37,8 +38,7 @@
 import com.google.javascript.jscomp.deps.ModuleLoader;
 import com.google.javascript.jscomp.deps.ModuleLoader.PathEscaper;
 import com.google.javascript.jscomp.deps.ModuleLoader.ResolutionMode;
-import com.google.javascript.jscomp.parsing.parser.FeatureSet;
-import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
+import com.google.javascript.rhino.Node;
 import java.io.IOException;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -75,17 +75,11 @@ public String runtime() {
     return sb.toString();
   }
 
-  public static final BaseTranspiler LATEST_TRANSPILER = to(FeatureSet.latest(), "");
+  public static final BaseTranspiler ES5_TRANSPILER = new BaseTranspiler(
+      new CompilerSupplier(), "es6_runtime");
 
-  public static final BaseTranspiler ES5_TRANSPILER = to(LanguageMode.ECMASCRIPT5.toFeatureSet());
-
-  public static final BaseTranspiler to(FeatureSet featureSet, String runtime) {
-    return new BaseTranspiler(new CompilerSupplier(featureSet), runtime);
-  }
-
-  public static final BaseTranspiler to(FeatureSet featureSet) {
-    return to(featureSet, "es6_runtime");
-  }
+  public static final BaseTranspiler ES_MODULE_TO_CJS_TRANSPILER =
+      new BaseTranspiler(new EsmToCjsCompilerSupplier(), "");
 
   /**
    * Wraps the Compiler into a more relevant interface, making it
@@ -99,19 +93,10 @@ public static class CompilerSupplier {
     protected final ResolutionMode moduleResolution;
     protected final ImmutableList<String> moduleRoots;
     protected final ImmutableMap<String, String> prefixReplacements;
-    protected final FeatureSet outputFeatureSet;
 
     public CompilerSupplier() {
-      this(LanguageMode.ECMASCRIPT5.toFeatureSet());
-    }
-
-    public CompilerSupplier(FeatureSet outputFeatureSet) {
       // Use the default resolution mode
-      this(
-          outputFeatureSet,
-          new CompilerOptions().getModuleResolutionMode(),
-          ImmutableList.of(),
-          ImmutableMap.of());
+      this(new CompilerOptions().getModuleResolutionMode(), ImmutableList.of(), ImmutableMap.of());
     }
 
     /**
@@ -122,11 +107,9 @@ public CompilerSupplier(FeatureSet outputFeatureSet) {
      *     ModuleLoader.ResolutionMode#BROWSER_WITH_TRANSFORMED_PREFIXES}
      */
     public CompilerSupplier(
-        FeatureSet outputFeatureSet,
         ModuleLoader.ResolutionMode moduleResolution,
         ImmutableList<String> moduleRoots,
         ImmutableMap<String, String> prefixReplacements) {
-      this.outputFeatureSet = outputFeatureSet;
       this.moduleResolution = moduleResolution;
       this.moduleRoots = moduleRoots;
       this.prefixReplacements = prefixReplacements;
@@ -175,8 +158,16 @@ protected CompilerOptions options() {
 
     protected void setOptions(CompilerOptions options) {
       options.setLanguageIn(LanguageMode.ECMASCRIPT_NEXT);
-      options.setOutputFeatureSet(outputFeatureSet.without(Feature.MODULES));
-      options.setEmitUseStrict(false);
+      // TODO(sdh): It would be nice to allow people to output code in
+      // strict mode.  But currently we swallow all the input language
+      // strictness checks, and there are various tests that are never
+      // compiled and so are broken when we output 'use strict'.  We
+      // could consider adding some sort of logging/warning/error in
+      // cases where the input was not strict, though there could still
+      // be semantic differences even if syntax is strict.  Possibly
+      // the first step would be to allow the option of outputting strict
+      // and then change the default and see what breaks.  b/33005948
+      options.setLanguageOut(LanguageMode.ECMASCRIPT5);
       options.setQuoteKeywordProperties(true);
       options.setSkipNonTranspilationPasses(true);
       options.setVariableRenaming(VariableRenamingPolicy.OFF);
@@ -216,6 +207,76 @@ protected void setOptions(CompilerOptions options) {
         DiagnosticType.error("JSC_CANNOT_CONVERT", ""));
   }
 
+  /**
+   * CompilerSupplier that only transforms EcmaScript Modules into a form that can be safely
+   * transformed on a file by file basis and concatenated.
+   */
+  public static class EsmToCjsCompilerSupplier extends CompilerSupplier {
+
+    public EsmToCjsCompilerSupplier() {
+      super();
+    }
+
+    public EsmToCjsCompilerSupplier(
+        ModuleLoader.ResolutionMode moduleResolution,
+        ImmutableList<String> moduleRoots,
+        ImmutableMap<String, String> prefixReplacements) {
+      super(moduleResolution, moduleRoots, prefixReplacements);
+    }
+
+
+    @Override
+    public CompileResult compile(URI path, String code) {
+      CompilerOptions options = new CompilerOptions();
+      options.setLanguageIn(LanguageMode.ECMASCRIPT_NEXT);
+      options.setEmitUseStrict(false);
+      options.setSourceMapOutputPath("/dev/null");
+      options.setSourceMapIncludeSourcesContent(true);
+      options.setPrettyPrint(true);
+      options.setModuleResolutionMode(moduleResolution);
+      options.setModuleRoots(moduleRoots);
+      options.setBrowserResolverPrefixReplacements(prefixReplacements);
+
+      // Create a compiler and run specifically this one pass on it.
+      Compiler compiler = compiler();
+      compiler.init(
+          ImmutableList.of(),
+          ImmutableList.of(SourceFile.fromCode(path.toString(), code)),
+          options);
+      compiler.parseForCompilation();
+
+      boolean transpiled = false;
+
+      if (!compiler.hasErrors()
+          && compiler.getRoot().getSecondChild().getFirstFirstChild().isModuleBody()
+          && !compiler
+              .getRoot()
+              .getSecondChild()
+              .getFirstChild()
+              .getBooleanProp(Node.GOOG_MODULE)) {
+        new Es6RewriteModulesToCommonJsModules(compiler)
+            .process(null, compiler.getRoot().getSecondChild());
+        compiler.getRoot().getSecondChild().getFirstChild().putBooleanProp(Node.TRANSPILED, true);
+        transpiled = true;
+      }
+
+      Result result = compiler.getResult();
+      String source = compiler.toSource();
+      StringBuilder sourceMap = new StringBuilder();
+      if (result.sourceMap != null) {
+        try {
+          result.sourceMap.appendTo(sourceMap, path.toString());
+        } catch (IOException e) {
+          // impossible, and not a big deal even if it did happen.
+        }
+      }
+      if (result.errors.length > 0) {
+        throw new TranspilationException(compiler, result.errors, result.warnings);
+      }
+      return new CompileResult(source, transpiled, transpiled ? sourceMap.toString() : "");
+    }
+  }
+
   /**
    * The source together with the additional compilation results.
    */

diff --git a/test/com/google/javascript/jscomp/deps/ClosureBundlerTest.java b/test/com/google/javascript/jscomp/deps/ClosureBundlerTest.java
@@ -20,8 +20,6 @@
 import static org.mockito.Mockito.when;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.javascript.jscomp.bundle.TranspilationException;
-import com.google.javascript.jscomp.transpile.BaseTranspiler;
 import com.google.javascript.jscomp.transpile.TranspileResult;
 import com.google.javascript.jscomp.transpile.Transpiler;
 import java.io.IOException;
@@ -142,10 +140,9 @@ public void testEs6Module() throws IOException {
     String input =
         "import {x} from './other.js';\n"
             + "export {x as y};"
-            + "let local;\n"
+            + "var local;\n"
             + "export function foo() { return local; }\n";
-    ClosureBundler bundler =
-        new ClosureBundler(BaseTranspiler.LATEST_TRANSPILER).withPath("nested/path/foo.js");
+    ClosureBundler bundler = new ClosureBundler().withPath("nested/path/foo.js");
     StringBuilder sb = new StringBuilder();
     bundler.appendRuntimeTo(sb);
     bundler.appendTo(
@@ -166,61 +163,10 @@ public void testEs6Module() throws IOException {
                 + "    return module$nested$path$other.x;\n"
                 + "  }}});\n"
                 + "  var module$nested$path$other = $$require(\"nested/path/other.js\");\n"
-                + "  let local;\n"
+                + "  var local;\n"
                 + "  function foo() {\n"
                 + "    return local;\n"
                 + "  }\n"
                 + "}, \"nested/path/foo.js\", [\"nested/path/other.js\"]);\n");
   }
-
-  public void testPassThroughIfNoTranspilationNeeded() throws Exception {
-    String input = "/** Hello Comments! */ const s = 0;\n  let intended;";
-    ClosureBundler bundler = new ClosureBundler(BaseTranspiler.LATEST_TRANSPILER);
-    StringBuilder sb = new StringBuilder();
-    bundler.appendTo(
-        sb,
-        SimpleDependencyInfo.builder("", "").build(),
-        input);
-    assertThat(sb.toString()).isEqualTo(input);
-  }
-
-  public void testCommentsAndFormattingRemovedWithTranspilation() throws Exception {
-    String input = "/** Hello Comments! */ const s = 0;\n  let intended;";
-    ClosureBundler bundler = new ClosureBundler(BaseTranspiler.ES5_TRANSPILER);
-    StringBuilder sb = new StringBuilder();
-    bundler.appendTo(
-        sb,
-        SimpleDependencyInfo.builder("", "").build(),
-        input);
-    assertThat(sb.toString()).isEqualTo("var s = 0;\nvar intended;\n");
-  }
-
-  public void testFullWidthLowLineWithDefaultTranspilerIsOkay() throws Exception {
-    // The last character is something the compiler doesn't handle correctly
-    String input = "var ａｅｓｔｈｅｔｉｃ＿";
-    ClosureBundler bundler = new ClosureBundler();
-    StringBuilder sb = new StringBuilder();
-    bundler.appendTo(
-        sb,
-        SimpleDependencyInfo.builder("", "").build(),
-        input);
-    assertThat(sb.toString()).isEqualTo(input);
-  }
-
-  // TODO(johnplaisted): If / when the compiler can parse full width low line in identifiers
-  // this should be okay to be transpiled.
-  public void testFullWidthLowLineInTranspiledCodeIsError() throws Exception {
-    // The last character is something the compiler doesn't handle correctly
-    String input = "let ａｅｓｔｈｅｔｉｃ＿";
-    ClosureBundler bundler = new ClosureBundler(BaseTranspiler.ES5_TRANSPILER);
-    StringBuilder sb = new StringBuilder();
-    try {
-      bundler.appendTo(sb, SimpleDependencyInfo.builder("", "").build(), input);
-      fail("Expected an exception");
-    } catch (TranspilationException e) {
-      assertThat(e)
-          .hasMessageThat()
-          .contains("Parse error. Character '＿' (U+FF3F) is not a valid identifier start char");
-    }
-  }
 }