Updates AmbientNamingDetector for the Ambient rename to CompositionLocal
This lint check now warns if the property is not prefixed with Local.
This lint check might not be needed in the future, but for now it will help developers migrate to CompositionLocal.
Bug: b/178501597
Test: CompositionLocalNamingDetectorTest
Change-Id: Ibc586c5eba8c2d5edc42958d859f76f2a7b6cc51
diff --git a/buildSrc/src/main/kotlin/androidx/build/AndroidXUiPlugin.kt b/buildSrc/src/main/kotlin/androidx/build/AndroidXUiPlugin.kt
index 11cf0a0..8477cb4 100644
--- a/buildSrc/src/main/kotlin/androidx/build/AndroidXUiPlugin.kt
+++ b/buildSrc/src/main/kotlin/androidx/build/AndroidXUiPlugin.kt
@@ -146,6 +146,7 @@
error("ComposableNaming")
error("ComposableLambdaParameterNaming")
error("ComposableLambdaParameterPosition")
+ error("CompositionLocalNaming")
}
// TODO(148540713): remove this exclusion when Lint can support using multiple lint jars
diff --git a/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/AmbientNamingDetector.kt b/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/AmbientNamingDetector.kt
deleted file mode 100644
index 91882fa..0000000
--- a/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/AmbientNamingDetector.kt
+++ /dev/null
@@ -1,93 +0,0 @@
-/*
- * Copyright 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-@file:Suppress("UnstableApiUsage")
-
-package androidx.compose.runtime.lint
-
-import com.android.tools.lint.client.api.UElementHandler
-import com.android.tools.lint.detector.api.Category
-import com.android.tools.lint.detector.api.Detector
-import com.android.tools.lint.detector.api.Implementation
-import com.android.tools.lint.detector.api.Issue
-import com.android.tools.lint.detector.api.JavaContext
-import com.android.tools.lint.detector.api.LintFix
-import com.android.tools.lint.detector.api.Scope
-import com.android.tools.lint.detector.api.Severity
-import com.android.tools.lint.detector.api.SourceCodeScanner
-import com.intellij.psi.util.InheritanceUtil
-import org.jetbrains.uast.UElement
-import org.jetbrains.uast.UVariable
-import java.util.EnumSet
-import java.util.Locale
-
-/**
- * [Detector] that checks the naming of Ambient properties for consistency with guidelines.
- *
- * - `Ambient` should not be used as a noun (suffix) in the name of an Ambient property. It may
- * be used as an adjective (prefix) in lieu of a more descriptive adjective.
- */
-class AmbientNamingDetector : Detector(), SourceCodeScanner {
- override fun getApplicableUastTypes() = listOf(UVariable::class.java)
-
- override fun createUastHandler(context: JavaContext) = object : UElementHandler() {
- override fun visitVariable(node: UVariable) {
- val type = node.type
- if (!InheritanceUtil.isInheritor(type, AmbientFqn)) return
-
- val name = node.name
- if (!name!!.endsWith(AmbientShortName)) return
-
- val newName = AmbientShortName + name.replace(AmbientShortName, "")
- .capitalize(Locale.getDefault())
-
- // Kotlinc can't disambiguate overloads for report / getNameLocation otherwise
- val uElementNode: UElement = node
-
- context.report(
- AmbientNaming,
- uElementNode,
- context.getNameLocation(uElementNode),
- "`Ambient` should not be used as a noun when naming Ambient properties",
- LintFix.create()
- .replace()
- .name("Use Ambient as an adjective (prefix)")
- .text(name)
- .with(newName)
- .autoFix()
- .build()
- )
- }
- }
-
- companion object {
- val AmbientNaming = Issue.create(
- "AmbientNaming",
- "Incorrect naming for Ambient properties",
- "`Ambient` should not be used as a anoun when naming Ambient properties. It may be " +
- "used as an adjective in lieu of a more descriptive adjective. Otherwise Ambients" +
- " follow standard property naming guidelines.",
- Category.CORRECTNESS, 3, Severity.ERROR,
- Implementation(
- AmbientNamingDetector::class.java,
- EnumSet.of(Scope.JAVA_FILE, Scope.TEST_SOURCES)
- )
- )
- }
-}
-
-private const val AmbientFqn = "androidx.compose.runtime.Ambient"
-private val AmbientShortName get() = AmbientFqn.split(".").last()
diff --git a/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/CompositionLocalNamingDetector.kt b/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/CompositionLocalNamingDetector.kt
new file mode 100644
index 0000000..f9250bc
--- /dev/null
+++ b/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/CompositionLocalNamingDetector.kt
@@ -0,0 +1,91 @@
+/*
+ * Copyright 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+@file:Suppress("UnstableApiUsage")
+
+package androidx.compose.runtime.lint
+
+import com.android.tools.lint.client.api.UElementHandler
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Implementation
+import com.android.tools.lint.detector.api.Issue
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.Scope
+import com.android.tools.lint.detector.api.Severity
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import com.intellij.psi.util.InheritanceUtil
+import org.jetbrains.kotlin.psi.KtParameter
+import org.jetbrains.kotlin.psi.KtProperty
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UParameter
+import org.jetbrains.uast.UVariable
+import java.util.EnumSet
+
+/**
+ * [Detector] that checks the naming of CompositionLocal properties for consistency with guidelines.
+ *
+ * CompositionLocal properties should be prefixed with `Local` to make it clear that their value
+ * is local to the current composition.
+ */
+class CompositionLocalNamingDetector : Detector(), SourceCodeScanner {
+ override fun getApplicableUastTypes() = listOf(UVariable::class.java)
+
+ override fun createUastHandler(context: JavaContext) = object : UElementHandler() {
+ override fun visitVariable(node: UVariable) {
+ // Ignore parameters of type CompositionLocal
+ if (node is UParameter) return
+ if (node.sourcePsi is KtParameter) return
+ // Ignore local properties
+ if ((node.sourcePsi as? KtProperty)?.isLocal == true) return
+
+ val type = node.type
+ if (!InheritanceUtil.isInheritor(type, CompositionLocalFqn)) return
+
+ val name = node.name
+ if (name!!.startsWith(CompositionLocalPrefix, ignoreCase = true)) return
+
+ // Kotlinc can't disambiguate overloads for report / getNameLocation otherwise
+ val uElementNode: UElement = node
+
+ context.report(
+ CompositionLocalNaming,
+ uElementNode,
+ context.getNameLocation(uElementNode),
+ "CompositionLocal properties should be prefixed with `Local`",
+ )
+ }
+ }
+
+ companion object {
+ val CompositionLocalNaming = Issue.create(
+ "CompositionLocalNaming",
+ "CompositionLocal properties should be prefixed with `Local`",
+ "CompositionLocal properties should be prefixed with `Local`. This helps make " +
+ "it clear at their use site that these values are local to the current " +
+ "composition. Typically the full name will be `Local` + the type of the " +
+ "CompositionLocal, for example val LocalFoo = compositionLocalOf { Foo() }.",
+ Category.CORRECTNESS, 3, Severity.WARNING,
+ Implementation(
+ CompositionLocalNamingDetector::class.java,
+ EnumSet.of(Scope.JAVA_FILE, Scope.TEST_SOURCES)
+ )
+ )
+ }
+}
+
+private const val CompositionLocalFqn = "androidx.compose.runtime.CompositionLocal"
+private const val CompositionLocalPrefix = "Local"
diff --git a/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/RuntimeIssueRegistry.kt b/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/RuntimeIssueRegistry.kt
index 6d5dcad..a860b1a 100644
--- a/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/RuntimeIssueRegistry.kt
+++ b/compose/runtime/runtime-lint/src/main/java/androidx/compose/runtime/lint/RuntimeIssueRegistry.kt
@@ -27,7 +27,7 @@
override val api = 8
override val minApi = CURRENT_API
override val issues get() = listOf(
- AmbientNamingDetector.AmbientNaming,
+ CompositionLocalNamingDetector.CompositionLocalNaming,
ComposableLambdaParameterDetector.ComposableLambdaParameterNaming,
ComposableLambdaParameterDetector.ComposableLambdaParameterPosition,
ComposableNamingDetector.ComposableNaming,
diff --git a/compose/runtime/runtime-lint/src/test/java/androidx/compose/runtime/lint/AmbientNamingDetectorTest.kt b/compose/runtime/runtime-lint/src/test/java/androidx/compose/runtime/lint/AmbientNamingDetectorTest.kt
deleted file mode 100644
index 7290145..0000000
--- a/compose/runtime/runtime-lint/src/test/java/androidx/compose/runtime/lint/AmbientNamingDetectorTest.kt
+++ /dev/null
@@ -1,182 +0,0 @@
-/*
- * Copyright 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-@file:Suppress("UnstableApiUsage")
-
-package androidx.compose.runtime.lint
-
-import com.android.tools.lint.checks.infrastructure.LintDetectorTest
-import com.android.tools.lint.detector.api.Detector
-import com.android.tools.lint.detector.api.Issue
-import org.junit.Test
-import org.junit.runner.RunWith
-import org.junit.runners.JUnit4
-
-/* ktlint-disable max-line-length */
-@RunWith(JUnit4::class)
-
-/**
- * Test for [AmbientNamingDetector].
- */
-class AmbientNamingDetectorTest : LintDetectorTest() {
- override fun getDetector(): Detector = AmbientNamingDetector()
-
- override fun getIssues(): MutableList<Issue> =
- mutableListOf(AmbientNamingDetector.AmbientNaming)
-
- // Simplified Ambient.kt stubs
- private val ambientStub = kotlin(
- """
- package androidx.compose.runtime
-
- sealed class Ambient<T> constructor(defaultFactory: (() -> T)? = null)
-
- abstract class ProvidableAmbient<T> internal constructor(
- defaultFactory: (() -> T)?
- ) : Ambient<T>(defaultFactory)
-
- internal class DynamicProvidableAmbient<T> constructor(
- defaultFactory: (() -> T)?
- ) : ProvidableAmbient<T>(defaultFactory)
-
- internal class StaticProvidableAmbient<T>(
- defaultFactory: (() -> T)?
- ) : ProvidableAmbient<T>(defaultFactory)
-
- fun <T> ambientOf(
- defaultFactory: (() -> T)? = null
- ): ProvidableAmbient<T> = DynamicProvidableAmbient(defaultFactory)
-
- fun <T> staticAmbientOf(
- defaultFactory: (() -> T)? = null
- ): ProvidableAmbient<T> = StaticProvidableAmbient(defaultFactory)
- """
- )
-
- @Test
- fun ambientUsedAsNoun() {
- lint().files(
- kotlin(
- """
- package androidx.compose.runtime.foo
-
- import androidx.compose.runtime.*
-
- val FooAmbient = ambientOf { 5 }
-
- object Test {
- val BarAmbient: Ambient<String?> = staticAmbientOf { null }
- }
-
- class Test2 {
- companion object {
- val BazAmbient: ProvidableAmbient<Int> = ambientOf()
- }
- }
- """
- ),
- ambientStub
- )
- .run()
- .expect(
- """
-src/androidx/compose/runtime/foo/Test.kt:6: Error: Ambient should not be used as a noun when naming Ambient properties [AmbientNaming]
- val FooAmbient = ambientOf { 5 }
- ~~~~~~~~~~
-src/androidx/compose/runtime/foo/Test.kt:9: Error: Ambient should not be used as a noun when naming Ambient properties [AmbientNaming]
- val BarAmbient: Ambient<String?> = staticAmbientOf { null }
- ~~~~~~~~~~
-src/androidx/compose/runtime/foo/Test.kt:14: Error: Ambient should not be used as a noun when naming Ambient properties [AmbientNaming]
- val BazAmbient: ProvidableAmbient<Int> = ambientOf()
- ~~~~~~~~~~
-3 errors, 0 warnings
- """
- )
- .expectFixDiffs(
- """
-Fix for src/androidx/compose/runtime/foo/Test.kt line 6: Use Ambient as an adjective (prefix):
-@@ -6 +6
-- val FooAmbient = ambientOf { 5 }
-+ val AmbientFoo = ambientOf { 5 }
-Fix for src/androidx/compose/runtime/foo/Test.kt line 9: Use Ambient as an adjective (prefix):
-@@ -9 +9
-- val BarAmbient: Ambient<String?> = staticAmbientOf { null }
-+ val AmbientBar: Ambient<String?> = staticAmbientOf { null }
-Fix for src/androidx/compose/runtime/foo/Test.kt line 14: Use Ambient as an adjective (prefix):
-@@ -14 +14
-- val BazAmbient: ProvidableAmbient<Int> = ambientOf()
-+ val AmbientBaz: ProvidableAmbient<Int> = ambientOf()
- """
- )
- }
-
- @Test
- fun ambientUsedAsAdjective() {
- lint().files(
- kotlin(
- """
- package androidx.compose.runtime.foo
-
- import androidx.compose.runtime.*
-
- val AmbientFoo = ambientOf { 5 }
-
- object Test {
- val AmbientBar: Ambient<String?> = staticAmbientOf { null }
- }
-
- class Test2 {
- companion object {
- val AmbientBaz: ProvidableAmbient<Int> = ambientOf()
- }
- }
- """
- ),
- ambientStub
- )
- .run()
- .expectClean()
- }
-
- @Test
- fun descriptiveAdjectives() {
- lint().files(
- kotlin(
- """
- package androidx.compose.runtime.foo
-
- import androidx.compose.runtime.*
-
- val ThemeFoo = ambientOf { 5 }
-
- object Test {
- val ThemeBar: Ambient<String?> = staticAmbientOf { null }
- }
-
- class Test2 {
- companion object {
- val StyledBaz: ProvidableAmbient<Int> = ambientOf()
- }
- }
- """
- ),
- ambientStub
- )
- .run()
- .expectClean()
- }
-}
-/* ktlint-enable max-line-length */
diff --git a/compose/runtime/runtime-lint/src/test/java/androidx/compose/runtime/lint/CompositionLocalNamingDetectorTest.kt b/compose/runtime/runtime-lint/src/test/java/androidx/compose/runtime/lint/CompositionLocalNamingDetectorTest.kt
new file mode 100644
index 0000000..5e09544
--- /dev/null
+++ b/compose/runtime/runtime-lint/src/test/java/androidx/compose/runtime/lint/CompositionLocalNamingDetectorTest.kt
@@ -0,0 +1,141 @@
+/*
+ * Copyright 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+@file:Suppress("UnstableApiUsage")
+
+package androidx.compose.runtime.lint
+
+import com.android.tools.lint.checks.infrastructure.LintDetectorTest
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+
+/* ktlint-disable max-line-length */
+@RunWith(JUnit4::class)
+
+/**
+ * Test for [CompositionLocalNamingDetector].
+ */
+class CompositionLocalNamingDetectorTest : LintDetectorTest() {
+ override fun getDetector(): Detector = CompositionLocalNamingDetector()
+
+ override fun getIssues(): MutableList<Issue> =
+ mutableListOf(CompositionLocalNamingDetector.CompositionLocalNaming)
+
+ // Simplified CompositionLocal.kt stubs
+ private val compositionLocalStub = kotlin(
+ """
+ package androidx.compose.runtime
+
+ sealed class CompositionLocal<T> constructor(defaultFactory: (() -> T)? = null)
+
+ abstract class ProvidableCompositionLocal<T> internal constructor(
+ defaultFactory: (() -> T)?
+ ) : CompositionLocal<T>(defaultFactory)
+
+ internal class DynamicProvidableCompositionLocal<T> constructor(
+ defaultFactory: (() -> T)?
+ ) : ProvidableCompositionLocal<T>(defaultFactory)
+
+ internal class StaticProvidableCompositionLocal<T>(
+ defaultFactory: (() -> T)?
+ ) : ProvidableCompositionLocal<T>(defaultFactory)
+
+ fun <T> compositionLocalOf(
+ defaultFactory: (() -> T)? = null
+ ): ProvidableCompositionLocal<T> = DynamicProvidableCompositionLocal(defaultFactory)
+
+ fun <T> staticCompositionLocalOf(
+ defaultFactory: (() -> T)? = null
+ ): ProvidableCompositionLocal<T> = StaticProvidableCompositionLocal(defaultFactory)
+ """
+ )
+
+ @Test
+ fun noLocalPrefix() {
+ lint().files(
+ kotlin(
+ """
+ package androidx.compose.runtime.foo
+
+ import androidx.compose.runtime.*
+
+ val FooCompositionLocal = compositionLocalOf { 5 }
+
+ object Test {
+ val BarCompositionLocal: CompositionLocal<String?> = staticCompositionLocalOf {
+ null
+ }
+ }
+
+ class Test2 {
+ companion object {
+ val BazCompositionLocal: ProvidableCompositionLocal<Int> =
+ compositionLocalOf()
+ }
+ }
+ """
+ ),
+ compositionLocalStub
+ )
+ .run()
+ .expect(
+ """
+src/androidx/compose/runtime/foo/Test.kt:6: Warning: CompositionLocal properties should be prefixed with Local [CompositionLocalNaming]
+ val FooCompositionLocal = compositionLocalOf { 5 }
+ ~~~~~~~~~~~~~~~~~~~
+src/androidx/compose/runtime/foo/Test.kt:9: Warning: CompositionLocal properties should be prefixed with Local [CompositionLocalNaming]
+ val BarCompositionLocal: CompositionLocal<String?> = staticCompositionLocalOf {
+ ~~~~~~~~~~~~~~~~~~~
+src/androidx/compose/runtime/foo/Test.kt:16: Warning: CompositionLocal properties should be prefixed with Local [CompositionLocalNaming]
+ val BazCompositionLocal: ProvidableCompositionLocal<Int> =
+ ~~~~~~~~~~~~~~~~~~~
+0 errors, 3 warnings
+ """
+ )
+ }
+
+ @Test
+ fun prefixedWithLocal() {
+ lint().files(
+ kotlin(
+ """
+ package androidx.compose.runtime.foo
+
+ import androidx.compose.runtime.*
+
+ val LocalFoo = compositionLocalOf { 5 }
+
+ object Test {
+ val LocalBar: CompositionLocal<String?> = staticCompositionLocalOf { null }
+ }
+
+ class Test2 {
+ companion object {
+ val LocalBaz: ProvidableCompositionLocal<Int> = compositionLocalOf()
+ }
+ }
+ """
+ ),
+ compositionLocalStub
+ )
+ .run()
+ .expectClean()
+ }
+}
+/* ktlint-enable max-line-length */
diff --git a/compose/runtime/runtime/samples/src/main/java/androidx/compose/runtime/samples/CompositionLocalSamples.kt b/compose/runtime/runtime/samples/src/main/java/androidx/compose/runtime/samples/CompositionLocalSamples.kt
index 0522c57..e9c0fc3 100644
--- a/compose/runtime/runtime/samples/src/main/java/androidx/compose/runtime/samples/CompositionLocalSamples.kt
+++ b/compose/runtime/runtime/samples/src/main/java/androidx/compose/runtime/samples/CompositionLocalSamples.kt
@@ -55,6 +55,7 @@
}
}
+@Suppress("CompositionLocalNaming")
private val ActiveUser = compositionLocalOf<User> { error("No active user found!") }
@Composable private fun SomeScreen() {}
diff --git a/development/build_log_simplifier/messages.ignore b/development/build_log_simplifier/messages.ignore
index ee13e90..7fe5781 100644
--- a/development/build_log_simplifier/messages.ignore
+++ b/development/build_log_simplifier/messages.ignore
@@ -882,6 +882,7 @@
\$SUPPORT/.*/build\.gradle: Ignore: Unknown issue id "ComposableNaming" \[UnknownIssueId\]
\$SUPPORT/.*/build\.gradle: Ignore: Unknown issue id "ComposableLambdaParameterNaming" \[UnknownIssueId\]
\$SUPPORT/.*/build\.gradle: Ignore: Unknown issue id "ComposableLambdaParameterPosition" \[UnknownIssueId\]
+\$SUPPORT/.*/build\.gradle: Ignore: Unknown issue id "CompositionLocalNaming" \[UnknownIssueId\]
Explanation for issues of type "UnknownIssueId":
Lint will report this issue if it is configured with an issue id it does
not recognize in for example Gradle files or lint\.xml configuration files\.
