Add lint check for repeatOnLifecycle in Fragments
The repeatOnLifecycle APIs should be used with the
viewLifecycleOwner in Fragments as opposed to
lifecycleOwner.
Fixes: b/187891353
Change-Id: I8af3a94139abdf436fa5653e6ac37ed80c6f3b70
diff --git a/fragment/fragment-lint/src/main/java/androidx/fragment/lint/FragmentIssueRegistry.kt b/fragment/fragment-lint/src/main/java/androidx/fragment/lint/FragmentIssueRegistry.kt
index c0dc727..392fc36 100644
--- a/fragment/fragment-lint/src/main/java/androidx/fragment/lint/FragmentIssueRegistry.kt
+++ b/fragment/fragment-lint/src/main/java/androidx/fragment/lint/FragmentIssueRegistry.kt
@@ -32,6 +32,7 @@
UnsafeFragmentLifecycleObserverDetector.LIVEDATA_ISSUE,
UseRequireInsteadOfGet.ISSUE,
UseGetLayoutInflater.ISSUE,
- OnCreateDialogIncorrectCallbackDetector.ISSUE
+ OnCreateDialogIncorrectCallbackDetector.ISSUE,
+ UnsafeRepeatOnLifecycleDetector.ISSUE
)
}
diff --git a/fragment/fragment-lint/src/main/java/androidx/fragment/lint/UnsafeRepeatOnLifecycleDetector.kt b/fragment/fragment-lint/src/main/java/androidx/fragment/lint/UnsafeRepeatOnLifecycleDetector.kt
new file mode 100644
index 0000000..500c462
--- /dev/null
+++ b/fragment/fragment-lint/src/main/java/androidx/fragment/lint/UnsafeRepeatOnLifecycleDetector.kt
@@ -0,0 +1,96 @@
+/*
+ * Copyright 2021 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.
+ */
+
+package androidx.fragment.lint
+
+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.PsiMethod
+import com.intellij.psi.PsiType
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UClass
+import org.jetbrains.uast.getParentOfType
+
+/**
+ * Lint check for detecting calls to the suspend `repeatOnLifecycle` APIs using `lifecycleOwner`
+ * instead of `viewLifecycleOwner` in [androidx.fragment.app.Fragment] classes but not in
+ * [androidx.fragment.app.DialogFragment] classes.
+ *
+ * DialogFragments are allowed to use `lifecycleOwner` since they don't always have a `view`
+ * attached to them.
+ */
+class UnsafeRepeatOnLifecycleDetector : Detector(), SourceCodeScanner {
+
+ companion object {
+ val ISSUE = Issue.create(
+ id = "UnsafeRepeatOnLifecycleDetector",
+ briefDescription = "RepeatOnLifecycle should be used with viewLifecycleOwner in " +
+ "Fragments.",
+ explanation = """The repeatOnLifecycle APIs should be used with the viewLifecycleOwner \
+ in Fragments as opposed to lifecycleOwner.""",
+ category = Category.CORRECTNESS,
+ severity = Severity.ERROR,
+ implementation = Implementation(
+ UnsafeRepeatOnLifecycleDetector::class.java, Scope.JAVA_FILE_SCOPE
+ ),
+ androidSpecific = true
+ )
+ }
+
+ override fun getApplicableMethodNames() = listOf("repeatOnLifecycle")
+
+ override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
+ // Check that repeatOnLifecycle is called in a Fragment
+ if (!hasFragmentAsAncestorType(node.getParentOfType<UClass>())) return
+
+ // Report issue if the receiver is not using viewLifecycleOwner
+ if (node.receiver?.sourcePsi?.text?.contains(SAFE_RECEIVER, ignoreCase = true) != true) {
+ context.report(
+ ISSUE,
+ context.getLocation(node),
+ "The repeatOnLifecycle API should be used with viewLifecycleOwner"
+ )
+ }
+ }
+
+ /**
+ * Check if `uClass` has FRAGMENT as a super type but not DIALOG_FRAGMENT
+ */
+ @Suppress("UNCHECKED_CAST")
+ private fun hasFragmentAsAncestorType(uClass: UClass?): Boolean {
+ if (uClass == null) return false
+ return hasFragmentAsSuperType(uClass.superTypes as Array<PsiType>)
+ }
+
+ private fun hasFragmentAsSuperType(superTypes: Array<PsiType>): Boolean {
+ for (superType in superTypes) {
+ if (superType.canonicalText == DIALOG_FRAGMENT_CLASS) return false
+ if (superType.canonicalText == FRAGMENT_CLASS) return true
+ if (hasFragmentAsSuperType(superType.superTypes)) return true
+ }
+ return false
+ }
+}
+
+private const val SAFE_RECEIVER = "viewLifecycleOwner"
+private const val FRAGMENT_CLASS = "androidx.fragment.app.Fragment"
+private const val DIALOG_FRAGMENT_CLASS = "androidx.fragment.app.DialogFragment"
diff --git a/fragment/fragment-lint/src/test/java/androidx/fragment/lint/UnsafeRepeatOnLifecycleDetectorTest.kt b/fragment/fragment-lint/src/test/java/androidx/fragment/lint/UnsafeRepeatOnLifecycleDetectorTest.kt
new file mode 100644
index 0000000..bb2084d
--- /dev/null
+++ b/fragment/fragment-lint/src/test/java/androidx/fragment/lint/UnsafeRepeatOnLifecycleDetectorTest.kt
@@ -0,0 +1,158 @@
+/*
+ * Copyright 2021 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.
+ */
+
+package androidx.fragment.lint
+
+import androidx.fragment.lint.stubs.REPEAT_ON_LIFECYCLE_STUBS
+import com.android.tools.lint.checks.infrastructure.TestFiles
+import com.android.tools.lint.checks.infrastructure.TestLintTask
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+
+@RunWith(Parameterized::class)
+class UnsafeRepeatOnLifecycleDetectorTest(private val config: TestConfig) {
+
+ data class TestConfig(
+ val fragmentExtensions: String,
+ val apiCall: String,
+ val shouldWarn: Boolean
+ )
+
+ companion object {
+ @JvmStatic
+ @Parameterized.Parameters(name = "{0}")
+ fun spec(): List<TestConfig> =
+ // Adding permutations manually to manually control if something should give a warning
+ listOf(
+ TestConfig(
+ "Fragment()",
+ "repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ true
+ ),
+ TestConfig(
+ "Fragment()",
+ "lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ true
+ ),
+ TestConfig(
+ "Fragment()",
+ "viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ false
+ ),
+ TestConfig(
+ "Fragment()",
+ "lifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ true
+ ),
+ TestConfig(
+ "Fragment()",
+ "getViewLifecycleOwner().repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ false
+ ),
+ TestConfig(
+ "Fragment()",
+ "getLifecycleOwner().repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ true
+ ),
+ TestConfig(
+ "Fragment()",
+ "viewLifecycleOwner.lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ false
+ ),
+ TestConfig(
+ "Fragment()",
+ "lifecycleOwner.lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ true
+ ),
+ TestConfig(
+ "Fragment()",
+ "getViewLifecycleOwner().lifecycle." +
+ "repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ false
+ ),
+ TestConfig(
+ "Fragment()",
+ "getLifecycleOwner().lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ true
+ ),
+ TestConfig(
+ "BaseFragment(), FragmentListener",
+ "getLifecycleOwner().lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ true
+ ),
+ TestConfig(
+ "FragmentListener, OtherFragmentListener, BaseFragment()",
+ "getLifecycleOwner().lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ true
+ ),
+ TestConfig(
+ "DialogFragment()",
+ "getLifecycleOwner().lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) { }",
+ false
+ ),
+ )
+ }
+
+ private val TEMPLATE = """
+ package foo
+
+ import androidx.lifecycle.Lifecycle
+ import androidx.lifecycle.LifecycleOwner
+ import androidx.lifecycle.repeatOnLifecycle
+ import kotlinx.coroutines.CoroutineScope
+ import kotlinx.coroutines.GlobalScope
+ import androidx.fragment.app.Fragment
+
+ class MyFragment : %s {
+ fun onCreateView() {
+ GlobalScope.launch {
+ %s
+ }
+ }
+ }
+
+ class BaseFragment : Fragment()
+ interface FragmentListener
+ interface OtherFragmentListener
+ """.trimIndent()
+
+ @Test
+ fun basicTest() {
+ val testLintResult = TestLintTask.lint()
+ .files(
+ *REPEAT_ON_LIFECYCLE_STUBS,
+ TestFiles.kt(TEMPLATE.format(config.fragmentExtensions, config.apiCall))
+ )
+ .issues(UnsafeRepeatOnLifecycleDetector.ISSUE)
+ .run()
+
+ if (config.shouldWarn) {
+ /* ktlint-disable max-line-length */
+ testLintResult.expect(
+ """
+ src/foo/MyFragment.kt:13: Error: The repeatOnLifecycle API should be used with viewLifecycleOwner [UnsafeRepeatOnLifecycleDetector]
+ ${config.apiCall}
+ ${"~".repeat(config.apiCall.length)}
+ 1 errors, 0 warnings
+ """.trimIndent()
+ )
+ /* ktlint-enable max-line-length */
+ } else {
+ testLintResult.expectClean()
+ }
+ }
+}
diff --git a/fragment/fragment-lint/src/test/java/androidx/fragment/lint/stubs/Stubs.kt b/fragment/fragment-lint/src/test/java/androidx/fragment/lint/stubs/Stubs.kt
index d9f8b42..4e94f6f 100644
--- a/fragment/fragment-lint/src/test/java/androidx/fragment/lint/stubs/Stubs.kt
+++ b/fragment/fragment-lint/src/test/java/androidx/fragment/lint/stubs/Stubs.kt
@@ -18,6 +18,7 @@
import com.android.tools.lint.checks.infrastructure.LintDetectorTest.java
import com.android.tools.lint.checks.infrastructure.LintDetectorTest.kotlin
+import com.android.tools.lint.checks.infrastructure.TestFiles
private val BACK_PRESSED_CALLBACK = java(
"""
@@ -43,23 +44,22 @@
"""
package androidx.fragment.app;
+ import androidx.lifecycle.Lifecycle;
import androidx.lifecycle.LifecycleOwner;
- public class Fragment {
+ public class Fragment implements LifecycleOwner {
public LifecycleOwner getViewLifecycleOwner() {}
+ public LifecycleOwner getLifecycleOwner() {}
+ public Lifecycle getLifecycle() {}
}
-"""
+ """
)
private val DIALOG_FRAGMENT = java(
"""
package androidx.fragment.app;
- import androidx.lifecycle.LifecycleOwner;
-
- public class DialogFragment extends Fragment {
- public LifecycleOwner getViewLifecycleOwner() {}
- }
+ public class DialogFragment extends Fragment { }
"""
)
@@ -67,7 +67,9 @@
"""
package androidx.lifecycle;
- public interface LifecycleOwner {}
+ public interface LifecycleOwner {
+ Lifecycle getLifecycle();
+ }
"""
)
@@ -101,6 +103,20 @@
"""
)
+private val LIFECYCLE = TestFiles.kt(
+ "androidx/lifecycle/Lifecycle.kt",
+ """
+ package androidx.lifecycle;
+
+ abstract class Lifecycle {
+ enum class State { CREATED, STARTED }
+ fun isAtLeast(state: State): Boolean {
+ return true
+ }
+ }
+ """
+).indented().within("src")
+
private val LIVEDATA_OBSERVE_EXTENSION = kotlin(
"androidx/lifecycle/LiveDataKt.kt",
"""
@@ -119,10 +135,52 @@
"""
).indented().within("src")
+private val COROUTINES = TestFiles.kt(
+ "kotlinx/coroutines/GlobalScope.kt",
+ """
+ package kotlinx.coroutines;
+
+ import kotlinx.coroutines.CoroutineScope
+
+ interface CoroutineScope {}
+
+ object GlobalScope {
+ fun launch(block: suspend () -> Unit) {}
+ }
+
+ """
+).indented().within("src")
+
+private val REPEAT_ON_LIFECYCLE = TestFiles.kt(
+ "androidx/lifecycle/RepeatOnLifecycle.kt",
+ """
+ package androidx.lifecycle;
+
+ import androidx.lifecycle.Lifecycle
+ import androidx.lifecycle.LifecycleOwner
+ import kotlinx.coroutines.CoroutineScope
+
+ public suspend fun Lifecycle.repeatOnLifecycle(
+ state: Lifecycle.State,
+ block: suspend CoroutineScope.() -> Unit
+ ) {
+ throw Error()
+ }
+
+ public suspend fun LifecycleOwner.repeatOnLifecycle(
+ state: Lifecycle.State,
+ block: suspend CoroutineScope.() -> Unit
+ ) {
+ throw Error()
+ }
+ """
+).indented().within("src")
+
// stubs for testing calls to LiveData.observe calls
internal val LIVEDATA_STUBS = arrayOf(
FRAGMENT,
DIALOG_FRAGMENT,
+ LIFECYCLE,
LIFECYCLE_OWNER,
LIVEDATA,
MUTABLE_LIVEDATA,
@@ -135,5 +193,16 @@
BACK_PRESSED_CALLBACK,
BACK_PRESSED_DISPATCHER,
FRAGMENT,
+ LIFECYCLE,
+ LIFECYCLE_OWNER
+)
+
+// stubs for testing calls to LifecycleOwner.repeatOnLifecycle
+internal val REPEAT_ON_LIFECYCLE_STUBS = arrayOf(
+ REPEAT_ON_LIFECYCLE,
+ DIALOG_FRAGMENT,
+ FRAGMENT,
+ COROUTINES,
+ LIFECYCLE,
LIFECYCLE_OWNER
)