Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ViewModelInjectionDetector to support a lint option #53

Merged
merged 5 commits into from
Feb 18, 2023
Merged

Update ViewModelInjectionDetector to support a lint option #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
de606d8
Update ViewModelInjectionDetector to support a lint option
WhosNickDoglio Feb 10, 2023
29f851b
Update compose-lint-checks/src/main/java/slack/lint/compose/ViewModel…
WhosNickDoglio Feb 15, 2023
a86a5d8
Simplify filtering allowed and known ViewModel factory functions
WhosNickDoglio Feb 15, 2023
bf8422e
Avoid unnecessarily creating a Set on each property checked
WhosNickDoglio Feb 15, 2023
d39522a
Use more accurate name for user added viewModel factory list
WhosNickDoglio Feb 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions compose-lint-checks/src/main/java/slack/lint/compose/ViewModelInjectionDetector.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,28 @@ import com.android.tools.lint.detector.api.Issue
import com.android.tools.lint.detector.api.JavaContext
import com.android.tools.lint.detector.api.Severity
import com.android.tools.lint.detector.api.SourceCodeScanner
import com.android.tools.lint.detector.api.StringOption
import org.jetbrains.kotlin.psi.KtCallExpression
import org.jetbrains.kotlin.psi.KtFunction
import org.jetbrains.kotlin.psi.KtProperty
import slack.lint.compose.util.*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for myself - we shouldn't have star imports, wonder why spotless isn't catching this 🤔

import slack.lint.compose.util.sourceImplementation

class ViewModelInjectionDetector : ComposableFunctionDetector(), SourceCodeScanner {
class ViewModelInjectionDetector
@JvmOverloads
constructor(private val userFactories: StringSetLintOption = StringSetLintOption(USER_FACTORIES)) :
ComposableFunctionDetector(userFactories), SourceCodeScanner {

companion object {

internal val USER_FACTORIES =
StringOption(
"viewmodel-factories",
"A comma-separated list of viewModel factories.",
null,
"This property should define comma-separated list of allowed viewModel factory function names."
)

private fun errorMessage(factoryName: String): String =
"""
Implicit dependencies of composables should be made explicit.
Expand All @@ -27,14 +40,15 @@ class ViewModelInjectionDetector : ComposableFunctionDetector(), SourceCodeScann

val ISSUE =
Issue.create(
id = "ComposeViewModelInjection",
briefDescription = "Implicit dependencies of composables should be made explicit",
explanation = "Replaced when reporting",
category = Category.CORRECTNESS,
priority = Priorities.NORMAL,
severity = Severity.ERROR,
implementation = sourceImplementation<ViewModelInjectionDetector>()
)
id = "ComposeViewModelInjection",
briefDescription = "Implicit dependencies of composables should be made explicit",
explanation = "Replaced when reporting",
category = Category.CORRECTNESS,
priority = Priorities.NORMAL,
severity = Severity.ERROR,
implementation = sourceImplementation<ViewModelInjectionDetector>()
)
.setOptions(listOf(USER_FACTORIES))

private val KnownViewModelFactories by lazy {
setOf(
Expand All @@ -51,13 +65,14 @@ class ViewModelInjectionDetector : ComposableFunctionDetector(), SourceCodeScann
if (function.isOverride || function.definedInInterface) return

val bodyBlock = function.bodyBlockExpression ?: return
val allFactoryNames = KnownViewModelFactories + userFactories.value

bodyBlock
.findChildrenByClass<KtProperty>()
.flatMap { property ->
property
.findDirectChildrenByClass<KtCallExpression>()
.filter { KnownViewModelFactories.contains(it.calleeExpression?.text) }
.filter { it.calleeExpression?.text in allFactoryNames }
.map { property to it.calleeExpression!!.text }
}
.forEach { (property, viewModelFactoryName) ->
Expand Down
9 changes: 8 additions & 1 deletion compose-lint-checks/src/test/java/slack/lint/compose/ViewModelInjectionDetectorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// SPDX-License-Identifier: Apache-2.0
package slack.lint.compose

import com.android.tools.lint.checks.infrastructure.TestLintTask
import com.android.tools.lint.checks.infrastructure.TestMode
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Issue
Expand All @@ -23,7 +24,8 @@ class ViewModelInjectionDetectorTest(private val viewModel: String) : BaseSlackL
arrayOf("weaverViewModel"),
arrayOf("hiltViewModel"),
arrayOf("injectedViewModel"),
arrayOf("mavericksViewModel")
arrayOf("mavericksViewModel"),
arrayOf("tangleViewModel"),
)
}
}
Expand All @@ -34,6 +36,11 @@ class ViewModelInjectionDetectorTest(private val viewModel: String) : BaseSlackL
// This mode is irrelevant to our test and totally untestable with stringy outputs
override val skipTestModes: Array<TestMode> = arrayOf(TestMode.SUPPRESSIBLE, TestMode.TYPE_ALIAS)

override fun lint(): TestLintTask {
return super.lint()
.configureOption(ViewModelInjectionDetector.USER_FACTORIES, "tangleViewModel")
}

@Test
fun `passes when a weaverViewModel is used as a default param`() {
@Language("kotlin")
Expand Down