Propose a lint rule to detect missing `@Serializable` annotation on type-safe routes passed to navigation functions for improved compile-time safety. [362725816]

Fixed

Feature Request

Status Update

No update yet.

Description

jb...@google.com

created issue #1

Aug 28, 2024 03:20PM

The current lint rules for detecting incorrect type-safe routes passed into navigation functions don't account for if the type-safe route is actually marked as `@Serializable`. So they may give developers suggested fixes that actually end up crashing at runtime once navigation attempts to use the kotlinx.serialization APIs on it.

Instead of the lint rule having users correct what they pass in then fail anyway, it would be nice if there was a lint rule that indicated that the `@Serializable` annotation needs to be added to the route class/object. This would provide some compile time safety to ensure that navigation only gets passed the correct objects.

Comments

jb...@google.com <jb...@google.com> Sep 16, 2024 08:25PM

Assigned to cl...@google.com.

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

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 23, 2024 04:50PM

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> #4Sep 23, 2024 04:50PM

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

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

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

commit 49c974dc53e902b0fb2b105e72c60b276fe22afe
Author: Clara Fok <clarafok@google.com>
Date: Wed Sep 18 10:49:48 2024

Add annotation lint for NavDestinationBuilder.deepLink

Lint for @Serializable and @Keep with NavDestinationBuilder.deepLink as entry point

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

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/TestStub.kt

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

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

cl...@google.com <cl...@google.com> #7Sep 25, 2024 04:56AM

Marked as fixed.

Fixed internally, available in navigation 2.8.2.

pr...@google.com <pr...@google.com> #8Oct 2, 2024 05:37PM

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.annotation:annotation-wasm-js:1.9.0-rc01
androidx.annotation:annotation-watchosdevicearm64:1.9.0-rc01