From 4bc2151a1761bd61e5c95917dcbcac9d3cd9f079 Mon Sep 17 00:00:00 2001 From: Rick Busarow Date: Tue, 2 Apr 2024 13:29:45 -0500 Subject: [PATCH] delay `@ContributesSubcomponent` generation until the last analysis rounds The `ContributesSubcomponentHandlerGenerator` is responsible for generating these: ```kotlin @MergeSubcomponent(scope = Any::class) public interface SubcomponentInterfaceA : SubcomponentInterface { // ... } ``` It used to generate those interfaces as soon as it scanned the triggers, but that can cause problems if another `@ContributesSubcomponent` interface is being generated, and that intends to replace a human-written one. Now, the handler is a `PrivateCodeGenerator` and it doesn't start parsing/generating until all the "public" code generation is done. This means it gets the full picture of the possible graph, but it still finishes generating before any IR merging happens. --- .../codegen/CodeGenerationExtension.kt | 71 ++++++++++--------- ...ContributesSubcomponentHandlerGenerator.kt | 56 +++++++-------- ...ributesSubcomponentHandlerGeneratorTest.kt | 24 +++---- 3 files changed, 76 insertions(+), 75 deletions(-) diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/CodeGenerationExtension.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/CodeGenerationExtension.kt index 5bc748941..ac3cbcfc2 100644 --- a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/CodeGenerationExtension.kt +++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/CodeGenerationExtension.kt @@ -89,10 +89,10 @@ internal class CodeGenerationExtension( // Either sync the local file state with our internal cache, // or delete any existing generated files (if caching isn't enabled). - val createdChanges = syncGeneratedDir(files) + val syncCreatedChanges = syncGeneratedDir(files) didSyncGeneratedDir = true - if (createdChanges) { + if (syncCreatedChanges) { // Tell the compiler to analyze the generated files *before* calling `analysisCompleted()`. // This ensures that the compiler will update/delete any references to stale code in the // ModuleDescriptor, as well as updating/deleting any stale .class files. @@ -118,10 +118,8 @@ internal class CodeGenerationExtension( anvilModule.addFiles(files) val generatedFiles = generateCode( - codeGenerators = codeGenerators, psiManager = psiManager, anvilModule = anvilModule, - files = files, ) if (trackSourceFiles) { @@ -189,10 +187,8 @@ internal class CodeGenerationExtension( } private fun generateCode( - codeGenerators: List, psiManager: PsiManager, anvilModule: RealAnvilModuleDescriptor, - files: Collection, ): MutableCollection { val anvilContext = commandLineOptions.toAnvilContext(anvilModule) @@ -227,47 +223,56 @@ internal class CodeGenerationExtension( } } - fun Collection.generateCode(files: Collection): List = + fun Collection.generateAndCache(files: Collection): List = flatMap { codeGenerator -> codeGenerator.generateCode(generatedDir, anvilModule, files) + .onEach { file -> + onGenerated( + generatedFile = file, + codeGenerator = codeGenerator, + allowOverwrites = false, + ) + } + .toKtFiles(psiManager, anvilModule) + } + + fun Collection.flush(): List = + flatMap { codeGenerator -> + codeGenerator.flush(generatedDir, anvilModule) .onEach { onGenerated( generatedFile = it, codeGenerator = codeGenerator, - allowOverwrites = false, + // flushing code generators write the files but no content during normal rounds. + allowOverwrites = true, ) } .toKtFiles(psiManager, anvilModule) } - fun Collection.flush(): List = - filterIsInstance() - .flatMap { codeGenerator -> - codeGenerator.flush(generatedDir, anvilModule) - .onEach { - onGenerated( - generatedFile = it, - codeGenerator = codeGenerator, - // flushing code generators write the files but no content during normal rounds. - allowOverwrites = true, - ) - } - .toKtFiles(psiManager, anvilModule) - } - - var newFiles = nonPrivateCodeGenerators.generateCode(files) - - while (newFiles.isNotEmpty()) { - // Parse the KtFile for each generated file. Then feed the code generators with the new - // parsed files until no new files are generated. - newFiles = nonPrivateCodeGenerators.generateCode(newFiles) + fun List.loopGeneration() { + var newFiles = generateAndCache(anvilModule.allFiles.toList()) + while (newFiles.isNotEmpty()) { + // Parse the KtFile for each generated file. Then feed the code generators with the new + // parsed files until no new files are generated. + newFiles = generateAndCache(newFiles) + } } - nonPrivateCodeGenerators.flush() + // All non-private code generators are batched together. + // They will execute against the initial set of files, + // then loop until no generator produces any new files. + nonPrivateCodeGenerators.loopGeneration() + + // Flushing generators are next. + // They have already seen all generated code. + // Their output may be consumed by a private generator. + codeGenerators.filterIsInstance().flush() - // PrivateCodeGenerators don't impact other code generators. Therefore, they can be called a - // single time at the end. - privateCodeGenerators.generateCode(anvilModule.allFiles.toList()) + // Private generators do not affect each other, so they're invoked last. + // They may require multiple iterations of their own logic, though, + // so we loop them individually until there are no more changes. + privateCodeGenerators.forEach { listOf(it).loopGeneration() } return generatedFiles.values } diff --git a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGenerator.kt b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGenerator.kt index a1743919c..ac629f20c 100644 --- a/compiler/src/main/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGenerator.kt +++ b/compiler/src/main/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGenerator.kt @@ -8,7 +8,6 @@ import com.squareup.anvil.compiler.PARENT_COMPONENT import com.squareup.anvil.compiler.SUBCOMPONENT_FACTORY import com.squareup.anvil.compiler.SUBCOMPONENT_MODULE import com.squareup.anvil.compiler.api.AnvilContext -import com.squareup.anvil.compiler.api.CodeGenerator import com.squareup.anvil.compiler.api.GeneratedFileWithSources import com.squareup.anvil.compiler.api.createGeneratedFile import com.squareup.anvil.compiler.contributesSubcomponentFactoryFqName @@ -63,7 +62,7 @@ import java.io.File */ internal class ContributesSubcomponentHandlerGenerator( private val classScanner: ClassScanner, -) : CodeGenerator { +) : PrivateCodeGenerator() { private val triggers = mutableListOf() private val contributions = mutableSetOf() @@ -74,7 +73,7 @@ internal class ContributesSubcomponentHandlerGenerator( override fun isApplicable(context: AnvilContext): Boolean = !context.generateFactoriesOnly - override fun generateCode( + override fun generateCodePrivate( codeGenDir: File, module: ModuleDescriptor, projectFiles: Collection, @@ -98,23 +97,18 @@ internal class ContributesSubcomponentHandlerGenerator( .flatMap { it.annotations } .filter { it.fqName == contributesSubcomponentFqName } .map { Contribution(it) } - .toList() - .also { contributions -> - // Find all replaced subcomponents and remember them. - replacedReferences += contributions.flatMap { contribution -> - contribution.annotation - .replaces() - .also { - checkReplacedSubcomponentWasNotAlreadyGenerated(contribution.clazz, it) - } - } - } - // Remove any contribution, if it was replaced by another contribution. - contributions.removeAll { contribution -> - contribution.clazz in replacedReferences + // Find all replaced subcomponents and remember them. + replacedReferences += contributions + .flatMap { contribution -> contribution.annotation.replaces() } + + for (contribution in contributions) { + checkReplacedSubcomponentWasNotAlreadyGenerated(contribution.clazz, replacedReferences) } + // Remove any contribution that was replaced by another contribution. + contributions.removeAll { it.clazz in replacedReferences } + return contributions .flatMap { contribution -> triggers @@ -127,9 +121,7 @@ internal class ContributesSubcomponentHandlerGenerator( } // Don't generate code for the same event twice. .minus(processedEvents) - .also { - processedEvents += it - } + .also { processedEvents += it } .map { generateCodeEvent -> val contribution = generateCodeEvent.contribution val generatedAnvilSubcomponent = generateCodeEvent.generatedAnvilSubcomponent @@ -157,7 +149,7 @@ internal class ContributesSubcomponentHandlerGenerator( val template = classes .joinToString(prefix = "[", postfix = "]") { "%T::class" } - addMember("$name = $template", *classes.toTypedArray()) + addMember("$name = $template", *classes.toTypedArray()) } } @@ -378,7 +370,7 @@ internal class ContributesSubcomponentHandlerGenerator( private fun checkReplacedSubcomponentWasNotAlreadyGenerated( contributedReference: ClassReference, - replacedReferences: List, + replacedReferences: Collection, ) { replacedReferences.forEach { replacedReference -> if (processedEvents.any { it.contribution.clazz == replacedReference }) { @@ -430,15 +422,21 @@ internal class ContributesSubcomponentHandlerGenerator( } private class Trigger( - annotation: AnnotationReference, + val clazz: ClassReference, + val scope: ClassReference, + val exclusions: List, ) { - val clazz = annotation.declaringClass() - val scope = annotation.scope() - val exclusions = annotation.exclude() + + constructor(annotation: AnnotationReference) : this( + clazz = annotation.declaringClass(), + scope = annotation.scope(), + exclusions = annotation.exclude(), + ) + val clazzFqName = clazz.fqName override fun toString(): String { - return "Trigger(clazz=$clazzFqName, scope=$scope)" + return "Trigger(clazz=$clazzFqName, scope=${scope.fqName})" } override fun equals(other: Any?): Boolean { @@ -460,9 +458,7 @@ internal class ContributesSubcomponentHandlerGenerator( } } - private class Contribution( - val annotation: AnnotationReference, - ) { + private class Contribution(val annotation: AnnotationReference) { val clazz = annotation.declaringClass() val scope = annotation.scope() val parentScope = annotation.parentScope() diff --git a/compiler/src/test/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGeneratorTest.kt b/compiler/src/test/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGeneratorTest.kt index cfd79751f..c939ad3be 100644 --- a/compiler/src/test/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGeneratorTest.kt +++ b/compiler/src/test/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGeneratorTest.kt @@ -1891,10 +1891,6 @@ class ContributesSubcomponentHandlerGeneratorTest { @Test fun `a previously generated contributed subcomponent cannot be replaced in a later round of generations`() { - // This test verifies an edge case where we generate the code for a contributed subcomponent, - // but then later in a new code generation round another code generator generates code that - // is supposed to replace the already generated subcomponent. We can't revert the code and - // don't want to support that use case. val codeGenerator = simpleCodeGenerator { clazz -> clazz .takeIf { it.isAnnotatedWith(mergeComponentFqName) } @@ -1904,7 +1900,6 @@ class ContributesSubcomponentHandlerGeneratorTest { package com.squareup.test import com.squareup.anvil.annotations.ContributesSubcomponent - import com.squareup.test.SubcomponentInterface1 @ContributesSubcomponent( scope = Any::class, @@ -1925,7 +1920,7 @@ class ContributesSubcomponentHandlerGeneratorTest { @ContributesSubcomponent( scope = Any::class, - parentScope = Unit::class + parentScope = Unit::class, ) interface SubcomponentInterface1 @@ -1934,12 +1929,17 @@ class ContributesSubcomponentHandlerGeneratorTest { """, codeGenerators = listOf(codeGenerator), ) { - assertThat(exitCode).isError() - assertThat(messages).contains( - "com.squareup.test.SubcomponentInterface2 tries to replace " + - "com.squareup.test.SubcomponentInterface1, but the code for " + - "com.squareup.test.SubcomponentInterface1 was already generated. This is not supported.", - ) + assertThat(exitCode).isEqualTo(OK) + + val parentComponentInterface2 = subcomponentInterface2 + .anvilComponent(componentInterface) + .parentComponentInterface + + assertThat(componentInterface extends parentComponentInterface2).isTrue() + + assertFailsWith { + subcomponentInterface1.anvilComponent(componentInterface) + } } }