Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

delay @ContributesSubcomponent generation until the last analysis rounds #946

Merged
merged 1 commit into from
Apr 3, 2024
Merged

delay @ContributesSubcomponent generation until the last analysis rounds #946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 38 additions & 33 deletions compiler/src/main/java/com/squareup/anvil/compiler/codegen/CodeGenerationExtension.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -118,10 +118,8 @@ internal class CodeGenerationExtension(
anvilModule.addFiles(files)

val generatedFiles = generateCode(
codeGenerators = codeGenerators,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just class-level, so no need to pass the variable around internally.

psiManager = psiManager,
anvilModule = anvilModule,
files = files,
)

if (trackSourceFiles) {
Expand Down Expand Up @@ -189,10 +187,8 @@ internal class CodeGenerationExtension(
}

private fun generateCode(
codeGenerators: List<CodeGenerator>,
psiManager: PsiManager,
anvilModule: RealAnvilModuleDescriptor,
files: Collection<KtFile>,
): MutableCollection<FileWithContent> {

val anvilContext = commandLineOptions.toAnvilContext(anvilModule)
Expand Down Expand Up @@ -227,47 +223,56 @@ internal class CodeGenerationExtension(
}
}

fun Collection<CodeGenerator>.generateCode(files: Collection<KtFile>): List<KtFile> =
fun Collection<CodeGenerator>.generateAndCache(files: Collection<KtFile>): List<KtFile> =
flatMap { codeGenerator ->
codeGenerator.generateCode(generatedDir, anvilModule, files)
.onEach { file ->
onGenerated(
generatedFile = file,
codeGenerator = codeGenerator,
allowOverwrites = false,
)
}
.toKtFiles(psiManager, anvilModule)
}

fun Collection<FlushingCodeGenerator>.flush(): List<KtFile> =
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<CodeGenerator>.flush(): List<KtFile> =
filterIsInstance<FlushingCodeGenerator>()
.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<CodeGenerator>.loopGeneration() {
var newFiles = generateAndCache(anvilModule.allFiles.toList())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously generateAndCache (which was called generateCode before) was called once with files (like 258 of the removed code var newFiles = nonPrivateCodeGenerators.generateCode(files)). Always using anvilModule.allFiles.toList() can break custom code generators that use topLevelFunctionReferences or topLevelPropertyReferences because it won't contain any KtFile that doesn't contain a KtClassOrObject. So a KtFile that only contains top level functions and/or properties will not be passed to code generators anymore and because of that topLevel*References won't include functions/properties from those 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 = 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<FlushingCodeGenerator>().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
}
Expand Down
56 changes: 26 additions & 30 deletions .../main/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,7 +62,7 @@ import java.io.File
*/
internal class ContributesSubcomponentHandlerGenerator(
private val classScanner: ClassScanner,
) : CodeGenerator {
) : PrivateCodeGenerator() {

private val triggers = mutableListOf<Trigger>()
private val contributions = mutableSetOf<Contribution>()
Expand All @@ -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<KtFile>,
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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<ClassName>())
}
}

Expand Down Expand Up @@ -378,7 +370,7 @@ internal class ContributesSubcomponentHandlerGenerator(

private fun checkReplacedSubcomponentWasNotAlreadyGenerated(
contributedReference: ClassReference,
replacedReferences: List<ClassReference>,
replacedReferences: Collection<ClassReference>,
) {
replacedReferences.forEach { replacedReference ->
if (processedEvents.any { it.contribution.clazz == replacedReference }) {
Expand Down Expand Up @@ -430,15 +422,21 @@ internal class ContributesSubcomponentHandlerGenerator(
}

private class Trigger(
annotation: AnnotationReference,
val clazz: ClassReference,
val scope: ClassReference,
val exclusions: List<ClassReference>,
) {
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 {
Expand All @@ -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()
Expand Down
24 changes: 12 additions & 12 deletions ...t/java/com/squareup/anvil/compiler/codegen/ContributesSubcomponentHandlerGeneratorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
Expand All @@ -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,
Expand All @@ -1925,7 +1920,7 @@ class ContributesSubcomponentHandlerGeneratorTest {

@ContributesSubcomponent(
scope = Any::class,
parentScope = Unit::class
parentScope = Unit::class,
)
interface SubcomponentInterface1

Expand All @@ -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<ClassNotFoundException> {
subcomponentInterface1.anvilComponent(componentInterface)
}
}
}

Expand Down