Skip to content

Commit

Permalink
Add a rule to warn against invalid methods on component interfaces
Browse files Browse the repository at this point in the history
  • Loading branch information
WhosNickDoglio committed May 28, 2024
1 parent b0db042 commit d69252a
Show file tree
Hide file tree
Showing 4 changed files with 393 additions and 3 deletions.
37 changes: 34 additions & 3 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down Expand Up @@ -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<MyOtherThing>

@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`
Expand Down Expand Up @@ -383,7 +414,7 @@ class MyFragment : Fragment() {
}
}

// Unsafe will throw UninitializedPropertyAccessException
// Unsafe will throw UninitializedPropertyAccessException
// when trying to use `something`
class MyOtherFragment : Fragment() {

Expand Down Expand Up @@ -459,7 +490,7 @@ interface MyUnsafeEntryPoint {
fun getMyClass(): MyClass
}

// Safe
// Safe
@InstallIn(SingletonComponent::class)
@EntryPoint
interface MySafeEntryPoint {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -29,6 +30,7 @@ public class DaggerRulesIssueRegistry : IssueRegistry() {
MultipleScopesDetector.ISSUE,
StaticProvidesDetector.ISSUE,
ScopedWithoutInjectAnnotationDetector.ISSUE,
ValidComponentMethodDetector.ISSUE,

Check warning on line 33 in lint/dagger/src/main/java/dev/whosnickdoglio/dagger/DaggerRulesIssueRegistry.kt

View check run for this annotation

Codecov / codecov/patch

lint/dagger/src/main/java/dev/whosnickdoglio/dagger/DaggerRulesIssueRegistry.kt#L33

Added line #L33 was not covered by tests
)

override val api: Int = CURRENT_API
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright (C) 2023 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.shared.COMPONENT
import dev.whosnickdoglio.lint.shared.SUBCOMPONENT
import org.jetbrains.uast.UAnnotation
import org.jetbrains.uast.UClass
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UMethod

// TODO can apply this to EntryPoint and ContributesTo
internal class ValidComponentMethodDetector : Detector(), SourceCodeScanner {

override fun getApplicableUastTypes(): List<Class<out UElement>> =
listOf(UAnnotation::class.java)

override fun createUastHandler(context: JavaContext): UElementHandler =
object : UElementHandler() {
override fun visitAnnotation(node: UAnnotation) {
if (node.qualifiedName == COMPONENT || node.qualifiedName == SUBCOMPONENT) {
val component = node.uastParent as? UClass ?: return

component.methods.forEach { method ->
val isValidProvisionMethod = method.isValidProvisionMethod()
val isValidMemberInjectionMethod = method.isValidMemberInjectionMethod()

if (!isValidProvisionMethod && !isValidMemberInjectionMethod) {
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 {
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,
)
}
}
Loading

0 comments on commit d69252a

Please sign in to comment.