-
Notifications
You must be signed in to change notification settings - Fork 969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
, thesetOnCancelListener
andsetOnDismissListener
callback functions within theonCreateDialog
function must not be used because theDialogFragment
owns these callbacks. Instead the respectiveonCancel
andonDismiss
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 being fixed