Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Enable ConstructorInjectionOverFieldInjectionDetector to be configure…
Browse files Browse the repository at this point in the history
…d when using AppComponentFactory
WhosNickDoglio committed Apr 27, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 7a76bf8 commit b89d788
Showing 2 changed files with 143 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -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,30 +27,27 @@ internal class ConstructorInjectionOverFieldInjectionDetector : Detector(), Sour
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(
@@ -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,14 @@ 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",
@@ -113,5 +112,6 @@ internal class ConstructorInjectionOverFieldInjectionDetector : Detector(), Sour
severity = Severity.WARNING,
implementation = implementation,
)
.setOptions(listOf(usingAppComponentFactory, allowList))
}
}
Original file line number Diff line number Diff line change
@@ -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,

0 comments on commit b89d788

Please sign in to comment.