RFR: 8332497: javac prints an AssertionError when annotation processing runs on program with module imports [v17]

[Vicente Romero]

On Fri, 24 May 2024 18:25:32 GMT, Evemose <duke at openjdk.org> wrote:

>> Fix is pretty simple: visitModuleImport in com.sun.tools.javac.tree.TreeScanner has notbeen overriden, so defaulted to Visitor::visitModuleImport, which forwards to Visitor::visitTree, which is also not overriden, and, therefore, threw AssertionError. 
>> 
>> PS: Im not even sure how it worked before without crashing, seems like there is some intermidiate implementation between this TreeScanner and actual scanners because otherwise it should have resultedin compile error the moment it encounter module importin any visitor
>
> Evemose has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Addressed review comments

ok this is what I would do, please feel free to use it, it is applicable on top of your current code. As you can see I have reformatted the test to align it better with the rest of similar tests in our code base. Also as Jan suggested I have added an equivalent method to TreeTranslator for completeness.


diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeTranslator.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeTranslator.java
index 59a7457e6d0..63778fb42ff 100644
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeTranslator.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeTranslator.java
@@ -131,6 +131,11 @@ public void visitImport(JCImport tree) {
         result = tree;
     }
 
+    public void visitModuleImport(JCModuleImport tree) {
+        tree.module = translate(tree.module);
+        result = tree;
+    }
+
     public void visitClassDef(JCClassDecl tree) {
         tree.mods = translate(tree.mods);
         tree.typarams = translateTypeParams(tree.typarams);
diff --git a/test/langtools/tools/javac/processing/ModuleImportProcessingTest.java b/test/langtools/tools/javac/processing/ModuleImportProcessingTest.java
index 88c7974a26e..760ec30820e 100644
--- a/test/langtools/tools/javac/processing/ModuleImportProcessingTest.java
+++ b/test/langtools/tools/javac/processing/ModuleImportProcessingTest.java
@@ -21,7 +21,16 @@
  * questions.
  */
 
-import toolbox.*;
+/**
+ * @test
+ * @bug 8332497
+ * @summary javac prints an AssertionError when annotation processing runs on program with module imports
+ * @library /tools/lib
+ * @modules jdk.compiler/com.sun.tools.javac.api
+ *          jdk.compiler/com.sun.tools.javac.main
+ * @build toolbox.JavacTask toolbox.ToolBox toolbox.Task
+ * @run main ModuleImportProcessingTest
+ */
 
 import javax.annotation.processing.AbstractProcessor;
 import javax.annotation.processing.RoundEnvironment;
@@ -32,40 +41,49 @@
 import java.nio.file.Paths;
 import java.util.Set;
 
-/**
- * @test
- * @bug 8332497
- * @summary error: javac prints an AssertionError when annotation processing runs on program with module imports
- * @library /tools/lib
- * @modules jdk.compiler/com.sun.tools.javac.api
- *          jdk.compiler/com.sun.tools.javac.main
- * @build toolbox.JavacTask toolbox.ToolBox toolbox.Task
- * @run main ModuleImportProcessingTest
- */
-public class ModuleImportProcessingTest {
-    final toolbox.ToolBox tb = new ToolBox();
-    final Path base = Paths.get(".");
-    final String processedSource = """
-        import module java.base;
-        import java.lang.annotation.*;
-        public class Main {
-          public static void main(String[] args) {
-            List.of();
-          }
-          @Ann
-          private void test() {}
-          @Retention(RetentionPolicy.RUNTIME)
-          @Target(ElementType.METHOD)
-          public @interface Ann {}
-        }
-        """;
+import toolbox.TestRunner;
+import toolbox.ToolBox;
+import toolbox.JavacTask;
+import toolbox.Task;
+
+public class ModuleImportProcessingTest extends TestRunner {
+    ToolBox tb = new ToolBox();
+
+    public ModuleImportProcessingTest() {
+        super(System.err);
+    }
+
+    protected void runTests() throws Exception {
+        runTests(m -> new Object[] { Paths.get(m.getName()) });
+    }
 
-    public static void main(String[] args) throws Exception {
-        new ModuleImportProcessingTest().test();
+    Path[] findJavaFiles(Path... paths) throws Exception {
+        return tb.findJavaFiles(paths);
     }
 
-    public void test() throws Exception {
-        tb.writeJavaFiles(base, processedSource);
+    public static void main(String... args) throws Exception {
+        new ModuleImportProcessingTest().runTests();
+    }
+
+    @Test
+    public void test(Path base) throws Exception {
+        Path src = base.resolve("src");
+        tb.writeJavaFiles(src,
+                """
+                import module java.base;
+                import java.lang.annotation.*;
+                public class Main {
+                    public static void main(String[] args) {
+                        List.of();
+                    }
+                    @Ann
+                    private void test() {}
+
+                    @Retention(RetentionPolicy.RUNTIME)
+                    @Target(ElementType.METHOD)
+                    public @interface Ann {}
+                }
+                """);
         new toolbox.JavacTask(tb)
                 .options(
                         "-processor", AP.class.getName(),
@@ -73,24 +91,15 @@ public void test() throws Exception {
                         "-source", Integer.toString(Runtime.version().feature()),
                         "-proc:only"
                 )
-                .outdir(base.toString())
-                .files(base.resolve("Main.java"))
-                .run(Task.Expect.SUCCESS)
-                .writeAll();
+                .files(findJavaFiles(src))
+                .run();
     }
 
     @SupportedAnnotationTypes("*")
     public static final class AP extends AbstractProcessor {
-
         @Override
         public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
             return false;
         }
-
-        @Override
-        public SourceVersion getSupportedSourceVersion() {
-            return SourceVersion.latest();
-        }
-
     }
-}
\ No newline at end of file
+}

-------------

PR Comment: https://git.openjdk.org/jdk/pull/19292#issuecomment-2130390062