diff --git a/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetector.kt b/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetector.kt index 9ff5128d..3c3e5129 100644 --- a/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetector.kt +++ b/lint/dagger/src/main/java/dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetector.kt @@ -5,6 +5,7 @@ package dev.whosnickdoglio.dagger.detectors import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.BooleanOption import com.android.tools.lint.detector.api.Category import com.android.tools.lint.detector.api.Detector import com.android.tools.lint.detector.api.Implementation @@ -26,22 +27,20 @@ internal class ConstructorInjectionOverFieldInjectionDetector : Detector(), Sour override fun getApplicableUastTypes(): List> = listOf(UAnnotation::class.java) - override fun createUastHandler(context: JavaContext): UElementHandler { - return object : UElementHandler() { + override fun createUastHandler(context: JavaContext): UElementHandler = + object : UElementHandler() { override fun visitAnnotation(node: UAnnotation) { if (node.qualifiedName == INJECT) { val annotatedElement = node.uastParent as? UField ?: return - // val fullAllowList: List = - // if (context.mainProject.minSdk >= MIN_SDK) { - // - // allowList.defaultValue?.split(",").orEmpty() - // } else { - // - // allowList.defaultValue?.split(",").orEmpty() + androidComponents - // } + val fullAllowList: List = + if (usingAppComponentFactory.getValue(context)) { + allowList.getValue(context)?.split(",").orEmpty() + } else { + allowList.getValue(context)?.split(",").orEmpty() + androidComponents + } val isAllowedFieldInjection = - androidComponents.any { className -> + fullAllowList.any { className -> context.evaluator.extendsClass( cls = annotatedElement.getContainingUClass(), className = className, @@ -49,7 +48,6 @@ internal class ConstructorInjectionOverFieldInjectionDetector : Detector(), Sour ) } - // minSdkLessThan(28) if (!isAllowedFieldInjection) { context.report( Incident( @@ -63,13 +61,8 @@ internal class ConstructorInjectionOverFieldInjectionDetector : Detector(), Sour } } } - } companion object { - // private const val MIN_SDK = 28 - - // TODO make this configurable - @Suppress("UnusedPrivateMember") private val allowList = StringOption( name = "allowList", @@ -78,8 +71,10 @@ internal class ConstructorInjectionOverFieldInjectionDetector : Detector(), Sour explanation = "", ) - // - private val androidComponents = + internal val usingAppComponentFactory = + BooleanOption(name = "usingAppComponentFactory", description = "TOOD", explanation = "") + + internal val androidComponents = setOf( // https://developer.android.com/reference/android/app/AppComponentFactory "android.app.Activity", @@ -100,18 +95,19 @@ internal class ConstructorInjectionOverFieldInjectionDetector : Detector(), Sour internal val ISSUE = Issue.create( - id = "ConstructorOverField", - briefDescription = "Class is using field injection over constructor injection", - explanation = - """ + id = "ConstructorOverField", + briefDescription = "Class is using field injection over constructor injection", + explanation = + """ Constructor injection should be favored over field injection for classes that support it. See https://whosnickdoglio.dev/dagger-rules/rules/#prefer-constructor-injection-over-field-injection for more information. """, - category = Category.CORRECTNESS, - priority = 5, - severity = Severity.WARNING, - implementation = implementation, - ) + category = Category.CORRECTNESS, + priority = 5, + severity = Severity.WARNING, + implementation = implementation, + ) + .setOptions(listOf(usingAppComponentFactory, allowList)) } } diff --git a/lint/dagger/src/test/java/dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetectorTest.kt b/lint/dagger/src/test/java/dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetectorTest.kt index 9cb193ad..fe2f2c4a 100644 --- a/lint/dagger/src/test/java/dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetectorTest.kt +++ b/lint/dagger/src/test/java/dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetectorTest.kt @@ -188,6 +188,130 @@ class ConstructorInjectionOverFieldInjectionDetectorTest { .expectWarningCount(1) } + @Test + fun `android subclass in kotlin does triggers member injection warning when usingAppComponentFactory is enabled`() { + TestLintTask.lint() + .files( + javaxAnnotations, + TestFiles.kotlin( + """ + package ${component.classPackage} + + class ${component.className} + + """ + ) + .indented(), + TestFiles.kotlin( + """ + package androidx + + import ${component.classImport} + + class AndroidX${component.className}: ${component.className} + """ + ) + .indented(), + TestFiles.kotlin( + """ + package com.test.android + + import javax.inject.Inject + import androidx.AndroidX${component.className} + + class Something + + class My${component.className}: AndroidX${component.className} { + + @Inject lateinit var something: Something + + } + """ + ) + .indented(), + ) + .issues(ConstructorInjectionOverFieldInjectionDetector.ISSUE) + .configureOption( + ConstructorInjectionOverFieldInjectionDetector.usingAppComponentFactory, + true, + ) + .run() + .expect( + """ + src/com/test/android/Something.kt:10: Warning: Constructor injection should be favored over field injection for classes that support it. + + See https://whosnickdoglio.dev/dagger-rules/rules/#prefer-constructor-injection-over-field-injection for more information. [ConstructorOverField] + @Inject lateinit var something: Something + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 0 errors, 1 warnings + """ + .trimIndent() + ) + .expectWarningCount(1) + } + + @Test + fun `android subclass in java does triggers member injection warning when usingAppComponentFactory is enabled`() { + TestLintTask.lint() + .files( + javaxAnnotations, + TestFiles.java( + """ + package ${component.classPackage}; + + class ${component.className} {} + + """ + ) + .indented(), + TestFiles.java( + """ + package androidx; + + import ${component.classImport}; + + class AndroidX${component.className} extends ${component.className} {} + """ + ) + .indented(), + TestFiles.java( + """ + package com.test.android; + + import javax.inject.Inject; + import androidx.AndroidX${component.className}; + + class Something {} + + class My${component.className} extends AndroidX${component.className} { + + @Inject Something something; + + } + """ + ) + .indented(), + ) + .issues(ConstructorInjectionOverFieldInjectionDetector.ISSUE) + .configureOption( + ConstructorInjectionOverFieldInjectionDetector.usingAppComponentFactory, + true, + ) + .run() + .expect( + """ + src/com/test/android/Something.java:10: Warning: Constructor injection should be favored over field injection for classes that support it. + + See https://whosnickdoglio.dev/dagger-rules/rules/#prefer-constructor-injection-over-field-injection for more information. [ConstructorOverField] + @Inject Something something; + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 0 errors, 1 warnings + """ + .trimIndent() + ) + .expectWarningCount(1) + } + @Suppress("unused") enum class AndroidComponentParameters( val className: String,