Skip to content

Commit

Permalink
Move the Index annotation for KSP to annotations.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sjudd committed Oct 6, 2022
1 parent f9b508b commit 463688b
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 18 deletions.
Expand Up @@ -212,7 +212,8 @@ internal class AppGlideModuleParser(
}

private fun KSAnnotation.getModuleArgumentValues(): List<String> {
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<String>
}
Expand Down

This file was deleted.

Expand Up @@ -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
Expand All @@ -80,15 +80,18 @@ 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<LibraryModuleName>,
): FileSpec {
val libraryModuleQualifiedNames: List<String> = libraryModuleNames.map { it.qualifiedName }

val indexAnnotation: AnnotationSpec =
AnnotationSpec.builder(Index::class.java)
.addRepeatedMember(libraryModuleQualifiedNames)
.addRepeatedMember(INDEX_MODULES_NAME, libraryModuleQualifiedNames)
.build()
val indexName = generateUniqueName(libraryModuleQualifiedNames)

Expand Down Expand Up @@ -124,6 +127,8 @@ internal object IndexGenerator {
) { it.replace(".", "_") }
}

private fun AnnotationSpec.Builder.addRepeatedMember(repeatedMember: List<String>) =
addMember("[\n" + "%S,\n".repeat(repeatedMember.size) + "]", *repeatedMember.toTypedArray())
private fun AnnotationSpec.Builder.addRepeatedMember(name: String, repeatedMember: List<String>) =
addMember(
"$name = [\n" + "%S,\n".repeat(repeatedMember.size) + "]", *repeatedMember.toTypedArray()
)
}
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand Down
Expand Up @@ -19,14 +19,35 @@ internal class CompilationResult(

fun generatedAppGlideModuleContents() = readFile(findAppGlideModule())

private fun readFile(file: File) = file.readLines().joinToString("\n")
fun allGeneratedFiles(): List<File> {
val allFiles = mutableListOf<File>()
val parentDir = generatedFilesParentDir()
if (parentDir != null) {
findAllFilesRecursive(parentDir, allFiles)
}
return allFiles
}

private fun findAppGlideModule(): File {
private fun findAllFilesRecursive(parent: File, allFiles: MutableList<File>) {
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"
Expand All @@ -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,
Expand All @@ -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
}
}

Expand Down
@@ -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.
*
* <p>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 {};
}

0 comments on commit 463688b

Please sign in to comment.