[GH] [Fragment] Add lint warning for calling setOnCancelListener and setOnDismissListener in onCreateDialog()
## Proposed Changes
When using a `DialogFragment`, the `setOnCancelListener` and `setOnDismissListener` callback functions within the `onCreateDialog` function __must not be used__ because the `DialogFragment` owns these callbacks. Instead the respective `onCancel` and `onDismiss` functions can be used to achieve the desired effect.
## Testing
Test: `./gradlew fragment:fragment-lint:test`
## Issues Fixed
Fixes: The bug on [https://issuetracker.google.com/issues/181780047](https://issuetracker.google.com/issues/181780047) being fixed
This is an imported pull request from https://github.com/androidx/androidx/pull/171.
Resolves #171
Github-Pr-Head-Sha: 92f6ef9d142d57909e0ffb10b473673f840d7d3f
GitOrigin-RevId: ab96d00aee90a600607fa7e0c55386e445dfa3fa
Change-Id: Iccbd039116b8d7fae70332ceb93d1ca40b8e371c
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 5384ce0..c0dc727 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
@@ -31,6 +31,7 @@
UnsafeFragmentLifecycleObserverDetector.BACK_PRESSED_ISSUE,
UnsafeFragmentLifecycleObserverDetector.LIVEDATA_ISSUE,
UseRequireInsteadOfGet.ISSUE,
- UseGetLayoutInflater.ISSUE
+ UseGetLayoutInflater.ISSUE,
+ OnCreateDialogIncorrectCallbackDetector.ISSUE
)
}
diff --git a/fragment/fragment-lint/src/main/java/androidx/fragment/lint/OnCreateDialogIncorrectCallbackDetector.kt b/fragment/fragment-lint/src/main/java/androidx/fragment/lint/OnCreateDialogIncorrectCallbackDetector.kt
new file mode 100644
index 0000000..b462e88
--- /dev/null
+++ b/fragment/fragment-lint/src/main/java/androidx/fragment/lint/OnCreateDialogIncorrectCallbackDetector.kt
@@ -0,0 +1,156 @@
+/*
+ * 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.
+ */
+
+@file:Suppress("UnstableApiUsage")
+
+package androidx.fragment.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.android.tools.lint.detector.api.isKotlin
+import com.intellij.psi.impl.source.PsiClassReferenceType
+import org.jetbrains.kotlin.psi.psiUtil.getSuperNames
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UClass
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.kotlin.KotlinUClass
+import org.jetbrains.uast.visitor.AbstractUastVisitor
+
+/**
+ * When using a `DialogFragment`, the `setOnCancelListener` and `setOnDismissListener` callback
+ * functions within the `onCreateDialog` function __must not be used__
+ * because the `DialogFragment` owns these callbacks. Instead the respective `onCancel` and
+ * `onDismiss` functions can be used to achieve the desired effect.
+ */
+class OnCreateDialogIncorrectCallbackDetector : Detector(), SourceCodeScanner {
+
+ companion object Issues {
+ val ISSUE = Issue.create(
+ id = "DialogFragmentCallbacksDetector",
+ briefDescription = "Use onCancel() and onDismiss() instead of calling " +
+ "setOnCancelListener() and setOnDismissListener() from onCreateDialog()",
+ explanation = """When using a `DialogFragment`, the `setOnCancelListener` and \
+ `setOnDismissListener` callback functions within the `onCreateDialog` function \
+ __must not be used__ because the `DialogFragment` owns these callbacks. \
+ Instead the respective `onCancel` and `onDismiss` functions can be used to \
+ achieve the desired effect.""",
+ category = Category.CORRECTNESS,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ OnCreateDialogIncorrectCallbackDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ ),
+ androidSpecific = true
+ )
+ }
+
+ override fun getApplicableUastTypes(): List<Class<out UElement>>? {
+ return listOf(UClass::class.java)
+ }
+
+ override fun createUastHandler(context: JavaContext): UElementHandler? {
+ return UastHandler(context)
+ }
+
+ private inner class UastHandler(val context: JavaContext) : UElementHandler() {
+ override fun visitClass(node: UClass) {
+ if (isKotlin(context.psiFile) && (node as KotlinUClass).ktClass!!.getSuperNames()[0] !=
+ DIALOG_FRAGMENT_CLASS
+ ) {
+ return
+ }
+
+ if (!isKotlin(context.psiFile) &&
+ (node.uastSuperTypes[0].type as PsiClassReferenceType)
+ .className != DIALOG_FRAGMENT_CLASS
+ ) {
+ return
+ }
+
+ node.methods.forEach {
+ if (it.name == ENTRY_METHOD) {
+ val visitor = UastMethodsVisitor(context, it.name)
+ it.uastBody?.accept(visitor)
+ }
+ }
+ }
+ }
+
+ /**
+ * A UAST Visitor that explores all method calls within a
+ * [androidx.fragment.app.DialogFragment] callback to check for an incorrect method call.
+ *
+ * @param context The context of the lint request.
+ * @param containingMethodName The name of the originating Fragment lifecycle method.
+ */
+ private class UastMethodsVisitor(
+ private val context: JavaContext,
+ private val containingMethodName: String
+ ) : AbstractUastVisitor() {
+ private val visitedMethods = mutableSetOf<UCallExpression>()
+
+ override fun visitCallExpression(node: UCallExpression): Boolean {
+ if (visitedMethods.contains(node)) {
+ return super.visitCallExpression(node)
+ }
+
+ val methodName = node.methodIdentifier?.name ?: return super.visitCallExpression(node)
+
+ when (methodName) {
+ SET_ON_CANCEL_LISTENER -> {
+ report(
+ context = context,
+ node = node,
+ message = "Use onCancel() instead of calling setOnCancelListener() " +
+ "from onCreateDialog()"
+ )
+ visitedMethods.add(node)
+ }
+ SET_ON_DISMISS_LISTENER -> {
+ report(
+ context = context,
+ node = node,
+ message = "Use onDismiss() instead of calling setOnDismissListener() " +
+ "from onCreateDialog()"
+ )
+ visitedMethods.add(node)
+ }
+ }
+ return super.visitCallExpression(node)
+ }
+
+ private fun report(context: JavaContext, node: UCallExpression, message: String) {
+ context.report(
+ issue = ISSUE,
+ location = context.getLocation(node),
+ message = message,
+ quickfixData = null
+ )
+ }
+ }
+}
+
+private const val ENTRY_METHOD = "onCreateDialog"
+private const val DIALOG_FRAGMENT_CLASS = "DialogFragment"
+private const val SET_ON_CANCEL_LISTENER = "setOnCancelListener"
+private const val SET_ON_DISMISS_LISTENER = "setOnDismissListener"
diff --git a/fragment/fragment-lint/src/test/java/androidx/fragment/lint/OnCreateDialogIncorrectCallbackDetectorTest.kt b/fragment/fragment-lint/src/test/java/androidx/fragment/lint/OnCreateDialogIncorrectCallbackDetectorTest.kt
new file mode 100644
index 0000000..c67cd41
--- /dev/null
+++ b/fragment/fragment-lint/src/test/java/androidx/fragment/lint/OnCreateDialogIncorrectCallbackDetectorTest.kt
@@ -0,0 +1,271 @@
+/*
+ * 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.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)
+class OnCreateDialogIncorrectCallbackDetectorTest : LintDetectorTest() {
+
+ override fun getDetector(): Detector = OnCreateDialogIncorrectCallbackDetector()
+
+ override fun getIssues(): MutableList<Issue> {
+ return mutableListOf(OnCreateDialogIncorrectCallbackDetector.ISSUE)
+ }
+
+ private val dialogFragmentCorrectImplementationStubJava = java(
+ """
+ package foo;
+ import android.app.Dialog;
+ import android.content.DialogInterface;
+ import android.os.Bundle;
+ import androidx.appcompat.app.AlertDialog;
+ import androidx.fragment.app.DialogFragment;
+ public class TestFragment extends DialogFragment {
+ @NonNull
+ @Override
+ public Dialog onCreateDialog(@Nullable Bundle savedInstanceState) {
+ Dialog dialog = AlertDialog.Builder(requireActivity());
+ return dialog.create();
+ }
+ }
+ """
+ ).indented()
+
+ private val dialogFragmentCorrectImplementationStubKotlin = kotlin(
+ """
+ package foo
+ import android.app.Dialog
+ import android.content.DialogInterface
+ import android.os.Bundle
+ import androidx.appcompat.app.AlertDialog
+ import androidx.fragment.app.DialogFragment
+ class TestDialog : DialogFragment() {
+ override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
+ val dialog = AlertDialog.Builder(requireActivity())
+ return dialog.create()
+ }
+ override fun onCancel(dialog: DialogInterface) {
+ super.onCancel(dialog)
+ }
+
+ override fun onDismiss(dialog: DialogInterface) {
+ super.onDismiss(dialog)
+ }
+ }
+ """
+ ).indented()
+
+ private val dialogFragmentStubJavaWithCancelListener = java(
+ """
+ package foo;
+ import android.app.Dialog;
+ import android.content.DialogInterface;
+ import android.os.Bundle;
+ import androidx.appcompat.app.AlertDialog;
+ import androidx.fragment.app.DialogFragment;
+ public class TestFragment extends DialogFragment {
+ @Override
+ public Dialog onCreateDialog(Bundle savedInstanceState) {
+ Dialog dialog = AlertDialog.Builder(requireActivity());
+ dialog.setOnCancelListener({ });
+ return dialog.create();
+ }
+ }
+ """
+ ).indented()
+
+ private val dialogFragmentStubJavaWithDismissListener = java(
+ """
+ package foo;
+ import android.app.Dialog;
+ import android.content.DialogInterface;
+ import android.os.Bundle;
+ import androidx.appcompat.app.AlertDialog;
+ import androidx.fragment.app.DialogFragment;
+ public class TestFragment extends DialogFragment {
+ @Override
+ public Dialog onCreateDialog(Bundle savedInstanceState) {
+ Dialog dialog = AlertDialog.Builder(requireActivity());
+ dialog.setOnDismissListener({ });
+ return dialog.create();
+ }
+ }
+ """
+ ).indented()
+
+ private val dialogFragmentStubKotlinWithCancelListener = kotlin(
+ """
+ package foo
+ import android.app.Dialog
+ import android.content.DialogInterface
+ import android.os.Bundle
+ import androidx.appcompat.app.AlertDialog
+ import androidx.fragment.app.DialogFragment
+ class TestDialog : DialogFragment() {
+ override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
+ val dialog = AlertDialog.Builder(requireActivity())
+ dialog.setOnCancelListener { }
+ return dialog.create()
+ }
+
+ override fun onDismiss(dialog: DialogInterface) {
+ super.onDismiss(dialog)
+ }
+ }
+ """
+ ).indented()
+
+ private val dialogFragmentStubKotlinWithDismissListener = kotlin(
+ """
+ package foo
+ import android.app.Dialog
+ import android.content.DialogInterface
+ import android.os.Bundle
+ import androidx.appcompat.app.AlertDialog
+ import androidx.fragment.app.DialogFragment
+ class TestDialog : DialogFragment() {
+ override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
+ val dialog = AlertDialog.Builder(requireActivity())
+ dialog.setOnDismissListener { }
+ return dialog.create()
+ }
+
+ override fun onCancel(dialog: DialogInterface) {
+ super.onCancel(dialog)
+ }
+ }
+ """
+ ).indented()
+
+ private val dialogFragmentStubKotlinWithDismissAndCancelListeners = kotlin(
+ """
+ package foo
+ import android.app.Dialog
+ import android.content.DialogInterface
+ import android.os.Bundle
+ import androidx.appcompat.app.AlertDialog
+ import androidx.fragment.app.DialogFragment
+ class TestDialog : DialogFragment() {
+ override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
+ val dialog = AlertDialog.Builder(requireActivity())
+ dialog.setOnDismissListener { }
+ dialog.setOnCancelListener { }
+ return dialog.create()
+ }
+ }
+ """
+ ).indented()
+
+ @Test
+ fun `java expect fail dialog fragment with cancel listener`() {
+ lint().files(dialogFragmentStubJavaWithCancelListener)
+ .run()
+ .expect(
+ """
+src/foo/TestFragment.java:11: Warning: Use onCancel() instead of calling setOnCancelListener() from onCreateDialog() [DialogFragmentCallbacksDetector]
+ dialog.setOnCancelListener({ });
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+0 errors, 1 warnings
+ """
+ )
+ .expectWarningCount(1)
+ }
+
+ @Test
+ fun `java expect fail dialog fragment with dismiss listener`() {
+ lint().files(dialogFragmentStubJavaWithDismissListener)
+ .run()
+ .expect(
+ """
+src/foo/TestFragment.java:11: Warning: Use onDismiss() instead of calling setOnDismissListener() from onCreateDialog() [DialogFragmentCallbacksDetector]
+ dialog.setOnDismissListener({ });
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+0 errors, 1 warnings
+ """
+ )
+ .expectWarningCount(1)
+ }
+
+ @Test
+ fun `java expect clean dialog fragment`() {
+ lint().files(dialogFragmentCorrectImplementationStubJava)
+ .run()
+ .expectClean()
+ }
+
+ @Test
+ fun `kotlin expect fail dialog fragment with cancel listener`() {
+ lint().files(dialogFragmentStubKotlinWithCancelListener)
+ .run()
+ .expect(
+ """
+src/foo/TestDialog.kt:10: Warning: Use onCancel() instead of calling setOnCancelListener() from onCreateDialog() [DialogFragmentCallbacksDetector]
+ dialog.setOnCancelListener { }
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+0 errors, 1 warnings
+ """
+ )
+ .expectWarningCount(1)
+ }
+
+ @Test
+ fun `kotlin expect fail dialog fragment with dismiss listener`() {
+ lint().files(dialogFragmentStubKotlinWithDismissListener)
+ .run()
+ .expect(
+ """
+src/foo/TestDialog.kt:10: Warning: Use onDismiss() instead of calling setOnDismissListener() from onCreateDialog() [DialogFragmentCallbacksDetector]
+ dialog.setOnDismissListener { }
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+0 errors, 1 warnings
+ """
+ )
+ .expectWarningCount(1)
+ }
+
+ @Test
+ fun `kotlin expect fail dialog fragment with dismiss and cancel listeners`() {
+ lint().files(dialogFragmentStubKotlinWithDismissAndCancelListeners)
+ .run()
+ .expect(
+ """
+src/foo/TestDialog.kt:10: Warning: Use onDismiss() instead of calling setOnDismissListener() from onCreateDialog() [DialogFragmentCallbacksDetector]
+ dialog.setOnDismissListener { }
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+src/foo/TestDialog.kt:11: Warning: Use onCancel() instead of calling setOnCancelListener() from onCreateDialog() [DialogFragmentCallbacksDetector]
+ dialog.setOnCancelListener { }
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+0 errors, 2 warnings
+ """
+ )
+ .expectWarningCount(2)
+ }
+
+ @Test
+ fun `kotlin expect clean dialog fragment`() {
+ lint().files(dialogFragmentCorrectImplementationStubKotlin)
+ .run()
+ .expectClean()
+ }
+}
