Skip to content

Commit

Permalink
Enforce scope parameter on all @Contributes* annotations
Browse files Browse the repository at this point in the history 
Enforce scope parameter on all `@Contributes*` annotations and stop using the kotlin-inject scope implicitly. This aligns the library with the original Anvil implementation for Dagger 2 and brings more consistency.

Resolves #36
  • Loading branch information
@vRallev
vRallev committed Sep 23, 2024
1 parent c0a9b5a commit d7cdd7e
Show file tree
Hide file tree
Showing 20 changed files with 340 additions and 1,112 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

### Changed

* **BREAKING CHANGE:** Enforce scope parameter on all `@Contributes*` annotations and stop using the kotlin-inject scope implicitly, see #36.

### Deprecated

### Removed
Expand Down
91 changes: 37 additions & 54 deletions compiler/src/main/kotlin/software/amazon/lastmile/kotlin/inject/anvil/ContextAware.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,31 +71,24 @@ internal interface ContextAware {

fun KSClassDeclaration.scope(): MergeScope {
return requireNotNull(scopeOrNull(), this) {
"Couldn't find scope annotation for $this."
"Couldn't find scope for $this."
}
}

private fun KSClassDeclaration.scopeOrNull(): MergeScope? {
val annotationsWithScopeParameter = annotations.filter {
// Avoid scope annotations themselves, e.g. that skips `@SingleIn` and include
// annotations with a "scope" parameter, e.g. `@ContributesTo`.
!isScopeAnnotation(it) && it.hasScopeParameter()
}.toList()

return if (annotationsWithScopeParameter.isEmpty()) {
annotations.firstOrNull { isScopeAnnotation(it) }
?.let { MergeScope(this@ContextAware, it) }
} else {
scopeForAnnotationsWithScopeParameters(this, annotationsWithScopeParameter)
}
val annotationsWithScopeParameter = annotations.filter { it.hasScopeParameter() }
.toList()
.ifEmpty { return null }

return scopeForAnnotationsWithScopeParameters(this, annotationsWithScopeParameter)
}

fun isScopeAnnotation(annotation: KSAnnotation): Boolean {
return isScopeAnnotation(annotation.annotationType.resolve())
fun KSAnnotation.isKotlinInjectScopeAnnotation(): Boolean {
return annotationType.resolve().isKotlinInjectScopeAnnotation()
}

private fun isScopeAnnotation(type: KSType): Boolean {
return type.declaration.annotations.any {
private fun KSType.isKotlinInjectScopeAnnotation(): Boolean {
return declaration.annotations.any {
it.annotationType.resolve().declaration.requireQualifiedName() == scopeFqName
}
}
Expand All @@ -109,50 +102,31 @@ internal interface ContextAware {
clazz: KSClassDeclaration,
annotations: List<KSAnnotation>,
): MergeScope {
val explicitScopes = annotations.mapNotNull { annotation ->
annotation.scopeParameter(this)
val explicitScopes = annotations.map { annotation ->
annotation.scopeParameter()
}

val classScope = clazz.annotations.firstOrNull { isScopeAnnotation(it) }
?.let { MergeScope(this, it) }

if (explicitScopes.isNotEmpty()) {
check(explicitScopes.size == annotations.size, clazz) {
"If one annotation has an explicit scope, then all " +
"annotations must specify an explicit scope."
explicitScopes.scan(
explicitScopes.first().declaration.requireQualifiedName(),
) { previous, next ->
check(previous == next.declaration.requireQualifiedName(), clazz) {
"All scopes on annotations must be the same."
}
previous
}

explicitScopes.scan(
explicitScopes.first().declaration.requireQualifiedName(),
) { previous, next ->
check(previous == next.declaration.requireQualifiedName(), clazz) {
"All explicit scopes on annotations must be the same."
}
previous
}
return MergeScope(explicitScopes.first())
}

val explicitScope = explicitScopes.first()
val explicitScopeIsScope = isScopeAnnotation(explicitScope)

return if (explicitScopeIsScope) {
MergeScope(
contextAware = this,
annotationType = explicitScope,
markerType = null,
)
} else {
MergeScope(
contextAware = this,
annotationType = null,
markerType = explicitScope,
)
}
private fun KSAnnotation.scopeParameter(): KSType {
return requireNotNull(scopeParameterOrNull(), this) {
"Couldn't find a scope parameter."
}
}

return requireNotNull(classScope, clazz) {
"Couldn't find scope for ${clazz.simpleName.asString()}. For unscoped " +
"objects it is required to specify the target scope on the annotation."
}
private fun KSAnnotation.scopeParameterOrNull(): KSType? {
return arguments.firstOrNull { it.name?.asString() == "scope" }
?.let { it.value as? KSType }
}

fun KSClassDeclaration.origin(): KSClassDeclaration {
Expand Down Expand Up @@ -184,6 +158,15 @@ internal interface ContextAware {
return getDeclaredFunctions().filter { it.isAbstract }
}

fun requireKotlinInjectScope(clazz: KSClassDeclaration): KSAnnotation {
return requireNotNull(
clazz.annotations.firstOrNull { it.isKotlinInjectScopeAnnotation() },
clazz,
) {
"A kotlin-inject scope like @SingleIn(Abc::class) is missing."
}
}

fun KSDeclaration.requireContainingFile(): KSFile = requireNotNull(containingFile, this) {
"Containing file was null for $this"
}
Expand Down
147 changes: 5 additions & 142 deletions compiler/src/main/kotlin/software/amazon/lastmile/kotlin/inject/anvil/MergeScope.kt
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,10 @@
package software.amazon.lastmile.kotlin.inject.anvil

import com.google.devtools.ksp.symbol.KSAnnotation
import com.google.devtools.ksp.symbol.KSType
import com.squareup.kotlinpoet.AnnotationSpec
import com.squareup.kotlinpoet.ksp.toAnnotationSpec
import com.squareup.kotlinpoet.ksp.toClassName

/**
* Represents the destination of contributed types and which types should be merged during
* the merge phase. There is complexity to this problem, because `kotlin-inject` didn't
* support parameters for scopes initially and our Anvil extensions added support for that. Later,
* we started supporting parameters, which changed the API. E.g. one could use:
* the merge phase, e.g.
* ```
* @ContributesTo(AppScope::class)
* interface ContributedComponentInterface
Expand All @@ -19,139 +13,8 @@ import com.squareup.kotlinpoet.ksp.toClassName
* @MergeComponent(AppScope::class)
* interface MergedComponent
* ```
* Or the old way:
* ```
* @ContributesTo
* @Singleton
* interface ContributedComponentInterface
*
* @Component
* @MergeComponent
* @Singleton
* interface MergedComponent
* ```
* Where `AppScope` would represent the "MergeScope".
*/
internal sealed class MergeScope {
/**
* The fully qualified name of the annotation used as scope, e.g.
* ```
* @ContributesTo
* @Singleton
* interface Abc
* ```
* Note that the annotation itself is annotated with `@Scope`.
*
* The value is `null`, when only a marker is used, e.g.
* ```
* @ContributesTo(AppScope::class)
* interface Abc
* ```
*
* If the `scope` parameter is used and the argument is annotated with `@Scope`, then
* this value is non-null, e.g. for this:
* ```
* @ContributesBinding(scope = Singleton::class)
* class Binding : SuperType
* ```
*/
abstract val annotationFqName: String?

/**
* A marker for a scope that isn't itself annotated with `@Scope`, e.g.
* ```
* @ContributesTo(AppScope::class)
* interface Abc
* ```
*
* The value is null, if no marker is used, e.g.
* ```
* @ContributesTo
* @Singleton
* interface Abc
* ```
*
* The value is also null, when the `scope` parameter is used and the argument is annotated
* with `@Scope`, e.g.
* ```
* @ContributesBinding(scope = Singleton::class)
* class Binding : SuperType
* ```
*/
abstract val markerFqName: String?

/**
* A reference to the scope.
*
* [markerFqName] is preferred, because it allows us to decouple contributions from
* kotlin-inject's scoping mechanism. E.g. imagine someone using `@Singleton` as a scope, and
* they'd like to adopt kotlin-inject-anvil with `@ContributesTo(AppScope::class)`. Because we
* prefer the marker, this would be supported.
*/
val fqName: String get() = requireNotNull(markerFqName ?: annotationFqName)

abstract fun toAnnotationSpec(): AnnotationSpec

override fun equals(other: Any?): Boolean {
if (this === other) return true
if (other !is MergeScope) return false

if (fqName != other.fqName) return false

return true
}

override fun hashCode(): Int {
return fqName.hashCode()
}

private class MarkerBasedMergeScope(
override val annotationFqName: String,
override val markerFqName: String?,
private val ksAnnotation: KSAnnotation,
) : MergeScope() {
override fun toAnnotationSpec(): AnnotationSpec {
return ksAnnotation.toAnnotationSpec()
}
}

private class AnnotationBasedMergeScope(
override val annotationFqName: String?,
override val markerFqName: String?,
private val ksType: KSType,
) : MergeScope() {
override fun toAnnotationSpec(): AnnotationSpec {
return AnnotationSpec.builder(ksType.toClassName()).build()
}
}

companion object {
operator fun invoke(
contextAware: ContextAware,
annotationType: KSType?,
markerType: KSType?,
): MergeScope {
val nonNullType = contextAware.requireNotNull(markerType ?: annotationType, null) {
"Couldn't determine scope. No scope annotation nor marker found."
}

return AnnotationBasedMergeScope(
annotationFqName = annotationType?.declaration?.requireQualifiedName(contextAware),
markerFqName = markerType?.declaration?.requireQualifiedName(contextAware),
ksType = nonNullType,
)
}

operator fun invoke(
contextAware: ContextAware,
ksAnnotation: KSAnnotation,
): MergeScope {
return MarkerBasedMergeScope(
annotationFqName = ksAnnotation.annotationType.resolve().declaration
.requireQualifiedName(contextAware),
markerFqName = ksAnnotation.scopeParameter(contextAware)?.declaration
?.requireQualifiedName(contextAware),
ksAnnotation = ksAnnotation,
)
}
}
}
internal data class MergeScope(
val type: KSType,
)
10 changes: 0 additions & 10 deletions compiler/src/main/kotlin/software/amazon/lastmile/kotlin/inject/anvil/Util.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import com.google.devtools.ksp.isDefault
import com.google.devtools.ksp.symbol.KSAnnotation
import com.google.devtools.ksp.symbol.KSClassDeclaration
import com.google.devtools.ksp.symbol.KSDeclaration
import com.google.devtools.ksp.symbol.KSType
import com.google.devtools.ksp.symbol.KSValueArgument
import com.squareup.kotlinpoet.Annotatable
import com.squareup.kotlinpoet.AnnotationSpec
Expand Down Expand Up @@ -74,12 +73,3 @@ internal fun KSDeclaration.requireQualifiedName(contextAware: ContextAware): Str
internal fun KClass<*>.requireQualifiedName(): String = requireNotNull(qualifiedName) {
"Qualified name was null for $this"
}

internal fun KSAnnotation.scopeParameter(contextAware: ContextAware): KSType? {
return arguments.firstOrNull { it.name?.asString() == "scope" }
?.let { it.value as? KSType }
?.takeIf {
it.declaration.requireQualifiedName(contextAware) !=
Unit::class.requireQualifiedName()
}
}
2 changes: 2 additions & 0 deletions .../amazon/lastmile/kotlin/inject/anvil/processor/ContributesSubcomponentFactoryProcessor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ internal class ContributesSubcomponentFactoryProcessor(
"Factory interfaces must be inner classes of the contributed subcomponent, which " +
"need to be annotated with @ContributesSubcomponent."
}

requireKotlinInjectScope(subcomponent)
}

private fun checkSingleFunction(factory: KSClassDeclaration) {
Expand Down
35 changes: 11 additions & 24 deletions ...oftware/amazon/lastmile/kotlin/inject/anvil/processor/ContributesSubcomponentProcessor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import com.squareup.kotlinpoet.KModifier
import com.squareup.kotlinpoet.ParameterSpec
import com.squareup.kotlinpoet.PropertySpec
import com.squareup.kotlinpoet.TypeSpec
import com.squareup.kotlinpoet.ksp.toAnnotationSpec
import com.squareup.kotlinpoet.ksp.toClassName
import com.squareup.kotlinpoet.ksp.toTypeName
import com.squareup.kotlinpoet.ksp.writeTo
Expand All @@ -22,7 +23,6 @@ import software.amazon.lastmile.kotlin.inject.anvil.ContextAware
import software.amazon.lastmile.kotlin.inject.anvil.ContributesSubcomponent
import software.amazon.lastmile.kotlin.inject.anvil.LOOKUP_PACKAGE
import software.amazon.lastmile.kotlin.inject.anvil.MergeComponent
import software.amazon.lastmile.kotlin.inject.anvil.MergeScope
import software.amazon.lastmile.kotlin.inject.anvil.addOriginAnnotation
import software.amazon.lastmile.kotlin.inject.anvil.internal.Subcomponent

Expand Down Expand Up @@ -117,14 +117,7 @@ internal class ContributesSubcomponentProcessor(
factoryInterface: KSClassDeclaration,
generatedFactoryInterface: KSClassDeclaration,
): ClassName {
val scope = requireNotNull(
value = subcomponent.annotations
.firstOrNull { isScopeAnnotation(it) }
?.let { MergeScope(this, it) },
symbol = subcomponent,
) {
"A scope like @SingleIn(Abc::class) is missing."
}
val kotlinInjectScope = requireKotlinInjectScope(subcomponent)

val function = factoryInterface.factoryFunctions().single()

Expand All @@ -142,22 +135,16 @@ internal class ContributesSubcomponentProcessor(
TypeSpec
.classBuilder(finalComponentClassName)
.addAnnotation(Component::class)
.apply {
val scopeOnSubcomponentAnnotation = subcomponent.scope()
if (scopeOnSubcomponentAnnotation.markerFqName != null) {
addAnnotation(
AnnotationSpec.builder(MergeComponent::class)
.addMember(
"scope = %T::class",
ClassName.bestGuess(scopeOnSubcomponentAnnotation.fqName),
)
.build(),
.addAnnotation(
AnnotationSpec
.builder(MergeComponent::class)
.addMember(
"scope = %T::class",
subcomponent.scope().type.toClassName(),
)
} else {
addAnnotation(MergeComponent::class)
}
}
.addAnnotation(scope.toAnnotationSpec())
.build(),
)
.addAnnotation(kotlinInjectScope.toAnnotationSpec())
.addOriginAnnotation(subcomponent)
.addModifiers(KModifier.ABSTRACT)
.primaryConstructor(
Expand Down
Loading

0 comments on commit d7cdd7e

Please sign in to comment.