Skip to content

Commit 33a0e33

Browse files
mrjameshamiltonmrjameshamilton
committed
Clean-up dangling metadata with dangling type alias references
1 parent 16deb73 commit 33a0e33

File tree

5 files changed

+188
-16
lines changed

5 files changed

+188
-16
lines changed

‎base/src/main/java/proguard/classfile/kotlin/KotlinDeclarationContainerMetadata.java‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/*
22
* ProGuardCORE -- library to process Java bytecode.
33
*
4-
* Copyright (c) 2002-2020 Guardsquare NV
4+
* Copyright (c) 2002-2022 Guardsquare NV
55
*
66
* Licensed under the Apache License, Version 2.0 (the "License");
77
* you may not use this file except in compliance with the License.
@@ -18,7 +18,11 @@
1818
package proguard.classfile.kotlin;
1919

2020
import proguard.classfile.Clazz;
21-
import proguard.classfile.kotlin.visitor.*;
21+
import proguard.classfile.kotlin.visitor.KotlinFunctionVisitor;
22+
import proguard.classfile.kotlin.visitor.KotlinMetadataVisitor;
23+
import proguard.classfile.kotlin.visitor.KotlinPropertyVisitor;
24+
import proguard.classfile.kotlin.visitor.KotlinTypeAliasVisitor;
25+
import proguard.classfile.visitor.ClassVisitor;
2226
import proguard.resources.kotlinmodule.KotlinModule;
2327
import proguard.resources.kotlinmodule.visitor.KotlinModuleVisitor;
2428

@@ -98,4 +102,20 @@ public void moduleAccept(KotlinModuleVisitor kotlinModuleVisitor)
98102
this.referencedModule.accept(kotlinModuleVisitor);
99103
}
100104
}
105+
106+
public void referencedOwnerClassAccept(ClassVisitor classVisitor)
107+
{
108+
if (this.ownerReferencedClass != null)
109+
{
110+
this.ownerReferencedClass.accept(classVisitor);
111+
}
112+
}
113+
114+
public void referencedOwnerClassAccept(KotlinMetadataVisitor kotlinMetadataVisitor)
115+
{
116+
if (this.ownerReferencedClass != null)
117+
{
118+
this.ownerReferencedClass.kotlinMetadataAccept(kotlinMetadataVisitor);
119+
}
120+
}
101121
}

‎base/src/main/java/proguard/classfile/kotlin/KotlinTypeMetadata.java‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,14 @@
2020
import proguard.classfile.Clazz;
2121
import proguard.classfile.kotlin.flags.KotlinCommonFlags;
2222
import proguard.classfile.kotlin.flags.KotlinTypeFlags;
23-
import proguard.classfile.kotlin.visitor.*;
23+
import proguard.classfile.kotlin.visitor.KotlinAnnotationVisitor;
24+
import proguard.classfile.kotlin.visitor.KotlinTypeAliasVisitor;
25+
import proguard.classfile.kotlin.visitor.KotlinTypeVisitor;
2426
import proguard.classfile.visitor.ClassVisitor;
25-
import proguard.util.*;
27+
import proguard.util.Processable;
28+
import proguard.util.SimpleProcessable;
2629

27-
import java.util.*;
30+
import java.util.List;
2831

2932
public class KotlinTypeMetadata
3033
extends SimpleProcessable
@@ -189,6 +192,15 @@ public void referencedClassAccept(ClassVisitor classVisitor)
189192
this.referencedClass.accept(classVisitor);
190193
}
191194
}
195+
196+
public void referencedTypeAliasAccept(Clazz clazz, KotlinTypeAliasVisitor kotlinTypeAliasVisitor)
197+
{
198+
if (this.referencedTypeAlias != null &&
199+
this.referencedTypeAlias.referencedDeclarationContainer != null)
200+
{
201+
this.referencedTypeAlias.accept(clazz, this.referencedTypeAlias.referencedDeclarationContainer, kotlinTypeAliasVisitor);
202+
}
203+
}
192204

193205
// Implementations for Object.
194206
@Override

‎base/src/main/java/proguard/util/kotlin/asserter/KotlinMetadataAsserter.java‎

Lines changed: 99 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,20 @@
1818

1919
package proguard.util.kotlin.asserter;
2020

21-
import java.util.Arrays;
22-
import java.util.List;
2321
import proguard.classfile.ClassPool;
2422
import proguard.classfile.Clazz;
23+
import proguard.classfile.kotlin.KotlinDeclarationContainerMetadata;
2524
import proguard.classfile.kotlin.KotlinMetadata;
25+
import proguard.classfile.kotlin.KotlinTypeAliasMetadata;
26+
import proguard.classfile.kotlin.KotlinTypeMetadata;
27+
import proguard.classfile.kotlin.visitor.AllTypeVisitor;
2628
import proguard.classfile.kotlin.visitor.KotlinMetadataRemover;
2729
import proguard.classfile.kotlin.visitor.KotlinMetadataVisitor;
30+
import proguard.classfile.kotlin.visitor.KotlinTypeAliasVisitor;
31+
import proguard.classfile.kotlin.visitor.KotlinTypeVisitor;
2832
import proguard.classfile.kotlin.visitor.ReferencedKotlinMetadataVisitor;
2933
import proguard.classfile.util.WarningLogger;
34+
import proguard.classfile.visitor.ClassVisitor;
3035
import proguard.resources.file.ResourceFile;
3136
import proguard.resources.file.ResourceFilePool;
3237
import proguard.resources.file.visitor.ResourceFileProcessingFlagFilter;
@@ -49,6 +54,9 @@
4954
import proguard.util.kotlin.asserter.constraint.TypeIntegrity;
5055
import proguard.util.kotlin.asserter.constraint.ValueParameterIntegrity;
5156

57+
import java.util.Arrays;
58+
import java.util.List;
59+
5260
/**
5361
* Performs a series of checks to see whether the kotlin metadata is intact.
5462
*/
@@ -86,6 +94,11 @@ public void execute(WarningLogger warningLogger,
8694
programClassPool.classesAccept(new ReferencedKotlinMetadataVisitor(kotlinMetadataAsserter));
8795
libraryClassPool.classesAccept(new ReferencedKotlinMetadataVisitor(kotlinMetadataAsserter));
8896

97+
ClassVisitor aliasReferenceCleaner =
98+
new ReferencedKotlinMetadataVisitor(new KotlinTypeAliasReferenceCleaner(reporter));
99+
programClassPool.classesAccept(aliasReferenceCleaner);
100+
libraryClassPool.classesAccept(aliasReferenceCleaner);
101+
89102
reporter.setErrorMessage("Warning: Kotlin module errors encountered in module %s. Not processing the metadata for this module.");
90103
resourceFilePool.resourceFilesAccept(new ResourceFileProcessingFlagFilter(0,
91104
ProcessingFlags.DONT_PROCESS_KOTLIN_MODULE,
@@ -95,10 +108,10 @@ public void execute(WarningLogger warningLogger,
95108
/**
96109
* This class performs a series of checks to see whether the kotlin metadata is intact
97110
*/
98-
public static class MyKotlinMetadataAsserter
99-
implements KotlinMetadataVisitor,
100-
ResourceFileVisitor,
101-
KotlinModuleVisitor
111+
private static class MyKotlinMetadataAsserter
112+
implements KotlinMetadataVisitor,
113+
ResourceFileVisitor,
114+
KotlinModuleVisitor
102115
{
103116
private final List<? extends KotlinAsserterConstraint> constraints;
104117
private final Reporter reporter;
@@ -145,4 +158,84 @@ public void visitKotlinModule(KotlinModule kotlinModule)
145158
@Override
146159
public void visitResourceFile(ResourceFile resourceFile) {}
147160
}
161+
162+
/**
163+
* // TODO: This may leave behind further invalid metadata.
164+
*
165+
* Cleans up any dangling references caused by removing metadata.
166+
* <p>
167+
* A {@link KotlinTypeMetadata} can have a referencedTypeAlias, which
168+
* refers to a type alias that is declared in a declaration container
169+
* of which the metadata was invalid and therefore removed, from it's "owner" class.
170+
* <p>
171+
* To avoid visiting invalid metadata later, we check if there is a link to
172+
* a class without metadata and remove the metadata from the class using the type alias
173+
* if necessary.
174+
* <p>Example, assuming `kotlin.Exception` is not available:
175+
* <code>
176+
* // AClass visited first.
177+
* // AClass.kt <--- Has a reference to MyFileFacade where MyAlias is declared.
178+
* class MyClass : MyAlias()
179+
*
180+
* // MyFileFacade.kt <--- Invalid because reference for kotlin.Exception not found
181+
* typealias MyAlias = kotlin.Exception
182+
*
183+
* // MyClass hangs onto the reference to MyFileFacade metadata, even though the
184+
* // metadata from its owner class (MyFileFacadeKt.class) has been removed.
185+
* </code>
186+
* </p>
187+
*/
188+
private static class KotlinTypeAliasReferenceCleaner implements KotlinMetadataVisitor,
189+
KotlinTypeVisitor,
190+
KotlinTypeAliasVisitor
191+
{
192+
private final Reporter reporter;
193+
private int count;
194+
195+
private KotlinTypeAliasReferenceCleaner(Reporter reporter)
196+
{
197+
this.reporter = reporter;
198+
}
199+
200+
@Override
201+
public void visitAnyKotlinMetadata(Clazz clazz, KotlinMetadata kotlinMetadata)
202+
{
203+
reporter.resetCounter(clazz.getName());
204+
kotlinMetadata.accept(clazz, new AllTypeVisitor(this));
205+
if (reporter.getCount() > 0)
206+
{
207+
clazz.accept(new KotlinMetadataRemover());
208+
}
209+
}
210+
211+
212+
@Override
213+
public void visitAnyType(Clazz clazz, KotlinTypeMetadata kotlinTypeMetadata)
214+
{
215+
if (kotlinTypeMetadata.aliasName != null &&
216+
kotlinTypeMetadata.referencedTypeAlias != null)
217+
{
218+
// The count will be increase if the alias' declaration container has no metadata.
219+
count = 0;
220+
kotlinTypeMetadata.referencedTypeAliasAccept(clazz, this);
221+
222+
boolean declarationContainerClazzHasNoMetadata = count == 0;
223+
224+
if (declarationContainerClazzHasNoMetadata)
225+
{
226+
reporter.report("Type alias '" + kotlinTypeMetadata.aliasName + "' is declared in a container with no metadata");
227+
}
228+
}
229+
}
230+
231+
232+
@Override
233+
public void visitTypeAlias(Clazz clazz,
234+
KotlinDeclarationContainerMetadata kotlinDeclarationContainerMetadata,
235+
KotlinTypeAliasMetadata kotlinTypeAliasMetadata)
236+
{
237+
// The counter will only increase if the owner class has metadata attached.
238+
kotlinDeclarationContainerMetadata.referencedOwnerClassAccept((__, metadata) -> count++);
239+
}
240+
}
148241
}

‎base/src/test/kotlin/proguard/util/kotlin/asserter/KotlinMetadataAsserterTest.kt‎

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,19 @@ import proguard.classfile.Clazz
1717
import proguard.classfile.ProgramClass
1818
import proguard.classfile.kotlin.KotlinClassKindMetadata
1919
import proguard.classfile.kotlin.KotlinMetadata
20+
import proguard.classfile.kotlin.visitor.KotlinMetadataPrinter
21+
import proguard.classfile.kotlin.visitor.KotlinMetadataRemover
2022
import proguard.classfile.kotlin.visitor.KotlinMetadataVisitor
23+
import proguard.classfile.kotlin.visitor.ReferencedKotlinMetadataVisitor
24+
import proguard.classfile.util.ClassReferenceInitializer
2125
import proguard.classfile.util.WarningLogger
26+
import proguard.classfile.util.kotlin.KotlinMetadataInitializer
2227
import proguard.resources.file.ResourceFilePool
2328
import proguard.testutils.ClassPoolBuilder
2429
import proguard.testutils.KotlinSource
2530

2631
class KotlinMetadataAsserterTest : FreeSpec({
32+
val warningLogger = WarningLogger(LogManager.getLogger(KotlinMetadataAsserter::class.java))
2733
"Given an interface with default implementation" - {
2834
val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(
2935
KotlinSource(
@@ -40,8 +46,7 @@ class KotlinMetadataAsserterTest : FreeSpec({
4046

4147
"When the KotlinMetadataAsserter is run" - {
4248

43-
// remove invalid kotlin metadata
44-
val warningLogger = WarningLogger(LogManager.getLogger(KotlinMetadataAsserter::class.java))
49+
// Remove invalid kotlin metadata
4550
KotlinMetadataAsserter().execute(warningLogger, programClassPool, libraryClassPool, ResourceFilePool())
4651
"Then the metadata should not be thrown away" {
4752
val visitor = spyk<KotlinMetadataVisitor>()
@@ -77,7 +82,6 @@ class KotlinMetadataAsserterTest : FreeSpec({
7782

7883
programClassPool.removeClass("Test\$DefaultImpls")
7984

80-
val warningLogger = WarningLogger(LogManager.getLogger(KotlinMetadataAsserter::class.java))
8185
KotlinMetadataAsserter().execute(warningLogger, programClassPool, libraryClassPool, ResourceFilePool())
8286

8387
"Then the metadata should be thrown away" {
@@ -112,7 +116,6 @@ class KotlinMetadataAsserterTest : FreeSpec({
112116
)
113117

114118
"When the KotlinMetadataAsserter is run" - {
115-
val warningLogger = WarningLogger(LogManager.getLogger(KotlinMetadataAsserter::class.java))
116119
KotlinMetadataAsserter().execute(warningLogger, programClassPool, libraryClassPool, ResourceFilePool())
117120
"Then the metadata should not be thrown away" {
118121
val visitor = spyk<KotlinMetadataVisitor>()
@@ -163,10 +166,53 @@ class KotlinMetadataAsserterTest : FreeSpec({
163166
(programClassPool.getClass("Test") as ProgramClass).kotlinMetadata shouldNotBe null
164167
// KotlinMetadataAsserter should remove Test enum's metadata because
165168
// null entries of kotlinClassKindMetadata.referencedEnumEntries violates the ClassIntegrity.
166-
val warningLogger = WarningLogger(LogManager.getLogger(KotlinMetadataAsserter::class.java))
167169
KotlinMetadataAsserter().execute(warningLogger, programClassPool, libraryClassPool, ResourceFilePool())
168170
(programClassPool.getClass("Test") as ProgramClass).kotlinMetadata shouldBe null
169171
}
170172
}
171173
}
174+
175+
"Given a user of a type alias for a library class" - {
176+
val (programClassPool, libraryClassPool) = ClassPoolBuilder.fromSource(
177+
KotlinSource(
178+
"ATest.kt",
179+
"""
180+
class ATest : MyAlias()
181+
""".trimIndent()
182+
),
183+
KotlinSource(
184+
"FileFacade.kt",
185+
"""
186+
typealias MyAlias = Exception
187+
""".trimIndent()
188+
)
189+
)
190+
191+
"When the library class has no metadata" - {
192+
// Simulate missing metadata by removing the metadata
193+
// from the library classes and re-initialize.
194+
with(KotlinMetadataInitializer(warningLogger)) {
195+
libraryClassPool.classesAccept(KotlinMetadataRemover())
196+
programClassPool.classesAccept(this)
197+
libraryClassPool.classesAccept(this)
198+
programClassPool.classesAccept(ClassReferenceInitializer(programClassPool, libraryClassPool))
199+
}
200+
201+
// Run the asserter to remove invalid metadata.
202+
KotlinMetadataAsserter().execute(warningLogger, programClassPool, libraryClassPool, ResourceFilePool())
203+
204+
// Re-initialize after running the asserter.
205+
programClassPool.classesAccept(ClassReferenceInitializer(programClassPool, libraryClassPool))
206+
207+
"Then the asserter should remove metadata for file facade where the alias is defined" {
208+
(programClassPool.getClass("FileFacadeKt") as ProgramClass).kotlinMetadata shouldBe null
209+
}
210+
211+
"Then the asserter should remove metadata for the class where the alias is used" {
212+
// Since the metadata where the alias is declared is now invalid,
213+
// ATest references now invalid metadata and is itself invalid.
214+
(programClassPool.getClass("ATest") as ProgramClass).kotlinMetadata shouldBe null
215+
}
216+
}
217+
}
172218
})

‎base/src/testFixtures/kotlin/proguard/testutils/ClassPoolBuilder.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ private class LibraryClassPoolBuilder(private val compiler: KotlinCompilation) {
206206
false,
207207
false,
208208
true,
209+
true,
209210
null,
210211
ClassPoolFiller(this)
211212
)

0 commit comments

Comments
 (0)