SafeArgs: partial default values in an action produce wrong parameters order [188855904]

Fixed

Bug

Status Update

No update yet.

Description

gm...@gmail.com

created issue #1

May 21, 2021 08:43AM

Component used: Navigation Version used: 2.3.5

It is often convenient to specify default argument values in an action, for example when multiple actions point to the same destination.

Sometimes we only specify values for some arguments:

<fragment
    android:id="@+id/firstFragment"
    android:name="package.FirstFragment">
    <action
        android:id="@+id/goToSecond"
        app:destination="@id/secondFragment">
        <argument
            android:name="argTwo"
            android:defaultValue="42" />
    </action>
</fragment>
<fragment
    android:id="@+id/secondFragment"
    android:name="package.SecondFragment">
    <argument
        android:name="argOne"
        app:argType="string" />
    <argument
        android:name="argTwo"
        app:argType="integer" />
</fragment>

We then would like to call it like this: FirstFragmentDirections.goToSecond("one!").

But unfortunately, SafeArgs generates code with parameters in the wrong order: argTwo (with default value), then argOne. See attached project.

I suppose better options would be to:

Respect arguments order from the destination (seems more intuitive), or
Always place default parameters last (looks nicer in Kotlin).

TestSafeArgs3.zip

141 KB

Download

Comments

il...@google.com <il...@google.com> #2May 21, 2021 06:51PM

Assigned to jb...@google.com.

We should always put arguments without a default value before those with a default value, following the order on the destination itself within each set.

ap...@google.com <ap...@google.com> #3Aug 18, 2021 07:19PM

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

commit 008b8791f45b61a6f537aef62a12d775f5f55420
Author: Jeremy Woods <jbwoods@google.com>
Date: Thu Jul 29 18:23:30 2021

Make generated params with defaults last

When generating an action in kotlin from safe args, when there are
multiple parameters we should always make default arguments the last
parameters on the action.

RelNote: "When generating actions, safe args now puts arguments without
default values before those with default values as parameters."
Test: ./gradlew --rerun-tasks navigation:navigation-safe-args-generator:test
Bug: 188855904

Change-Id: Idb69783a3b0efe53de8f3e04a978dd6b979a0e04

M navigation/navigation-safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/kotlin/KotlinNavWriter.kt
M navigation/navigation-safe-args-generator/src/test/kotlin/androidx/navigation/safe/args/generator/KotlinNavWriterTest.kt
A navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/MainFragmentDefaultParamDirections.kt
M navigation/navigation-safe-args-generator/src/test/test-data/expected/kotlin_nav_writer_test/Next.kt

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

jb...@google.com <jb...@google.com> #4Aug 18, 2021 07:22PM

Marked as fixed.

This has been fixed internally and will be available as part of the Navigation 2.4.0-alpha08 release.

gm...@gmail.com <gm...@gmail.com> #5Nov 10, 2021 03:11PM

Just tested with Navigation 2.4.0-beta02:

Project compiles and runs fine, but the Arctic Fox still highlights goToSecond("one!") as a type mismatch error. Cleaning the project does not help. Could you check it please?

gm...@gmail.com <gm...@gmail.com> #6Apr 25, 2022 09:03AM

Re-tested with 2.4.2 & 2.5.0-beta01 in Bumblebee:

It still works, but Lint now shows two warnings. The second one is in nav_graph.xml for action argument: Element argument is not allowed here

Issue 188855904