From ddac23a2b0e52f6a2bb2cc983ff94ee08fdbd258 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Sun, 5 Mar 2023 16:42:19 -0500 Subject: [PATCH 1/7] Implement M2ApiDetector This implements an opt-in check that can be used to error against use of M2 APIs. This is intended to be useful for apps that have completed their M3 migrations or baseline existing M2 usages while completing their migrations. --- .../lint/compose/ComposeLintsIssueRegistry.kt | 1 + .../java/slack/lint/compose/M2ApiDetector.kt | 70 +++++++++++++++++++ docs/rules.md | 19 +++++ 3 files changed, 90 insertions(+) create mode 100644 compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt diff --git a/compose-lint-checks/src/main/java/slack/lint/compose/ComposeLintsIssueRegistry.kt b/compose-lint-checks/src/main/java/slack/lint/compose/ComposeLintsIssueRegistry.kt index c6bc126a..b95d3769 100644 --- a/compose-lint-checks/src/main/java/slack/lint/compose/ComposeLintsIssueRegistry.kt +++ b/compose-lint-checks/src/main/java/slack/lint/compose/ComposeLintsIssueRegistry.kt @@ -31,6 +31,7 @@ class ComposeLintsIssueRegistry : IssueRegistry() { ModifierMissingDetector.ISSUE, ModifierReusedDetector.ISSUE, ModifierWithoutDefaultDetector.ISSUE, + M2ApiDetector.ISSUE, MultipleContentEmittersDetector.ISSUE, MutableParametersDetector.ISSUE, ParameterOrderDetector.ISSUE, diff --git a/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt b/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt new file mode 100644 index 00000000..c5a14c08 --- /dev/null +++ b/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt @@ -0,0 +1,70 @@ +// Copyright (C) 2022 Salesforce, Inc. +// SPDX-License-Identifier: Apache-2.0 +package slack.lint.compose + +import com.android.tools.lint.client.api.UElementHandler +import com.android.tools.lint.detector.api.Category.Companion.CORRECTNESS +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Severity.IGNORE +import com.android.tools.lint.detector.api.SourceCodeScanner +import com.android.tools.lint.detector.api.TextFormat +import org.jetbrains.uast.UCallExpression +import org.jetbrains.uast.UElement +import org.jetbrains.uast.UImportStatement +import org.jetbrains.uast.UQualifiedReferenceExpression +import org.jetbrains.uast.UResolvable +import slack.lint.compose.util.Priorities.NORMAL +import slack.lint.compose.util.sourceImplementation + +internal class M2ApiDetector : Detector(), SourceCodeScanner { + + override fun getApplicableUastTypes() = + listOf>( + UCallExpression::class.java, + UImportStatement::class.java, + UQualifiedReferenceExpression::class.java, + ) + + override fun createUastHandler(context: JavaContext) = + object : UElementHandler() { + override fun visitCallExpression(node: UCallExpression) = checkNode(node) + + override fun visitImportStatement(node: UImportStatement) = checkNode(node) + + override fun visitQualifiedReferenceExpression(node: UQualifiedReferenceExpression) = + checkNode(node) + + private fun checkNode(node: UResolvable) { + val resolved = node.resolve() ?: return + val packageName = context.evaluator.getPackage(resolved)?.qualifiedName ?: return + if (packageName == M2Package) { + context.report( + issue = ISSUE, + location = context.getLocation(node), + message = ISSUE.getExplanation(TextFormat.TEXT), + ) + } + } + } + + companion object { + private const val M2Package = "androidx.compose.material" + + val ISSUE = + Issue.create( + id = "ComposeM2Api", + briefDescription = "Using a Compose M2 API is not recommended", + explanation = + """ + Compose Material 2 (M2) is succeeded by Material 3 (M3). Please use M3 APIs. + See https://slackhq.github.io/compose-lints/rules/#use-material-3 for more information. + """, + category = CORRECTNESS, + priority = NORMAL, + severity = IGNORE, + implementation = sourceImplementation() + ) + } +} diff --git a/docs/rules.md b/docs/rules.md index 74eb2695..81d2ebad 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -294,3 +294,22 @@ Composed modifiers may be created outside of composition, shared across elements More info: [Modifier extensions](https://developer.android.com/reference/kotlin/androidx/compose/ui/package-summary#extension-functions), [Composed modifiers in Jetpack Compose by Jorge Castillo](https://jorgecastillo.dev/composed-modifiers-in-jetpack-compose) and [Composed modifiers in API guidelines](https://github.com/androidx/androidx/blob/androidx-main/compose/docs/compose-api-guidelines.md#composed-modifiers) Related rule: [`ComposeComposableModifier`](https://github.com/slackhq/compose-lints/blob/main/compose-lint-checks/src/main/java/slack/lint/compose/ModifierComposableDetector.kt) + +### Use Material 3 + +Rule: [`ComposeM2Api`](https://github.com/slackhq/compose-lints/blob/main/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt) + +Material 3 (M3) reached stable in October 2022. In apps that have migrated to M3, there may be `androidx.compose.material` (M2) APIs still remaining on the classpath from libraries or dependencies that can cause confusing imports due to the many similar or colliding Composable names in the two libraries. The `ComposeM2Api` can be enabled + set to `ERROR` to prevent these from being used. + +Note that this rule is set to `IGNORE` by default and is opt-in. + + +**Related docs links** + +- Announcement post: https://material.io/blog/material-3-compose-stable +- Docs: https://m3.material.io/develop/android/jetpack-compose +- Migration guide: https://developer.android.com/jetpack/compose/themes/material2-material3 +- Guidance: https://developer.android.com/jetpack/compose/themes/material3 +- Reply (primary sample app): https://github.com/android/compose-samples/tree/main/Reply +- More samples: https://github.com/android/compose-samples + From 1d2426a4f214f6ce74bce9e1a307ee81e63d69c4 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Mon, 6 Mar 2023 22:40:47 -0500 Subject: [PATCH 2/7] Update docs/rules.md Co-authored-by: Chris Banes --- docs/rules.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rules.md b/docs/rules.md index 81d2ebad..49f22907 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -299,7 +299,7 @@ Related rule: [`ComposeComposableModifier`](https://github.com/slackhq/compose-l Rule: [`ComposeM2Api`](https://github.com/slackhq/compose-lints/blob/main/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt) -Material 3 (M3) reached stable in October 2022. In apps that have migrated to M3, there may be `androidx.compose.material` (M2) APIs still remaining on the classpath from libraries or dependencies that can cause confusing imports due to the many similar or colliding Composable names in the two libraries. The `ComposeM2Api` can be enabled + set to `ERROR` to prevent these from being used. +Material 3 (M3) reached stable in October 2022. In apps that have migrated to M3, there may be `androidx.compose.material` (M2) APIs still remaining on the classpath from libraries or dependencies that can cause confusing imports due to the many similar or colliding Composable names in the two libraries. The `ComposeM2Api` rule can be enabled + set to `ERROR` to prevent these from being used. Note that this rule is set to `IGNORE` by default and is opt-in. From d1f8079a1c439d9c7986e64a1f52a2df20486a35 Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Tue, 7 Mar 2023 12:56:47 -0500 Subject: [PATCH 3/7] Add setup instructions --- docs/rules.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/rules.md b/docs/rules.md index 81d2ebad..317cc00c 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -301,8 +301,18 @@ Rule: [`ComposeM2Api`](https://github.com/slackhq/compose-lints/blob/main/compos Material 3 (M3) reached stable in October 2022. In apps that have migrated to M3, there may be `androidx.compose.material` (M2) APIs still remaining on the classpath from libraries or dependencies that can cause confusing imports due to the many similar or colliding Composable names in the two libraries. The `ComposeM2Api` can be enabled + set to `ERROR` to prevent these from being used. -Note that this rule is set to `IGNORE` by default and is opt-in. +Note that this rule is set to `IGNORE` by default and is opt-in. You can enable and make it an error like so. +```kotlin +android { + lint { + enable += "ComposeM2Api" + error += "ComposeM2Api" + } +} +``` + +More lint configuration docs can be found [here](https://developer.android.com/studio/write/lint#gradle). **Related docs links** From 88b5bf104306465c16a6799542d88a9b4739e68c Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Tue, 7 Mar 2023 12:57:11 -0500 Subject: [PATCH 4/7] Don't check imports since there's no file-level suppression --- .../src/main/java/slack/lint/compose/M2ApiDetector.kt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt b/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt index c5a14c08..67cbe300 100644 --- a/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt +++ b/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt @@ -12,7 +12,6 @@ import com.android.tools.lint.detector.api.SourceCodeScanner import com.android.tools.lint.detector.api.TextFormat import org.jetbrains.uast.UCallExpression import org.jetbrains.uast.UElement -import org.jetbrains.uast.UImportStatement import org.jetbrains.uast.UQualifiedReferenceExpression import org.jetbrains.uast.UResolvable import slack.lint.compose.util.Priorities.NORMAL @@ -23,7 +22,6 @@ internal class M2ApiDetector : Detector(), SourceCodeScanner { override fun getApplicableUastTypes() = listOf>( UCallExpression::class.java, - UImportStatement::class.java, UQualifiedReferenceExpression::class.java, ) @@ -31,8 +29,6 @@ internal class M2ApiDetector : Detector(), SourceCodeScanner { object : UElementHandler() { override fun visitCallExpression(node: UCallExpression) = checkNode(node) - override fun visitImportStatement(node: UImportStatement) = checkNode(node) - override fun visitQualifiedReferenceExpression(node: UQualifiedReferenceExpression) = checkNode(node) From 43c41af3527a781503a6f8ac4d1d94b1b611597b Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Tue, 7 Mar 2023 13:18:04 -0500 Subject: [PATCH 5/7] Add allow-list option --- .../java/slack/lint/compose/M2ApiDetector.kt | 59 ++++++++++++------- docs/rules.md | 9 +++ 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt b/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt index 67cbe300..326b4747 100644 --- a/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt +++ b/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt @@ -4,20 +4,54 @@ package slack.lint.compose import com.android.tools.lint.client.api.UElementHandler import com.android.tools.lint.detector.api.Category.Companion.CORRECTNESS -import com.android.tools.lint.detector.api.Detector import com.android.tools.lint.detector.api.Issue import com.android.tools.lint.detector.api.JavaContext import com.android.tools.lint.detector.api.Severity.IGNORE import com.android.tools.lint.detector.api.SourceCodeScanner +import com.android.tools.lint.detector.api.StringOption import com.android.tools.lint.detector.api.TextFormat +import com.intellij.psi.PsiNamedElement import org.jetbrains.uast.UCallExpression import org.jetbrains.uast.UElement import org.jetbrains.uast.UQualifiedReferenceExpression import org.jetbrains.uast.UResolvable +import slack.lint.compose.util.OptionLoadingDetector import slack.lint.compose.util.Priorities.NORMAL +import slack.lint.compose.util.StringSetLintOption import slack.lint.compose.util.sourceImplementation -internal class M2ApiDetector : Detector(), SourceCodeScanner { +internal class M2ApiDetector +@JvmOverloads +constructor(private val allowList: StringSetLintOption = StringSetLintOption(ALLOW_LIST)) : + OptionLoadingDetector(allowList), SourceCodeScanner { + + companion object { + private const val M2Package = "androidx.compose.material" + + internal val ALLOW_LIST = + StringOption( + "allowed-m2-apis", + "A comma-separated list of APIs in androidx.compose.material that should be allowed.", + null, + "This property should define a comma-separated list of APIs in androidx.compose.material that should be allowed." + ) + + val ISSUE = + Issue.create( + id = "ComposeM2Api", + briefDescription = "Using a Compose M2 API is not recommended", + explanation = + """ + Compose Material 2 (M2) is succeeded by Material 3 (M3). Please use M3 APIs. + See https://slackhq.github.io/compose-lints/rules/#use-material-3 for more information. + """, + category = CORRECTNESS, + priority = NORMAL, + severity = IGNORE, + implementation = sourceImplementation() + ) + .setOptions(listOf(ALLOW_LIST)) + } override fun getApplicableUastTypes() = listOf>( @@ -36,6 +70,8 @@ internal class M2ApiDetector : Detector(), SourceCodeScanner { val resolved = node.resolve() ?: return val packageName = context.evaluator.getPackage(resolved)?.qualifiedName ?: return if (packageName == M2Package) { + // Ignore any in the allow-list. + if (resolved is PsiNamedElement && resolved.name in allowList.value) return context.report( issue = ISSUE, location = context.getLocation(node), @@ -44,23 +80,4 @@ internal class M2ApiDetector : Detector(), SourceCodeScanner { } } } - - companion object { - private const val M2Package = "androidx.compose.material" - - val ISSUE = - Issue.create( - id = "ComposeM2Api", - briefDescription = "Using a Compose M2 API is not recommended", - explanation = - """ - Compose Material 2 (M2) is succeeded by Material 3 (M3). Please use M3 APIs. - See https://slackhq.github.io/compose-lints/rules/#use-material-3 for more information. - """, - category = CORRECTNESS, - priority = NORMAL, - severity = IGNORE, - implementation = sourceImplementation() - ) - } } diff --git a/docs/rules.md b/docs/rules.md index 317cc00c..995f0be5 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -314,6 +314,15 @@ android { More lint configuration docs can be found [here](https://developer.android.com/studio/write/lint#gradle). +!!! note "Allow-list Configuration" +To allow certain APIs (i.e. for incremental migration), you can configure a `allowed-m2-apis` option in `lint.xml`. + + ```xml + + + ``` + **Related docs links** - Announcement post: https://material.io/blog/material-3-compose-stable From 804a925909da179f8ecace804dbafb41302f8a5d Mon Sep 17 00:00:00 2001 From: Zac Sweers Date: Tue, 7 Mar 2023 13:22:34 -0500 Subject: [PATCH 6/7] Doc cleanups --- docs/rules.md | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/docs/rules.md b/docs/rules.md index 394bd0ba..a5bc30b1 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -299,24 +299,22 @@ Related rule: [`ComposeComposableModifier`](https://github.com/slackhq/compose-l Rule: [`ComposeM2Api`](https://github.com/slackhq/compose-lints/blob/main/compose-lint-checks/src/main/java/slack/lint/compose/M2ApiDetector.kt) -Material 3 (M3) reached stable in October 2022. In apps that have migrated to M3, there may be `androidx.compose.material` (M2) APIs still remaining on the classpath from libraries or dependencies that can cause confusing imports due to the many similar or colliding Composable names in the two libraries. The `ComposeM2Api` rule can be enabled + set to `ERROR` to prevent these from being used. - -Note that this rule is set to `IGNORE` by default and is opt-in. You can enable and make it an error like so. - -```kotlin -android { - lint { - enable += "ComposeM2Api" - error += "ComposeM2Api" - } -} -``` - -More lint configuration docs can be found [here](https://developer.android.com/studio/write/lint#gradle). +Material 3 (M3) reached stable in October 2022. In apps that have migrated to M3, there may be `androidx.compose.material` (M2) APIs still remaining on the classpath from libraries or dependencies that can cause confusing imports due to the many similar or colliding Composable names in the two libraries. The `ComposeM2Api` rule can prevent these from being used. + +!!! warning "Lint Configuration" + This rule is set to `IGNORE` by default and is **opt-in**. You can enable and make it an error via `lint` in Gradle. + ```kotlin + android { + lint { + enable += "ComposeM2Api" + error += "ComposeM2Api" + } + } + ``` + More lint configuration docs can be found [here](https://developer.android.com/studio/write/lint#gradle). !!! note "Allow-list Configuration" -To allow certain APIs (i.e. for incremental migration), you can configure a `allowed-m2-apis` option in `lint.xml`. - + To allow certain APIs (i.e. for incremental migration), you can configure a `allowed-m2-apis` option in `lint.xml`. ```xml