Use AST-based parser for dependency info in JSCompiler unit tests

This exposes a few small behavioral differences, like module load errors being reported in more tests.

This also fixes some crashes in some parsing/deps finding code on malformed goog.modules, that were previously covered up by the regex parser throwing an error before we got to AST-based parsing.

A followup change will use the AST-based parser in even more places.

PiperOrigin-RevId: 506049938

Author: lauraharker

src/com/google/javascript/jscomp/AbstractCompiler.java

@@ -260,6 +260,15 @@ void afterPass(String passName) {}
   /** Prints a node to source code. */
   public abstract String toSource(Node root);
 
+  /**
+   * Whether to prefer using {@link JSFileRegexParser} when calculating {@link DependencyInfo} as
+   * opposed to triggering an AST-based parse.
+   *
+   * <p>This is primarily for performance reasons. The two should produce the same results except in
+   * some edge cases.
+   */
+  abstract boolean preferRegexParser();
+
   /** Gets a default error reporter for injecting into Rhino. */
   abstract ErrorReporter getDefaultErrorReporter();
 

src/com/google/javascript/jscomp/Compiler.java

@@ -1979,6 +1979,31 @@ Node parseInputs() {
     }
   }
 
+  /**
+   * This hints to other parts of the compiler as to whether every input will eventually require a
+   * full AST parse.
+   *
+   * <p>NOTE: this is just intended as a hint towards improving performance. Returning true or false
+   * shouldn't affect any non-performance behavior.
+   */
+  @Override
+  boolean preferRegexParser() {
+    return preferRegexParser;
+  }
+
+  /**
+   * Artificially sets the return value of `preferRegexParser`
+   *
+   * <p>Non-public and not exposed on AbstractCompiler, as for now we want to maintain control of
+   * when the regex vs. AST parser is used.
+   */
+  void setPreferRegexParser(boolean preferRegexParser) {
+    this.preferRegexParser = preferRegexParser;
+  }
+
+  // TODO(b/264916633): make this default to false.
+  private boolean preferRegexParser = true;
+
   void orderInputsWithLargeStack() {
     runInCompilerThread(
         () -> {
@@ -2035,8 +2060,8 @@ void orderInputs() {
   /**
    * Find modules by recursively traversing dependencies starting with the entry points.
    *
-   * <p>Causes a regex parse of every file, and a full parse of every file reachable from the entry
-   * points (which would be required by later compilation passes regardless).
+   * <p>Triggers either a regex parse or full AST-based parse of each file in order to gather
+   * dependency information.
    *
    * <p>If the dependency mode is set to PRUNE_LEGACY, inputs which the regex parse does not
    * identify as ES modules and which do not contain any provide statements are considered to be

src/com/google/javascript/jscomp/CompilerInput.java

@@ -341,7 +341,7 @@ private DependencyInfo generateDependencyInfo() {
 
     // If the code is a JsAst, then it was originally JS code, and is compatible with the
     // regex-based parsing of JsFileRegexParser.
-    if (ast instanceof JsAst && JsFileRegexParser.isSupported()) {
+    if (ast instanceof JsAst && JsFileRegexParser.isSupported() && compiler.preferRegexParser()) {
       // Look at the source code.
       // Note: it's OK to use getName() instead of
       // getPathRelativeToClosureBase() here because we're not using

src/com/google/javascript/jscomp/GatherModuleMetadata.java

@@ -387,6 +387,7 @@ private void visitGoogCall(NodeTraversal t, Node n) {
           addNamespace(currentModule, ModuleType.GOOG_PROVIDE, namespace, t, n);
         } else {
           t.report(n, ClosureRewriteModule.INVALID_PROVIDE_NAMESPACE);
+          currentModule.metadataBuilder.usesClosure(false);
         }
       } else if (getprop.matchesQualifiedName(GOOG_MODULE)) {
         currentModule.moduleType(ModuleType.GOOG_MODULE, t, n);
@@ -395,6 +396,7 @@ private void visitGoogCall(NodeTraversal t, Node n) {
           addNamespace(currentModule, ModuleType.GOOG_MODULE, namespace, t, n);
         } else {
           t.report(n, ClosureRewriteModule.INVALID_MODULE_ID_ARG);
+          currentModule.metadataBuilder.usesClosure(false);
         }
       } else if (getprop.matchesQualifiedName(GOOG_MODULE_DECLARELEGACYNAMESPACE)) {
         currentModule.recordDeclareLegacyNamespace(n);

src/com/google/javascript/jscomp/modules/ModuleMapCreator.java

@@ -255,7 +255,13 @@ private void process(ModuleMetadata moduleMetadata) {
         break;
       case GOOG_MODULE:
       case LEGACY_GOOG_MODULE:
-        processor = closureModuleProcessor;
+        if (moduleMetadata.googNamespaces().size() == 1) {
+          processor = closureModuleProcessor;
+        } else {
+          // this indicates some malformed Closure module. We should already have reported an
+          // error.
+          processor = nonEsModuleProcessor;
+        }
         break;
       default:
         processor = nonEsModuleProcessor;

src/com/google/javascript/jscomp/parsing/IRFactory.java

@@ -1309,7 +1309,9 @@ private boolean isGoogModuleFile(Node scriptNode) {
       if (!call.isCall()) {
         return false;
       }
-      return GOOG_MODULE.matches(call.getFirstChild());
+      return GOOG_MODULE.matches(call.getFirstChild())
+          && call.hasTwoChildren()
+          && call.getSecondChild().isStringLit();
     }
 
     /**

test/com/google/javascript/jscomp/ClosureRewriteModuleTest.java

@@ -26,6 +26,7 @@
 import static com.google.javascript.jscomp.ClosureRewriteModule.LOAD_MODULE_FN_MISSING_RETURN;
 import static com.google.javascript.jscomp.modules.ModuleMapCreator.DOES_NOT_HAVE_EXPORT_WITH_DETAILS;
 import static com.google.javascript.rhino.testing.NodeSubject.assertNode;
+import static org.junit.Assert.assertThrows;
 
 import com.google.common.base.Predicates;
 import com.google.javascript.jscomp.testing.TestExternsBuilder;
@@ -1092,7 +1093,9 @@ public void testGoogLoadModule_missingReturn() {
 
   @Test
   public void testGoogLoadModuleString() {
-    testSame("goog.loadModule(\"goog.module('a.b.c'); exports = class {};\");");
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> testSame("goog.loadModule(\"goog.module('a.b.c'); exports = class {};\");"));
   }
 
   @Test

test/com/google/javascript/jscomp/CodePrinterTest.java

@@ -17,6 +17,7 @@
 package com.google.javascript.jscomp;
 
 import static com.google.common.truth.Truth.assertThat;
+import static com.google.javascript.jscomp.DiagnosticGroups.MODULE_LOAD;
 import static com.google.javascript.rhino.testing.NodeSubject.assertNode;
 import static java.nio.charset.StandardCharsets.UTF_8;
 
@@ -3568,19 +3569,21 @@ public void testDeclarations() {
 
   @Test
   public void testImports() {
-    assertPrintSame("import x from\"foo\"");
-    assertPrintSame("import\"foo\"");
-    assertPrintSame("import x,{a as b}from\"foo\"");
-    assertPrintSame("import{a as b,c as d}from\"foo\"");
-    assertPrintSame("import x,{a}from\"foo\"");
-    assertPrintSame("import{a,c}from\"foo\"");
-    assertPrintSame("import x,*as f from\"foo\"");
-    assertPrintSame("import*as f from\"foo\"");
+    diagnosticsToIgnore = ImmutableList.of(MODULE_LOAD); // allow importing nonexistent modules
+    assertPrintSame("import x from\"./foo\"");
+    assertPrintSame("import\"./foo\"");
+    assertPrintSame("import x,{a as b}from\"./foo\"");
+    assertPrintSame("import{a as b,c as d}from\"./foo\"");
+    assertPrintSame("import x,{a}from\"./foo\"");
+    assertPrintSame("import{a,c}from\"./foo\"");
+    assertPrintSame("import x,*as f from\"./foo\"");
+    assertPrintSame("import*as f from\"./foo\"");
   }
 
   @Test
   public void testExports() {
     // export declarations
+    diagnosticsToIgnore = ImmutableList.of(MODULE_LOAD); // allow importing nonexistent modules
     assertPrintSame("export var x=1");
     assertPrintSame("export var x;export var y");
     assertPrintSame("export let x=1");
@@ -3590,13 +3593,13 @@ public void testExports() {
     assertPrintSame("export class f{}export class b{}");
 
     // export all from
-    assertPrint("export * from 'a.b.c'", "export*from\"a.b.c\"");
+    assertPrint("export * from './a.b.c'", "export*from\"./a.b.c\"");
 
     // from
-    assertPrintSame("export{a}from\"a.b.c\"");
-    assertPrintSame("export{a as x}from\"a.b.c\"");
-    assertPrintSame("export{a,b}from\"a.b.c\"");
-    assertPrintSame("export{a as x,b as y}from\"a.b.c\"");
+    assertPrintSame("export{a}from\"./a.b.c\"");
+    assertPrintSame("export{a as x}from\"./a.b.c\"");
+    assertPrintSame("export{a,b}from\"./a.b.c\"");
+    assertPrintSame("export{a as x,b as y}from\"./a.b.c\"");
     assertPrintSame("export{a}");
     assertPrintSame("export{a as x}");
 

test/com/google/javascript/jscomp/CodePrinterTestBase.java

@@ -38,6 +38,7 @@ public abstract class CodePrinterTestBase {
   protected LanguageMode languageMode = LanguageMode.ECMASCRIPT5;
   protected @Nullable Compiler lastCompiler = null;
   protected @Nullable Charset outputCharset = null;
+  protected ImmutableList<DiagnosticGroup> diagnosticsToIgnore = ImmutableList.of();
 
   @Before
   public void setUp() throws Exception {
@@ -48,6 +49,7 @@ public void setUp() throws Exception {
     lastCompiler = null;
     languageMode = LanguageMode.UNSUPPORTED;
     outputCharset = null;
+    diagnosticsToIgnore = ImmutableList.of();
   }
 
   Node parse(String js) {
@@ -66,6 +68,8 @@ Node parse(String js, boolean typeChecked) {
       options.setParseJsDocDocumentation(Config.JsDocParsing.INCLUDE_ALL_COMMENTS);
       options.setPreserveNonJSDocComments(true);
     }
+    options.setContinueAfterErrors(true);
+    compiler.setPreferRegexParser(false);
     compiler.init(
         ImmutableList.of(SourceFile.fromCode("externs", CompilerTestCase.MINIMAL_EXTERNS)),
         ImmutableList.of(SourceFile.fromCode("testcode", js)),
@@ -87,21 +91,25 @@ Node parse(String js, boolean typeChecked) {
 
   private void checkUnexpectedErrorsOrWarnings(
       Compiler compiler, int expected) {
-    int actual = compiler.getErrors().size();
-    if (!allowWarnings) {
-      actual += compiler.getWarnings().size();
-    }
-
-    if (actual != expected) {
+    int actual = 0;
       String msg = "";
       for (JSError err : compiler.getErrors()) {
+      if (shouldIgnore(err)) {
+        continue;
+      }
+      actual++;
         msg += "Error:" + err + "\n";
       }
       if (!allowWarnings) {
         for (JSError err : compiler.getWarnings()) {
+        if (shouldIgnore(err)) {
+          continue;
+        }
+        actual++;
           msg += "Warning:" + err + "\n";
         }
       }
+    if (actual != expected) {
       assertWithMessage("Unexpected warnings or errors.\n " + msg).that(actual).isEqualTo(expected);
     }
   }
@@ -110,6 +118,15 @@ String parsePrint(String js, CompilerOptions options) {
     return new CodePrinter.Builder(parse(js)).setCompilerOptions(options).build();
   }
 
+  private boolean shouldIgnore(JSError error) {
+    for (DiagnosticGroup diagnosticGroup : diagnosticsToIgnore) {
+      if (diagnosticGroup.matches(error)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   abstract static class CompilerOptionBuilder {
     abstract void setOptions(CompilerOptions options);
   }

test/com/google/javascript/jscomp/CompilerTest.java

@@ -2346,7 +2346,7 @@ public void testProperGoogBaseOrdering() throws Exception {
         SourceFile.fromCode(
             "base.js",
             lines(
-                "/** @provideGoog */",
+                "/** @fileoverview @provideGoog */",
                 "/** @const */ var goog = goog || {};",
                 "var COMPILED = false;")));
     sources.add(