Adding a rule to suggest @Binds over @Provides

docs/rules.md

@@ -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.

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

@@ -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
@@ -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,

lint/dagger/src/main/java/dev/whosnickdoglio/dagger/detectors/FavorBindsOverProvidesDetector.kt

@@ -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)
+    }
+}

lint/dagger/src/test/java/dev/whosnickdoglio/dagger/detectors/FavorBindsOverProvidesDetectorTest.kt

@@ -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")
+    }
+}