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 b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019

Change the way to detect mismatch between POJO and query

This fixes cursor mismatch warnings with expandProjection.

Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8

M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt

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

https://goto.google.com/android-sha1/b90079595f33f58fece04026a97faa0d243acdb1

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

The following changes were cherrypicked through Coastguard (

https://android-build.googleplex.com/coastguard/)
Release Track: (

https://android-build.googleplex.com/coastguard/dashboard/6093116828680192/#/cherrypickHistory/all)
Created: 2019-10-14 17:23:42.610596+00:00
Changes: aosp/1123258

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

Project: platform/frameworks/support
Branch: androidx-master-dev

commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020

Re-factor how errors are dismissed when query is re-written

This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (

b/140759491

).
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.

This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.

Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f

M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt

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

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!