Fix constant analysis for package-info and module-info

subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/listeners/constants/ConstantsTreeVisitor.java

@@ -16,12 +16,11 @@
 
 package org.gradle.internal.compiler.java.listeners.constants;
 
-import com.sun.source.tree.AssignmentTree;
 import com.sun.source.tree.ClassTree;
 import com.sun.source.tree.CompilationUnitTree;
 import com.sun.source.tree.IdentifierTree;
 import com.sun.source.tree.MemberSelectTree;
-import com.sun.source.tree.PackageTree;
+import com.sun.source.tree.ModuleTree;
 import com.sun.source.tree.VariableTree;
 import com.sun.source.util.TreePathScanner;
 import com.sun.source.util.Trees;
@@ -38,6 +37,10 @@
 
 public class ConstantsTreeVisitor extends TreePathScanner<ConstantsVisitorContext, ConstantsVisitorContext> {
 
+    private static final String PACKAGE_INFO_JAVA = "package-info.java";
+    private static final String PACKAGE_INFO = "package-info";
+    private static final String MODULE_INFO = "module-info";
+
     private final Elements elements;
     private final Trees trees;
     private final ConstantDependentsConsumer consumer;
@@ -49,52 +52,44 @@ public ConstantsTreeVisitor(Elements elements, Trees trees, ConstantDependentsCo
     }
 
     @Override
-    public ConstantsVisitorContext visitCompilationUnit(CompilationUnitTree node, ConstantsVisitorContext constantConsumer) {
-        return super.visitCompilationUnit(node, constantConsumer);
-    }
-
-    @Override
-    public ConstantsVisitorContext visitAssignment(AssignmentTree node, ConstantsVisitorContext constantConsumer) {
-        return super.visitAssignment(node, constantConsumer);
+    public ConstantsVisitorContext visitCompilationUnit(CompilationUnitTree node, ConstantsVisitorContext context) {
+        // For JDK8 visitPackage is not called for package-info.java, so we have to resolve package-info from compilation unit
+        String sourceName = node.getSourceFile().getName();
+        if (sourceName.endsWith(PACKAGE_INFO_JAVA)) {
+            PackageElement packageElement = elements.getPackageOf(trees.getElement(getCurrentPath()));
+            String visitedPackageInfo = packageElement == null || packageElement.getQualifiedName().toString().isEmpty()
+                ? PACKAGE_INFO
+                : packageElement.getQualifiedName().toString() + "." + PACKAGE_INFO;
+            return super.visitCompilationUnit(node, new ConstantsVisitorContext(visitedPackageInfo, consumer::consumeAccessibleDependent));
+        }
+        return super.visitCompilationUnit(node, context);
     }
 
     @Override
     @SuppressWarnings("Since15")
-    public ConstantsVisitorContext visitPackage(PackageTree node, ConstantsVisitorContext constantConsumer) {
-        Element element = trees.getElement(getCurrentPath());
-
-        // Collect classes for visited class
-        String visitedClass = ((PackageElement) element).getQualifiedName().toString();
-        // Always add self, so we know this class was visited
-        consumer.consumePrivateDependent(visitedClass, visitedClass);
-        super.visitPackage(node, new ConstantsVisitorContext(visitedClass, consumer::consumePrivateDependent));
-
-        // Return back previous collected classes
-        return constantConsumer;
+    public ConstantsVisitorContext visitModule(ModuleTree node, ConstantsVisitorContext context) {
+        super.visitModule(node, new ConstantsVisitorContext(MODULE_INFO, consumer::consumeAccessibleDependent));
+        return context;
     }
 
     @Override
-    public ConstantsVisitorContext visitClass(ClassTree node, ConstantsVisitorContext constantConsumer) {
+    public ConstantsVisitorContext visitClass(ClassTree node, ConstantsVisitorContext context) {
         Element element = trees.getElement(getCurrentPath());
 
-        // Collect classes for visited class
         String visitedClass = getBinaryClassName((TypeElement) element);
-        // Always add self, so we know this class was visited
-        consumer.consumePrivateDependent(visitedClass, visitedClass);
         super.visitClass(node, new ConstantsVisitorContext(visitedClass, consumer::consumePrivateDependent));
 
-        // Return back previous collected classes
-        return constantConsumer;
+        return context;
     }
 
     @Override
-    public ConstantsVisitorContext visitVariable(VariableTree node, ConstantsVisitorContext constantConsumer) {
+    public ConstantsVisitorContext visitVariable(VariableTree node, ConstantsVisitorContext context) {
         if (isAccessibleConstantVariableDeclaration(node) && node.getInitializer() != null && node.getInitializer().getKind() != METHOD_INVOCATION) {
             // We now just check, that constant declaration is not `static {}` or `CONSTANT = methodInvocation()`,
             // but it could be further optimized to check if expression is one that can be inlined or not.
-            return super.visitVariable(node, new ConstantsVisitorContext(constantConsumer.getVisitedClass(), consumer::consumeAccessibleDependent));
+            return super.visitVariable(node, new ConstantsVisitorContext(context.getVisitedClass(), consumer::consumeAccessibleDependent));
         } else {
-            return super.visitVariable(node, constantConsumer);
+            return super.visitVariable(node, context);
         }
     }
 

subprojects/java-compiler-plugin/src/test/groovy/com/gradle/internal/compiler/java/AbstractCompilerPluginTest.groovy

@@ -62,4 +62,13 @@ class AbstractCompilerPluginTest extends Specification {
         return [f]
     }
 
+    List<File> toModuleSourceFile(String body) {
+        def className = "module-info"
+        File parent = Paths.get(sourceFolder.absolutePath, "src", "main", "java").toFile()
+        File f = Paths.get(parent.absolutePath, "${className}.java").toFile()
+        parent.mkdirs()
+        f.text = body
+        return [f]
+    }
+
 }

subprojects/java-compiler-plugin/src/test/groovy/com/gradle/internal/compiler/java/listeners/ConstantsCollectorTest.groovy

@@ -42,11 +42,12 @@ class ConstantsCollectorTest extends AbstractCompilerPluginTest {
         )
     }
 
-    def "should always return self as a private constant origin class"() {
+    def "should not return self as a constant origin class"() {
         given:
         String clazz = """
 class A {
     public static final int CONSTANT = 1;
+    public static final int CONSTANT2 = A.CONSTANT;
 
     @Annotation(A.CONSTANT)
     public final int field = A.CONSTANT;
@@ -57,7 +58,7 @@ class A {
         compiler.compile(toSourceFile(clazz) + getAnnotation("int"))
 
         then:
-        privateDependentToConstants["A"] == ["A"] as Set
+        privateDependentToConstants["A"] == null
         accessibleDependentToConstants["A"] == null
     }
 
@@ -79,7 +80,7 @@ class A<@Annotation(Constant2.CONSTANT2 $addition) T> {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1", "Constant2", "Constant3", "Constant4", "Constant5", "Constant6"] as Set
+        privateDependentToConstants["A"] == ["Constant1", "Constant2", "Constant3", "Constant4", "Constant5", "Constant6"] as Set
         accessibleDependentToConstants.isEmpty()
 
         where:
@@ -115,7 +116,7 @@ class A {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1", "Constant2", "Constant3", "Constant5", "Constant6", "Constant7"] as Set
+        privateDependentToConstants["A"] == ["Constant1", "Constant2", "Constant3", "Constant5", "Constant6", "Constant7"] as Set
         accessibleDependentToConstants["A"] == ["Constant4"] as Set
 
         where:
@@ -148,7 +149,7 @@ class A {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1", "Constant2", "Constant3", "Constant4", "Constant5", "Constant6", "Constant7"] as Set
+        privateDependentToConstants["A"] == ["Constant1", "Constant2", "Constant3", "Constant4", "Constant5", "Constant6", "Constant7"] as Set
         accessibleDependentToConstants.isEmpty()
 
         where:
@@ -181,7 +182,7 @@ class A {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1", "Constant2", "Constant3", "Constant4"] as Set
+        privateDependentToConstants["A"] == ["Constant1", "Constant2", "Constant3", "Constant4"] as Set
 
         where:
         constantType | constantValue | addition
@@ -223,7 +224,7 @@ class A {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1", "Constant2", "Constant3", "Constant4", "Constant5",
+        privateDependentToConstants["A"] == ["Constant1", "Constant2", "Constant3", "Constant4", "Constant5",
                                              "Constant6", "Constant7", "Constant8", "Constant9", "Constant10"] as Set
         accessibleDependentToConstants.isEmpty()
 
@@ -279,7 +280,7 @@ class A {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "gradle.unit.test.Constant1", "gradle.unit.test.Constant2",
+        privateDependentToConstants["A"] == ["gradle.unit.test.Constant1", "gradle.unit.test.Constant2",
                                              "gradle.unit.test.Constant3", "gradle.unit.test.Constant4", "gradle.unit.test.Constant5",
                                              "gradle.unit.test.Constant6", "gradle.unit.test.Constant7", "gradle.unit.test.Constant8",
                                              "gradle.unit.test.Constant9", "gradle.unit.test.Constant10"] as Set
@@ -313,7 +314,7 @@ public @interface A {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1", "Constant2", "Constant3", "Constant4"] as Set
+        privateDependentToConstants["A"] == ["Constant1", "Constant2", "Constant3", "Constant4"] as Set
         accessibleDependentToConstants.isEmpty()
 
         where:
@@ -341,7 +342,7 @@ class A {
 
         then:
         accessibleDependentToConstants["A"] == ["Constant1"] as Set
-        privateDependentToConstants["A"] == ["A"] as Set
+        privateDependentToConstants["A"] == null
     }
 
     def "collects constants from private static fields as private dependents"() {
@@ -357,7 +358,7 @@ class A {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1"] as Set
+        privateDependentToConstants["A"] == ["Constant1"] as Set
         accessibleDependentToConstants.isEmpty()
     }
 
@@ -381,7 +382,7 @@ class A {
         accessibleDependentToConstants["C"] == ["B"] as Set
         accessibleDependentToConstants["D"] == ["C"] as Set
         accessibleDependentToConstants["E"] == null
-        privateDependentToConstants["A"] == ["A"] as Set
+        privateDependentToConstants["A"] == null
     }
 
     def "collect constants from accessible static fields as private dependents when static block is used"() {
@@ -400,7 +401,7 @@ class A {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1"] as Set
+        privateDependentToConstants["A"] == ["Constant1"] as Set
         accessibleDependentToConstants.isEmpty()
     }
 
@@ -420,7 +421,7 @@ class A {
         compiler.compile(classes)
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1"] as Set
+        privateDependentToConstants["A"] == ["Constant1"] as Set
         accessibleDependentToConstants.isEmpty()
     }
 
@@ -461,11 +462,11 @@ class OuterClass {
         compiler.compile(sourceFiles)
 
         then:
-        privateDependentToConstants["OuterClass\$1"] == ["OuterClass\$1", "Constant1", "OuterClass\$C", "OuterClass\$D"] as Set
-        privateDependentToConstants["OuterClass\$A"] == ["OuterClass\$A", "Constant2", "OuterClass\$C", "OuterClass\$D"] as Set
-        privateDependentToConstants["OuterClass\$B"] == ["OuterClass\$B", "Constant3", "OuterClass\$C", "OuterClass\$D"] as Set
-        privateDependentToConstants["OuterClass\$C"] == ["OuterClass\$C"] as Set
-        privateDependentToConstants["OuterClass\$D"] == ["OuterClass\$D"] as Set
+        privateDependentToConstants["OuterClass\$1"] == ["Constant1", "OuterClass\$C", "OuterClass\$D"] as Set
+        privateDependentToConstants["OuterClass\$A"] == ["Constant2", "OuterClass\$C", "OuterClass\$D"] as Set
+        privateDependentToConstants["OuterClass\$B"] == ["Constant3", "OuterClass\$C", "OuterClass\$D"] as Set
+        privateDependentToConstants["OuterClass\$C"] == null
+        privateDependentToConstants["OuterClass\$D"] == null
         accessibleDependentToConstants.isEmpty()
     }
 
@@ -482,7 +483,7 @@ interface A {
         compiler.compile(toSourceFile(clazz) + getAnnotation("int") + getConstants("int", "1"))
 
         then:
-        privateDependentToConstants["A"] == ["A", "Constant1"] as Set
+        privateDependentToConstants["A"] == ["Constant1"] as Set
         accessibleDependentToConstants["A"] == ["Constant2"] as Set
     }
 
@@ -496,8 +497,8 @@ interface A {
         compiler.compile(toSourceFiles(classes))
 
         then:
-        privateDependentToConstants["A"] == ["A"] as Set
-        privateDependentToConstants["B"] == ["B"] as Set
+        privateDependentToConstants["A"] == null
+        privateDependentToConstants["B"] == null
         accessibleDependentToConstants.isEmpty()
     }
 
@@ -520,10 +521,9 @@ class C {
 
         then:
         accessibleDependentToConstants["C"] == ["A"] as Set
-        privateDependentToConstants["C"] == ["C"] as Set
+        privateDependentToConstants["C"] == null
     }
 
-    @Requires({ javaVersion >= 12 })
     def "collect all constants for package-info class"() {
         String packageDefinition = """
 @PackageInfoAnnotation(Constant.CONSTANT + " world")
@@ -557,8 +557,32 @@ public class Constant {
         compiler.compile(classesSourceFiles + packageSourceFile)
 
         then:
-        privateDependentToConstants["gradle.unit.test"] == ["gradle.unit.test", "gradle.unit.test.Constant"] as Set
-        accessibleDependentToConstants.isEmpty()
+        privateDependentToConstants.isEmpty()
+        accessibleDependentToConstants["gradle.unit.test.package-info"] == ["gradle.unit.test.Constant"] as Set
+    }
+
+    @Requires({ javaVersion >= 9 })
+    def "collect all constants for module-info class"() {
+        String moduleDefinition = """
+    import gradle.unit.test.Constant;
+
+    @SuppressWarnings(Constant.CONSTANT)
+    module module {
+    }
+"""
+        List<String> classes = ["""
+package gradle.unit.test;
+public class Constant {
+    public static final String CONSTANT = "unchecked";
+}
+"""]
+
+        when:
+        compiler.compile(toModuleSourceFile(moduleDefinition) + toSourceFiles(classes))
+
+        then:
+        privateDependentToConstants.isEmpty()
+        accessibleDependentToConstants["module-info"] == ["gradle.unit.test.Constant"] as Set
     }
 
     List<File> getAnnotation(String valueType, String packageName = "") {