ZacSweers / MoshiX

Extensions for Moshi including IR plugins, moshi-sealed, and more.
Apache License 2.0
518 stars 37 forks source link

MoshiX generates malformed proguard rule #415

Closed sav007 closed 5 months ago

sav007 commented 1 year ago

Latest version of MoshiX sealed leads to malformed proguard rules. The cause of issue was the data package name like: com.example.data

The generated proguard rule:

# Conditionally keep this adapter for every possible nested subtype that uses it.
-if class com.example.`data`.Model
-keep class com.example.data.ModelJsonAdapter {
    public <init>(com.squareup.moshi.Moshi);
}

Error:

AGPBI: {"kind":"error","text":"Expecting '-keep' option after '-if' option.","sources":[{"file":".../build/intermediates/consumer_proguard_dir/release/lib0/proguard.txt","position":{"startLine":57,"startColumn":0,"startOffset":2994,"endColumn":41,"endOffset":3035}}],"tool":"R8"}

Not 100% sure what the cause of this but it looks like KotlinPoet changed the returned value in reflectionName():

https://github.com/ZacSweers/MoshiX/blob/612e921c70b0a9292873a92e3513d39c56afbd67/moshi-sealed/codegen/src/main/kotlin/dev/zacsweers/moshix/sealed/codegen/ProguardConfig.kt#L57

ZacSweers commented 1 year ago

Which version of Kotlinpoet are you using?

sav007 commented 1 year ago

We tried the latest one 1.13.0, but I believe we saw similar with 1.12.0, but not quite sure I have to double check as it might that some other library that uses KPoet updated it to 1.13.0.

Btw, the way how we addressed this issue in our project is to avoid using data in the name of package.

ZacSweers commented 1 year ago

Got it. This is a Kotlinpoet bug, so could you please file there? The reflection name should not be escaped

sav007 commented 1 year ago

Yeah, I guess you are right will reference this one in that issue.

sav007 commented 1 year ago

I wasn't able to reproduce this issue with plain KPoet, but have one guess:

  1. If you create a class like ClassName("com.sample.data", "Model").reflectionName() it returns the name without escaping: com.sample.data.Model
  2. But if you call on that class toString ClassName("com.sample.data", "Model").toString() it returns an escaped string com.sample.'data'.Model
  3. Next if you create class name with a escaped string like ClassName("", "com.sample.data.Model").reflectionName() then it returns escaped com.sample.'data'.Model

That makes me think if there any possible transformation in the codegen that leads to passing targetClass:ClassName with escaped package name to ProguardConfig. It might be a case where ClassName has been created with escaped package name somewhere ?

I also tried to reproduce the original issue in MoshiSealedSymbolProcessorProviderTest but no luck, though I see a little different result. If I change the test fixture to package test.data the generated adapter content is following:

// Code generated by moshi-sealed. Do not edit.
package test.`data`
....

even this valid source code, but I guess it should be non escaped package name.

I still will try to repro issue we see in our project with escaped proguard rule, but does above ^^^ give any idea where the issue is?

ZacSweers commented 1 year ago

I did not understand your third bullet, maybe because the formatting broke. Why would you make a class like ClassName("", "com.sample.data.Model")? That seems like a programmer error

ZacSweers commented 5 months ago

Closing as wontfix as there's been no followup