Incremental compiler hotfixes

subprojects/core/src/main/java/org/gradle/api/internal/tasks/compile/ApiMemberSelector.java

@@ -83,7 +83,7 @@ public void visitEnd() {
         }
         for (FieldMember field : fields) {
             FieldVisitor fieldVisitor = apiMemberAdapter.visitField(
-                field.getAccess(), field.getName(), field.getTypeDesc(), field.getSignature(), null);
+                field.getAccess(), field.getName(), field.getTypeDesc(), field.getSignature(), field.getValue());
             visitAnnotationMembers(fieldVisitor, field.getAnnotations());
             fieldVisitor.visitEnd();
         }
@@ -188,7 +188,8 @@ public AnnotationVisitor visitParameterAnnotation(int parameter, String desc, bo
     @Override
     public FieldVisitor visitField(int access, String name, String desc, String signature, Object value) {
         if (isCandidateApiMember(access, apiIncludesPackagePrivateMembers)) {
-            final FieldMember fieldMember = new FieldMember(access, name, signature, desc);
+            Object keepValue = (access & ACC_STATIC) == ACC_STATIC && ((access & ACC_FINAL) == ACC_FINAL) ? value : null;
+            final FieldMember fieldMember = new FieldMember(access, name, signature, desc, keepValue);
             fields.add(fieldMember);
             return new FieldVisitor(ASM5) {
                 @Override

subprojects/core/src/main/java/org/gradle/api/internal/tasks/compile/FieldMember.java

@@ -16,24 +16,32 @@
 
 package org.gradle.api.internal.tasks.compile;
 
+import com.google.common.collect.Ordering;
 import org.objectweb.asm.Type;
 
 import java.lang.reflect.Modifier;
 
 public class FieldMember extends TypedMember implements Comparable<FieldMember> {
 
-    public FieldMember(int access, String name, String signature, String typeDesc) {
+    private final Object value;
+
+    public FieldMember(int access, String name, String signature, String typeDesc, Object value) {
         super(access, name, signature, typeDesc);
+        this.value = value;
     }
 
     @Override
     public int compareTo(FieldMember o) {
-        return super.compare(o).result();
+        return super.compare(o).compare(value, o.value, Ordering.arbitrary()).result();
     }
 
     @Override
     public String toString() {
         return String.format(
             "%s %s %s", Modifier.toString(getAccess()), Type.getType(getTypeDesc()).getClassName(), getName());
     }
+
+    public Object getValue() {
+        return value;
+    }
 }

subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/AbstractCrossTaskIncrementalJavaCompilationIntegrationTest.groovy

@@ -165,6 +165,8 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
         impl.noneRecompiled()
     }
 
+    @NotYetImplemented
+    // used to work in 3.4, not anymore in 3.4.1. Can re-enable with compiler plugins. See gradle/gradle#1474
     def "deletion of jar with non-private constant causes rebuild if constant is used"() {
         java api: ["class A { public final static int x = 1; }"], impl: ["class X { int x() { return 1;} }", "class Y {}"]
         impl.snapshot { run "compileJava" }
@@ -181,6 +183,8 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
         impl.recompiledClasses("X")
     }
 
+    @NotYetImplemented
+    // used to work in 3.4, not anymore in 3.4.1. Can re-enable with compiler plugins. See gradle/gradle#1474
     def "change in an upstream class with non-private constant doesn't cause full rebuild if constant is not used"() {
         java api: ["class A {}", "class B { final static int x = 1; }"], impl: ["class ImplA extends A {}", "class ImplB extends B {}"]
         impl.snapshot { run "compileJava" }
@@ -203,7 +207,8 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
         run "impl:compileJava"
 
         then:
-        impl.recompiledClasses('ImplA')
+        // impl.recompiledClasses('ImplA') // used to work in 3.4, not anymore in 3.4.1. Can re-enable with compiler plugins. See gradle/gradle#1474
+        impl.recompiledClasses('ImplA', 'ImplB')
 
         where:
         constantType | constantValue
@@ -219,6 +224,8 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
     }
 
     @Unroll
+    @NotYetImplemented
+    // used to work in 3.4, not anymore in 3.4.1. Can re-enable with compiler plugins. See gradle/gradle#1474
     def "change in an upstream class with non-private constant causes rebuild only if same constant is used and no direct dependency (#constantType)"() {
         java api: ["class A {}", "class B { final static $constantType x = $constantValue; }"], impl: ["class X { $constantType foo() { return $constantValue; }}", "class Y {int foo() { return -2; }}"]
         impl.snapshot { run "compileJava" }
@@ -244,6 +251,8 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
     }
 
     @Unroll
+    @NotYetImplemented
+    // used to work in 3.4, not anymore in 3.4.1. Can re-enable with compiler plugins. See gradle/gradle#1474
     def "constant value change in an upstream class causes rebuild if previous constant value was used in previous build (#constantType)"() {
         java api: ["class A {}", "class B { final static $constantType x = $constantValue; }"], impl: ["class X { $constantType foo() { return $constantValue; }}", "class Y {int foo() { return -2; }}"]
         impl.snapshot { run "compileJava" }
@@ -268,6 +277,8 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
         'String'     | '"foo" + "bar"' | '"bar"'
     }
 
+    @NotYetImplemented
+    // used to work in 3.4, not anymore in 3.4.1. Can re-enable with compiler plugins. See gradle/gradle#1474
     def "ignores irrelevant changes to constant values"() {
         java api: ["class A {}", "class B { final static int x = 3; final static int y = -2; }"],
             impl: ["class X { int foo() { return 3; }}", "class Y {int foo() { return -2; }}"]
@@ -596,7 +607,7 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
         run("impl:compileJava")
 
         then:
-        impl.recompiledClasses('C', 'C$Inner', 'D', 'D$Inner', 'E', 'E$1', 'F', 'F$Inner')
+        impl.recompiledClasses('B', 'C', 'C$Inner', 'D', 'D$Inner', 'E', 'E$1', 'F', 'F$Inner')
 
         where:
         visibility << ['public', 'private' , '']
@@ -648,4 +659,46 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
         !output.contains(':api:compileJava - is not incremental (e.g. outputs have changed, no previous execution, etc.).')
         impl.recompiledClasses("ImplA")
     }
+
+    @NotYetImplemented
+    // used to work in 3.4, not anymore in 3.4.1. Can re-enable with compiler plugins. See gradle/gradle#1474
+    def "only recompiles classes potentially affected by constant change"() {
+        java api: ["class A { public static final int FOO = 10; public static final int BAR = 20; }"],
+            impl: ['class B { void foo() { int x = 10; } }', 'class C { void foo() { int x = 20; } }']
+        impl.snapshot { run 'compileJava' }
+
+        when:
+        java api: ['class A { public static final int FOO = 100; public static final int BAR = 20; }']
+        run 'impl:compileJava'
+
+        then:
+        impl.recompiledClasses 'B'
+    }
+
+    def "recompiles dependent class in case a constant is switched"() {
+        java api: ["class A { public static final int FOO = 10; public static final int BAR = 20; }"],
+            impl: ['class B { void foo() { int x = 10; } }', 'class C { void foo() { int x = 20; } }']
+        impl.snapshot { run 'compileJava' }
+
+        when:
+        java api: ['class A { public static final int FOO = 20; public static final int BAR = 10; }']
+        run 'impl:compileJava'
+
+        then:
+        impl.recompiledClasses 'B', 'C'
+    }
+
+    @Issue("gradle/gradle#1474")
+    def "recompiles dependent class in case a constant is computed from another constant"() {
+        java api: ["class A { public static final int FOO = 10; }"], impl: ['class B { public static final int BAR = 2 + A.FOO; } ']
+        impl.snapshot { run 'compileJava' }
+
+        when:
+        java api: ['class A { public static final int FOO = 100; }']
+        run 'impl:compileJava'
+
+        then:
+        impl.recompiledClasses 'B'
+
+    }
 }

subprojects/language-java/src/integTest/groovy/org/gradle/java/compile/incremental/SourceIncrementalJavaCompilationIntegrationTest.groovy

@@ -257,6 +257,8 @@ class SourceIncrementalJavaCompilationIntegrationTest extends AbstractIntegratio
         outputs.recompiledClasses 'B', 'A'
     }
 
+    @NotYetImplemented
+    // used to work in 3.4, not anymore in 3.4.1. Can re-enable with compiler plugins. See gradle/gradle#1474
     def "changed class with unused non-private constant incurs partial rebuild"() {
         java "class A { int foo() { return 2; } }", "class B { final static int x = 1;}"
         outputs.snapshot { run "compileJava" }

subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/asm/ClassDependenciesVisitor.java

@@ -152,7 +152,10 @@ private boolean isAnnotationType(String[] interfaces) {
     public FieldVisitor visitField(int access, String name, String desc, String signature, Object value) {
         maybeAddDependentType(descTypeOf(desc));
         if (isAccessibleConstant(access, value) && constants != null) {
-            constants.add(value.hashCode()); //non-private const
+            // we need to compute a hash for a constant, which is based on the name of the constant + its value
+            // otherwise we miss the case where a class defines several constants with the same value, or when
+            // two values are switched
+            constants.add((name + '|' + value).hashCode()); //non-private const
         }
         return null;
     }

subprojects/language-java/src/main/java/org/gradle/api/internal/tasks/compile/incremental/deps/ClassSetAnalysis.java

@@ -46,7 +46,7 @@ public DependentsSet getRelevantDependents(Iterable<String> classes, Set<Integer
             for (String dependentClass : dependentClasses) {
                 result.add(dependentClass);
                 Set<String> children = data.classesToChildren.get(dependentClass);
-                if (children!=null && children.contains(cls)) {
+                if (children != null && children.contains(cls)) {
                     System.out.println("children = " + children);
                 }
             }
@@ -63,19 +63,26 @@ public DependentsSet getRelevantDependents(String className, Set<Integer> consta
         if (deps == null && constants.isEmpty()) {
             return DefaultDependentsSet.EMPTY;
         }
+        // since 3.4.1, the Set<Integer> of constants here doesn't match the set of literals, so we cannot use them
+        // anymore. If the set above is not empty, it means a constant has been added or changed, and we need to
+        // recompile everything
+        if (!constants.isEmpty()) {
+            return DependencyToAll.INSTANCE;
+        }
         Set<String> result = new HashSet<String>();
         if (deps != null && !deps.isDependencyToAll()) {
             recurseDependents(new HashSet<String>(), result, deps.getDependentClasses());
         }
-        for (Integer constant : constants) {
+        /*for (Integer constant : constants) {
             Set<String> classes = data.literalsToClasses.get(constant);
             if (classes != null) {
                 result.addAll(classes);
             }
         }
-
+        */
         result.remove(className);
         return new DefaultDependentsSet(result);
+
     }
 
     public boolean isDependencyToAll(String className) {

subprojects/language-java/src/test/groovy/org/gradle/api/internal/tasks/compile/incremental/analyzer/DefaultClassDependenciesAnalyzerTest.groovy

@@ -69,7 +69,7 @@ class DefaultClassDependenciesAnalyzerTest extends Specification {
         then:
         analysis.classDependencies == [UsedByNonPrivateConstantsClass.name] as Set
         !analysis.dependencyToAll
-        analysis.constants == [1] as Set
+        analysis.constants == ['X|1'.hashCode()] as Set
         analysis.literals == [] as Set
 
         when:
@@ -78,7 +78,7 @@ class DefaultClassDependenciesAnalyzerTest extends Specification {
         then:
         analysis.classDependencies.isEmpty()
         !analysis.dependencyToAll
-        analysis.constants == [1] as Set
+        analysis.constants == ['X|1'.hashCode()] as Set
         analysis.literals == [] as Set
 
         when:

subprojects/language-java/src/test/groovy/org/gradle/api/internal/tasks/compile/incremental/deps/ClassSetAnalysisTest.groovy

@@ -16,6 +16,7 @@
 
 package org.gradle.api.internal.tasks.compile.incremental.deps
 
+import groovy.transform.NotYetImplemented
 import spock.lang.Specification
 
 import static org.gradle.api.internal.tasks.compile.incremental.deps.DefaultDependentsSet.dependents
@@ -180,6 +181,8 @@ class ClassSetAnalysisTest extends Specification {
         !a.isDependencyToAll("Unknown")
     }
 
+    @NotYetImplemented
+    // used to work in 3.4, not anymore in 3.4.1. Can re-enable with compiler plugins. See gradle/gradle#1474
     def "adds classes with literals as dependents"() {
         def a = analysis([:], [A: [1,2] as Set], [1: ['B', 'C'] as Set, 2: ['D'] as Set])
 

subprojects/plugins/src/integTest/groovy/org/gradle/java/compile/AbstractJavaCompileAvoidanceIntegrationSpec.groovy

@@ -385,6 +385,42 @@ public class ToolImpl {
         executedAndNotSkipped ':b:compileJava'
     }
 
+    def "recompiles when constant value of API changes"() {
+        given:
+        buildFile << """
+            project(':b') {
+                dependencies {
+                    compile project(':a')
+                }
+            }
+        """
+        def sourceFile = file("a/src/main/java/ToolImpl.java")
+        sourceFile << """ 
+            public class ToolImpl { public static final int CONST = 1; }
+        """
+        file("b/src/main/java/Main.java") << """
+            public class Main { public static final int CONST2 = 1 + ToolImpl.CONST; }
+        """
+
+        when:
+        succeeds ':b:compileJava'
+
+        then:
+        executedAndNotSkipped ':a:compileJava'
+        executedAndNotSkipped ':b:compileJava'
+
+        when:
+        // change to constant value
+        sourceFile.text = """
+            public class ToolImpl { public static final int CONST = 10; }
+"""
+
+        then:
+        succeeds ':b:compileJava'
+        executedAndNotSkipped ':a:compileJava'
+        executedAndNotSkipped ':b:compileJava'
+    }
+
     def "recompiles when generic type signatures of implementation class changes"() {
         given:
         buildFile << """