Unable to use enums with type-safe navigation in production (minified) builds without explicitly annotating them with @androidx.annotation.Keep [358687142]

Fixed

Bug

Status Update

No update yet.

Description

st...@gmail.com

created issue #1

Aug 10, 2024 07:33PM

Component used: Navigation
Version used: Latest snapshot
Devices/Android versions reproduced on: Any

This bug report is to track the discussion that has started over at https://issuetracker.google.com/issues/358137294#comment6

The problem is that in minified builds, even though navigation now has support for enum classes, kotlinx.serialization at that point knows the name of the class before it was obfuscated by r8, and the navigation library is trying to find it by using the obfuscated name. That's my understanding of the issue.

The real effect this has when using the type-safe apis is that one is simply not able to have enums inside their serializable type-safe destinations.

The one workaround which works is to annotate the enum class with @androidx.annotation.Keep, however this is not mentioned anywhere, there is no compile-time warning for it, nor any lint which could help.

I am reporting this here with the hopes that there can be some improvement in this area. Optimally this should "just work", even in minified builds, but if that turns out to be infeasible, perhaps some lint could help us out. And if that is infeasible too, perhaps a special error message at compile time could inform the user that they should use this annotation, or whatever other solution may be considered best.

There is a repro project here https://github.com/StylianosGakis/navigation-predictive-back-breaking-shared-element/tree/b401ce64c0d2b48585d92f96f626b970fb01bb4a which as seen here https://issuetracker.google.com/issues/358137294#comment9 shows this crash happening.

Comments

jb...@google.com <jb...@google.com> Aug 12, 2024 06:21PM

Assigned to cl...@google.com.

cl...@google.com <cl...@google.com> #2Sep 5, 2024 11:27PM

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

ap...@google.com <ap...@google.com> #3Sep 18, 2024 02:50PM

Project: platform/frameworks/support
Branch: androidx-main

commit 4940c45ab1632128129bfb64b4fd42320b081c92
Author: Clara Fok <clarafok@google.com>
Date: Mon Sep 16 13:04:40 2024

Add navigation-common lint for Keep annotation

We need to make sure that Enums that are used as nav arguments in type-safe routes are annotated with @Keep. The e
ntry point to locating said Enums are rotue declarations that contains Enum arguments.

Since the entry point is essentially the same lint location for linting @Serializable annotation, this lint uses the same detector but expands upon it.

Test: ./gradlew navigation:navigation-common-lint:test
Bug:358687142
Change-Id: I2b46fbc3b5be6251d89e902e315f50f20c46ce19

M navigation/navigation-common-lint/src/main/java/androidx/navigation/common/lint/NavigationCommonIssueRegistry.kt
M navigation/navigation-common-lint/src/main/java/androidx/navigation/common/lint/TypeSafeDestinationMissingAnnotationDetector.kt
A navigation/navigation-common-lint/src/test/java/androidx/navigation/common/lint/MissingKeepAnnotationDetectorTest.kt

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

ap...@google.com <ap...@google.com> #4Sep 23, 2024 04:49PM

Project: platform/frameworks/support
Branch: androidx-main

commit 33f3eee080a07079f9ce7b34d58ee210869fe180
Author: Clara Fok <clarafok@google.com>
Date: Wed Sep 18 23:32:56 2024

Add navigation-compose lint for type safe annotations

The lint checks if routes that are used to create NavDestinations are annotated with kotlinx @Serializable, and checks that enum arg types are annotated with @Keep.

Test: ./gradlew navigation:navigation-compose-lint:test
Bug: 362725816
Bug: 358687142
Change-Id: I954023cd0cb4a5ee7d08647cf9a8139b0c8ae9d9

M navigation/navigation-compose-lint/src/main/java/androidx/navigation/compose/lint/NavigationComposeIssueRegistry.kt
A navigation/navigation-compose-lint/src/main/java/androidx/navigation/compose/lint/TypeSafeDestinationMissingAnnotationDetector.kt
A navigation/navigation-compose-lint/src/test/java/androidx/navigation/compose/lint/MissingKeepAnnotationDetectorTest.kt
A navigation/navigation-compose-lint/src/test/java/androidx/navigation/compose/lint/MissingSerializableAnnotationDetectorTest.kt
M navigation/navigation-compose-lint/src/test/java/androidx/navigation/compose/lint/Stubs.kt

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

ap...@google.com <ap...@google.com> #5Sep 23, 2024 04:49PM

Project: platform/frameworks/support
Branch: androidx-main

commit 50b1c751929ecb0bfafa59afc1a6101b39668e9b
Author: Clara Fok <clarafok@google.com>
Date: Wed Sep 18 20:55:45 2024

Add navigation-runtime lint for type safe annotations

The lint checks if routes that are used to create NavDestinations are annotated with kotlinx @Serializable, and checks that enum arg types are annotated with @Keep.

Test: ./gradlew navigation:navigation-runtime-lint:test
Bug: 362725816
Bug: 358687142
Change-Id: Ie601a15449b1561154bd3ae5d990e006a371c7c6

M navigation/navigation-runtime-lint/src/main/java/androidx/navigation/runtime/lint/NavigationRuntimeIssueRegistry.kt
A navigation/navigation-runtime-lint/src/main/java/androidx/navigation/runtime/lint/TypeSafeDestinationMissingAnnotationDetector.kt
A navigation/navigation-runtime-lint/src/test/java/androidx/navigation/runtime/lint/MissingKeepAnnotationDetectorTest.kt
A navigation/navigation-runtime-lint/src/test/java/androidx/navigation/runtime/lint/MissingSerializableAnnotationDetectorTest.kt
A navigation/navigation-runtime-lint/src/test/java/androidx/navigation/runtime/lint/Stubs.kt

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

cl...@google.com <cl...@google.com> Sep 24, 2024 05:30PM

Accepted by cl...@google.com.

ap...@google.com <ap...@google.com> #6Sep 24, 2024 06:13PM

Project: platform/frameworks/support
Branch: androidx-main

commit 8b4f5f4e8219232187ac103252715c7213215625
Author: Clara Fok <clarafok@google.com>
Date: Tue Sep 24 04:42:33 2024

Add annotation lint for NavDeepLink builders

Lint for @Serializable and @Keep with for navDeepLink builders

Test: ./gradlew navigation:navigation-common-lint:test
Bug: 362725816
Bug: 358687142
Change-Id: I4a259b5544e4e4655c4bdee812d64f3b2a1947bd

M navigation/navigation-common-lint/src/main/java/androidx/navigation/common/lint/TypeSafeDestinationMissingAnnotationDetector.kt
M navigation/navigation-common-lint/src/test/java/androidx/navigation/common/lint/MissingKeepAnnotationDetectorTest.kt
M navigation/navigation-common-lint/src/test/java/androidx/navigation/common/lint/MissingSerializableAnnotationDetectorTest.kt
M navigation/navigation-lint-common/src/main/java/androidx/navigation/lint/common/BaseTypeSafeDestinationMissingAnnotationDetector.kt
M navigation/navigation-lint-common/src/main/java/androidx/navigation/lint/common/TestStub.kt

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

jb...@google.com <jb...@google.com> #7Oct 14, 2024 04:17AM

Marked as fixed.

This has been fixed internally and will be available in the Navigation 2.8.3 release.