Keep Kotlin interface method when default implementation is used

Guardsquare · Feb 13, 2024 · 1c421bf · 1c421bf
1 parent d4692c3
commit 1c421bf
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 16 deletions.
diff --git a/base/src/main/java/proguard/shrink/ClassUsageMarker.java b/base/src/main/java/proguard/shrink/ClassUsageMarker.java
@@ -1971,23 +1971,28 @@ public void visitFunction(Clazz clazz,
         {
             visitAnyFunction(clazz, kotlinDeclarationContainerMetadata, kotlinFunctionMetadata);
 
-            // Non-abstract functions in interfaces should have default implementations, so keep it if the
-            // user kept the original function.
-            if (isUsed(kotlinFunctionMetadata))
-            {
-                if (kotlinDeclarationContainerMetadata.k == KotlinConstants.METADATA_KIND_CLASS &&
-                    ((KotlinClassKindMetadata)kotlinDeclarationContainerMetadata).flags.isInterface &&
-                    !kotlinFunctionMetadata.flags.modality.isAbstract &&
-                    (kotlinFunctionMetadata.referencedMethod.getProcessingFlags() & ProcessingFlags.DONT_SHRINK) != 0)
-                {
-                    kotlinFunctionMetadata.referencedDefaultImplementationMethodAccept(
-                        new MultiMemberVisitor(
-                            ClassUsageMarker.this,
-                            new MemberToClassVisitor(ClassUsageMarker.this)
-                        )
-                    );
-                }
+            boolean isInterface =
+                kotlinDeclarationContainerMetadata.k == KotlinConstants.METADATA_KIND_CLASS
+                    && ((KotlinClassKindMetadata) kotlinDeclarationContainerMetadata).flags.isInterface
+                    && !kotlinFunctionMetadata.flags.modality.isAbstract;
+
+            if (isUsed(kotlinFunctionMetadata)
+                && isInterface
+                && (kotlinFunctionMetadata.referencedMethod.getProcessingFlags()
+                        & ProcessingFlags.DONT_SHRINK)
+                    != 0) {
+                kotlinFunctionMetadata.referencedDefaultImplementationMethodAccept(
+                   new MultiMemberVisitor(
+                       ClassUsageMarker.this, new MemberToClassVisitor(ClassUsageMarker.this)));
             }
+
+            // If a default implementation is called directly,
+            // the interface should be marked as used as well.
+            if (kotlinFunctionMetadata.referencedDefaultImplementationMethod != null
+                && isInterface
+                && isUsed(kotlinFunctionMetadata.referencedDefaultImplementationMethod)) {
+                kotlinFunctionMetadata.referencedMethodAccept(ClassUsageMarker.this);
+            } 
         }
 
         // Implementations for KotlinTypeAliasVisitor.

diff --git a/base/src/test/kotlin/proguard/shrink/ClassUsageMarkerTest.kt b/base/src/test/kotlin/proguard/shrink/ClassUsageMarkerTest.kt
@@ -10,11 +10,17 @@ import io.kotest.matchers.shouldNot
 import io.kotest.matchers.shouldNotBe
 import proguard.AppView
 import proguard.Configuration
+import proguard.classfile.Clazz
+import proguard.classfile.Member
 import proguard.classfile.attribute.annotation.visitor.AllElementValueVisitor
 import proguard.classfile.attribute.visitor.AllAttributeVisitor
+import proguard.classfile.kotlin.visitor.ReferencedKotlinMetadataVisitor
 import proguard.classfile.util.EnumFieldReferenceInitializer
 import proguard.classfile.visitor.AllMethodVisitor
+import proguard.classfile.visitor.MemberVisitor
 import proguard.classfile.visitor.MultiClassVisitor
+import proguard.classfile.visitor.NamedClassVisitor
+import proguard.classfile.visitor.NamedMethodVisitor
 import proguard.testutils.ClassPoolBuilder
 import proguard.testutils.JavaSource
 import proguard.testutils.KotlinSource
@@ -212,4 +218,41 @@ class ClassUsageMarkerTest : StringSpec({
             programClassPool.classAccept("Test", MultiClassVisitor(classUsageMarker, AllMethodVisitor(classUsageMarker)))
         }
     }
+    "Given a Kotlin interface with default method implementation" {
+        val (programClassPool, _) = ClassPoolBuilder.fromSource(
+            KotlinSource(
+                "Interface.kt",
+                """
+        package test;
+        interface Interface {
+            fun foo() : Int {
+                return 42;
+            }
+        }
+                """.trimIndent(),
+            ),
+        )
+
+        // Necessary to force marking methods that are not actually used and have not been processed by the Marker.
+        class CustomMarker(var marker: SimpleUsageMarker) : MemberVisitor {
+            override fun visitAnyMember(clazz: Clazz, member: Member) {
+                marker.markAsUsed(member)
+            }
+        }
+
+        val usageMarker = SimpleUsageMarker()
+        val classUsageMarker = ClassUsageMarker(usageMarker)
+
+        // Mark the classes as used.
+        programClassPool.classesAccept(classUsageMarker)
+
+        // Mark the default implementation as used.
+        programClassPool.accept(NamedClassVisitor(NamedMethodVisitor("foo", null, CustomMarker(usageMarker)), "test/Interface\$DefaultImpls"))
+
+        // Process Kotlin metadata: this should cause the interface method to be kept as well.
+        programClassPool.classesAccept(ReferencedKotlinMetadataVisitor(classUsageMarker))
+
+        val fooInterface = programClassPool.getClass("test/Interface").findMethod("foo", null)
+        fooInterface should beMarkedWith(usageMarker)
+    }
 })
diff --git a/docs/md/manual/releasenotes.md b/docs/md/manual/releasenotes.md
@@ -1,3 +1,9 @@
+## Version 7.4.3
+
+### Bugfixes
+
+- Prevent unwanted name collision leading to missing methods in Kotlin DefaultImpls classes.
+
 ## Version 7.4.2
 
 ### Bugfixes