From 5245e821e0b61a99685dc129ac83e1bb044275d2 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Wed, 5 Oct 2022 15:33:39 -0700 Subject: [PATCH] Move the Index annotation for KSP to annotations. The processor will produce a Index annotated class for each LibraryGlideModule in given compilation unit. If the Index annotation is in the ksp module, it will not be accessible in parent modules, causing a compilation failure. To work around that, we'll follow the same pattern as we did for Java and place the annotation in the annotations package, but with package private visibility. Any compilation unit that uses our annotation processor must already have a dependency on annotation, so the Index annotation will be available. Fixes #4911. --- .../glide/annotation/ksp/AppGlideModules.kt | 3 +- .../bumptech/glide/annotation/ksp/Index.kt | 5 -- .../annotation/ksp/LibraryGlideModules.kt | 15 +++-- .../annotation/ksp/LibraryGlideModuleTests.kt | 63 +++++++++++++++++++ .../glide/annotation/ksp/SourceTestHelpers.kt | 49 ++++++++++++--- .../bumptech/glide/annotation/ksp/Index.java | 19 ++++++ 6 files changed, 136 insertions(+), 18 deletions(-) delete mode 100644 annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/Index.kt create mode 100644 annotation/src/main/java/com/bumptech/glide/annotation/ksp/Index.java diff --git a/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/AppGlideModules.kt b/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/AppGlideModules.kt index 264f67d283..0426bdc270 100644 --- a/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/AppGlideModules.kt +++ b/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/AppGlideModules.kt @@ -212,7 +212,8 @@ internal class AppGlideModuleParser( } private fun KSAnnotation.getModuleArgumentValues(): List { - val result = arguments.find { it.name?.getShortName().equals("modules") }?.value + val result = + arguments.find { it.name?.getShortName().equals(IndexGenerator.INDEX_MODULES_NAME) }?.value if (result is List<*> && result.all { it is String }) { @Suppress("UNCHECKED_CAST") return result as List } diff --git a/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/Index.kt b/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/Index.kt deleted file mode 100644 index 59f6a5f7b3..0000000000 --- a/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/Index.kt +++ /dev/null @@ -1,5 +0,0 @@ -package com.bumptech.glide.annotation.ksp - -@Target(AnnotationTarget.CLASS) -@Retention(AnnotationRetention.BINARY) -annotation class Index(val modules: Array) diff --git a/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModules.kt b/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModules.kt index 2bbf0cd866..fdd3445e97 100644 --- a/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModules.kt +++ b/annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModules.kt @@ -68,7 +68,7 @@ internal class LibraryGlideModulesParser( * The output file generated by this class with a single LibraryGlideModule looks like this: * * ``` - * @com.bumptech.glide.annotation.compiler.Index( + * @com.bumptech.glide.annotation.ksp.Index( * ["com.bumptech.glide.integration.okhttp3.OkHttpLibraryGlideModule"] * ) * class Indexer_GlideModule_com_bumptech_glide_integration_okhttp3_OkHttpLibraryGlideModule @@ -80,7 +80,10 @@ internal object IndexGenerator { private const val INDEXER_NAME_PREFIX = "GlideIndexer_" private const val MAXIMUM_FILE_NAME_LENGTH = 255 - @OptIn(DelicateKotlinPoetApi::class) // We're using AnnotationSpec.builder + // The name of the parameter in the Index annotation that points to the list of modules + internal const val INDEX_MODULES_NAME = "modules" + + @OptIn(DelicateKotlinPoetApi::class) // For AnnotationSpec.builder fun generate( libraryModuleNames: List, ): FileSpec { @@ -88,7 +91,7 @@ internal object IndexGenerator { val indexAnnotation: AnnotationSpec = AnnotationSpec.builder(Index::class.java) - .addRepeatedMember(libraryModuleQualifiedNames) + .addRepeatedMember(INDEX_MODULES_NAME, libraryModuleQualifiedNames) .build() val indexName = generateUniqueName(libraryModuleQualifiedNames) @@ -124,6 +127,8 @@ internal object IndexGenerator { ) { it.replace(".", "_") } } - private fun AnnotationSpec.Builder.addRepeatedMember(repeatedMember: List) = - addMember("[\n" + "%S,\n".repeat(repeatedMember.size) + "]", *repeatedMember.toTypedArray()) + private fun AnnotationSpec.Builder.addRepeatedMember(name: String, repeatedMember: List) = + addMember( + "$name = [\n" + "%S,\n".repeat(repeatedMember.size) + "]", *repeatedMember.toTypedArray() + ) } diff --git a/annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModuleTests.kt b/annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModuleTests.kt index 8f23edd173..0671dd4a8c 100644 --- a/annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModuleTests.kt +++ b/annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModuleTests.kt @@ -2,9 +2,12 @@ package com.bumptech.glide.annotation.ksp.test import com.bumptech.glide.annotation.ksp.AppGlideModuleConstants import com.bumptech.glide.annotation.ksp.GlideSymbolProcessorConstants +import com.bumptech.glide.annotation.ksp.GlideSymbolProcessorProvider import com.google.common.truth.Truth.assertThat import com.tschuchort.compiletesting.KotlinCompilation import com.tschuchort.compiletesting.KotlinCompilation.ExitCode +import com.tschuchort.compiletesting.SourceFile +import com.tschuchort.compiletesting.symbolProcessorProviders import java.io.FileNotFoundException import kotlin.test.assertFailsWith import org.intellij.lang.annotations.Language @@ -598,6 +601,66 @@ class LibraryGlideModuleTests(override val sourceType: SourceType) : PerSourceTy .contains(AppGlideModuleConstants.INVALID_EXCLUDES_ANNOTATION_MESSAGE.format("AppModule")) } } + + @Test + fun compile_withLibraryGlideModule_compiledSeparately_includesLibraryGlideModule_2() { + val kotlinLibraryModule = + KotlinSourceFile( + "LibraryModule.kt", + """ + import com.bumptech.glide.annotation.GlideModule + import com.bumptech.glide.module.LibraryGlideModule + + @GlideModule class LibraryModule : LibraryGlideModule() + """ + ) + val javaLibraryModule = + JavaSourceFile( + "LibraryModule.java", + """ + import com.bumptech.glide.annotation.GlideModule; + import com.bumptech.glide.module.LibraryGlideModule; + + @GlideModule public class LibraryModule extends LibraryGlideModule {} + """ + ) + + val libraryCompilationResult = compileCurrentSourceType(kotlinLibraryModule, javaLibraryModule) + + val kotlinAppModule = + KotlinSourceFile( + "AppModule.kt", + """ + import com.bumptech.glide.annotation.GlideModule + import com.bumptech.glide.module.AppGlideModule + + @GlideModule class AppModule : AppGlideModule() + """ + ) + val javaAppModule = + JavaSourceFile( + "AppModule.java", + """ + import com.bumptech.glide.annotation.GlideModule; + import com.bumptech.glide.module.AppGlideModule; + + @GlideModule public class AppModule extends AppGlideModule { + public AppModule() {} + } + """ + ) + + val generatedLibrarySources = + libraryCompilationResult.allGeneratedFiles() + .map { GeneratedSourceFile(it, sourceType) } + + compileCurrentSourceType( + *(listOf(kotlinAppModule, javaAppModule) + generatedLibrarySources).toTypedArray(), + ) { + assertThat(it.generatedAppGlideModuleContents()) + .hasSourceEqualTo(appGlideModuleWithLibraryModule) + } + } } @Language("kotlin") diff --git a/annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/SourceTestHelpers.kt b/annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/SourceTestHelpers.kt index 3c81e9f939..40cb64d7d9 100644 --- a/annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/SourceTestHelpers.kt +++ b/annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/SourceTestHelpers.kt @@ -19,14 +19,35 @@ internal class CompilationResult( fun generatedAppGlideModuleContents() = readFile(findAppGlideModule()) - private fun readFile(file: File) = file.readLines().joinToString("\n") + fun allGeneratedFiles(): List { + val allFiles = mutableListOf() + val parentDir = generatedFilesParentDir() + if (parentDir != null) { + findAllFilesRecursive(parentDir, allFiles) + } + return allFiles + } - private fun findAppGlideModule(): File { + private fun findAllFilesRecursive(parent: File, allFiles: MutableList) { + if (parent.isFile) { + allFiles.add(parent) + return + } + parent.listFiles()?.map { findAllFilesRecursive(it, allFiles) } + } + + private fun generatedFilesParentDir(): File? { var currentDir: File? = compilation.kspSourcesDir listOf("kotlin", "com", "bumptech", "glide").forEach { directoryName -> currentDir = currentDir?.listFiles()?.find { it.name.equals(directoryName) } } - return currentDir?.listFiles()?.find { it.name.equals("GeneratedAppGlideModuleImpl.kt") } + return currentDir + } + + private fun readFile(file: File) = file.readLines().joinToString("\n") + + private fun findAppGlideModule(): File { + return generatedFilesParentDir()?.listFiles()?.find { it.name.equals("GeneratedAppGlideModuleImpl.kt") } ?: throw FileNotFoundException( "GeneratedAppGlideModuleImpl.kt was not generated or not generated in the expected" + "location" @@ -44,6 +65,19 @@ sealed interface TypedSourceFile { fun sourceType(): SourceType } +internal class GeneratedSourceFile( + private val file: File, private val currentSourceType: SourceType, +) : TypedSourceFile { + override fun sourceFile(): SourceFile = SourceFile.fromPath(file) + + // Hack alert: We use this class only for generated output of some previous compilation. We rely + // on the type in that previous compilation to select the proper source. The output however is + // always Kotlin, regardless of source. But we always want to include whatever the generated + // output is in the next step. That means we need our sourceType here to match the + // currentSourceType in the test. + override fun sourceType(): SourceType = currentSourceType +} + internal class KotlinSourceFile( val name: String, @Language("kotlin") val content: String, @@ -65,11 +99,12 @@ internal interface PerSourceTypeTest { fun compileCurrentSourceType( vararg sourceFiles: TypedSourceFile, - test: (input: CompilationResult) -> Unit, - ) { - test( + test: (input: CompilationResult) -> Unit = {}, + ) : CompilationResult { + val result = compile(sourceFiles.filter { it.sourceType() == sourceType }.map { it.sourceFile() }.toList()) - ) + test(result) + return result } } diff --git a/annotation/src/main/java/com/bumptech/glide/annotation/ksp/Index.java b/annotation/src/main/java/com/bumptech/glide/annotation/ksp/Index.java new file mode 100644 index 0000000000..44222755ec --- /dev/null +++ b/annotation/src/main/java/com/bumptech/glide/annotation/ksp/Index.java @@ -0,0 +1,19 @@ +package com.bumptech.glide.annotation.ksp; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Used to retrieve LibraryGlideModule and GlideExtension classes in our annotation processor from + * libraries and applications. + * + *

Part of the internals of Glide's annotation processor and not for public use. + */ +@Target(ElementType.TYPE) +// Needs to be parsed from class files in JAR. +@Retention(RetentionPolicy.CLASS) +@interface Index { + String[] modules() default {}; +}