Fix dispatcher qualifier target#2114
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the @dispatcher qualifier annotation in NiaDispatchers.kt to explicitly define its targets as FUNCTION and VALUE_PARAMETER. Feedback suggests expanding these targets to include FIELD and PROPERTY to support field injection, which is a common requirement in Android development.
| import kotlin.annotation.AnnotationTarget.FUNCTION | ||
| import kotlin.annotation.AnnotationTarget.VALUE_PARAMETER | ||
|
|
||
| @Qualifier | ||
| @Target(FUNCTION, VALUE_PARAMETER) |
There was a problem hiding this comment.
When explicitly defining annotation targets for a Dagger qualifier in Kotlin, it is important to include FIELD and PROPERTY in addition to FUNCTION and VALUE_PARAMETER. This ensures that the qualifier can be used for field injection (e.g., in Activities or Fragments using lateinit var), which is a common pattern in Android development. Restricting it to only FUNCTION and VALUE_PARAMETER might break existing or future field injections.
| import kotlin.annotation.AnnotationTarget.FUNCTION | |
| import kotlin.annotation.AnnotationTarget.VALUE_PARAMETER | |
| @Qualifier | |
| @Target(FUNCTION, VALUE_PARAMETER) | |
| import kotlin.annotation.AnnotationTarget.FIELD | |
| import kotlin.annotation.AnnotationTarget.FUNCTION | |
| import kotlin.annotation.AnnotationTarget.PROPERTY | |
| import kotlin.annotation.AnnotationTarget.VALUE_PARAMETER | |
| @Qualifier | |
| @Target(FIELD, VALUE_PARAMETER, FUNCTION, PROPERTY) |
There was a problem hiding this comment.
Thanks for the suggestion. I kept the target list scoped to the current usages in this project and to #2001.
@Dispatcher is currently used on Hilt provider functions and injected value parameters, so this PR includes FUNCTION and VALUE_PARAMETER. I left out FIELD and PROPERTY because there are no current field/property injection usages for this qualifier, and adding them would broaden the allowed targets beyond the issue's requested restriction.
There was a problem hiding this comment.
Code Review
This pull request updates the @dispatcher annotation in NiaDispatchers.kt to explicitly define its target as FUNCTION and VALUE_PARAMETER. Feedback suggests expanding these targets to include FIELD and PROPERTY to better support field injection in Android components and prevent potential compilation errors.
| import kotlin.annotation.AnnotationTarget.FUNCTION | ||
| import kotlin.annotation.AnnotationTarget.VALUE_PARAMETER | ||
|
|
||
| @Qualifier | ||
| @Target(FUNCTION, VALUE_PARAMETER) |
There was a problem hiding this comment.
To support field injection (common in Android components like Activities and Fragments) and to ensure compatibility with various Kotlin property usage patterns, it is recommended to include FIELD and PROPERTY in the @Target list. This prevents potential compilation errors when the qualifier is used on properties outside of constructor parameters.
| import kotlin.annotation.AnnotationTarget.FUNCTION | |
| import kotlin.annotation.AnnotationTarget.VALUE_PARAMETER | |
| @Qualifier | |
| @Target(FUNCTION, VALUE_PARAMETER) | |
| import kotlin.annotation.AnnotationTarget.FIELD | |
| import kotlin.annotation.AnnotationTarget.FUNCTION | |
| import kotlin.annotation.AnnotationTarget.PROPERTY | |
| import kotlin.annotation.AnnotationTarget.VALUE_PARAMETER | |
| @Qualifier | |
| @Target(FIELD, FUNCTION, PROPERTY, VALUE_PARAMETER) |
What I have done and why
This PR adds explicit Kotlin annotation targets to the
@Dispatcherqualifier.@Dispatcheris used on injected value parameters and on Hilt provider functions, so the target list includes bothVALUE_PARAMETERandFUNCTION. This keeps the qualifier usage explicit while preserving the existing provider declarations.Fixes #2001
Testing
./gradlew :core:common:test./gradlew :app:compileDemoDebugKotlin./gradlew :build-logic:convention:check./gradlew spotlessCheckJAVA_HOME='/Applications/Android Studio.app/Contents/jbr/Contents/Home' ./gradlew testDemoDebug :lint:testJAVA_HOME='/Applications/Android Studio.app/Contents/jbr/Contents/Home' ./gradlew dependencyGuardJAVA_HOME='/Applications/Android Studio.app/Contents/jbr/Contents/Home' ./gradlew verifyRoborazziDemoDebugJAVA_HOME='/Applications/Android Studio.app/Contents/jbr/Contents/Home' ./gradlew :app:assemble -PminifyWithR8=falseJAVA_HOME='/Applications/Android Studio.app/Contents/jbr/Contents/Home' ./gradlew :app:lintProdRelease :app-nia-catalog:lintRelease :lint:lintJAVA_HOME='/Applications/Android Studio.app/Contents/jbr/Contents/Home' ./gradlew :app:checkProdReleaseBadging