Skip to content

Commit

Permalink
Enable ConstructorInjectionOverFieldInjectionDetector to be configure…
Browse files Browse the repository at this point in the history 
…d when using AppComponentFactory
  • Loading branch information
@WhosNickDoglio
WhosNickDoglio committed Jul 13, 2024
1 parent af403fe commit 5117451
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 19 deletions.
38 changes: 19 additions & 19 deletions ...ava/dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetector.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,30 +29,27 @@ internal class ConstructorInjectionOverFieldInjectionDetector :
override fun getApplicableUastTypes(): List<Class<out UElement>> =
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<String> =
// if (context.mainProject.minSdk >= MIN_SDK) {
//
// allowList.defaultValue?.split(",").orEmpty()
// } else {
//
// allowList.defaultValue?.split(",").orEmpty() + androidComponents
// }
val fullAllowList: List<String> =
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,
strict = true,
)
}

// minSdkLessThan(28)
if (!isAllowedFieldInjection) {
context.report(
Incident(
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -115,5 +114,6 @@ internal class ConstructorInjectionOverFieldInjectionDetector :
severity = Severity.WARNING,
implementation = implementation,
)
.setOptions(listOf(usingAppComponentFactory, allowList))
}
}
124 changes: 124 additions & 0 deletions ...dev/whosnickdoglio/dagger/detectors/ConstructorInjectionOverFieldInjectionDetectorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 5117451

Please sign in to comment.