Remove constant usage analysis in favor of full recompilation.

This commit fixes #1474, by not relying on the analysis of literals (which still exists),
but rather recompiling everything whenever we detect that a constant effectively changed in a class.
This doesn't prevent us from using a smarter strategy later (say, using a compiler plugin).

The commit also changes the hash that is used to store constants, so that the name of the constant is used.
This prevents another bug which was that if a class has multiple variants of the same value, or that values
of several constants are switched, we wouldn't detect the change.

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

@@ -20,9 +20,7 @@ package org.gradle.java.compile.incremental
 import groovy.transform.NotYetImplemented
 import org.gradle.integtests.fixtures.AbstractIntegrationSpec
 import org.gradle.integtests.fixtures.CompilationOutputsFixture
-import org.gradle.util.TestPrecondition
 import spock.lang.Issue
-import spock.lang.Requires
 import spock.lang.Unroll
 
 abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extends AbstractIntegrationSpec {
@@ -167,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" }
@@ -183,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" }
@@ -205,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
@@ -221,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" }
@@ -246,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" }
@@ -270,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; }}"]
@@ -598,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' , '']
@@ -651,8 +660,35 @@ abstract class AbstractCrossTaskIncrementalJavaCompilationIntegrationTest extend
         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")
-    @Requires({TestPrecondition.JDK8_OR_LATER}) // todo: also fix for JDK < 8
     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' }

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])