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.

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

Enforce scope parameter on all @Contributes* annotations #42

Merged
merged 1 commit into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
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
60 changes: 19 additions & 41 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,61 +179,39 @@ every component in your project.

### Scopes

The plugin builds a connection between contributions and merged components through the scope.
How scopes function with `kotlin-inject` is described in the
[documentation](https://github.com/evant/kotlin-inject#scopes).

`kotlin-inject` supports scopes with and without parameters. For `kotlin-inject-anvil` we decided
to prefer scope references as parameter to contribute and merge types as the
[`@SingleIn` annotation](runtime-optional/src/commonMain/kotlin/software/amazon/lastmile/kotlin/inject/anvil/SingleIn.kt)
defines it:
The plugin builds a connection between contributions and merged components through the scope
parameters. Scope classes are only markers and have no further meaning besides building a
connection between contributions and merging them. The class `AppScope` from the sample could
look like this:
```kotlin
object AppScope
```

Scope classes are independent of the `kotlin-inject`
[scopes](https://github.com/evant/kotlin-inject#scopes). It's still necessary to set a scope for
the `kotlin-inject` components or to make instances a singleton in a scope, e.g.
```kotlin
@Inject
@SingleIn(AppScope::class)
@SingleIn(AppScope::class) // scope for kotlin-inject
@ContributesBinding(AppScope::class)
class RealAuthenticator : Authenticator

@Component
@MergeComponent(AppScope::class)
@SingleIn(AppScope::class)
interface AppComponent : AppComponentMerged
@SingleIn(AppScope::class) // scope for kotlin-inject
interface AppComponent
```
The `@SingleIn` annotation needs to be explicitly imported!

`kotlin-inject-anvil` provides the
[`@SingleIn` scope annotation](runtime-optional/src/commonMain/kotlin/software/amazon/lastmile/kotlin/inject/anvil/SingleIn.kt)
optionally by importing following module. We strongly recommend to use the annotation for
consistency.
```groovy
dependencies {
commonMainImplementation "software.amazon.lastmile.kotlin.inject.anvil:runtime-optional:$version"
}
```

However, scopes without parameters are also supported, such as:
```kotlin
import me.tatarka.inject.annotations.Scope

@Scope
annotation class Singleton
```
Instead of using the `scope` parameter on the `@Contributes*` annotations, you'd add this
annotation on the class itself and `kotlin-anvil-inject` will still build the correct
connections and merge the code:
```kotlin
@ContributesTo
@Singleton
interface AppIdComponent {
@Provides
fun provideAppId(): String = "demo app"
}

@Inject
@Singleton
@ContributesBinding
class RealAuthenticator : Authenticator

@Component
@MergeComponent
@Singleton
interface AppComponent : AppComponentMerged
```

## Sample

A [sample project](sample) for Android and iOS is available.
Expand Down
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
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,
)
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()
}
}
Loading