Merge pull request #25183 [Backport] Use compiler API data for increm…

…ental compilation after failure

Backport of #22608, #23226, #23661 and #24880

Fixes #23922

Co-authored-by: Anže Sodja <asodja@gradle.com>

subprojects/docs/src/docs/userguide/jvm/java_plugin.adoc

@@ -509,26 +509,6 @@ To help you understand how incremental compilation works, the following provides
 * Having a source structure that does not match the package names, while legal for compilation, might end up causing trouble in the toolchain.
     Even more if annotation processing and <<build_cache.adoc#build_cache,caching>> are involved.
 
-Additionally, Gradle might temporarily change the output location while running incremental compilation.
-This might affect some annotation processors that inspect output locations or accept file paths as arguments (e.g., `-XepExcludedPaths` in Error Prone).
-There are two options:
-
- - Disable `incremental after failure` by setting link:{javadocPath}/org/gradle/api/tasks/compile/CompileOptions.html#getIncrementalAfterFailure--[`options.incrementalAfterFailure`] to `false` on the JavaCompile task.
--  Keep `incremental after failure` enabled by instead passing a temporary output value to the annotation processor as a parameter so that the annotation processor is aware of the temporary path. 
-
-In the case of Error Prone, `-XepExcludedPaths` must be set. Given an existing value of `-XepExcludedPaths=.\*/build/generated/.*`, the updated value would be
-`-XepExcludedPaths=.\*/(build/generated|compileJava/compileTransaction/annotation-output)/.*`.
-
-The following table shows the mapping from default output location to temporary output location:
-
-.Compiler output to temporary output location
-|===
-| JavaCompile location property | Default output location | Temporary output location
-| `destinationDirectory` | `build/classes/java/main` | `build/tmp/<task-name>/compileTransaction/compile-output`
-| `options.generatedSourceOutputDirectory` | `build/generated/sources/annotationProcessor/java/main` | `build/tmp/<task-name>/compileTransaction/annotation-output`
-| `options.headerOutputDirectory` | `build/generated/sources/headers/java/main` | `build/tmp/<task-name>/compileTransaction/header-output`
-|===
-
 [[sec:incremental_annotation_processing]]
 == Incremental annotation processing
 

subprojects/docs/src/docs/userguide/migration/upgrading_version_7.adoc

@@ -173,13 +173,6 @@ Use `implementation.bundle(libs.bundles.testing)` instead.
 
 For more information, see the updated <<jvm_test_suite_plugin.adoc#sec:declare_an_additional_test_suite, declare an additional test suite>> example in the JVM Test Suite Plugin section of the user guide and the link:{groovyDslPath}/org.gradle.api.artifacts.dsl.DependencyAdder.html[`DependencyAdder`] page in the DSL reference.
 
-==== Incremental compilation temporarily changes the output location
-
-Incremental Java and Groovy compilation may now change the compiler output location.
-This might affect some annotation processors that allow users to wire some action to file paths (e.g. `-XepExcludedPaths` in Error Prone).
-This behaviour can be disabled by setting `options.incrementalAfterFailure` to `false`.
-Please refer to the <<java_plugin#sec:incremental_compilation_known_issues, userguide section about known incremental compilation issues>> for more details.
-
 === Deprecations
 
 [[invalid_toolchain_specification_deprecation]]

subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/IncrementalCompileTask.java

@@ -46,15 +46,18 @@ public class IncrementalCompileTask implements JavaCompiler.CompilationTask {
 
     private final Function<File, Optional<String>> relativize;
     private final Consumer<Map<String, Set<String>>> classNameConsumer;
+    private final Consumer<String> classBackupService;
     private final ConstantDependentsConsumer constantDependentsConsumer;
     private final JavacTask delegate;
 
     public IncrementalCompileTask(JavaCompiler.CompilationTask delegate,
                                   Function<File, Optional<String>> relativize,
+                                  Consumer<String> classBackupService,
                                   Consumer<Map<String, Set<String>>> classNamesConsumer,
                                   BiConsumer<String, String> publicDependentDelegate,
                                   BiConsumer<String, String> privateDependentDelegate) {
         this.relativize = relativize;
+        this.classBackupService = classBackupService;
         this.classNameConsumer = classNamesConsumer;
         this.constantDependentsConsumer = new ConstantDependentsConsumer(publicDependentDelegate, privateDependentDelegate);
         if (delegate instanceof JavacTask) {
@@ -81,7 +84,7 @@ public void setLocale(Locale locale) {
 
     @Override
     public Boolean call() {
-        ClassNameCollector classNameCollector = new ClassNameCollector(relativize, delegate.getElements());
+        ClassNameCollector classNameCollector = new ClassNameCollector(relativize, classBackupService, delegate.getElements());
         ConstantsCollector constantsCollector = new ConstantsCollector(delegate, constantDependentsConsumer);
         delegate.addTaskListener(classNameCollector);
         delegate.addTaskListener(constantsCollector);

subprojects/java-compiler-plugin/src/main/java/org/gradle/internal/compiler/java/listeners/classnames/ClassNameCollector.java

@@ -28,16 +28,19 @@
 import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.function.Consumer;
 import java.util.function.Function;
 
 public class ClassNameCollector implements TaskListener {
     private final Map<File, Optional<String>> relativePaths = new HashMap<>();
+    private final Consumer<String> classBackupService;
     private final Map<String, Set<String>> mapping = new HashMap<>();
     private final Function<File, Optional<String>> relativize;
     private final Elements elements;
 
-    public ClassNameCollector(Function<File, Optional<String>> relativize, Elements elements) {
+    public ClassNameCollector(Function<File, Optional<String>> relativize, Consumer<String> classBackupService, Elements elements) {
         this.relativize = relativize;
+        this.classBackupService = classBackupService;
         this.elements = elements;
     }
 
@@ -47,11 +50,6 @@ public Map<String, Set<String>> getMapping() {
 
     @Override
     public void started(TaskEvent e) {
-
-    }
-
-    @Override
-    public void finished(TaskEvent e) {
         JavaFileObject sourceFile = e.getSourceFile();
         if (isSourceFile(sourceFile)) {
             File asSourceFile = new File(sourceFile.getName());
@@ -63,6 +61,10 @@ public void finished(TaskEvent e) {
         }
     }
 
+    @Override
+    public void finished(TaskEvent e) {
+    }
+
     private static boolean isSourceFile(JavaFileObject sourceFile) {
         return sourceFile != null && sourceFile.getKind() == JavaFileObject.Kind.SOURCE;
     }
@@ -73,6 +75,7 @@ private void processSourceFile(TaskEvent e, File sourceFile) {
             String key = relativePath.get();
             String symbol = normalizeName(e.getTypeElement());
             registerMapping(key, symbol);
+            classBackupService.accept(symbol);
         }
     }
 

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

@@ -40,6 +40,7 @@ class ConstantsCollectorTest extends AbstractCompilerPluginTest {
             Files.createTempDirectory(temporaryFolder.toPath(), null).toFile(),
             { f -> Optional.empty() },
             {},
+            {},
             consumer
         )
     }

subprojects/java-compiler-plugin/src/testFixtures/java/org/gradle/internal/compiler/java/TestCompiler.java

@@ -38,15 +38,18 @@ public class TestCompiler {
 
     private final File outputFolder;
     private final Function<File, Optional<String>> relativize;
+    private final Consumer<String> classBackupService;
     private final Consumer<Map<String, Set<String>>> classNameConsumer;
     private final ConstantDependentsConsumer constantDependentsConsumer;
 
     public TestCompiler(File outputFolder,
                         Function<File, Optional<String>> relativize,
+                        Consumer<String> classBackupService,
                         Consumer<Map<String, Set<String>>> classNamesConsumer,
                         ConstantDependentsConsumer constantDependentsConsumer) {
         this.outputFolder = outputFolder;
         this.relativize = relativize;
+        this.classBackupService = classBackupService;
         this.classNameConsumer = classNamesConsumer;
         this.constantDependentsConsumer = constantDependentsConsumer;
     }
@@ -58,7 +61,7 @@ public void compile(List<File> sourceFiles) {
         Iterable<? extends JavaFileObject> compilationUnits = fileManager.getJavaFileObjectsFromFiles(sourceFiles);
         List<String> arguments = Arrays.asList("-d", outputFolder.getAbsolutePath());
         JavaCompiler.CompilationTask delegate = compiler.getTask(output, fileManager, null, arguments, null, compilationUnits);
-        IncrementalCompileTask task = new IncrementalCompileTask(delegate, relativize, classNameConsumer, constantDependentsConsumer::consumeAccessibleDependent, constantDependentsConsumer::consumePrivateDependent);
+        IncrementalCompileTask task = new IncrementalCompileTask(delegate, relativize, classBackupService, classNameConsumer, constantDependentsConsumer::consumeAccessibleDependent, constantDependentsConsumer::consumePrivateDependent);
         if (!task.call()) {
             throw new RuntimeException(output.toString());
         }

subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/ApiGroovyCompiler.java

@@ -29,9 +29,11 @@
 import org.codehaus.groovy.control.CompilationUnit;
 import org.codehaus.groovy.control.CompilePhase;
 import org.codehaus.groovy.control.CompilerConfiguration;
+import org.codehaus.groovy.control.MultipleCompilationErrorsException;
 import org.codehaus.groovy.control.SourceUnit;
 import org.codehaus.groovy.control.customizers.CompilationCustomizer;
 import org.codehaus.groovy.control.customizers.ImportCustomizer;
+import org.codehaus.groovy.control.messages.ExceptionMessage;
 import org.codehaus.groovy.control.messages.SimpleMessage;
 import org.codehaus.groovy.tools.javac.JavaAwareCompilationUnit;
 import org.codehaus.groovy.tools.javac.JavaCompiler;
@@ -58,6 +60,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 
 import static org.gradle.internal.FileUtils.hasExtension;
 
@@ -73,7 +76,7 @@ public ApiGroovyCompiler(Compiler<JavaCompileSpec> javaCompiler, ProjectLayout p
     private static abstract class IncrementalCompilationCustomizer extends CompilationCustomizer {
         static IncrementalCompilationCustomizer fromSpec(GroovyJavaJointCompileSpec spec, ApiCompilerResult result) {
             if (spec.incrementalCompilationEnabled()) {
-                return new TrackingClassGenerationCompilationCustomizer(new CompilationSourceDirs(spec), result);
+                return new TrackingClassGenerationCompilationCustomizer(new CompilationSourceDirs(spec), result, new CompilationClassBackupService(spec, result));
             } else {
                 return new NoOpCompilationCustomizer();
             }
@@ -101,10 +104,12 @@ public void call(SourceUnit source, GeneratorContext context, ClassNode classNod
     private static class TrackingClassGenerationCompilationCustomizer extends IncrementalCompilationCustomizer {
         private final CompilationSourceDirs compilationSourceDirs;
         private final ApiCompilerResult result;
+        private final CompilationClassBackupService compilationClassBackupService;
 
-        private TrackingClassGenerationCompilationCustomizer(CompilationSourceDirs compilationSourceDirs, ApiCompilerResult result) {
+        private TrackingClassGenerationCompilationCustomizer(CompilationSourceDirs compilationSourceDirs, ApiCompilerResult result, CompilationClassBackupService compilationClassBackupService) {
             this.compilationSourceDirs = compilationSourceDirs;
             this.result = result;
+            this.compilationClassBackupService = compilationClassBackupService;
         }
 
         @Override
@@ -113,8 +118,10 @@ public void call(SourceUnit source, GeneratorContext context, ClassNode classNod
         }
 
         private void inspectClassNode(SourceUnit sourceUnit, ClassNode classNode) {
+            String classFqName = classNode.getName();
             String relativePath = compilationSourceDirs.relativize(new File(sourceUnit.getSource().getURI().getPath())).orElseThrow(IllegalStateException::new);
-            result.getSourceClassesMapping().computeIfAbsent(relativePath, key -> new HashSet<>()).add(classNode.getName());
+            result.getSourceClassesMapping().computeIfAbsent(relativePath, key -> new HashSet<>()).add(classFqName);
+            compilationClassBackupService.maybeBackupClassFile(classFqName);
             Iterator<InnerClassNode> iterator = classNode.getInnerClasses();
             while (iterator.hasNext()) {
                 inspectClassNode(sourceUnit, iterator.next());
@@ -257,24 +264,37 @@ public boolean apply(File file) {
                         try {
                             WorkResult javaCompilerResult = javaCompiler.execute(spec);
                             if (javaCompilerResult instanceof ApiCompilerResult) {
-                                result.getSourceClassesMapping().putAll(((ApiCompilerResult) javaCompilerResult).getSourceClassesMapping());
+                                copyJavaCompilerResult((ApiCompilerResult) javaCompilerResult);
                             }
                         } catch (CompilationFailedException e) {
+                            Optional<ApiCompilerResult> partialResult = e.getCompilerPartialResult();
+                            partialResult.ifPresent(result -> copyJavaCompilerResult(result));
                             cu.getErrorCollector().addFatalError(new SimpleMessage(e.getMessage(), cu));
                         }
                     }
                 };
             }
+
+            private void copyJavaCompilerResult(ApiCompilerResult javaCompilerResult) {
+                result.getSourceClassesMapping().putAll(javaCompilerResult.getSourceClassesMapping());
+                result.getBackupClassFiles().putAll(javaCompilerResult.getBackupClassFiles());
+            }
         });
 
         try {
             unit.compile();
             return result;
         } catch (org.codehaus.groovy.control.CompilationFailedException e) {
+            if (isFatalException(e)) {
+                // This indicates a compiler bug and not a user error,
+                // so we cannot recover from such error: we need to force full recompilation.
+                throw new CompilationFatalException(e);
+            }
+
             System.err.println(e.getMessage());
             // Explicit flush, System.err is an auto-flushing PrintWriter unless it is replaced.
             System.err.flush();
-            throw new CompilationFailedException();
+            throw new CompilationFailedException(result);
         } finally {
             // Remove compile and AST types from the Groovy loader
             compilerGroovyLoader.discardTypesFrom(classPathLoader);
@@ -285,6 +305,27 @@ public boolean apply(File file) {
         }
     }
 
+    /**
+     * Returns true if the exception is fatal, unrecoverable for the incremental compilation. Example of such error:
+     * <pre>
+     * error: startup failed:
+     * General error during instruction selection: java.lang.NoClassDefFoundError: Unable to load class ClassName due to missing dependency DependencyName
+     *   java.lang.RuntimeException: java.lang.NoClassDefFoundError: Unable to load class ClassName due to missing dependency DependencyName
+     *      at org.codehaus.groovy.control.CompilationUnit$IPrimaryClassNodeOperation.doPhaseOperation(CompilationUnit.java:977)
+     *      at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:672)
+     *      at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:636)
+     *      at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:611)
+     * </pre>
+     */
+    private static boolean isFatalException(org.codehaus.groovy.control.CompilationFailedException e) {
+        if (e instanceof MultipleCompilationErrorsException) {
+            // Groovy compiler wraps any uncontrolled exception (e.g. IOException, NoClassDefFoundError and similar) in a `ExceptionMessage`
+            return ((MultipleCompilationErrorsException) e).getErrorCollector().getErrors().stream()
+                .anyMatch(message -> message instanceof ExceptionMessage);
+        }
+        return false;
+    }
+
     private static boolean shouldProcessAnnotations(GroovyJavaJointCompileSpec spec) {
         return spec.getGroovyCompileOptions().isJavaAnnotationProcessing() && spec.annotationProcessingConfigured();
     }

subprojects/language-groovy/src/main/java/org/gradle/api/internal/tasks/compile/incremental/recomp/GroovyRecompilationSpecProvider.java

@@ -19,13 +19,18 @@
 import com.google.common.collect.ImmutableSet;
 import org.gradle.api.file.FileTree;
 import org.gradle.api.internal.file.FileOperations;
+import org.gradle.api.internal.tasks.compile.GroovyJavaJointCompileSpec;
+import org.gradle.api.internal.tasks.compile.JavaCompileSpec;
 import org.gradle.internal.file.Deleter;
 import org.gradle.work.FileChange;
 
 import java.util.Set;
+import java.util.stream.Collectors;
 
 public class GroovyRecompilationSpecProvider extends AbstractRecompilationSpecProvider {
 
+    private static final Set<String> SUPPORTED_FILE_EXTENSIONS = ImmutableSet.of(".java", ".groovy");
+
     public GroovyRecompilationSpecProvider(
         Deleter deleter,
         FileOperations fileOperations,
@@ -36,9 +41,42 @@ public GroovyRecompilationSpecProvider(
         super(deleter, fileOperations, sources, sourceChanges, incremental);
     }
 
+    /**
+     * For all classes with Java source that we will be recompiled due to some change, we need to recompile all subclasses.
+     * This is because Groovy might try to load some subclass when analysing Groovy classes before Java compilation, but if parent class was stale,
+     * it has been deleted, so class loading of a subclass will fail.
+     *
+     * Fix for issue <a href="https://github.com/gradle/gradle/issues/22531">#22531</a>.
+     */
+    @Override
+    protected void processCompilerSpecificDependencies(
+        JavaCompileSpec spec,
+        RecompilationSpec recompilationSpec,
+        SourceFileChangeProcessor sourceFileChangeProcessor,
+        SourceFileClassNameConverter sourceFileClassNameConverter
+    ) {
+        if (!supportsGroovyJavaJointCompilation(spec)) {
+            return;
+        }
+        Set<String> classesWithJavaSource = recompilationSpec.getClassesToCompile().stream()
+            .flatMap(classToCompile -> sourceFileClassNameConverter.getRelativeSourcePaths(classToCompile).stream())
+            .filter(sourcePath -> sourcePath.endsWith(".java"))
+            .flatMap(sourcePath -> sourceFileClassNameConverter.getClassNames(sourcePath).stream())
+            .collect(Collectors.toSet());
+        if (!classesWithJavaSource.isEmpty()) {
+            // We need to collect just accessible dependents, since it seems
+            // private references to classes are not problematic when Groovy compiler loads a class
+            sourceFileChangeProcessor.processOnlyAccessibleChangeOfClasses(classesWithJavaSource, recompilationSpec);
+        }
+    }
+
+    private boolean supportsGroovyJavaJointCompilation(JavaCompileSpec spec) {
+        return spec instanceof GroovyJavaJointCompileSpec && ((GroovyJavaJointCompileSpec) spec).getGroovyCompileOptions().getFileExtensions().contains("java");
+    }
+
     @Override
     protected Set<String> getFileExtensions() {
-        return ImmutableSet.of(".java", ".groovy");
+        return SUPPORTED_FILE_EXTENSIONS;
     }
 
     @Override