Skip to content
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

Fix proguard rule gen not capturing non-sealed subtypes. #603

Merged
merged 3 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ Changelog
- Improve moshi-sealed KSP error messages.
- Fix fallback adapter support in IR code gen not recognizing Moshi parameters to primary constructors.
- Check for same subtypes before erroring on duplicate labels in moshi-sealed IR.
- Fix proguard rule gen not capturing non-sealed subtypes.
- Don't write empty proguard rule files if not rules were necessary.

0.27.0
------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ public class MoshiProguardGenSymbolProcessor(private val environment: SymbolProc
targetConstructorHasDefaults = false,
targetConstructorParams = emptyList(),
)

if (!enableMoshi && nestedSealedClassNames.isEmpty()) {
return emptyList()
}

environment.logger.info(
"MOSHI: Writing proguard rules for ${targetType.canonicalName}: $config",
clazz,
Expand Down Expand Up @@ -163,6 +168,8 @@ public class MoshiProguardGenSymbolProcessor(private val environment: SymbolProc
for (nested in getSealedSubclasses()) {
nested.walkSealedSubtypes(elements, skipAnnotationCheck = false)
}
} else {
elements += toClassName()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,85 @@ class MoshiProguardGenSymbolProcessorTest(private val useKSP2: Boolean) {
generatedFile.assertInCorrectPath()
when (generatedFile.nameWithoutExtension) {
"moshi-test.BaseType" ->
assertThat(generatedFile.readText())
.contains(
assertThat(generatedFile.readText().trimIndent())
.isEqualTo(
"""
# Conditionally keep this adapter for every possible nested subtype that uses it.
-if class test.BaseType.TypeA
-keep class test.BaseTypeJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
-if class test.BaseType.TypeB
-keep class test.BaseTypeJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
-if class test.BaseType.TypeC
-keep class test.BaseTypeJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
-if class test.BaseType.TypeC.TypeCImpl
-keep class test.BaseTypeJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
"""
.trimIndent()
)
else -> error("Unrecognized proguard file: $generatedFile")
}
}
}

@Test
fun ensureNestedSealedClassesAreAdded() {
val source =
kotlin(
"BaseType.kt",
"""
package test
import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass
import dev.zacsweers.moshix.sealed.annotations.TypeLabel
import dev.zacsweers.moshix.sealed.annotations.NestedSealed
import dev.zacsweers.moshix.sealed.annotations.DefaultObject

@JsonClass(generateAdapter = true, generator = "sealed:type")
sealed class Message {

@TypeLabel("success")
@JsonClass(generateAdapter = true)
data class Success(
@Json(name = "value")
val value: String
) : Message()

@DefaultObject
object Unknown : Message()
}
""",
)

val compilation = prepareCompilation(source)
val result = compilation.compile()
assertThat(result.exitCode).isEqualTo(ExitCode.OK)
val generatedSourcesDir = compilation.kspSourcesDir
val proguardFiles = generatedSourcesDir.walkTopDown().filter { it.extension == "pro" }.toList()
check(proguardFiles.isNotEmpty())
for (generatedFile in proguardFiles) {
generatedFile.assertInCorrectPath()
when (generatedFile.nameWithoutExtension) {
"moshi-test.Message" ->
assertThat(generatedFile.readText().trimIndent())
.isEqualTo(
"""
# Conditionally keep this adapter for every possible nested subtype that uses it.
-if class test.Message.Success
-keep class test.MessageJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
-if class test.Message.Unknown
-keep class test.MessageJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
"""
.trimIndent()
)
Expand Down Expand Up @@ -163,8 +234,8 @@ sealed class BaseType {
generatedFile.assertInCorrectPath()
when (generatedFile.nameWithoutExtension) {
"moshi-test.BaseType" ->
assertThat(generatedFile.readText())
.contains(
assertThat(generatedFile.readText().trimIndent())
.isEqualTo(
"""
-if class test.BaseType
-keepnames class test.BaseType
Expand All @@ -174,10 +245,22 @@ sealed class BaseType {
}

# Conditionally keep this adapter for every possible nested subtype that uses it.
-if class test.BaseType.TypeA
-keep class test.BaseTypeJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
-if class test.BaseType.TypeB
-keep class test.BaseTypeJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
-if class test.BaseType.TypeC
-keep class test.BaseTypeJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
-if class test.BaseType.TypeC.TypeCImpl
-keep class test.BaseTypeJsonAdapter {
public <init>(com.squareup.moshi.Moshi);
}
"""
.trimIndent()
)
Expand Down