I hope to add method in UnsafeFragmentLifecycleObserverDetector [148996309]

Fixed

Bug

Status Update

No update yet.

Description

uc...@gmail.com

created issue #1

Feb 6, 2020 12:01PM

Component used: Fragment
Version used:
androidx.fragment:fragment:1.2.0
androidx.lifecycle:lifecycle-lifedata:2.2.0

Devices/Android versions reproduced on: Android studio lint issue. (Android Studio 4.0 canary 9)

I like your lint detector and I agree UnsafeFragmentLifecycleObserverDetector's direction.

UNSAFE_METHODS is like that.
internal val UNSAFE_METHODS = mapOf(
Method("androidx.lifecycle.LiveData", "observe") to LIVEDATA_ISSUE,
Method("androidx.activity.OnBackPressedDispatcher", "addCallback") to BACK_PRESSED_ISSUE
)
(SourceCode referenced is :

https://android.googlesource.com/platform/frameworks/support/+/androidx-master-dev/fragment/fragment-lint/src/main/java/androidx/fragment/lint/UnsafeFragmentLifecycleObserverDetector.kt )

And 'inline fun <T> LiveData<T>.observe' was add in 'androidx.lifecycle:lifecycle-livedata-core-ktx:2.2.0'.
It isn't catch by lint in "onCreateView", "onViewCreated", "onActivityCreated", "onViewStateRestored". When I used viewModel.liveData.observe(this) { } in Fragment.

So I hope to add one unsafe method in map of 'UNSAFE_METHODS'.
Method("androidx.lifecycle.LiveDataKt", "observe") to LIVEDATA_ISSUE,

I was test in my example and works good.

Let me know your opinion.

Best Regards.

If this is a bug in the library, we would appreciate if you could attach:
- Sample project to trigger the issue.
- A screenrecord or screenshots showing the issue (if UI related).

Comments

il...@google.com <il...@google.com> #2Feb 6, 2020 05:28PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

uc...@gmail.com <uc...@gmail.com> #3Feb 6, 2020 07:55PM

This is my example

https://github.com/uchun-dev/TestLintForViewLifecycle with ScreenShot

il...@google.com <il...@google.com> Feb 7, 2020 10:03PM

Assigned to jb...@google.com.

jb...@google.com <jb...@google.com> #4Feb 10, 2020 11:01PM

Thank you for sending this in and you're right.

We filed a bug for writing lint tests for the kotlin extension, but we will try to get this fix into the Fragment 1.2.2 release.

ap...@google.com <ap...@google.com> #5Feb 11, 2020 12:00AM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit 918b7b274374a2998b70622de9194f73862b0903
Author: Jeremy Woods <jbwoods@google.com>
Date: Mon Feb 10 13:26:19 2020

Add lint for LiveData.observe() extension function

The lint rule to replace the fragment LifecycleOwner with the
viewLifecycleOwner in calls to LiveData.observe works fine, but it misses
cases that use the LiveData.observe extension function.

This changes adds the extension function to the list of lint rules.

Test: Verified in sample app
Bug: 148996309
Change-Id: I05d1fe78d088b0d6cce5aa79454b98d25c50faef

M fragment/fragment-lint/src/main/java/androidx/fragment/lint/UnsafeFragmentLifecycleObserverDetector.kt

https://android-review.googlesource.com/1231871

an...@google.com <an...@google.com> #6Feb 11, 2020 06:19AM

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6552735953321984/#/history/all)
Created: 2020-02-11 06:16:43.754400+00:00
Changes: aosp/1231871

jb...@google.com <jb...@google.com> #7Feb 12, 2020 12:42AM

Marked as fixed.

This has been fixed internally and will be available in the Fragment 1.2.2 release.