Safeargs kotlin plugin using manifest package id for generating Directions class [217414933]

Fixed

Feature Request

Status Update

No update yet.

Description

li...@gmail.com

created issue #1

Feb 2, 2022 01:45PM

Component used: Navigation SafeArgs Kotlin Plugin

Version used: 2.3.5, 2.5.0-alpha01

Reproduced on: Build step

My problem is that the plugin generates classes using the package field from the manifest and substitutes in the path to import the R class.

If you remove the name package field from the manifest and move it to gradle script namespace(AGP 7.1.0). The plugin will generate Directions files with this R import:

import R

And I receive Unresolved reference: R

Therefore will not find the required ActionId from xml When I ran into returning package in the manifest file, it generated the correct R class imports

import com.example.app.R

Comments

jb...@google.com <jb...@google.com> #2Feb 2, 2022 04:55PM

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

li...@gmail.com <li...@gmail.com> #3Feb 3, 2022 02:47PM

Created sample project, see here

https://github.com/LinDevHard/SafeArgsKotlinPluginSample

jb...@google.com <jb...@google.com> Feb 10, 2022 10:28PM

Assigned to jb...@google.com.

sv...@whisp.de <sv...@whisp.de> #4Feb 16, 2022 02:40PM

Can confirm, this happened to me as well. Had to both define namespace in my app's build.gradle and keep the since AGP 7.1.0 deprecated package definition in the manifest.

ap...@google.com <ap...@google.com> #5Feb 16, 2022 10:23PM

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

commit ec4a3af4e420e32da145d4c2e991ce86cfb6de57
Author: Jeremy Woods <jbwoods@google.com>
Date: Thu Feb 10 15:10:25 2022

Add namespace support in safeArgs

We currently retrieve the package from the namespace for
ApplicationVariants, but we fail to apply it to the R resources when a
package name is missing from the manifest.

We should support using namespace when the package is missing from the
manifest to ensure we can retrieve xml resources from the proper
location.

If you would like to alternative packages for different product flavors
you still must use the package in the manifest.

RelNote: "Navigation SafeArgs now supports using the namespace defined
in the `build.gradle` in place of the package in the AndroidManifest."
Test: changed the kotlin test to use namespace instead of package
Bug: 217414933

Change-Id: I659efc0f5b74619f5fe3ffb882c96ccb360670b7

M navigation/navigation-safe-args-gradle-plugin/src/test/kotlin/androidx/navigation/safeargs/gradle/BasePluginTest.kt
M navigation/navigation-safe-args-gradle-plugin/src/main/kotlin/androidx/navigation/safeargs/gradle/SafeArgsPlugin.kt
M navigation/navigation-safe-args-gradle-plugin/src/test/test-data/app-project-kotlin/src/main/AndroidManifest.xml

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

jb...@google.com <jb...@google.com> #6Feb 16, 2022 10:24PM

Marked as fixed.

This has been fixed internally and will be available in the Navigation 2.4.2 and 2.5.0-alpha03 releases.

hn...@formerstudents.ucdavis.edu <hn...@formerstudents.ucdavis.edu> #7May 10, 2022 01:51AM

This issue is not fixed in `2.4.2`

Mon May 09 2022 18:50:48 GMT-0700 (Pacific Daylight Time).png

238 KB

View

Download

sp...@google.com <sp...@google.com> #8May 10, 2022 03:37PM

Re: #7, you might be hitting an error if your applicationId is different than your namespace because the navigation-safe-args-gradle-plugin is incorrectly using the applicationId instead of the namespace to namespace the R class. There's an internal bug tracking this (Issue 230786658)

sv...@whisp.de <sv...@whisp.de> #9May 10, 2022 05:52PM

Can you please post a comment on this issue so that everyone subscribed here will be notified when the internal bug has been fixed and the fix has been shipped? Thanks in advance!

sp...@google.com <sp...@google.com> #10May 11, 2022 03:47PM

Re #9, Issue 232107688 is a public bug tracking the same issue.

sv...@whisp.de <sv...@whisp.de> #11May 11, 2022 04:07PM

Neat, many thanks!