Support Nullable Primitives or Lint Warning in Typesafe Navigation [351994237]

Fixed

Feature Request

Status Update

No update yet.

Description

ag...@wayfair.com

created issue #1

Jul 9, 2024 01:51PM

Component used: Navigation Version used: 2.8.0-beta03 Devices/Android versions reproduced on:

Simply make a class with a nullable primitive like an Int?

@Serializable data class ExampleRoute(val accountMethod: Int?)

and runtime crash:

java.lang.IllegalArgumentException: integer does not allow nullable values
at androidx.navigation.NavArgument.<init>(NavArgument.kt:210)
at androidx.navigation.NavArgument$Builder.build(NavArgument.kt:199)
at androidx.navigation.NavArgumentBuilder.build(NavDestinationBuilder.kt:394)
at androidx.navigation.NamedNavArgumentKt.navArgument(NamedNavArgument.kt:21)
at androidx.navigation.serialization.RouteSerializerKt.generateNavArguments(RouteSerializer.kt:109)
at androidx.navigation.NavDestinationBuilder.<init>(NavDestinationBuilder.kt:94)
at androidx.navigation.compose.ComposeNavigatorDestinationBuilder.<init>(ComposeNavigatorDestinationBuilder.kt:95)

In the IDE, there is no indication this is wrong, and is perfectly valid in Kotlinx serialization. I understand complex types requiring a barrier, but a nullable primitive seems logical in kotlin. A lint warning or error would be also highly helpful to prevent this from potentially crashing at runtime.

Comments

jb...@google.com <jb...@google.com> Jul 9, 2024 01:51PM

Assigned to cl...@google.com.

ap...@google.com <ap...@google.com> #2Jul 23, 2024 11: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

cl...@google.com <cl...@google.com> Jul 24, 2024 01:06AM

Accepted by cl...@google.com.

ap...@google.com <ap...@google.com> #3Jul 24, 2024 06:47PM

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

commit 3672e5d8699e643cc156bcc841b3515821d51c34
Author: Clara Fok <clarafok@google.com>
Date: Wed Jul 24 10:40:00 2024

Add support for nullable Boolean in safe args

Test: ./gradlew navigation:navigation-runtime:cC
Bug: 351994237
Relnote: "Navigation in safe args has added built-in support for argumnets of type `Boolean?`."
Change-Id: Ifba6679f9214b477802567403d8ae97203a30303

M navigation/navigation-common/src/androidTest/java/androidx/navigation/serialization/RouteFilledTest.kt
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/NavTypeConverter.kt
M navigation/navigation-common/src/test/java/androidx/navigation/serialization/NavArgumentGeneratorTest.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerRouteTest.kt

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

ap...@google.com <ap...@google.com> #4Jul 24, 2024 06:50PM

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

commit 055c9b9f41871ed7edf4a3d2508eb0ed2f553a47
Author: Clara Fok <clarafok@google.com>
Date: Wed Jul 24 10:56:03 2024

Add support for nullable Float in safe args

Test: ./gradlew navigation:navigation-runtime:cC
Bug: 351994237
Relnote: "Navigation in safe args has added built-in support for argumnets of type `Float?`."
Change-Id: I1c580e271cab940b11378f85bd0a5175c503e2f5

M navigation/navigation-common/src/main/java/androidx/navigation/serialization/NavTypeConverter.kt
M navigation/navigation-common/src/test/java/androidx/navigation/serialization/NavArgumentGeneratorTest.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerRouteTest.kt

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

ap...@google.com <ap...@google.com> #5Jul 24, 2024 06:52PM

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

commit b4551b59c205fda966eb62ecdf769717585287e5
Author: Clara Fok <clarafok@google.com>
Date: Wed Jul 24 11:10:36 2024

Add support for nullable Long in safe args

Test: ./gradlew navigation:navigation-runtime:cC
Bug: 351994237
Relnote: "Navigation in safe args has added built-in support for argumnets of type `Long?`."
Change-Id: I32d131926d08b01151c22d35f7a2d3389d5a1f56

M navigation/navigation-common/src/main/java/androidx/navigation/serialization/NavTypeConverter.kt
M navigation/navigation-common/src/test/java/androidx/navigation/serialization/NavArgumentGeneratorTest.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerRouteTest.kt

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

cl...@google.com <cl...@google.com> #6Jul 24, 2024 07:06PM

Marked as fixed.

Added built-in support of the following nullable primitives

Int?, Boolean?, Float?, Long?

These primitives will no longer require custom NavTypes.

Note that this support only applies to safe args in Kotlin DSL. XML-based navigation will continue to only support non-nullable primitives.

Will be available in the navigation release on Aug 7th.

ap...@google.com <ap...@google.com> #7Jul 25, 2024 11:56PM

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

commit 3cba58164e32afa04d092ba3b4ab8dc326451290
Author: Clara Fok <clarafok@google.com>
Date: Wed Jul 24 17:10:19 2024

Add support for nullable Enums in safe args

Refactored helper function `parseSerializableOrParcelableType` to take a `clazz: Class<*>` instead of `className: String` so that it can be better reused by NavTypeConverter.

Test: ./gradlew navigation:navigation-common:test
Test: ./gradlew navigation:navigation-common:cC
Test: ./gradlew navigation:navigation-runtime:cC
Bug: 351994237
Relnote: "Navigation in safe args has added built-in support for argumnets of type `Enum<*>?`."
Change-Id: Ide076fd4f77daf8373d71e093f833920aba9b911

M navigation/navigation-common/src/androidTest/java/androidx/navigation/serialization/RouteFilledTest.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavType.kt
M navigation/navigation-common/src/main/java/androidx/navigation/serialization/NavTypeConverter.kt
M navigation/navigation-common/src/test/java/androidx/navigation/serialization/NavArgumentGeneratorTest.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavControllerRouteTest.kt

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

pr...@google.com <pr...@google.com> #8Aug 13, 2024 06:59PM

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

androidx.navigation:navigation-common:2.8.0-beta07
androidx.navigation:navigation-runtime:2.8.0-beta07