Improve parse error handling in compile-testing

RELNOTES=Improve parse error handling PiperOrigin-RevId: 592691296
google · Dec 21, 2023 · 044a5a5 · 044a5a5
1 parent e794c75
commit 044a5a5
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 98 deletions.
diff --git a/pom.xml b/pom.xml
@@ -251,6 +251,9 @@
           --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
           --add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED
           --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+          --add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
+          --add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED
+          --add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
         </test.jvm.flags>
       </properties>
     </profile>

diff --git a/src/main/java/com/google/testing/compile/Parser.java b/src/main/java/com/google/testing/compile/Parser.java
@@ -15,37 +15,33 @@
  */
 package com.google.testing.compile;
 
-import static com.google.common.base.MoreObjects.firstNonNull;
-import static java.lang.Boolean.TRUE;
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.function.Predicate.isEqual;
 import static javax.tools.Diagnostic.Kind.ERROR;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimaps;
 import com.sun.source.tree.CompilationUnitTree;
-import com.sun.source.tree.ErroneousTree;
-import com.sun.source.tree.Tree;
 import com.sun.source.util.JavacTask;
-import com.sun.source.util.TreeScanner;
 import com.sun.source.util.Trees;
-import com.sun.tools.javac.api.JavacTool;
+import com.sun.tools.javac.api.JavacTrees;
+import com.sun.tools.javac.file.JavacFileManager;
+import com.sun.tools.javac.parser.JavacParser;
+import com.sun.tools.javac.parser.ParserFactory;
+import com.sun.tools.javac.tree.JCTree.JCCompilationUnit;
 import com.sun.tools.javac.util.Context;
+import com.sun.tools.javac.util.Log;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
-import java.util.Locale;
 import javax.tools.Diagnostic;
 import javax.tools.DiagnosticCollector;
-import javax.tools.JavaCompiler;
+import javax.tools.DiagnosticListener;
 import javax.tools.JavaFileObject;
-import javax.tools.ToolProvider;
 
 /** Methods to parse Java source files. */
-public final class Parser {
+final class Parser {
 
   /**
    * Parses {@code sources} into {@linkplain CompilationUnitTree compilation units}. This method
@@ -55,89 +51,42 @@ public final class Parser {
    * @throws IllegalStateException if any parsing errors occur.
    */
   static ParseResult parse(Iterable<? extends JavaFileObject> sources, String sourcesDescription) {
-    JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
     DiagnosticCollector<JavaFileObject> diagnosticCollector = new DiagnosticCollector<>();
-    InMemoryJavaFileManager fileManager =
-        new InMemoryJavaFileManager(
-            compiler.getStandardFileManager(diagnosticCollector, Locale.getDefault(), UTF_8));
     Context context = new Context();
-    JavacTask task =
-        ((JavacTool) compiler)
-            .getTask(
-                null, // explicitly use the default because old javac logs some output on stderr
-                fileManager,
-                diagnosticCollector,
-                ImmutableSet.<String>of(),
-                ImmutableSet.<String>of(),
-                sources,
-                context);
+    context.put(DiagnosticListener.class, diagnosticCollector);
+    Log log = Log.instance(context);
+    // The constructor registers the instance in the Context
+    JavacFileManager unused = new JavacFileManager(context, true, UTF_8);
+    ParserFactory parserFactory = ParserFactory.instance(context);
     try {
-      Iterable<? extends CompilationUnitTree> parsedCompilationUnits = task.parse();
+      List<CompilationUnitTree> parsedCompilationUnits = new ArrayList<>();
+      for (JavaFileObject source : sources) {
+        log.useSource(source);
+        JavacParser parser =
+            parserFactory.newParser(
+                source.getCharContent(false),
+                /* keepDocComments= */ true,
+                /* keepEndPos= */ true,
+                /* keepLineMap= */ true);
+        JCCompilationUnit unit = parser.parseCompilationUnit();
+        unit.sourcefile = source;
+        parsedCompilationUnits.add(unit);
+      }
       List<Diagnostic<? extends JavaFileObject>> diagnostics = diagnosticCollector.getDiagnostics();
-      if (foundParseErrors(parsedCompilationUnits, diagnostics)) {
+      if (foundParseErrors(diagnostics)) {
         String msgPrefix = String.format("Error while parsing %s:\n", sourcesDescription);
         throw new IllegalStateException(msgPrefix + Joiner.on('\n').join(diagnostics));
       }
       return new ParseResult(
-          sortDiagnosticsByKind(diagnostics), parsedCompilationUnits, Trees.instance(task));
+          sortDiagnosticsByKind(diagnostics), parsedCompilationUnits, JavacTrees.instance(context));
     } catch (IOException e) {
       throw new RuntimeException(e);
-    } finally {
-      DummyJavaCompilerSubclass.closeCompiler(context);
     }
   }
 
-  /**
-   * Returns {@code true} if errors were found while parsing source files.
-   *
-   * <p>Normally, the parser reports error diagnostics, but in some cases there are no diagnostics;
-   * instead the parse tree contains {@linkplain ErroneousTree "erroneous"} nodes.
-   */
-  private static boolean foundParseErrors(
-      Iterable<? extends CompilationUnitTree> parsedCompilationUnits,
-      List<Diagnostic<? extends JavaFileObject>> diagnostics) {
-    return diagnostics.stream().map(Diagnostic::getKind).anyMatch(isEqual(ERROR))
-        || Iterables.any(parsedCompilationUnits, Parser::hasErrorNode);
-  }
-
-  /**
-   * Returns {@code true} if the tree contains at least one {@linkplain ErroneousTree "erroneous"}
-   * node.
-   */
-  private static boolean hasErrorNode(Tree tree) {
-    return isTrue(HAS_ERRONEOUS_NODE.scan(tree, false));
-  }
-
-  private static final TreeScanner<Boolean, Boolean> HAS_ERRONEOUS_NODE =
-      new TreeScanner<Boolean, Boolean>() {
-        @Override
-        public Boolean visitErroneous(ErroneousTree node, Boolean p) {
-          return true;
-        }
-
-        @Override
-        public Boolean scan(Iterable<? extends Tree> nodes, Boolean p) {
-          for (Tree node : firstNonNull(nodes, ImmutableList.<Tree>of())) {
-            if (isTrue(scan(node, p))) {
-              return true;
-            }
-          }
-          return p;
-        }
-
-        @Override
-        public Boolean scan(Tree tree, Boolean p) {
-          return isTrue(p) ? p : super.scan(tree, p);
-        }
-
-        @Override
-        public Boolean reduce(Boolean r1, Boolean r2) {
-          return isTrue(r1) || isTrue(r2);
-        }
-      };
-
-  private static boolean isTrue(Boolean p) {
-    return TRUE.equals(p);
+  /** Returns {@code true} if errors were found while parsing source files. */
+  private static boolean foundParseErrors(List<Diagnostic<? extends JavaFileObject>> diagnostics) {
+    return diagnostics.stream().anyMatch(d -> d.getKind().equals(ERROR));
   }
 
   private static ImmutableListMultimap<Diagnostic.Kind, Diagnostic<? extends JavaFileObject>>
@@ -184,20 +133,5 @@ Trees trees() {
     }
   }
 
-  // JavaCompiler.compilerKey has protected access until Java 9, so this is a workaround.
-  private static final class DummyJavaCompilerSubclass extends com.sun.tools.javac.main.JavaCompiler {
-    private static void closeCompiler(Context context) {
-      com.sun.tools.javac.main.JavaCompiler compiler = context.get(compilerKey);
-      if (compiler != null) {
-        compiler.close();
-      }
-    }
-
-    private DummyJavaCompilerSubclass() {
-      // not instantiable
-      super(null);
-    }
-  }
-
   private Parser() {}
 }
diff --git a/src/test/java/com/google/testing/compile/ParserTest.java b/src/test/java/com/google/testing/compile/ParserTest.java
@@ -0,0 +1,27 @@
+package com.google.testing.compile;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import com.google.common.collect.ImmutableList;
+import javax.tools.JavaFileObject;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class ParserTest {
+
+  private static final JavaFileObject HELLO_WORLD_BROKEN =
+      JavaFileObjects.forSourceLines(
+          "test.HelloWorld", "package test;", "", "public class HelloWorld {", "}}");
+
+  @Test
+  public void failsToParse() {
+    IllegalStateException expected =
+        assertThrows(
+            IllegalStateException.class,
+            () -> Parser.parse(ImmutableList.of(HELLO_WORLD_BROKEN), "hello world"));
+    assertThat(expected).hasMessageThat().contains("HelloWorld.java:4: error");
+  }
+}