diff --git a/docs/rules.md b/docs/rules.md index d6ee8ee2..a55cc393 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -34,7 +34,7 @@ or `Fragments`) if the creation of the class you're trying to add to the Dagger graph isn't managed by something else like the Android OS you should be using constructor injection. -```kotlin +```kotlin class MyClass { // BAD! don't do this! @@ -223,6 +223,37 @@ interface AppComponent @Singleton MyOtherClass @Inject constructor() ``` +### Valid `@Component` methods + +`@Components` and `@Subcomponents` only support two types of methods, provision methods and members-injection methods. +Trying to add any other kind of method to your component will lead to a crash at compile time. + +#### Provision methods + +Provision methods cover exposing specific dependencies from your graph via the `@Component` +and are defined as "Provision methods have no parameters and return an injected or provided type." + +```kotlin + +// All valid provision methods +@Component +interface AppComponent { + + fun myThing(): MyThing + + fun myMultipleOtherThings(): Set + + @Qualified + fun MyQualifiedThing(): MyQualifiedThing +} +``` + +#### Member-injection methods + +Member injection methods + +More information here: [`@Component` Dagger documentation](https://dagger.dev/api/latest/dagger/Component.html) + ## Anvil Rules ### Prefer using `@ContributesBinding` over `@Binds` @@ -383,7 +414,7 @@ class MyFragment : Fragment() { } } -// Unsafe will throw UninitializedPropertyAccessException +// Unsafe will throw UninitializedPropertyAccessException // when trying to use `something` class MyOtherFragment : Fragment() { @@ -459,7 +490,7 @@ interface MyUnsafeEntryPoint { fun getMyClass(): MyClass } -// Safe +// Safe @InstallIn(SingletonComponent::class) @EntryPoint interface MySafeEntryPoint { diff --git a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/Stubs.kt b/lint/annotation-constants/src/testFixtures/java/dev/whosnickdoglio/stubs/Hilt.kt similarity index 94% rename from lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/Stubs.kt rename to lint/annotation-constants/src/testFixtures/java/dev/whosnickdoglio/stubs/Hilt.kt index a8df4de5..9bfdcae9 100644 --- a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/Stubs.kt +++ b/lint/annotation-constants/src/testFixtures/java/dev/whosnickdoglio/stubs/Hilt.kt @@ -2,7 +2,7 @@ * Copyright (C) 2024 Nicholas Doglio * SPDX-License-Identifier: MIT */ -package dev.whosnickdoglio.hilt.detectors +package dev.whosnickdoglio.stubs import com.android.tools.lint.checks.infrastructure.TestFiles diff --git a/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/DaggerRulesIssueRegistry.kt b/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/DaggerRulesIssueRegistry.kt index b1eb0b65..e0e30f68 100644 --- a/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/DaggerRulesIssueRegistry.kt +++ b/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/DaggerRulesIssueRegistry.kt @@ -16,6 +16,7 @@ import dev.whosnickdoglio.dagger.detectors.MissingModuleAnnotationDetector import dev.whosnickdoglio.dagger.detectors.MultipleScopesDetector import dev.whosnickdoglio.dagger.detectors.ScopedWithoutInjectAnnotationDetector import dev.whosnickdoglio.dagger.detectors.StaticProvidesDetector +import dev.whosnickdoglio.dagger.detectors.ValidComponentMethodDetector @AutoService(IssueRegistry::class) public class DaggerRulesIssueRegistry : IssueRegistry() { @@ -29,6 +30,7 @@ public class DaggerRulesIssueRegistry : IssueRegistry() { MultipleScopesDetector.ISSUE, StaticProvidesDetector.ISSUE, ScopedWithoutInjectAnnotationDetector.ISSUE, + ValidComponentMethodDetector.ISSUE, ) override val api: Int = CURRENT_API diff --git a/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/detectors/ValidComponentMethodDetector.kt b/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/detectors/ValidComponentMethodDetector.kt new file mode 100644 index 00000000..b627d751 --- /dev/null +++ b/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/detectors/ValidComponentMethodDetector.kt @@ -0,0 +1,105 @@ +/* + * Copyright (C) 2024 Nicholas Doglio + * SPDX-License-Identifier: MIT + */ +package dev.whosnickdoglio.dagger.detectors + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Incident +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.SourceCodeScanner +import com.android.tools.lint.detector.api.TextFormat +import com.intellij.psi.PsiTypes +import dev.whosnickdoglio.lint.annotations.anvil.CONTRIBUTES_SUBCOMPONENT +import dev.whosnickdoglio.lint.annotations.anvil.MERGE_COMPONENT +import dev.whosnickdoglio.lint.annotations.anvil.MERGE_SUBCOMPONENT +import dev.whosnickdoglio.lint.annotations.dagger.COMPONENT +import dev.whosnickdoglio.lint.annotations.dagger.SUBCOMPONENT +import dev.whosnickdoglio.lint.annotations.hilt.ENTRY_POINT +import org.jetbrains.uast.UAnnotation +import org.jetbrains.uast.UClass +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UMethod + +internal class ValidComponentMethodDetector : + Detector(), + SourceCodeScanner { + + override fun getApplicableUastTypes(): List> = + listOf(UAnnotation::class.java) + + override fun createUastHandler(context: JavaContext): UElementHandler = + object : UElementHandler() { + override fun visitAnnotation(node: UAnnotation) { + if (node.qualifiedName in components) { + val component = node.uastParent as? UClass ?: return + + component.methods.forEach { method -> + val isValidProvisionMethod = method.isValidProvisionMethod() + val isValidMemberInjectionMethod = method.isValidMemberInjectionMethod() + + if (!isValidProvisionMethod && !isValidMemberInjectionMethod) { + // TODO update message based on where usage is found + context.report( + Incident( + ISSUE, + context.getLocation(method), + ISSUE.getExplanation(TextFormat.RAW), + ), + ) + } + } + } + } + } + + // no parameter with actual return type + private fun UMethod.isValidProvisionMethod(): Boolean = + parameterList.isEmpty && returnType != PsiTypes.voidType() + + // One parameter with a Unit/Void return type + private fun UMethod.isValidMemberInjectionMethod(): Boolean { + // Return type can be the same type as injected type + val numberOfParameters = parameterList.parametersCount + return numberOfParameters == 1 && returnType == PsiTypes.voidType() + } + + companion object { + internal val components = setOf( + // Vanilla Dagger + COMPONENT, + SUBCOMPONENT, + // Anvil + MERGE_COMPONENT, + MERGE_SUBCOMPONENT, + CONTRIBUTES_SUBCOMPONENT, + // Hilt + ENTRY_POINT, + ) + + private val implementation = + Implementation(ValidComponentMethodDetector::class.java, Scope.JAVA_FILE_SCOPE) + + internal val ISSUE = + Issue.create( + id = "ValidComponentMethod", + briefDescription = "Invalid `@Component` method", + explanation = + "Methods in a `@Component` interface either need to take a single parameter with no " + + "return type (member injection methods) or take no parameters and return a injected or " + + "provided type (provision methods), anything else will create a compile time error." + + "See https://whosnickdoglio.dev/dagger-rules/rules/#valid-component-methods " + + "for more information.", + category = Category.CORRECTNESS, + priority = 5, + severity = Severity.ERROR, + implementation = implementation, + ) + } +} diff --git a/lint/dagger/src/test/java/dev/whosnickdoglio/dagger/detectors/ValidComponentMethodDetectorTest.kt b/lint/dagger/src/test/java/dev/whosnickdoglio/dagger/detectors/ValidComponentMethodDetectorTest.kt new file mode 100644 index 00000000..92f1f9cb --- /dev/null +++ b/lint/dagger/src/test/java/dev/whosnickdoglio/dagger/detectors/ValidComponentMethodDetectorTest.kt @@ -0,0 +1,293 @@ +/* + * Copyright (C) 2024 Nicholas Doglio + * SPDX-License-Identifier: MIT + */ +package dev.whosnickdoglio.dagger.detectors + +import com.android.tools.lint.checks.infrastructure.TestFiles +import com.android.tools.lint.checks.infrastructure.TestLintTask +import com.google.testing.junit.testparameterinjector.TestParameter +import com.google.testing.junit.testparameterinjector.TestParameterInjector +import com.google.testing.junit.testparameterinjector.TestParameterValuesProvider +import dev.whosnickdoglio.stubs.anvilAnnotations +import dev.whosnickdoglio.stubs.daggerAnnotations +import dev.whosnickdoglio.stubs.hiltAnnotations +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(TestParameterInjector::class) +class ValidComponentMethodDetectorTest { + + private class ValidComponentMethodDetectorTestParameterValueProvider : TestParameterValuesProvider() { + override fun provideValues(context: Context?): List<*> = + ValidComponentMethodDetector.components.toList() + } + + @TestParameter(valuesProvider = ValidComponentMethodDetectorTestParameterValueProvider::class) + lateinit var component: String + + @Test + fun `kotlin component has valid provision method does not trigger error`() { + TestLintTask.lint() + .files( + daggerAnnotations, + anvilAnnotations, + *hiltAnnotations, + + TestFiles.kotlin( + """ + package com.test.android + + import $component + + @${component.substringAfterLast(".")} + interface AppComponent { + fun myThing(): String + } + """, + ) + .indented(), + ) + .issues(ValidComponentMethodDetector.ISSUE) + .run() + .expectClean() + .expectWarningCount(0) + } + + @Test + fun `java component has valid provision method does not trigger error`() { + TestLintTask.lint() + .files( + daggerAnnotations, + anvilAnnotations, + *hiltAnnotations, + TestFiles.java( + """ + package com.test.android; + + import $component; + + @${component.substringAfterLast(".")} + interface MyModule { + String myString(); + } + """, + ) + .indented(), + ) + .issues(ValidComponentMethodDetector.ISSUE) + .run() + .expectClean() + .expectWarningCount(0) + } + + @Test + fun `kotlin component with valid member injection method does not trigger error`() { + TestLintTask.lint() + .files( + daggerAnnotations, + anvilAnnotations, + *hiltAnnotations, + TestFiles.kotlin( + """ + package com.test.android + + import $component + + @${component.substringAfterLast(".")} + interface AppComponent { + fun inject(target: String) + } + """, + ) + .indented(), + ) + .issues(ValidComponentMethodDetector.ISSUE) + .run() + .expectClean() + .expectWarningCount(0) + } + + @Test + fun `java component with valid member injection method does not trigger error`() { + TestLintTask.lint() + .files( + daggerAnnotations, + anvilAnnotations, + *hiltAnnotations, + TestFiles.java( + """ + package com.test.android; + + import $component; + + @${component.substringAfterLast(".")} + interface AppComponent { + void inject(String target); + } + """, + ) + .indented(), + ) + .issues(ValidComponentMethodDetector.ISSUE) + .run() + .expectClean() + .expectWarningCount(0) + } + + @Test + fun `kotlin component with invalid member injection methods triggers error`() { + TestLintTask.lint() + .files( + daggerAnnotations, + anvilAnnotations, + *hiltAnnotations, + TestFiles.kotlin( + """ + package com.test.android + + import $component + + @${component.substringAfterLast(".")} + interface AppComponent { + fun injectWithTwoParams(target: String, otherTarget: Int) + fun injectWithReturnType(target: String): String + } + """, + ) + .indented(), + ) + .issues(ValidComponentMethodDetector.ISSUE) + .run() + .expect( + """ + src/com/test/android/AppComponent.kt:7: Error: Methods in a @Component interface either need to take a single parameter with no return type (member injection methods) or take no parameters and return a injected or provided type (provision methods), anything else will create a compile time error.See https://whosnickdoglio.dev/dagger-rules/rules/#valid-component-methods for more information. [ValidComponentMethod] + fun injectWithTwoParams(target: String, otherTarget: Int) + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + src/com/test/android/AppComponent.kt:8: Error: Methods in a @Component interface either need to take a single parameter with no return type (member injection methods) or take no parameters and return a injected or provided type (provision methods), anything else will create a compile time error.See https://whosnickdoglio.dev/dagger-rules/rules/#valid-component-methods for more information. [ValidComponentMethod] + fun injectWithReturnType(target: String): String + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 2 errors, 0 warnings + """ + .trimIndent(), + ) + .expectErrorCount(2) + } + + @Test + fun `java component with invalid member injection methods triggers error`() { + TestLintTask.lint() + .files( + daggerAnnotations, + anvilAnnotations, + *hiltAnnotations, + TestFiles.java( + """ + package com.test.android; + + import $component; + + @${component.substringAfterLast(".")} + interface AppComponent { + void injectWithTwoParams(String target, Int otherTarget); + String injectWithReturnType(String target); + } + """, + ) + .indented(), + ) + .issues(ValidComponentMethodDetector.ISSUE) + .run() + .expect( + """ + src/com/test/android/AppComponent.java:7: Error: Methods in a @Component interface either need to take a single parameter with no return type (member injection methods) or take no parameters and return a injected or provided type (provision methods), anything else will create a compile time error.See https://whosnickdoglio.dev/dagger-rules/rules/#valid-component-methods for more information. [ValidComponentMethod] + void injectWithTwoParams(String target, Int otherTarget); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + src/com/test/android/AppComponent.java:8: Error: Methods in a @Component interface either need to take a single parameter with no return type (member injection methods) or take no parameters and return a injected or provided type (provision methods), anything else will create a compile time error.See https://whosnickdoglio.dev/dagger-rules/rules/#valid-component-methods for more information. [ValidComponentMethod] + String injectWithReturnType(String target); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 2 errors, 0 warnings + """ + .trimIndent(), + ) + .expectErrorCount(2) + } + + @Test + fun `kotlin component with invalid provision methods triggers error`() { + TestLintTask.lint() + .files( + daggerAnnotations, + anvilAnnotations, + *hiltAnnotations, + TestFiles.kotlin( + """ + package com.test.android + + import $component + + @${component.substringAfterLast(".")} + interface AppComponent { + fun myThingWithParameter(otherThing: String): String + fun myThingWithNoReturnType() + } + """, + ) + .indented(), + ) + .issues(ValidComponentMethodDetector.ISSUE) + .run() + .expect( + """ + src/com/test/android/AppComponent.kt:7: Error: Methods in a @Component interface either need to take a single parameter with no return type (member injection methods) or take no parameters and return a injected or provided type (provision methods), anything else will create a compile time error.See https://whosnickdoglio.dev/dagger-rules/rules/#valid-component-methods for more information. [ValidComponentMethod] + fun myThingWithParameter(otherThing: String): String + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + src/com/test/android/AppComponent.kt:8: Error: Methods in a @Component interface either need to take a single parameter with no return type (member injection methods) or take no parameters and return a injected or provided type (provision methods), anything else will create a compile time error.See https://whosnickdoglio.dev/dagger-rules/rules/#valid-component-methods for more information. [ValidComponentMethod] + fun myThingWithNoReturnType() + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 2 errors, 0 warnings + """ + .trimIndent(), + ) + .expectErrorCount(2) + } + + @Test + fun `java component with invalid provision methods triggers error`() { + TestLintTask.lint() + .files( + daggerAnnotations, + anvilAnnotations, + *hiltAnnotations, + TestFiles.java( + """ + package com.test.android; + + import $component; + + @${component.substringAfterLast(".")} + interface AppComponent { + String myThingWithParameter(String otherThing); + void myThingWithNoReturnType(); + } + """, + ) + .indented(), + ) + .issues(ValidComponentMethodDetector.ISSUE) + .run() + .expect( + """ + src/com/test/android/AppComponent.java:7: Error: Methods in a @Component interface either need to take a single parameter with no return type (member injection methods) or take no parameters and return a injected or provided type (provision methods), anything else will create a compile time error.See https://whosnickdoglio.dev/dagger-rules/rules/#valid-component-methods for more information. [ValidComponentMethod] + String myThingWithParameter(String otherThing); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + src/com/test/android/AppComponent.java:8: Error: Methods in a @Component interface either need to take a single parameter with no return type (member injection methods) or take no parameters and return a injected or provided type (provision methods), anything else will create a compile time error.See https://whosnickdoglio.dev/dagger-rules/rules/#valid-component-methods for more information. [ValidComponentMethod] + void myThingWithNoReturnType(); + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 2 errors, 0 warnings + """ + .trimIndent(), + ) + .expectErrorCount(2) + } +} diff --git a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/EntryPointMustBeAnInterfaceDetectorTest.kt b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/EntryPointMustBeAnInterfaceDetectorTest.kt index 48fd4ce3..3d857f6d 100644 --- a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/EntryPointMustBeAnInterfaceDetectorTest.kt +++ b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/EntryPointMustBeAnInterfaceDetectorTest.kt @@ -7,6 +7,7 @@ package dev.whosnickdoglio.hilt.detectors import com.android.tools.lint.checks.infrastructure.TestFiles import com.android.tools.lint.checks.infrastructure.TestLintTask import dev.whosnickdoglio.lint.annotations.hilt.ENTRY_POINT +import dev.whosnickdoglio.stubs.hiltAnnotations import org.junit.Test class EntryPointMustBeAnInterfaceDetectorTest { diff --git a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingAndroidEntryPointDetectorTest.kt b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingAndroidEntryPointDetectorTest.kt index 4aa9b1f4..5ce7ac50 100644 --- a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingAndroidEntryPointDetectorTest.kt +++ b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingAndroidEntryPointDetectorTest.kt @@ -9,6 +9,7 @@ import com.android.tools.lint.checks.infrastructure.TestLintTask import com.google.testing.junit.testparameterinjector.TestParameter import com.google.testing.junit.testparameterinjector.TestParameterInjector import dev.whosnickdoglio.lint.annotations.hilt.ANDROID_ENTRY_POINT +import dev.whosnickdoglio.stubs.hiltAnnotations import dev.whosnickdoglio.stubs.javaxAnnotations import org.junit.Test import org.junit.runner.RunWith diff --git a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingHiltAndroidAppAnnotationDetectorTest.kt b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingHiltAndroidAppAnnotationDetectorTest.kt index a1f92a6e..6324fece 100644 --- a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingHiltAndroidAppAnnotationDetectorTest.kt +++ b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingHiltAndroidAppAnnotationDetectorTest.kt @@ -7,6 +7,7 @@ package dev.whosnickdoglio.hilt.detectors import com.android.tools.lint.checks.infrastructure.TestFiles import com.android.tools.lint.checks.infrastructure.TestLintTask import dev.whosnickdoglio.lint.annotations.hilt.HILT_ANDROID_APP +import dev.whosnickdoglio.stubs.hiltAnnotations import org.junit.Test class MissingHiltAndroidAppAnnotationDetectorTest { diff --git a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingHiltViewModelAnnotationDetectorTest.kt b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingHiltViewModelAnnotationDetectorTest.kt index 0425dcb0..31a4c451 100644 --- a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingHiltViewModelAnnotationDetectorTest.kt +++ b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingHiltViewModelAnnotationDetectorTest.kt @@ -7,6 +7,7 @@ package dev.whosnickdoglio.hilt.detectors import com.android.tools.lint.checks.infrastructure.TestFiles import com.android.tools.lint.checks.infrastructure.TestLintTask import dev.whosnickdoglio.lint.annotations.hilt.HILT_VIEW_MODEL +import dev.whosnickdoglio.stubs.hiltAnnotations import dev.whosnickdoglio.stubs.javaxAnnotations import org.junit.Test diff --git a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingInstallInDetectorTest.kt b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingInstallInDetectorTest.kt index afe660d7..02191138 100644 --- a/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingInstallInDetectorTest.kt +++ b/lint/hilt/src/test/java/dev/whosnickdoglio/hilt/detectors/MissingInstallInDetectorTest.kt @@ -7,6 +7,7 @@ package dev.whosnickdoglio.hilt.detectors import com.android.tools.lint.checks.infrastructure.TestFiles import com.android.tools.lint.checks.infrastructure.TestLintTask import dev.whosnickdoglio.stubs.daggerAnnotations +import dev.whosnickdoglio.stubs.hiltAnnotations import org.junit.Test class MissingInstallInDetectorTest {