New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Easier to configure ProGuard rules #1721
Conversation
…med companion classes
These rules only require modification when serializable classed with named companion objects are used, and even then, less than the previous rules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
sorry for the delay, and thanks for your contribution.
The main idea behind specifying the application package in the rules was for the user to control which classes really needed to be serialized (not obfuscate them or shrink their members). This can be useful if some dependency contains serializable classes that the application does not serialize for some reason.
On the other hand, such cases are rare and it is more convenient for users to use the rules without changing anything.
It might be worth keeping the old rules, but adding the new ones as an alternative by placing them in an expanding block like this
<details>
<summary>Rules that apply to all serializable classes</summary>
...rules...
</details>
This way we can collect more feedback and be able to create rules that are suitable for the majority of users.
README.md
Outdated
# public static <1>$$serializer INSTANCE; | ||
#} | ||
|
||
# Keep `serializer()` on companion objects of serializable classes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please indicate in this comment that this rule applies to default (Companion) and named companions. This makes the rules easier to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a small side effect, this rule will keep the serializer
function for all nested classes, but this is a very rare case and this rule is the easiest way to support named companions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this rule will keep the
serializer
function for all nested classes
It actually doesn't, because of the pattern matching. It will pass beyond the if
check, but the <3>
in -keepclassmembers class <1>$<3>
will be the static field name defined in the if
condition.
I tried with the following:
@Serializable
class HasSimpleCompanion( val output: String )
{
class Test
{
fun answer() = 42
fun serializer(): KSerializer<*> = HasSimpleCompanion.serializer()
}
}
This would translate to -keepclassmembers class HasSimpleCompanion$Companion
, not -keepclassmembers class HasSimpleCompanion$Test
. There is no static field for normal inner classes.
serializer
is still removed (answer()
is used so the full class isn't removed) as shown in the usage.txt
output:
com.example.myapplication.HasSimpleCompanion$Test:
public final kotlinx.serialization.KSerializer serializer()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I believe that the new rules will be more convenient for Android developers.
These rules only require modification when serializable classed with named companion objects are used, and even then, less than the previous rules.
While enabling ProGuard on my Android project, I ran into the (seemingly recently changed) ProGuard example configuration of
kotlinx.serialization
. Given the way serializer lookup works at runtime, the ProGuard configuration seems unnecessarily complex to me in terms of the number of changes end users need to make. I noticed this by running into some alternatives provided by @ArcticLampyrid, which provided the base inspiration for this PR.As part of Hacktoberfest, I wanted to both challenge myself learning about ProGuard and contribute to my favorite Kotlin library!
Noticeable changes, more or less in order they appear in the rules:
-keepattributes *Annotation*
does not seem needed. I don't know of any annotations that are retrieved at runtime inkotlinx.serialization
, but maybe I missed something. If this is needed, it can probably be narrowed down toRuntimeVisibleAnnotations
or similar.-dontnote kotlinx.serialization.AnnotationsKt
is meant to do. This may be an omission on my part.-keepattributes InnerClasses
is only needed in case there are serializable classes with named companion objects.kotlinx.serialization-json
specific rules were to handle@Serializable
ofJsonElement
and the like. These are no longer needed as they are handled by the new all-encompassing**
rules.These changes were tested in an Android project which contained, and used, the following serializable classes. Maybe it is a good idea to set up a test for this?
In case you approve of these changes, you can slap a "hacktoberfest-accepted" label on this PR to thank me. :)