[Fragment] Add lint warning for calling setOnCancelListener and setOnDismissListener in onCreateDialog() #171
Conversation
…ener in onCreateDialog()
|
|
||
| private const val ENTRY_METHOD = "onCreateDialog" | ||
| private const val DIALOG_FRAGMENT_CLASS = "DialogFragment" | ||
| private val callbacks = setOf("setOnCancelListener", "setOnDismissListener") |
There was a problem hiding this comment.
Add an extra line at the end of the file.
| companion object Issues { | ||
| val ISSUE = Issue.create( | ||
| id = "DialogFragmentCallbacksDetector", | ||
| briefDescription = "Use onCancel() and onDismiss() callbacks to get the instead of " + |
There was a problem hiding this comment.
"Use onCancel() and onDismiss() instead of calling setOnCancelListener() and setOnDismissListener() from onCreateDialog()"
| Instead the respective `onCancel` and `onDismiss` functions can be used to \ | ||
| achieve the desired effect.""", | ||
| category = Category.CORRECTNESS, | ||
| priority = 9, |
There was a problem hiding this comment.
We don't normally set priority. You can remove this.
| .run() | ||
| .expect( | ||
| """ | ||
| src/foo/TestFragment.java:11: Warning: Use onCancel() and onDismiss() callbacks to get the instead of calling setOnCancelListener() and setOnDismissListener() from onCreateDialog() [DialogFragmentCallbacksDetector] |
There was a problem hiding this comment.
You will have to fix these to reflect the changed description above.
| context.report( | ||
| issue = ISSUE, | ||
| location = context.getLocation(node), | ||
| message = "Use onCancel() and onDismiss() callbacks to get the instead of " + |
There was a problem hiding this comment.
Can we actually separate this and report in the message based on whether setOnCancelListener or setOnDismissListener was called? I think being specific here would make it easier for developers to know what needs to be done instead.
| * because the `DialogFragment` owns these callbacks. Instead the respective `onCancel` and | ||
| * `onDismiss` functions can be used to achieve the desired effect. | ||
| */ | ||
| class DialogFragmentCallbacksDetector : Detector(), SourceCodeScanner { |
There was a problem hiding this comment.
Let's be more specific in the naming here. Something like OnCreateDialogIncorrectCallbackDetector
|
|
||
| /* ktlint-disable max-line-length */ | ||
| @RunWith(JUnit4::class) | ||
| class DialogFragmentCallbacksDetectorTest : LintDetectorTest() { |
There was a problem hiding this comment.
Once the reports are separated out, I think we should add individual tests for using single callbacks.
|
From our internal lint: fragment/fragment-lint/src/main/java/androidx/fragment/lint/OnCreateDialogIncorrectCallbackDetector.kt fragment/fragment-lint/src/test/java/androidx/fragment/lint/OnCreateDialogIncorrectCallbackDetectorTest.kt |
|
@jbw0033 |
|
Got more errors: $SUPPORT/fragment/fragment-lint/src/main/java/androidx/fragment/lint/OnCreateDialogIncorrectCallbackDetector.kt:27:1: Unused import You can try |
|
LOL, I was only checking Android Lint. |
|
Thank you for the pull request! |
* update runtime tests configuration for js and native Change-Id: I9de7f64c2657cbe9cde979867374d65cd508b8ca * add comments about ignored tests Change-Id: I7caa9ee0762c715deeef761c3851f5f52d91dbc5 Co-authored-by: Oleksandr Karpovich <[email protected]>
Proposed Changes
When using a
DialogFragment, thesetOnCancelListenerandsetOnDismissListenercallback functions within theonCreateDialogfunction must not be used because theDialogFragmentowns these callbacks. Instead the respectiveonCancelandonDismissfunctions can be used to achieve the desired effect.Testing
Test:
./gradlew fragment:fragment-lint:testIssues Fixed
Fixes: The bug on https://issuetracker.google.com/issues/181780047 being fixed