-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
- Loading branch information
1 parent
6a40054
commit 2dfbb58
Showing
4 changed files
with
284 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
109 changes: 109 additions & 0 deletions
109
...agger/src/main/java/dev/whosnickdoglio/dagger/detectors/FavorBindsOverProvidesDetector.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| /* | ||
| * 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.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.shared.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) { | ||
| 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) | ||
| } | ||
| } |
171 changes: 171 additions & 0 deletions
171
...r/src/test/java/dev/whosnickdoglio/dagger/detectors/FavorBindsOverProvidesDetectorTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,171 @@ | ||
| /* | ||
| * Copyright (C) 2023 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") | ||
| } | ||
| } |