Skip to content

Commit 9e920e9

Browse files
supersaiyansubtletysupersaiyansubtlety
committed
update record tests
1 parent 93b9cea commit 9e920e9

File tree

7 files changed

+183
-32
lines changed

7 files changed

+183
-32
lines changed

‎enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ final class RecordIndexingVisitor extends ClassVisitor {
4040
// visitation state fields; cleared in visitEnd()
4141
private ClassEntry clazz;
4242
private final Set<RecordComponentNode> recordComponents = new HashSet<>();
43+
// TODO investigate this; may need to replace clazz with a class stack and to change this to fieldsByNameByClass
4344
// this is a multimap because inner classes' fields go in the same map as their outer class's
4445
private final Multimap<String, FieldNode> fieldsByName = HashMultimap.create();
4546
private final Multimap<String, MethodNode> methodsByDescriptor = HashMultimap.create();

‎enigma/src/test/java/org/quiltmc/enigma/input/records/NameMismatchRecord.java‎

Lines changed: 0 additions & 13 deletions
This file was deleted.

‎enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/BridgeRecord.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package org.quiltmc.enigma.input.records.name_mismatch;
2+
3+
import org.quiltmc.enigma.impl.plugin.RecordIndexingService;
4+
5+
import java.util.function.Supplier;
6+
7+
/**
8+
* {@link #get()} should be found by {@link RecordIndexingService} because it's the only getter candidate: the
9+
* {@code Object get()} bridge method should not be a candidate because it has the wrong return type and wrong access.
10+
*/
11+
public record BridgeRecord(Double get) implements Supplier<Double> { }

‎enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterRightInstructionsRecord.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package org.quiltmc.enigma.input.records.name_mismatch;
2+
3+
import org.quiltmc.enigma.impl.plugin.RecordIndexingService;
4+
5+
public record FakeGetterRightInstructionsRecord(int component) {
6+
/**
7+
* This <em>should</em> be found by {@link RecordIndexingService} as the getter because it gets the same
8+
* obf name as the component field and it has the expected descriptor, access, and instructions as a default getter.
9+
*
10+
* <p> This behavior is important because it matches decompilers' behavior. Decompilers will consider this a
11+
* default getter and hide it, so it's important that we propose a name for it to prevent it from making stats
12+
* un-completable.
13+
*/
14+
public int fakeGetter() {
15+
return this.component;
16+
}
17+
}

‎enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterWrongInstructionsRecord.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package org.quiltmc.enigma.input.records.name_mismatch;
2+
3+
import org.quiltmc.enigma.impl.plugin.RecordIndexingService;
4+
5+
/**
6+
* {@link #component()} shouldn't be found by {@link RecordIndexingService} - despite being a default getter -
7+
* because its obf name doesn't match {@link #component}'s.
8+
*/
9+
public record FakeGetterWrongInstructionsRecord(int component) {
10+
/**
11+
* This shouldn't be found by {@link RecordIndexingService} - despite its obf name, access, and descriptor matching
12+
* expectations for a getter - because its instructions don't match that of a default getter.
13+
*/
14+
public int fakeGetter() {
15+
return 0;
16+
}
17+
}

‎enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/StringComponentOverrideGetterRecord.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package org.quiltmc.enigma.input.records.name_mismatch;
2+
3+
import org.quiltmc.enigma.impl.plugin.RecordIndexingService;
4+
5+
public record StringComponentOverrideGetterRecord(String string) {
6+
/**
7+
* This getter should be found by {@link RecordIndexingService} because it's the only getter candidate:
8+
* the only other public no-args method returning a {@link String} is {@link #toString()}, and {@code toString} is
9+
* no a legal component name.
10+
*/
11+
@Override
12+
public String string() {
13+
return "";
14+
}
15+
}

‎enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java‎

Lines changed: 122 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,18 @@
1515
import org.quiltmc.enigma.api.translation.representation.entry.ClassEntry;
1616
import org.quiltmc.enigma.api.translation.representation.entry.FieldEntry;
1717
import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry;
18+
import org.quiltmc.enigma.impl.plugin.RecordComponentProposalService;
1819

1920
import java.io.IOException;
2021
import java.io.Reader;
2122
import java.io.StringReader;
2223
import java.nio.file.Path;
2324

25+
/**
26+
* Many record tests rely on the fact that proguard consistently names things in order a, b, c... which results in
27+
* most default record component getters having the same name as their fields.<br>
28+
* Changing proguard's naming configs could break many tests.
29+
*/
2430
public class TestRecordComponentProposal {
2531
private static final Path JAR = TestUtil.obfJar("records");
2632
private static EnigmaProject project;
@@ -71,32 +77,129 @@ void testSimpleRecordComponentProposal() {
7177
}
7278

7379
@Test
74-
void testMismatchRecordComponentProposal() {
75-
// name of getter mismatches with name of field
76-
ClassEntry cClass = TestEntryFactory.newClass("d");
77-
FieldEntry aField = TestEntryFactory.newField(cClass, "a", "I");
78-
MethodEntry fakeAGetter = TestEntryFactory.newMethod(cClass, "a", "()I");
79-
MethodEntry realAGetter = TestEntryFactory.newMethod(cClass, "b", "()I");
80+
void testFakeGetterWrongInstructions() {
81+
final ClassEntry fakeGetterWrongInstructionsRecord = TestEntryFactory.newClass("h");
82+
final FieldEntry componentField = TestEntryFactory.newField(fakeGetterWrongInstructionsRecord, "a", "I");
83+
final MethodEntry fakeGetter = TestEntryFactory.newMethod(fakeGetterWrongInstructionsRecord, "a", "()I");
84+
final MethodEntry componentGetter = TestEntryFactory.newMethod(fakeGetterWrongInstructionsRecord, "b", "()I");
8085

81-
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(aField).tokenType());
82-
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(fakeAGetter).tokenType());
83-
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(realAGetter).tokenType());
86+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(componentField).tokenType());
87+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(fakeGetter).tokenType());
88+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(componentGetter).tokenType());
8489

85-
project.getRemapper().putMapping(TestUtil.newVC(), aField, new EntryMapping("mapped"));
90+
final String targetName = "mapped";
91+
project.getRemapper().putMapping(TestUtil.newVC(), componentField, new EntryMapping(targetName));
8692

87-
var fieldMapping = project.getRemapper().getMapping(aField);
88-
Assertions.assertEquals(TokenType.DEOBFUSCATED, fieldMapping.tokenType());
89-
Assertions.assertEquals("mapped", fieldMapping.targetName());
93+
final EntryMapping fieldMapping = project.getRemapper().getMapping(componentField);
94+
Assertions.assertSame(TokenType.DEOBFUSCATED, fieldMapping.tokenType());
95+
Assertions.assertEquals(targetName, fieldMapping.targetName());
9096

9197
// fake getter should NOT be mapped
92-
var fakeGetterMapping = project.getRemapper().getMapping(fakeAGetter);
98+
final EntryMapping fakeGetterMapping = project.getRemapper().getMapping(fakeGetter);
9399
Assertions.assertEquals(TokenType.OBFUSCATED, fakeGetterMapping.tokenType());
94100

95-
// real getter SHOULD be mapped
96-
var realGetterMapping = project.getRemapper().getMapping(realAGetter);
97-
Assertions.assertEquals(TokenType.DYNAMIC_PROPOSED, realGetterMapping.tokenType());
98-
Assertions.assertEquals("mapped", realGetterMapping.targetName());
99-
Assertions.assertEquals("enigma:record_component_proposer", realGetterMapping.sourcePluginId());
101+
// real getter should also NOT be mapped
102+
// it's impossible to determine that it's the real getter
103+
// this behavior matches decompilers'
104+
final EntryMapping componentGetterMapping = project.getRemapper().getMapping(componentGetter);
105+
Assertions.assertEquals(TokenType.OBFUSCATED, componentGetterMapping.tokenType());
106+
}
107+
108+
@Test
109+
void testFakeGetterRightInstructions() {
110+
final ClassEntry fakeGetterRightInstructionsRecord = TestEntryFactory.newClass("g");
111+
final FieldEntry componentField = TestEntryFactory.newField(fakeGetterRightInstructionsRecord, "a", "I");
112+
final MethodEntry fakeGetter = TestEntryFactory.newMethod(fakeGetterRightInstructionsRecord, "a", "()I");
113+
final MethodEntry componentGetter = TestEntryFactory.newMethod(fakeGetterRightInstructionsRecord, "b", "()I");
114+
115+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(componentField).tokenType());
116+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(fakeGetter).tokenType());
117+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(componentGetter).tokenType());
118+
119+
final String targetName = "mapped";
120+
project.getRemapper().putMapping(TestUtil.newVC(), componentField, new EntryMapping(targetName));
121+
122+
// FAKE getter SHOULD be mapped
123+
// Assuming it's the getter - based on name, access, descriptor and instructions - matches decompilers'
124+
// assumptions.
125+
// Decompilers assume it's a default getter and hide it, so we propose a name to prevent un-completable stats.
126+
final EntryMapping fakeGetterMappings = project.getRemapper().getMapping(fakeGetter);
127+
Assertions.assertEquals(TokenType.DYNAMIC_PROPOSED, fakeGetterMappings.tokenType());
128+
Assertions.assertEquals(targetName, fakeGetterMappings.targetName());
129+
Assertions.assertEquals(RecordComponentProposalService.ID, fakeGetterMappings.sourcePluginId());
130+
131+
// real getter should NOT be mapped
132+
final EntryMapping componentGetterMapping = project.getRemapper().getMapping(componentGetter);
133+
Assertions.assertEquals(TokenType.OBFUSCATED, componentGetterMapping.tokenType());
134+
}
135+
136+
@Test
137+
void testBridgeRecord() {
138+
final String doubleDesc = "Ljava/lang/Double;";
139+
final String stringGetterDesc = "()" + doubleDesc;
140+
141+
final ClassEntry bridgeRecord = TestEntryFactory.newClass("f");
142+
final FieldEntry getField = TestEntryFactory.newField(bridgeRecord, "a", doubleDesc);
143+
final MethodEntry getGetter = TestEntryFactory.newMethod(bridgeRecord, "a", stringGetterDesc);
144+
final MethodEntry getBridge = TestEntryFactory.newMethod(bridgeRecord, "get", "()Ljava/lang/Object;");
145+
146+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(getField).tokenType());
147+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(getGetter).tokenType());
148+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(getBridge).tokenType());
149+
150+
final String targetName = "mapped";
151+
project.getRemapper().putMapping(TestUtil.newVC(), getField, new EntryMapping(targetName));
152+
153+
final EntryMapping fieldMapping = project.getRemapper().getMapping(getField);
154+
Assertions.assertSame(TokenType.DEOBFUSCATED, fieldMapping.tokenType());
155+
Assertions.assertEquals(targetName, fieldMapping.targetName());
156+
157+
// getter should be mapped; it should be the only getter candidate
158+
final EntryMapping getterMapping = project.getRemapper().getMapping(getGetter);
159+
Assertions.assertSame(TokenType.DYNAMIC_PROPOSED, getterMapping.tokenType());
160+
Assertions.assertEquals(targetName, getterMapping.targetName());
161+
Assertions.assertEquals(RecordComponentProposalService.ID, getterMapping.sourcePluginId());
162+
163+
// bridge should not be mapped; it should not be a getter candidate because
164+
// it has the wrong access and descriptor
165+
final EntryMapping bridgeMapping = project.getRemapper().getMapping(getBridge);
166+
Assertions.assertEquals(TokenType.OBFUSCATED, bridgeMapping.tokenType());
167+
}
168+
169+
@Test
170+
void testIllegalGetterNameExclusion() {
171+
final String stringDesc = "Ljava/lang/String;";
172+
final String stringGetterDesc = "()" + stringDesc;
173+
174+
final ClassEntry stringComponentOverrideGetterRecord = TestEntryFactory.newClass("i");
175+
final FieldEntry stringField = TestEntryFactory.newField(stringComponentOverrideGetterRecord, "a", stringDesc);
176+
final MethodEntry stringGetter = TestEntryFactory
177+
.newMethod(stringComponentOverrideGetterRecord, "a", stringGetterDesc);
178+
final MethodEntry toString = TestEntryFactory
179+
.newMethod(stringComponentOverrideGetterRecord, "toString", stringGetterDesc);
180+
181+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(stringField).tokenType());
182+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(stringGetter).tokenType());
183+
Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(toString).tokenType());
184+
185+
final String targetName = "mapped";
186+
project.getRemapper().putMapping(TestUtil.newVC(), stringField, new EntryMapping(targetName));
187+
188+
final EntryMapping fieldMapping = project.getRemapper().getMapping(stringField);
189+
Assertions.assertSame(TokenType.DEOBFUSCATED, fieldMapping.tokenType());
190+
Assertions.assertEquals(targetName, fieldMapping.targetName());
191+
192+
// getter should be mapped; it should be the only getter candidate: toString should be excluded from candidates
193+
// because its name is not a legal component name
194+
final EntryMapping getterMapping = project.getRemapper().getMapping(stringGetter);
195+
Assertions.assertSame(TokenType.DYNAMIC_PROPOSED, getterMapping.tokenType());
196+
Assertions.assertEquals(targetName, getterMapping.targetName());
197+
Assertions.assertEquals(RecordComponentProposalService.ID, getterMapping.sourcePluginId());
198+
199+
// toString should not be mapped because it's name doesn't match the field,
200+
// its name is no a legal component name, and it's a library method (unmappable)
201+
final EntryMapping bridgeMapping = project.getRemapper().getMapping(toString);
202+
Assertions.assertEquals(TokenType.OBFUSCATED, bridgeMapping.tokenType());
100203
}
101204

102205
@Test

0 commit comments

Comments
 (0)