Skip to content

Commit 5245e82

Browse files
committed
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.
1 parent f3d6ff7 commit 5245e82

File tree

6 files changed

+136
-18
lines changed

6 files changed

+136
-18
lines changed

annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/AppGlideModules.kt

+2-1
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,8 @@ internal class AppGlideModuleParser(
212212
}
213213

214214
private fun KSAnnotation.getModuleArgumentValues(): List<String> {
215-
val result = arguments.find { it.name?.getShortName().equals("modules") }?.value
215+
val result =
216+
arguments.find { it.name?.getShortName().equals(IndexGenerator.INDEX_MODULES_NAME) }?.value
216217
if (result is List<*> && result.all { it is String }) {
217218
@Suppress("UNCHECKED_CAST") return result as List<String>
218219
}

annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/Index.kt

-5
This file was deleted.

annotation/ksp/src/main/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModules.kt

+10-5
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ internal class LibraryGlideModulesParser(
6868
* The output file generated by this class with a single LibraryGlideModule looks like this:
6969
*
7070
* ```
71-
* @com.bumptech.glide.annotation.compiler.Index(
71+
* @com.bumptech.glide.annotation.ksp.Index(
7272
* ["com.bumptech.glide.integration.okhttp3.OkHttpLibraryGlideModule"]
7373
* )
7474
* class Indexer_GlideModule_com_bumptech_glide_integration_okhttp3_OkHttpLibraryGlideModule
@@ -80,15 +80,18 @@ internal object IndexGenerator {
8080
private const val INDEXER_NAME_PREFIX = "GlideIndexer_"
8181
private const val MAXIMUM_FILE_NAME_LENGTH = 255
8282

83-
@OptIn(DelicateKotlinPoetApi::class) // We're using AnnotationSpec.builder
83+
// The name of the parameter in the Index annotation that points to the list of modules
84+
internal const val INDEX_MODULES_NAME = "modules"
85+
86+
@OptIn(DelicateKotlinPoetApi::class) // For AnnotationSpec.builder
8487
fun generate(
8588
libraryModuleNames: List<LibraryModuleName>,
8689
): FileSpec {
8790
val libraryModuleQualifiedNames: List<String> = libraryModuleNames.map { it.qualifiedName }
8891

8992
val indexAnnotation: AnnotationSpec =
9093
AnnotationSpec.builder(Index::class.java)
91-
.addRepeatedMember(libraryModuleQualifiedNames)
94+
.addRepeatedMember(INDEX_MODULES_NAME, libraryModuleQualifiedNames)
9295
.build()
9396
val indexName = generateUniqueName(libraryModuleQualifiedNames)
9497

@@ -124,6 +127,8 @@ internal object IndexGenerator {
124127
) { it.replace(".", "_") }
125128
}
126129

127-
private fun AnnotationSpec.Builder.addRepeatedMember(repeatedMember: List<String>) =
128-
addMember("[\n" + "%S,\n".repeat(repeatedMember.size) + "]", *repeatedMember.toTypedArray())
130+
private fun AnnotationSpec.Builder.addRepeatedMember(name: String, repeatedMember: List<String>) =
131+
addMember(
132+
"$name = [\n" + "%S,\n".repeat(repeatedMember.size) + "]", *repeatedMember.toTypedArray()
133+
)
129134
}

annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/LibraryGlideModuleTests.kt

+63
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@ package com.bumptech.glide.annotation.ksp.test
22

33
import com.bumptech.glide.annotation.ksp.AppGlideModuleConstants
44
import com.bumptech.glide.annotation.ksp.GlideSymbolProcessorConstants
5+
import com.bumptech.glide.annotation.ksp.GlideSymbolProcessorProvider
56
import com.google.common.truth.Truth.assertThat
67
import com.tschuchort.compiletesting.KotlinCompilation
78
import com.tschuchort.compiletesting.KotlinCompilation.ExitCode
9+
import com.tschuchort.compiletesting.SourceFile
10+
import com.tschuchort.compiletesting.symbolProcessorProviders
811
import java.io.FileNotFoundException
912
import kotlin.test.assertFailsWith
1013
import org.intellij.lang.annotations.Language
@@ -598,6 +601,66 @@ class LibraryGlideModuleTests(override val sourceType: SourceType) : PerSourceTy
598601
.contains(AppGlideModuleConstants.INVALID_EXCLUDES_ANNOTATION_MESSAGE.format("AppModule"))
599602
}
600603
}
604+
605+
@Test
606+
fun compile_withLibraryGlideModule_compiledSeparately_includesLibraryGlideModule_2() {
607+
val kotlinLibraryModule =
608+
KotlinSourceFile(
609+
"LibraryModule.kt",
610+
"""
611+
import com.bumptech.glide.annotation.GlideModule
612+
import com.bumptech.glide.module.LibraryGlideModule
613+
614+
@GlideModule class LibraryModule : LibraryGlideModule()
615+
"""
616+
)
617+
val javaLibraryModule =
618+
JavaSourceFile(
619+
"LibraryModule.java",
620+
"""
621+
import com.bumptech.glide.annotation.GlideModule;
622+
import com.bumptech.glide.module.LibraryGlideModule;
623+
624+
@GlideModule public class LibraryModule extends LibraryGlideModule {}
625+
"""
626+
)
627+
628+
val libraryCompilationResult = compileCurrentSourceType(kotlinLibraryModule, javaLibraryModule)
629+
630+
val kotlinAppModule =
631+
KotlinSourceFile(
632+
"AppModule.kt",
633+
"""
634+
import com.bumptech.glide.annotation.GlideModule
635+
import com.bumptech.glide.module.AppGlideModule
636+
637+
@GlideModule class AppModule : AppGlideModule()
638+
"""
639+
)
640+
val javaAppModule =
641+
JavaSourceFile(
642+
"AppModule.java",
643+
"""
644+
import com.bumptech.glide.annotation.GlideModule;
645+
import com.bumptech.glide.module.AppGlideModule;
646+
647+
@GlideModule public class AppModule extends AppGlideModule {
648+
public AppModule() {}
649+
}
650+
"""
651+
)
652+
653+
val generatedLibrarySources =
654+
libraryCompilationResult.allGeneratedFiles()
655+
.map { GeneratedSourceFile(it, sourceType) }
656+
657+
compileCurrentSourceType(
658+
*(listOf(kotlinAppModule, javaAppModule) + generatedLibrarySources).toTypedArray(),
659+
) {
660+
assertThat(it.generatedAppGlideModuleContents())
661+
.hasSourceEqualTo(appGlideModuleWithLibraryModule)
662+
}
663+
}
601664
}
602665

603666
@Language("kotlin")

annotation/ksp/test/src/test/kotlin/com/bumptech/glide/annotation/ksp/SourceTestHelpers.kt

+42-7
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,35 @@ internal class CompilationResult(
1919

2020
fun generatedAppGlideModuleContents() = readFile(findAppGlideModule())
2121

22-
private fun readFile(file: File) = file.readLines().joinToString("\n")
22+
fun allGeneratedFiles(): List<File> {
23+
val allFiles = mutableListOf<File>()
24+
val parentDir = generatedFilesParentDir()
25+
if (parentDir != null) {
26+
findAllFilesRecursive(parentDir, allFiles)
27+
}
28+
return allFiles
29+
}
2330

24-
private fun findAppGlideModule(): File {
31+
private fun findAllFilesRecursive(parent: File, allFiles: MutableList<File>) {
32+
if (parent.isFile) {
33+
allFiles.add(parent)
34+
return
35+
}
36+
parent.listFiles()?.map { findAllFilesRecursive(it, allFiles) }
37+
}
38+
39+
private fun generatedFilesParentDir(): File? {
2540
var currentDir: File? = compilation.kspSourcesDir
2641
listOf("kotlin", "com", "bumptech", "glide").forEach { directoryName ->
2742
currentDir = currentDir?.listFiles()?.find { it.name.equals(directoryName) }
2843
}
29-
return currentDir?.listFiles()?.find { it.name.equals("GeneratedAppGlideModuleImpl.kt") }
44+
return currentDir
45+
}
46+
47+
private fun readFile(file: File) = file.readLines().joinToString("\n")
48+
49+
private fun findAppGlideModule(): File {
50+
return generatedFilesParentDir()?.listFiles()?.find { it.name.equals("GeneratedAppGlideModuleImpl.kt") }
3051
?: throw FileNotFoundException(
3152
"GeneratedAppGlideModuleImpl.kt was not generated or not generated in the expected" +
3253
"location"
@@ -44,6 +65,19 @@ sealed interface TypedSourceFile {
4465
fun sourceType(): SourceType
4566
}
4667

68+
internal class GeneratedSourceFile(
69+
private val file: File, private val currentSourceType: SourceType,
70+
) : TypedSourceFile {
71+
override fun sourceFile(): SourceFile = SourceFile.fromPath(file)
72+
73+
// Hack alert: We use this class only for generated output of some previous compilation. We rely
74+
// on the type in that previous compilation to select the proper source. The output however is
75+
// always Kotlin, regardless of source. But we always want to include whatever the generated
76+
// output is in the next step. That means we need our sourceType here to match the
77+
// currentSourceType in the test.
78+
override fun sourceType(): SourceType = currentSourceType
79+
}
80+
4781
internal class KotlinSourceFile(
4882
val name: String,
4983
@Language("kotlin") val content: String,
@@ -65,11 +99,12 @@ internal interface PerSourceTypeTest {
6599

66100
fun compileCurrentSourceType(
67101
vararg sourceFiles: TypedSourceFile,
68-
test: (input: CompilationResult) -> Unit,
69-
) {
70-
test(
102+
test: (input: CompilationResult) -> Unit = {},
103+
) : CompilationResult {
104+
val result =
71105
compile(sourceFiles.filter { it.sourceType() == sourceType }.map { it.sourceFile() }.toList())
72-
)
106+
test(result)
107+
return result
73108
}
74109
}
75110

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package com.bumptech.glide.annotation.ksp;
2+
3+
import java.lang.annotation.ElementType;
4+
import java.lang.annotation.Retention;
5+
import java.lang.annotation.RetentionPolicy;
6+
import java.lang.annotation.Target;
7+
8+
/**
9+
* Used to retrieve LibraryGlideModule and GlideExtension classes in our annotation processor from
10+
* libraries and applications.
11+
*
12+
* <p>Part of the internals of Glide's annotation processor and not for public use.
13+
*/
14+
@Target(ElementType.TYPE)
15+
// Needs to be parsed from class files in JAR.
16+
@Retention(RetentionPolicy.CLASS)
17+
@interface Index {
18+
String[] modules() default {};
19+
}

0 commit comments

Comments
 (0)