Skip to content

Commit

Permalink
Adding a rule to suggest @BINDS over @provides
Browse files Browse the repository at this point in the history
  • Loading branch information
WhosNickDoglio committed Nov 16, 2024
1 parent c8e49c7 commit 08a1bdd
Show file tree
Hide file tree
Showing 4 changed files with 285 additions and 0 deletions.
2 changes: 2 additions & 0 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ here: [Keeping the Daggers Sharp](https://developer.squareup.com/blog/keeping-th

[//]: # (TODO mention `AppComponentFactory` and `FragmentFactory`)

## Prefer using @Binds over @Provides

### Methods annotated with `@Binds` must be abstract

Methods annotated with the [`@Binds` annotation](https://dagger.dev/api/latest/dagger/Binds.html) need to be abstract.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.google.auto.service.AutoService
import dev.whosnickdoglio.dagger.detectors.ComponentMustBeAbstractDetector
import dev.whosnickdoglio.dagger.detectors.ConstructorInjectionOverFieldInjectionDetector
import dev.whosnickdoglio.dagger.detectors.CorrectBindsUsageDetector
import dev.whosnickdoglio.dagger.detectors.FavorBindsOverProvidesDetector
import dev.whosnickdoglio.dagger.detectors.MissingModuleAnnotationDetector
import dev.whosnickdoglio.dagger.detectors.MultipleScopesDetector
import dev.whosnickdoglio.dagger.detectors.ScopedWithoutInjectAnnotationDetector
Expand All @@ -26,6 +27,7 @@ public class DaggerRulesIssueRegistry : IssueRegistry() {
ConstructorInjectionOverFieldInjectionDetector.ISSUE,
CorrectBindsUsageDetector.ISSUE_BINDS_ABSTRACT,
CorrectBindsUsageDetector.ISSUE_CORRECT_RETURN_TYPE,
FavorBindsOverProvidesDetector.ISSUE,
MissingModuleAnnotationDetector.ISSUE,
MultipleScopesDetector.ISSUE,
StaticProvidesDetector.ISSUE,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
* 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.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.android.tools.lint.detector.api.asCall
import com.intellij.psi.util.InheritanceUtil
import dev.whosnickdoglio.lint.annotations.dagger.PROVIDES
import org.jetbrains.uast.UAnnotation
import org.jetbrains.uast.UClass
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UMethod
import org.jetbrains.uast.toUElementOfType
import org.jetbrains.uast.util.isConstructorCall

// TODO make this configurable

internal class FavorBindsOverProvidesDetector : 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 == PROVIDES) {
// TODO check if provided class has `@Inject` on it's constructor
val providesMethod = node.uastParent as? UMethod ?: return

val parameterIsReturnedAsSuper =
providesMethod.checkIfMethodJustPassesThroughParameter(context)

val constructorCall =
providesMethod.checkIfMethodBodyJustCallsConstructor(context)

if (parameterIsReturnedAsSuper || constructorCall) {
context.report(
issue = ISSUE,
location = context.getNameLocation(providesMethod),
message = ISSUE.getExplanation(TextFormat.TEXT),
)
}
}
}
}

private fun UMethod.checkIfMethodJustPassesThroughParameter(context: JavaContext): Boolean {
val methodReturnType = context.evaluator.getTypeClass(this.returnType)
val parameterType = context.evaluator.getTypeClass(this.uastParameters.firstOrNull()?.type)

// CHeck if type is specifically the same type and not a supertype
return InheritanceUtil.isInheritor(
parameterType,
true,
methodReturnType?.qualifiedName.orEmpty(),
)
}

private fun UMethod.checkIfMethodBodyJustCallsConstructor(context: JavaContext): Boolean {
val methodReturnType =
context.evaluator.getTypeClass(this.returnType)?.toUElementOfType<UClass>()

val bodyReturnType =
context.evaluator
.getTypeClass(uastBody?.asCall()?.returnType)
?.toUElementOfType<UClass>()

val isConstructorCall = uastBody?.isConstructorCall() == true

val bodyReturnsSubClassOfReturnType =
InheritanceUtil.isInheritor(
bodyReturnType,
true,
methodReturnType?.qualifiedName.orEmpty(),
)
val returnCondition = isConstructorCall && bodyReturnsSubClassOfReturnType

return returnCondition
}

companion object {

internal val allowList = setOf("Factory", "Builder")

private val implementation =
Implementation(FavorBindsOverProvidesDetector::class.java, Scope.JAVA_FILE_SCOPE)
val ISSUE =
Issue.create(
id = "FavorBindsOverProvides",
briefDescription = "Using @Provides instead of @Binds",
explanation = "plz use @Binds instead of @Provides",
category = Category.CORRECTNESS,
priority = 5,
severity = Severity.WARNING,
implementation = implementation,
)
.setEnabledByDefault(false)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/*
* 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 dev.whosnickdoglio.stubs.daggerAnnotations
import org.junit.Test

class FavorBindsOverProvidesDetectorTest {

// TODO add more complicated methods (multiple params, setup before calling constructor,
// returning a lambda etc)
// or valid @Provides use cases

@Test
fun `kotlin returning @Provides method parameter directly with supertype as return type shows an warning`() {
TestLintTask.lint()
.files(
daggerAnnotations,
TestFiles.kotlin(
"""
import dagger.Provides
import dagger.Module
interface Greeter
class GreeterImpl: Greeter
@Module
object MyModule {
@Provides
fun provideAnotherGreeter(impl: GreeterImpl): Greeter = impl
}
"""
)
.indented(),
)
.issues(FavorBindsOverProvidesDetector.ISSUE)
.run()
.expect(
"""
src/Greeter.kt:11: Warning: plz use @Binds instead of @Provides [FavorBindsOverProvides]
fun provideAnotherGreeter(impl: GreeterImpl): Greeter = impl
~~~~~~~~~~~~~~~~~~~~~
0 errors, 1 warnings
"""
.trimIndent()
)
.expectWarningCount(1)
}

@Test
fun `java returning @Provides method parameter directly with supertype as return type shows an warning`() {
TestLintTask.lint()
.files(
daggerAnnotations,
TestFiles.java(
"""
import dagger.Provides;
import dagger.Module;
interface Greeter {}
class GreeterImpl extends Greeter {}
@Module
class MyModule {
@Provides
Greeter provideAnotherGreeter(GreeterImpl impl) {
return impl;
}
}
"""
)
.indented(),
)
.issues(FavorBindsOverProvidesDetector.ISSUE)
.run()
.expect(
"""
src/Greeter.java:11: Warning: plz use @Binds instead of @Provides [FavorBindsOverProvides]
Greeter provideAnotherGreeter(GreeterImpl impl) {
~~~~~~~~~~~~~~~~~~~~~
0 errors, 1 warnings
"""
.trimIndent()
)
.expectWarningCount(1)
}

@Test
fun `kotlin calling constructor directly in @Provides method instead of using @Binds method shows a warning`() {
TestLintTask.lint()
.files(
daggerAnnotations,
TestFiles.kotlin(
"""
import dagger.Provides
import dagger.Module
interface Greeter
class GreeterImpl: Greeter
@Module
object MyModule {
@Provides fun provideGreeter(): Greeter = GreeterImpl()
}
"""
)
.indented(),
)
.issues(FavorBindsOverProvidesDetector.ISSUE)
.run()
.expect("")
.expectWarningCount(1)
}

@Test
fun `java calling constructor directly in @Provides method instead of using @Binds method shows a warning`() {
TestLintTask.lint()
.files(
daggerAnnotations,
TestFiles.java(
"""
import dagger.Provides;
import dagger.Module;
interface Greeter {}
class GreeterImpl implements Greeter {}
@Module
class MyModule {
@Provides Greeter provideGreeter() {
return GreeterImpl();
}
}
"""
)
.indented(),
)
.issues(FavorBindsOverProvidesDetector.ISSUE)
.run()
.expectErrorCount(1)
.expect("")
}

@Test
fun `kotlin @Provides method that uses a Builder does not show a warning`() {
TODO("Not yet implemented")
}

@Test
fun `java @Provides method that uses a Builder does not show a warning`() {
TODO("Not yet implemented")
}

@Test
fun `kotlin @Provides method that uses a Factory does not show a warning`() {
TODO("Not yet implemented")
}

@Test
fun `java @Provides method that uses a Factory does not show a warning`() {
TODO("Not yet implemented")
}
}

0 comments on commit 08a1bdd

Please sign in to comment.