From f7b781904fc8d715fa90e8cb578a6d15fb13bf72 Mon Sep 17 00:00:00 2001 From: nicholasdoglio Date: Sun, 7 Apr 2024 10:31:48 -0400 Subject: [PATCH] Enable ConstructorInjectionOverFieldInjectionDetector to be configured when using AppComponentFactory --- ...ctorInjectionOverFieldInjectionDetector.kt | 38 +++--- ...InjectionOverFieldInjectionDetectorTest.kt | 124 ++++++++++++++++++ 2 files changed, 143 insertions(+), 19 deletions(-) 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 9af402ef..a4a499f4 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 @@ -28,22 +29,20 @@ internal class ConstructorInjectionOverFieldInjectionDetector : 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, @@ -51,7 +50,6 @@ internal class ConstructorInjectionOverFieldInjectionDetector : ) } - // minSdkLessThan(28) if (!isAllowedFieldInjection) { context.report( Incident( @@ -65,13 +63,8 @@ internal class ConstructorInjectionOverFieldInjectionDetector : } } } - } companion object { - // private const val MIN_SDK = 28 - - // TODO make this configurable - @Suppress("UnusedPrivateMember") private val allowList = StringOption( name = "allowList", @@ -80,8 +73,14 @@ internal class ConstructorInjectionOverFieldInjectionDetector : 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", @@ -115,5 +114,6 @@ internal class ConstructorInjectionOverFieldInjectionDetector : 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 8897537a..dd31a881 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,