From 318d5b7bfe503af3c1cc5f039b566ce3925725bd Mon Sep 17 00:00:00 2001 From: supersaiyansubtlety Date: Mon, 17 Nov 2025 11:56:29 -0800 Subject: [PATCH 1/7] better utilize RecordIndexingVisitor in RecordComponentProposalService RecordIndexingVisitor: only check bytecode if multiple methods are getter candidates; allows finding overrides in simple cases RecordIndexingVisitor: when there are multiple getter candidates, check name and descriptor in addition to bytecode; prevents some false-positives in getter finding (though false positives for non-getters are still possible) --- .../RecordComponentProposalService.java | 55 ++---- .../impl/plugin/RecordIndexingVisitor.java | 174 ++++++++++++------ 2 files changed, 137 insertions(+), 92 deletions(-) diff --git a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordComponentProposalService.java b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordComponentProposalService.java index be55bc664..dddca4038 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordComponentProposalService.java +++ b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordComponentProposalService.java @@ -2,20 +2,17 @@ import org.jspecify.annotations.Nullable; import org.quiltmc.enigma.api.Enigma; -import org.quiltmc.enigma.api.analysis.index.jar.EntryIndex; import org.quiltmc.enigma.api.analysis.index.jar.JarIndex; import org.quiltmc.enigma.api.service.NameProposalService; import org.quiltmc.enigma.api.source.TokenType; import org.quiltmc.enigma.api.translation.mapping.EntryMapping; import org.quiltmc.enigma.api.translation.mapping.EntryRemapper; -import org.quiltmc.enigma.api.translation.representation.entry.ClassDefEntry; -import org.quiltmc.enigma.api.translation.representation.entry.ClassEntry; +import org.quiltmc.enigma.api.translation.mapping.tree.EntryTreeNode; import org.quiltmc.enigma.api.translation.representation.entry.Entry; import org.quiltmc.enigma.api.translation.representation.entry.FieldEntry; import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry; import java.util.HashMap; -import java.util.List; import java.util.Map; public record RecordComponentProposalService(RecordIndexingVisitor visitor) implements NameProposalService { @@ -29,14 +26,18 @@ public Map, EntryMapping> getProposedNames(Enigma enigma, JarIndex inde @Nullable @Override - public Map, EntryMapping> getDynamicProposedNames(EntryRemapper remapper, @Nullable Entry obfEntry, @Nullable EntryMapping oldMapping, @Nullable EntryMapping newMapping) { + public Map, EntryMapping> getDynamicProposedNames( + EntryRemapper remapper, @Nullable Entry obfEntry, @Nullable EntryMapping oldMapping, + @Nullable EntryMapping newMapping + ) { if (obfEntry instanceof FieldEntry fieldEntry) { - return this.mapRecordComponentGetter(remapper, fieldEntry.getContainingClass(), fieldEntry, newMapping); + return this.mapRecordComponentGetter(fieldEntry, newMapping); } else if (obfEntry == null) { - Map, EntryMapping> mappings = new HashMap<>(); - for (var mapping : remapper.getMappings()) { + final Map, EntryMapping> mappings = new HashMap<>(); + for (final EntryTreeNode mapping : remapper.getMappings()) { if (mapping.getEntry() instanceof FieldEntry fieldEntry) { - var getter = this.mapRecordComponentGetter(remapper, fieldEntry.getContainingClass(), fieldEntry, mapping.getValue()); + final Map, EntryMapping> getter = + this.mapRecordComponentGetter(fieldEntry, mapping.getValue()); if (getter != null) { mappings.putAll(getter); } @@ -50,34 +51,17 @@ public Map, EntryMapping> getDynamicProposedNames(EntryRemapper remappe } @Nullable - private Map, EntryMapping> mapRecordComponentGetter(EntryRemapper remapper, ClassEntry parent, FieldEntry obfFieldEntry, EntryMapping mapping) { - EntryIndex entryIndex = remapper.getJarIndex().getIndex(EntryIndex.class); - ClassDefEntry parentDef = entryIndex.getDefinition(parent); - var def = entryIndex.getDefinition(obfFieldEntry); - if ((parentDef != null && !parentDef.isRecord()) || (def != null && def.getAccess().isStatic())) { - return null; - } - - List obfClassMethods = remapper.getJarIndex().getChildrenByClass().get(parentDef).stream() - .filter(e -> e instanceof MethodEntry) - .map(e -> (MethodEntry) e) - .toList(); - - MethodEntry obfMethodEntry = null; - for (MethodEntry method : obfClassMethods) { - if (this.isGetter(obfFieldEntry, method)) { - obfMethodEntry = method; - break; - } - } - - if (obfMethodEntry == null) { + private Map, EntryMapping> mapRecordComponentGetter(FieldEntry obfFieldEntry, EntryMapping mapping) { + final MethodEntry obfGetter = this.visitor.getComponentGetter(obfFieldEntry); + if (obfGetter == null) { return null; } // remap method to match field - EntryMapping newMapping = mapping.tokenType() == TokenType.OBFUSCATED ? new EntryMapping(null, null, TokenType.OBFUSCATED, null) : this.createMapping(mapping.targetName(), TokenType.DYNAMIC_PROPOSED); - return Map.of(obfMethodEntry, newMapping); + final EntryMapping getterMapping = mapping.tokenType() == TokenType.OBFUSCATED + ? EntryMapping.OBFUSCATED + : this.createMapping(mapping.targetName(), TokenType.DYNAMIC_PROPOSED); + return Map.of(obfGetter, getterMapping); } @Override @@ -89,11 +73,6 @@ public void validateProposedMapping(Entry entry, EntryMapping mapping, boolea NameProposalService.super.validateProposedMapping(entry, mapping, dynamic); } - public boolean isGetter(FieldEntry obfFieldEntry, MethodEntry method) { - final MethodEntry getter = this.visitor.getComponentGetter(obfFieldEntry); - return getter != null && getter.equals(method); - } - @Override public String getId() { return ID; diff --git a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java index 5c892eeda..7aedd79c4 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java +++ b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java @@ -3,6 +3,7 @@ import com.google.common.collect.BiMap; import com.google.common.collect.HashBiMap; import com.google.common.collect.HashMultimap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; import org.jspecify.annotations.Nullable; import org.objectweb.asm.ClassVisitor; @@ -23,40 +24,74 @@ import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.stream.Stream; +// TODO add tests +// TODO javadoc, including getter uncertainty final class RecordIndexingVisitor extends ClassVisitor { + private static final int REQUIRED_GETTER_ACCESS = Opcodes.ACC_PUBLIC; + private static final int ILLEGAL_GETTER_ACCESS = Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE | Opcodes.ACC_STATIC; + + private static final ImmutableSet ILLEGAL_GETTER_NAMES = ImmutableSet + .of("clone", "finalize", "getClass", "hashCode", "notify", "notifyAll", "toString", "wait"); + + // visitation state fields; cleared in visitEnd() private ClassEntry clazz; private final Set recordComponents = new HashSet<>(); - private final Set fields = new HashSet<>(); - private final Set methods = new HashSet<>(); - - private final BiMap gettersByField; - private final Multimap fieldsByClass = HashMultimap.create(); - private final Multimap methodsByClass = HashMultimap.create(); + // this is a multimap because inner classes' fields go in the same map as their outer class's + private final Multimap fieldsByName = HashMultimap.create(); + private final Multimap methodsByDescriptor = HashMultimap.create(); + + // index fields; contents publicly queryable + private final Multimap componentFieldsByClass = HashMultimap.create(); + // holds methods that are at least probably getters for their field keys; superset of definiteComponentGettersByField + private final BiMap componentGettersByField = HashBiMap.create(); + // holds methods that are definitely the getters for their field keys + private final BiMap definiteComponentGettersByField = HashBiMap.create(); + // holds methods that are at least probably getters; superset of definiteComponentGettersByClass + private final Multimap componentGettersByClass = HashMultimap.create(); + // holds methods that are definitely component getters + private final Multimap definiteComponentGettersByClass = HashMultimap.create(); RecordIndexingVisitor() { super(Enigma.ASM_VERSION); - this.gettersByField = HashBiMap.create(); } @Nullable public MethodEntry getComponentGetter(FieldEntry componentField) { - return this.gettersByField.get(componentField); + return this.componentGettersByField.get(componentField); } @Nullable public FieldEntry getComponentField(MethodEntry componentGetter) { - return this.gettersByField.inverse().get(componentGetter); + return this.componentGettersByField.inverse().get(componentGetter); + } + + // TODO javadoc, prevent directly naming method (always match field) + @Nullable + public MethodEntry getDefiniteComponentGetter(FieldEntry componentField) { + return this.definiteComponentGettersByField.get(componentField); + } + + // TODO javadoc + @Nullable + public FieldEntry getDefiniteComponentField(MethodEntry componentGetter) { + return this.definiteComponentGettersByField.inverse().get(componentGetter); } public Stream streamComponentFields(ClassEntry recordEntry) { - return this.fieldsByClass.get(recordEntry).stream(); + return this.componentFieldsByClass.get(recordEntry).stream(); } public Stream streamComponentMethods(ClassEntry recordEntry) { - return this.methodsByClass.get(recordEntry).stream(); + return this.componentGettersByClass.get(recordEntry).stream(); + } + + // TODO javadoc + public Stream streamDefiniteComponentMethods(ClassEntry recordEntry) { + return this.definiteComponentGettersByClass.get(recordEntry).stream(); } @Override @@ -74,8 +109,8 @@ public RecordComponentVisitor visitRecordComponent(final String name, final Stri @Override public FieldVisitor visitField(final int access, final String name, final String descriptor, final String signature, final Object value) { if (this.clazz != null && ((access & Opcodes.ACC_PRIVATE) != 0) && this.recordComponents.stream().anyMatch(component -> component.name.equals(name))) { - FieldNode node = new FieldNode(this.api, access, name, descriptor, signature, value); - this.fields.add(node); + final FieldNode node = new FieldNode(this.api, access, name, descriptor, signature, value); + this.fieldsByName.put(node.name, node); return node; } @@ -85,8 +120,8 @@ public FieldVisitor visitField(final int access, final String name, final String @Override public MethodVisitor visitMethod(final int access, final String name, final String descriptor, final String signature, final String[] exceptions) { if (this.clazz != null && ((access & Opcodes.ACC_PUBLIC) != 0)) { - MethodNode node = new MethodNode(this.api, access, name, descriptor, signature, exceptions); - this.methods.add(node); + final MethodNode node = new MethodNode(this.api, access, name, descriptor, signature, exceptions); + this.methodsByDescriptor.put(node.desc, node); return node; } @@ -103,8 +138,8 @@ public void visitEnd() { } finally { this.clazz = null; this.recordComponents.clear(); - this.fields.clear(); - this.methods.clear(); + this.fieldsByName.clear(); + this.methodsByDescriptor.clear(); } } @@ -113,43 +148,74 @@ private void collectResults() { return; } - for (RecordComponentNode component : this.recordComponents) { - FieldNode field = null; - for (FieldNode node : this.fields) { - if (node.name.equals(component.name) && node.desc.equals(component.descriptor)) { - field = node; - break; - } - } - - if (field == null) { - throw new RuntimeException("Field not found for record component: " + component.name); - } - - for (MethodNode method : this.methods) { - InsnList instructions = method.instructions; - - // match bytecode to exact expected bytecode for a getter - // only check important instructions (ignore new frame instructions, etc.) - if ( - instructions.size() == 6 - && instructions.get(2).getOpcode() == Opcodes.ALOAD - && instructions.get(3) instanceof FieldInsnNode fieldInsn - && fieldInsn.getOpcode() == Opcodes.GETFIELD - && fieldInsn.owner.equals(this.clazz.getFullName()) - && fieldInsn.desc.equals(field.desc) - && fieldInsn.name.equals(field.name) - && instructions.get(4).getOpcode() >= Opcodes.IRETURN - && instructions.get(4).getOpcode() <= Opcodes.ARETURN - ) { - final FieldEntry fieldEntry = new FieldEntry(this.clazz, field.name, new TypeDescriptor(field.desc)); - final MethodEntry methodEntry = new MethodEntry(this.clazz, method.name, new MethodDescriptor(method.desc)); - - this.gettersByField.put(fieldEntry, methodEntry); - this.fieldsByClass.put(this.clazz, fieldEntry); - this.methodsByClass.put(this.clazz, methodEntry); - } - } + this.recordComponents.stream() + .map(component -> this.fieldsByName.get(component.name).stream() + .filter(field -> field.desc.equals(component.descriptor)) + .findAny() + .orElseThrow(() -> new IllegalStateException( + "Field not found for record component: " + component.name + )) + ) + .forEach(field -> { + final List potentialGetters = this.methodsByDescriptor + .get("()" + field.desc) + .stream() + .filter(method -> (method.access & REQUIRED_GETTER_ACCESS) == REQUIRED_GETTER_ACCESS) + .filter(method -> (method.access & ILLEGAL_GETTER_ACCESS) == 0) + .filter(method -> !ILLEGAL_GETTER_NAMES.contains(method.name)) + .toList(); + + if (potentialGetters.isEmpty()) { + throw new IllegalStateException("No potential getters for field: " + field); + } else { + final FieldEntry fieldEntry = + new FieldEntry(this.clazz, field.name, new TypeDescriptor(field.desc)); + // index the field even if a corresponding getter can't be found + this.componentFieldsByClass.put(this.clazz, fieldEntry); + + if (potentialGetters.size() == 1) { + this.indexGetter(potentialGetters.get(0), fieldEntry, true); + } else { + // If there are multiple methods with the getter's descriptor and access, it's impossible to + // tell which is the getter because obfuscation can mismatch getter/field names. + // This matching produces as few false-positives as possible by matching name, descriptor, + // and the bytecode of a default (non-overriden) getter method. + // It can still give a false-positive if a non-getter method's obfuscated name matches the + // field's, and that non-getter the has expected descriptor and bytecode of the getter. + // It also has false-negatives for getter overrides with non-default bytecode. + potentialGetters.stream() + .filter(method -> method.name.equals(field.name)) + // match bytecode to exact expected bytecode for a getter + // only check important instructions (ignore new frame instructions, etc.) + .filter(method -> { + final InsnList instructions = method.instructions; + return instructions.size() == 6 + && instructions.get(2).getOpcode() == Opcodes.ALOAD + && instructions.get(3) instanceof FieldInsnNode fieldInsn + && fieldInsn.getOpcode() == Opcodes.GETFIELD + && fieldInsn.owner.equals(this.clazz.getFullName()) + && fieldInsn.desc.equals(field.desc) + && fieldInsn.name.equals(field.name) + && instructions.get(4).getOpcode() >= Opcodes.IRETURN + && instructions.get(4).getOpcode() <= Opcodes.ARETURN; + }) + .findAny() + .ifPresent(getter -> this.indexGetter(getter, fieldEntry, false)); + } + } + }); + } + + private void indexGetter(MethodNode getterNode, FieldEntry fieldEntry, boolean definite) { + final MethodEntry getterEntry = + new MethodEntry(this.clazz, getterNode.name, new MethodDescriptor(getterNode.desc)); + + this.componentGettersByField.put(fieldEntry, getterEntry); + this.componentGettersByClass.put(this.clazz, getterEntry); + + if (definite) { + this.definiteComponentGettersByField.put(fieldEntry, getterEntry); + this.definiteComponentGettersByClass.put(this.clazz, getterEntry); } } } From cb33c58aac8de1a3be520cfb89f53db6ee27c68c Mon Sep 17 00:00:00 2001 From: supersaiyansubtlety Date: Mon, 17 Nov 2025 12:11:20 -0800 Subject: [PATCH 2/7] expose probable component getter methods --- .../impl/plugin/RecordIndexingVisitor.java | 48 +++++++++++++++---- 1 file changed, 38 insertions(+), 10 deletions(-) diff --git a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java index 7aedd79c4..a32c078f3 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java +++ b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java @@ -46,27 +46,33 @@ final class RecordIndexingVisitor extends ClassVisitor { // index fields; contents publicly queryable private final Multimap componentFieldsByClass = HashMultimap.create(); - // holds methods that are at least probably getters for their field keys; superset of definiteComponentGettersByField - private final BiMap componentGettersByField = HashBiMap.create(); // holds methods that are definitely the getters for their field keys private final BiMap definiteComponentGettersByField = HashBiMap.create(); - // holds methods that are at least probably getters; superset of definiteComponentGettersByClass - private final Multimap componentGettersByClass = HashMultimap.create(); + // holds methods that are probably, but not certainly getters for their field keys + private final BiMap probableComponentGettersByField = HashBiMap.create(); // holds methods that are definitely component getters private final Multimap definiteComponentGettersByClass = HashMultimap.create(); + // holds methods that are probably, but not certainly getters + private final Multimap probableComponentGettersByClass = HashMultimap.create(); RecordIndexingVisitor() { super(Enigma.ASM_VERSION); } + // TODO javadoc @Nullable public MethodEntry getComponentGetter(FieldEntry componentField) { - return this.componentGettersByField.get(componentField); + final MethodEntry definiteGetter = this.definiteComponentGettersByField.get(componentField); + return definiteGetter == null ? this.probableComponentGettersByField.get(componentField) : definiteGetter; } + // TODO javadoc @Nullable public FieldEntry getComponentField(MethodEntry componentGetter) { - return this.componentGettersByField.inverse().get(componentGetter); + final FieldEntry definiteField = this.definiteComponentGettersByField.inverse().get(componentGetter); + return definiteField == null + ? this.probableComponentGettersByField.inverse().get(componentGetter) + : definiteField; } // TODO javadoc, prevent directly naming method (always match field) @@ -81,12 +87,29 @@ public FieldEntry getDefiniteComponentField(MethodEntry componentGetter) { return this.definiteComponentGettersByField.inverse().get(componentGetter); } + // TODO javadoc + @Nullable + public MethodEntry getProbableComponentGetter(FieldEntry componentField) { + return this.probableComponentGettersByField.get(componentField); + } + + // TODO javadoc + @Nullable + public FieldEntry getProbableComponentField(MethodEntry componentGetter) { + return this.probableComponentGettersByField.inverse().get(componentGetter); + } + + // TODO javadoc public Stream streamComponentFields(ClassEntry recordEntry) { return this.componentFieldsByClass.get(recordEntry).stream(); } + // TODO javadoc public Stream streamComponentMethods(ClassEntry recordEntry) { - return this.componentGettersByClass.get(recordEntry).stream(); + return Stream.concat( + this.definiteComponentGettersByClass.get(recordEntry).stream(), + this.probableComponentGettersByClass.get(recordEntry).stream() + ); } // TODO javadoc @@ -94,6 +117,11 @@ public Stream streamDefiniteComponentMethods(ClassEntry recordEntry return this.definiteComponentGettersByClass.get(recordEntry).stream(); } + // TODO javadoc + public Stream streamProbableComponentMethods(ClassEntry recordEntry) { + return this.probableComponentGettersByClass.get(recordEntry).stream(); + } + @Override public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { super.visit(version, access, name, signature, superName, interfaces); @@ -210,12 +238,12 @@ private void indexGetter(MethodNode getterNode, FieldEntry fieldEntry, boolean d final MethodEntry getterEntry = new MethodEntry(this.clazz, getterNode.name, new MethodDescriptor(getterNode.desc)); - this.componentGettersByField.put(fieldEntry, getterEntry); - this.componentGettersByClass.put(this.clazz, getterEntry); - if (definite) { this.definiteComponentGettersByField.put(fieldEntry, getterEntry); this.definiteComponentGettersByClass.put(this.clazz, getterEntry); + } else { + this.probableComponentGettersByField.put(fieldEntry, getterEntry); + this.probableComponentGettersByClass.put(this.clazz, getterEntry); } } } From 19a52326216a5623e639abb28d0e31abfc4bd7e1 Mon Sep 17 00:00:00 2001 From: supersaiyansubtlety Date: Mon, 17 Nov 2025 14:45:14 -0800 Subject: [PATCH 3/7] update record tests --- .../impl/plugin/RecordIndexingVisitor.java | 1 + .../input/records/NameMismatchRecord.java | 13 -- .../records/name_mismatch/BridgeRecord.java | 11 ++ .../FakeGetterRightInstructionsRecord.java | 17 +++ .../FakeGetterWrongInstructionsRecord.java | 17 +++ .../StringComponentOverrideGetterRecord.java | 15 ++ .../records/TestRecordComponentProposal.java | 141 +++++++++++++++--- 7 files changed, 183 insertions(+), 32 deletions(-) delete mode 100644 enigma/src/test/java/org/quiltmc/enigma/input/records/NameMismatchRecord.java create mode 100644 enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/BridgeRecord.java create mode 100644 enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterRightInstructionsRecord.java create mode 100644 enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterWrongInstructionsRecord.java create mode 100644 enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/StringComponentOverrideGetterRecord.java diff --git a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java index a32c078f3..425019d37 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java +++ b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java @@ -40,6 +40,7 @@ final class RecordIndexingVisitor extends ClassVisitor { // visitation state fields; cleared in visitEnd() private ClassEntry clazz; private final Set recordComponents = new HashSet<>(); + // TODO investigate this; may need to replace clazz with a class stack and to change this to fieldsByNameByClass // this is a multimap because inner classes' fields go in the same map as their outer class's private final Multimap fieldsByName = HashMultimap.create(); private final Multimap methodsByDescriptor = HashMultimap.create(); diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/records/NameMismatchRecord.java b/enigma/src/test/java/org/quiltmc/enigma/input/records/NameMismatchRecord.java deleted file mode 100644 index 6d3018a2d..000000000 --- a/enigma/src/test/java/org/quiltmc/enigma/input/records/NameMismatchRecord.java +++ /dev/null @@ -1,13 +0,0 @@ -package org.quiltmc.enigma.input.records; - -public record NameMismatchRecord(int i) { - public int a() { - return 103; - } - - // obfuscates to b(), mismatching with the record component name - @Override - public int i() { - return this.i; - } -} diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/BridgeRecord.java b/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/BridgeRecord.java new file mode 100644 index 000000000..713d44c95 --- /dev/null +++ b/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/BridgeRecord.java @@ -0,0 +1,11 @@ +package org.quiltmc.enigma.input.records.name_mismatch; + +import org.quiltmc.enigma.impl.plugin.RecordIndexingService; + +import java.util.function.Supplier; + +/** + * {@link #get()} should be found by {@link RecordIndexingService} because it's the only getter candidate: the + * {@code Object get()} bridge method should not be a candidate because it has the wrong return type and wrong access. + */ +public record BridgeRecord(Double get) implements Supplier { } diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterRightInstructionsRecord.java b/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterRightInstructionsRecord.java new file mode 100644 index 000000000..f3a883e26 --- /dev/null +++ b/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterRightInstructionsRecord.java @@ -0,0 +1,17 @@ +package org.quiltmc.enigma.input.records.name_mismatch; + +import org.quiltmc.enigma.impl.plugin.RecordIndexingService; + +public record FakeGetterRightInstructionsRecord(int component) { + /** + * This should be found by {@link RecordIndexingService} as the getter because it gets the same + * obf name as the component field and it has the expected descriptor, access, and instructions as a default getter. + * + *

This behavior is important because it matches decompilers' behavior. Decompilers will consider this a + * default getter and hide it, so it's important that we propose a name for it to prevent it from making stats + * un-completable. + */ + public int fakeGetter() { + return this.component; + } +} diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterWrongInstructionsRecord.java b/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterWrongInstructionsRecord.java new file mode 100644 index 000000000..edce5ebe3 --- /dev/null +++ b/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/FakeGetterWrongInstructionsRecord.java @@ -0,0 +1,17 @@ +package org.quiltmc.enigma.input.records.name_mismatch; + +import org.quiltmc.enigma.impl.plugin.RecordIndexingService; + +/** + * {@link #component()} shouldn't be found by {@link RecordIndexingService} - despite being a default getter - + * because its obf name doesn't match {@link #component}'s. + */ +public record FakeGetterWrongInstructionsRecord(int component) { + /** + * This shouldn't be found by {@link RecordIndexingService} - despite its obf name, access, and descriptor matching + * expectations for a getter - because its instructions don't match that of a default getter. + */ + public int fakeGetter() { + return 0; + } +} diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/StringComponentOverrideGetterRecord.java b/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/StringComponentOverrideGetterRecord.java new file mode 100644 index 000000000..9e4ea7698 --- /dev/null +++ b/enigma/src/test/java/org/quiltmc/enigma/input/records/name_mismatch/StringComponentOverrideGetterRecord.java @@ -0,0 +1,15 @@ +package org.quiltmc.enigma.input.records.name_mismatch; + +import org.quiltmc.enigma.impl.plugin.RecordIndexingService; + +public record StringComponentOverrideGetterRecord(String string) { + /** + * This getter should be found by {@link RecordIndexingService} because it's the only getter candidate: + * the only other public no-args method returning a {@link String} is {@link #toString()}, and {@code toString} is + * no a legal component name. + */ + @Override + public String string() { + return ""; + } +} diff --git a/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java b/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java index 1fdb6ad4c..ddae78c80 100644 --- a/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java +++ b/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java @@ -15,12 +15,18 @@ import org.quiltmc.enigma.api.translation.representation.entry.ClassEntry; import org.quiltmc.enigma.api.translation.representation.entry.FieldEntry; import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry; +import org.quiltmc.enigma.impl.plugin.RecordComponentProposalService; import java.io.IOException; import java.io.Reader; import java.io.StringReader; import java.nio.file.Path; +/** + * Many record tests rely on the fact that proguard consistently names things in order a, b, c... which results in + * most default record component getters having the same name as their fields.
+ * Changing proguard's naming configs could break many tests. + */ public class TestRecordComponentProposal { private static final Path JAR = TestUtil.obfJar("records"); private static EnigmaProject project; @@ -71,32 +77,129 @@ void testSimpleRecordComponentProposal() { } @Test - void testMismatchRecordComponentProposal() { - // name of getter mismatches with name of field - ClassEntry cClass = TestEntryFactory.newClass("d"); - FieldEntry aField = TestEntryFactory.newField(cClass, "a", "I"); - MethodEntry fakeAGetter = TestEntryFactory.newMethod(cClass, "a", "()I"); - MethodEntry realAGetter = TestEntryFactory.newMethod(cClass, "b", "()I"); + void testFakeGetterWrongInstructions() { + final ClassEntry fakeGetterWrongInstructionsRecord = TestEntryFactory.newClass("h"); + final FieldEntry componentField = TestEntryFactory.newField(fakeGetterWrongInstructionsRecord, "a", "I"); + final MethodEntry fakeGetter = TestEntryFactory.newMethod(fakeGetterWrongInstructionsRecord, "a", "()I"); + final MethodEntry componentGetter = TestEntryFactory.newMethod(fakeGetterWrongInstructionsRecord, "b", "()I"); - Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(aField).tokenType()); - Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(fakeAGetter).tokenType()); - Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(realAGetter).tokenType()); + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(componentField).tokenType()); + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(fakeGetter).tokenType()); + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(componentGetter).tokenType()); - project.getRemapper().putMapping(TestUtil.newVC(), aField, new EntryMapping("mapped")); + final String targetName = "mapped"; + project.getRemapper().putMapping(TestUtil.newVC(), componentField, new EntryMapping(targetName)); - var fieldMapping = project.getRemapper().getMapping(aField); - Assertions.assertEquals(TokenType.DEOBFUSCATED, fieldMapping.tokenType()); - Assertions.assertEquals("mapped", fieldMapping.targetName()); + final EntryMapping fieldMapping = project.getRemapper().getMapping(componentField); + Assertions.assertSame(TokenType.DEOBFUSCATED, fieldMapping.tokenType()); + Assertions.assertEquals(targetName, fieldMapping.targetName()); // fake getter should NOT be mapped - var fakeGetterMapping = project.getRemapper().getMapping(fakeAGetter); + final EntryMapping fakeGetterMapping = project.getRemapper().getMapping(fakeGetter); Assertions.assertEquals(TokenType.OBFUSCATED, fakeGetterMapping.tokenType()); - // real getter SHOULD be mapped - var realGetterMapping = project.getRemapper().getMapping(realAGetter); - Assertions.assertEquals(TokenType.DYNAMIC_PROPOSED, realGetterMapping.tokenType()); - Assertions.assertEquals("mapped", realGetterMapping.targetName()); - Assertions.assertEquals("enigma:record_component_proposer", realGetterMapping.sourcePluginId()); + // real getter should also NOT be mapped + // it's impossible to determine that it's the real getter + // this behavior matches decompilers' + final EntryMapping componentGetterMapping = project.getRemapper().getMapping(componentGetter); + Assertions.assertEquals(TokenType.OBFUSCATED, componentGetterMapping.tokenType()); + } + + @Test + void testFakeGetterRightInstructions() { + final ClassEntry fakeGetterRightInstructionsRecord = TestEntryFactory.newClass("g"); + final FieldEntry componentField = TestEntryFactory.newField(fakeGetterRightInstructionsRecord, "a", "I"); + final MethodEntry fakeGetter = TestEntryFactory.newMethod(fakeGetterRightInstructionsRecord, "a", "()I"); + final MethodEntry componentGetter = TestEntryFactory.newMethod(fakeGetterRightInstructionsRecord, "b", "()I"); + + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(componentField).tokenType()); + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(fakeGetter).tokenType()); + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(componentGetter).tokenType()); + + final String targetName = "mapped"; + project.getRemapper().putMapping(TestUtil.newVC(), componentField, new EntryMapping(targetName)); + + // FAKE getter SHOULD be mapped + // Assuming it's the getter - based on name, access, descriptor and instructions - matches decompilers' + // assumptions. + // Decompilers assume it's a default getter and hide it, so we propose a name to prevent un-completable stats. + final EntryMapping fakeGetterMappings = project.getRemapper().getMapping(fakeGetter); + Assertions.assertEquals(TokenType.DYNAMIC_PROPOSED, fakeGetterMappings.tokenType()); + Assertions.assertEquals(targetName, fakeGetterMappings.targetName()); + Assertions.assertEquals(RecordComponentProposalService.ID, fakeGetterMappings.sourcePluginId()); + + // real getter should NOT be mapped + final EntryMapping componentGetterMapping = project.getRemapper().getMapping(componentGetter); + Assertions.assertEquals(TokenType.OBFUSCATED, componentGetterMapping.tokenType()); + } + + @Test + void testBridgeRecord() { + final String doubleDesc = "Ljava/lang/Double;"; + final String stringGetterDesc = "()" + doubleDesc; + + final ClassEntry bridgeRecord = TestEntryFactory.newClass("f"); + final FieldEntry getField = TestEntryFactory.newField(bridgeRecord, "a", doubleDesc); + final MethodEntry getGetter = TestEntryFactory.newMethod(bridgeRecord, "a", stringGetterDesc); + final MethodEntry getBridge = TestEntryFactory.newMethod(bridgeRecord, "get", "()Ljava/lang/Object;"); + + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(getField).tokenType()); + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(getGetter).tokenType()); + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(getBridge).tokenType()); + + final String targetName = "mapped"; + project.getRemapper().putMapping(TestUtil.newVC(), getField, new EntryMapping(targetName)); + + final EntryMapping fieldMapping = project.getRemapper().getMapping(getField); + Assertions.assertSame(TokenType.DEOBFUSCATED, fieldMapping.tokenType()); + Assertions.assertEquals(targetName, fieldMapping.targetName()); + + // getter should be mapped; it should be the only getter candidate + final EntryMapping getterMapping = project.getRemapper().getMapping(getGetter); + Assertions.assertSame(TokenType.DYNAMIC_PROPOSED, getterMapping.tokenType()); + Assertions.assertEquals(targetName, getterMapping.targetName()); + Assertions.assertEquals(RecordComponentProposalService.ID, getterMapping.sourcePluginId()); + + // bridge should not be mapped; it should not be a getter candidate because + // it has the wrong access and descriptor + final EntryMapping bridgeMapping = project.getRemapper().getMapping(getBridge); + Assertions.assertEquals(TokenType.OBFUSCATED, bridgeMapping.tokenType()); + } + + @Test + void testIllegalGetterNameExclusion() { + final String stringDesc = "Ljava/lang/String;"; + final String stringGetterDesc = "()" + stringDesc; + + final ClassEntry stringComponentOverrideGetterRecord = TestEntryFactory.newClass("i"); + final FieldEntry stringField = TestEntryFactory.newField(stringComponentOverrideGetterRecord, "a", stringDesc); + final MethodEntry stringGetter = TestEntryFactory + .newMethod(stringComponentOverrideGetterRecord, "a", stringGetterDesc); + final MethodEntry toString = TestEntryFactory + .newMethod(stringComponentOverrideGetterRecord, "toString", stringGetterDesc); + + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(stringField).tokenType()); + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(stringGetter).tokenType()); + Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(toString).tokenType()); + + final String targetName = "mapped"; + project.getRemapper().putMapping(TestUtil.newVC(), stringField, new EntryMapping(targetName)); + + final EntryMapping fieldMapping = project.getRemapper().getMapping(stringField); + Assertions.assertSame(TokenType.DEOBFUSCATED, fieldMapping.tokenType()); + Assertions.assertEquals(targetName, fieldMapping.targetName()); + + // getter should be mapped; it should be the only getter candidate: toString should be excluded from candidates + // because its name is not a legal component name + final EntryMapping getterMapping = project.getRemapper().getMapping(stringGetter); + Assertions.assertSame(TokenType.DYNAMIC_PROPOSED, getterMapping.tokenType()); + Assertions.assertEquals(targetName, getterMapping.targetName()); + Assertions.assertEquals(RecordComponentProposalService.ID, getterMapping.sourcePluginId()); + + // toString should not be mapped because it's name doesn't match the field, + // its name is no a legal component name, and it's a library method (unmappable) + final EntryMapping bridgeMapping = project.getRemapper().getMapping(toString); + Assertions.assertEquals(TokenType.OBFUSCATED, bridgeMapping.tokenType()); } @Test From 22ffd627a8accfcad3afb9def777602bb46fc52c Mon Sep 17 00:00:00 2001 From: supersaiyansubtlety Date: Mon, 17 Nov 2025 19:33:19 -0800 Subject: [PATCH 4/7] update comments, javadoc RecordIndexingService --- .../impl/plugin/RecordIndexingService.java | 112 ++++++++++++++++++ .../impl/plugin/RecordIndexingVisitor.java | 48 +++++--- 2 files changed, 146 insertions(+), 14 deletions(-) diff --git a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingService.java b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingService.java index be2bbf042..845834310 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingService.java +++ b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingService.java @@ -12,6 +12,34 @@ import java.util.Set; import java.util.stream.Stream; +/** + * Indexes records, finding component getters and their corresponding fields. + * + *

While component fields can be reliably indexed, there can be uncertainty in determining their corresponding + * getters. Some getters can be definitively determined, some are classified as 'probable getters' + * (probabilistically determined), and some cannot be determined at all. + * + *

{@link RecordIndexingService} provides separate methods for accessing getters that are definitive, probabilistic, + * or either.
+ * Either: + *

    + *
  • {@link #getComponentGetter(FieldEntry)} + *
  • {@link #getComponentField(MethodEntry)} + *
  • {@link #streamComponentMethods(ClassEntry)} + *
+ * Definite: + *
    + *
  • {@link #getDefiniteComponentGetter(FieldEntry)} + *
  • {@link #getDefiniteComponentField(MethodEntry)} + *
  • {@link #streamDefiniteComponentMethods(ClassEntry)} + *
+ * Probable: + *
    + *
  • {@link #getProbableComponentGetter(FieldEntry)} + *
  • {@link #getProbableComponentField(MethodEntry)} + *
  • {@link #streamProbableComponentMethods(ClassEntry)} + *
+ */ public class RecordIndexingService implements JarIndexerService { public static final String ID = "enigma:record_component_indexer"; @@ -21,24 +49,108 @@ public class RecordIndexingService implements JarIndexerService { this.visitor = visitor; } + /** + * @return the {@link MethodEntry} representing the getter of the passed {@code componentField}, + * or {@code null} if the passed {@code componentField} is not a record component field + * or if its getter could not be determined; returns both + * {@linkplain #getDefiniteComponentGetter(FieldEntry) definitive} and + * {@linkplain #getProbableComponentGetter(FieldEntry) probable} getters + */ @Nullable public MethodEntry getComponentGetter(FieldEntry componentField) { return this.visitor.getComponentGetter(componentField); } + /** + * @return the {@link FieldEntry} representing the field of the passed {@code componentGetter}, + * or {@code null} if the passed {@code componentGetter} is not a record component getter + * or if its field could not be determined; returns both + * {@linkplain #getDefiniteComponentField(MethodEntry) definitive} and + * {@linkplain #getProbableComponentField(MethodEntry) probable} fields + */ @Nullable public FieldEntry getComponentField(MethodEntry componentGetter) { return this.visitor.getComponentField(componentGetter); } + /** + * @return the definitive {@link MethodEntry} representing the getter of the passed {@code componentField}, + * or {@code null} if the passed {@code componentField} is not a record component field + * or if its getter could not be definitively determined + */ + @Nullable + public MethodEntry getDefiniteComponentGetter(FieldEntry componentField) { + return this.visitor.getDefiniteComponentGetter(componentField); + } + + /** + * @return the definitive {@link FieldEntry} representing the field of the passed {@code componentGetter}, + * or {@code null} if the passed {@code componentGetter} is not a record component getter + * or if its field could not be definitively determined + */ + @Nullable + public FieldEntry getDefiniteComponentField(MethodEntry componentGetter) { + return this.visitor.getDefiniteComponentField(componentGetter); + } + + /** + * @return the probable {@link MethodEntry} representing the getter of the passed {@code componentField}, + * or {@code null} if the passed {@code componentField} is not a record component field + * or if its getter was not probabilistically determined; + * does not include {@linkplain #getDefiniteComponentGetter(FieldEntry) definitive} getters + */ + @Nullable + public MethodEntry getProbableComponentGetter(FieldEntry componentField) { + return this.visitor.getProbableComponentGetter(componentField); + } + + /** + * @return the probably {@link FieldEntry} representing the field of the passed {@code componentGetter}, + * or {@code null} if the passed {@code componentGetter} is not a record component getter + * or if its field was not probabilistically determined; + * does not include {@linkplain #getDefiniteComponentField(MethodEntry) definitive} fields + */ + @Nullable + public FieldEntry getProbableComponentField(MethodEntry componentGetter) { + return this.visitor.getProbableComponentField(componentGetter); + } + + /** + * @return a {@link Stream} of component fields of the passed {@code recordEntry}; + * there's no uncertainty in getter field determination, so all fields are always included; + * if the passed {@code recordEntry} does not represent a record, the stream is empty + */ public Stream streamComponentFields(ClassEntry recordEntry) { return this.visitor.streamComponentFields(recordEntry); } + /** + * @return a {@link Stream} of component getter methods of the passed {@code recordEntry}; + * includes both {@linkplain #streamDefiniteComponentMethods(ClassEntry) definitive} and + * {@linkplain #streamProbableComponentMethods(ClassEntry) probable} getters; + * if the passed {@code recordEntry} does not represent a record, the stream is empty + */ public Stream streamComponentMethods(ClassEntry recordEntry) { return this.visitor.streamComponentMethods(recordEntry); } + /** + * @return a {@link Stream} of definitive component getter methods of the passed {@code recordEntry}; + * if the passed {@code recordEntry} does not represent a record, the stream is empty + */ + public Stream streamDefiniteComponentMethods(ClassEntry recordEntry) { + return this.visitor.streamDefiniteComponentMethods(recordEntry); + } + + /** + * @return a {@link Stream} of probable component getter methods of the passed {@code recordEntry}; + * does not include {@linkplain #streamDefiniteComponentMethods(ClassEntry) definitive} getters; + * if the passed {@code recordEntry} does not represent a record, the stream is empty + */ + public Stream streamProbableComponentMethods(ClassEntry recordEntry) { + return this.visitor.streamProbableComponentMethods(recordEntry); + } + @Override public void acceptJar(Set scope, ProjectClassProvider classProvider, JarIndex jarIndex) { for (String className : scope) { diff --git a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java index 425019d37..6d97ad63d 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java +++ b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java @@ -28,8 +28,9 @@ import java.util.Set; import java.util.stream.Stream; -// TODO add tests -// TODO javadoc, including getter uncertainty +/** + * @see RecordIndexingService + */ final class RecordIndexingVisitor extends ClassVisitor { private static final int REQUIRED_GETTER_ACCESS = Opcodes.ACC_PUBLIC; private static final int ILLEGAL_GETTER_ACCESS = Opcodes.ACC_SYNTHETIC | Opcodes.ACC_BRIDGE | Opcodes.ACC_STATIC; @@ -40,8 +41,7 @@ final class RecordIndexingVisitor extends ClassVisitor { // visitation state fields; cleared in visitEnd() private ClassEntry clazz; private final Set recordComponents = new HashSet<>(); - // TODO investigate this; may need to replace clazz with a class stack and to change this to fieldsByNameByClass - // this is a multimap because inner classes' fields go in the same map as their outer class's + // this is a multimap because proguard can give component fields with different types the same name private final Multimap fieldsByName = HashMultimap.create(); private final Multimap methodsByDescriptor = HashMultimap.create(); @@ -60,14 +60,18 @@ final class RecordIndexingVisitor extends ClassVisitor { super(Enigma.ASM_VERSION); } - // TODO javadoc + /** + * @see RecordIndexingService#getComponentGetter(FieldEntry) + */ @Nullable public MethodEntry getComponentGetter(FieldEntry componentField) { final MethodEntry definiteGetter = this.definiteComponentGettersByField.get(componentField); return definiteGetter == null ? this.probableComponentGettersByField.get(componentField) : definiteGetter; } - // TODO javadoc + /** + * @see RecordIndexingService#getComponentField(MethodEntry) + */ @Nullable public FieldEntry getComponentField(MethodEntry componentGetter) { final FieldEntry definiteField = this.definiteComponentGettersByField.inverse().get(componentGetter); @@ -76,36 +80,48 @@ public FieldEntry getComponentField(MethodEntry componentGetter) { : definiteField; } - // TODO javadoc, prevent directly naming method (always match field) + /** + * @see RecordIndexingService#getDefiniteComponentGetter(FieldEntry) + */ @Nullable public MethodEntry getDefiniteComponentGetter(FieldEntry componentField) { return this.definiteComponentGettersByField.get(componentField); } - // TODO javadoc + /** + * @see RecordIndexingService#getDefiniteComponentField(MethodEntry) + */ @Nullable public FieldEntry getDefiniteComponentField(MethodEntry componentGetter) { return this.definiteComponentGettersByField.inverse().get(componentGetter); } - // TODO javadoc + /** + * @see RecordIndexingService#getProbableComponentGetter(FieldEntry) + */ @Nullable public MethodEntry getProbableComponentGetter(FieldEntry componentField) { return this.probableComponentGettersByField.get(componentField); } - // TODO javadoc + /** + * @see RecordIndexingService#getProbableComponentField(MethodEntry) + */ @Nullable public FieldEntry getProbableComponentField(MethodEntry componentGetter) { return this.probableComponentGettersByField.inverse().get(componentGetter); } - // TODO javadoc + /** + * @see RecordIndexingService#streamComponentFields(ClassEntry) + */ public Stream streamComponentFields(ClassEntry recordEntry) { return this.componentFieldsByClass.get(recordEntry).stream(); } - // TODO javadoc + /** + * @see RecordIndexingService#streamComponentMethods(ClassEntry) + */ public Stream streamComponentMethods(ClassEntry recordEntry) { return Stream.concat( this.definiteComponentGettersByClass.get(recordEntry).stream(), @@ -113,12 +129,16 @@ public Stream streamComponentMethods(ClassEntry recordEntry) { ); } - // TODO javadoc + /** + * @see RecordIndexingService#streamDefiniteComponentMethods(ClassEntry) + */ public Stream streamDefiniteComponentMethods(ClassEntry recordEntry) { return this.definiteComponentGettersByClass.get(recordEntry).stream(); } - // TODO javadoc + /** + * @see RecordIndexingService#streamProbableComponentMethods(ClassEntry) + */ public Stream streamProbableComponentMethods(ClassEntry recordEntry) { return this.probableComponentGettersByClass.get(recordEntry).stream(); } From bf00f87cbb16a29d50650f83cfcef7e53bcfd860 Mon Sep 17 00:00:00 2001 From: supersaiyansubtlety Date: Mon, 17 Nov 2025 19:37:20 -0800 Subject: [PATCH 5/7] checkstyle --- .../impl/plugin/RecordIndexingVisitor.java | 20 +++++++++---------- .../records/TestRecordComponentProposal.java | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java index 6d97ad63d..8e8e136bc 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java +++ b/enigma/src/main/java/org/quiltmc/enigma/impl/plugin/RecordIndexingVisitor.java @@ -76,8 +76,8 @@ public MethodEntry getComponentGetter(FieldEntry componentField) { public FieldEntry getComponentField(MethodEntry componentGetter) { final FieldEntry definiteField = this.definiteComponentGettersByField.inverse().get(componentGetter); return definiteField == null - ? this.probableComponentGettersByField.inverse().get(componentGetter) - : definiteField; + ? this.probableComponentGettersByField.inverse().get(componentGetter) + : definiteField; } /** @@ -239,14 +239,14 @@ private void collectResults() { .filter(method -> { final InsnList instructions = method.instructions; return instructions.size() == 6 - && instructions.get(2).getOpcode() == Opcodes.ALOAD - && instructions.get(3) instanceof FieldInsnNode fieldInsn - && fieldInsn.getOpcode() == Opcodes.GETFIELD - && fieldInsn.owner.equals(this.clazz.getFullName()) - && fieldInsn.desc.equals(field.desc) - && fieldInsn.name.equals(field.name) - && instructions.get(4).getOpcode() >= Opcodes.IRETURN - && instructions.get(4).getOpcode() <= Opcodes.ARETURN; + && instructions.get(2).getOpcode() == Opcodes.ALOAD + && instructions.get(3) instanceof FieldInsnNode fieldInsn + && fieldInsn.getOpcode() == Opcodes.GETFIELD + && fieldInsn.owner.equals(this.clazz.getFullName()) + && fieldInsn.desc.equals(field.desc) + && fieldInsn.name.equals(field.name) + && instructions.get(4).getOpcode() >= Opcodes.IRETURN + && instructions.get(4).getOpcode() <= Opcodes.ARETURN; }) .findAny() .ifPresent(getter -> this.indexGetter(getter, fieldEntry, false)); diff --git a/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java b/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java index ddae78c80..ccfe02f30 100644 --- a/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java +++ b/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java @@ -174,9 +174,9 @@ void testIllegalGetterNameExclusion() { final ClassEntry stringComponentOverrideGetterRecord = TestEntryFactory.newClass("i"); final FieldEntry stringField = TestEntryFactory.newField(stringComponentOverrideGetterRecord, "a", stringDesc); final MethodEntry stringGetter = TestEntryFactory - .newMethod(stringComponentOverrideGetterRecord, "a", stringGetterDesc); + .newMethod(stringComponentOverrideGetterRecord, "a", stringGetterDesc); final MethodEntry toString = TestEntryFactory - .newMethod(stringComponentOverrideGetterRecord, "toString", stringGetterDesc); + .newMethod(stringComponentOverrideGetterRecord, "toString", stringGetterDesc); Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(stringField).tokenType()); Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(stringGetter).tokenType()); From 5e8b0a5f6e26f691af52c9fe51ecb95bec817d56 Mon Sep 17 00:00:00 2001 From: supersaiyansubtlety Date: Tue, 18 Nov 2025 07:07:22 -0800 Subject: [PATCH 6/7] update comments --- .../quiltmc/enigma/records/TestRecordComponentProposal.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java b/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java index ccfe02f30..4dc983254 100644 --- a/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java +++ b/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java @@ -140,6 +140,7 @@ void testBridgeRecord() { final ClassEntry bridgeRecord = TestEntryFactory.newClass("f"); final FieldEntry getField = TestEntryFactory.newField(bridgeRecord, "a", doubleDesc); + // once Supplier is indexed as a lib, this should be named get final MethodEntry getGetter = TestEntryFactory.newMethod(bridgeRecord, "a", stringGetterDesc); final MethodEntry getBridge = TestEntryFactory.newMethod(bridgeRecord, "get", "()Ljava/lang/Object;"); @@ -197,7 +198,7 @@ void testIllegalGetterNameExclusion() { Assertions.assertEquals(RecordComponentProposalService.ID, getterMapping.sourcePluginId()); // toString should not be mapped because it's name doesn't match the field, - // its name is no a legal component name, and it's a library method (unmappable) + // its name is not a legal component name, and it's a library method (unmappable) final EntryMapping bridgeMapping = project.getRemapper().getMapping(toString); Assertions.assertEquals(TokenType.OBFUSCATED, bridgeMapping.tokenType()); } From 79020efb45b0f0bbf56847f93f233172366ce793 Mon Sep 17 00:00:00 2001 From: supersaiyansubtlety Date: Tue, 18 Nov 2025 14:40:57 -0800 Subject: [PATCH 7/7] checkstyle --- .../org/quiltmc/enigma/records/TestRecordComponentProposal.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java b/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java index 4dc983254..ece7575ee 100644 --- a/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java +++ b/enigma/src/test/java/org/quiltmc/enigma/records/TestRecordComponentProposal.java @@ -177,7 +177,7 @@ void testIllegalGetterNameExclusion() { final MethodEntry stringGetter = TestEntryFactory .newMethod(stringComponentOverrideGetterRecord, "a", stringGetterDesc); final MethodEntry toString = TestEntryFactory - .newMethod(stringComponentOverrideGetterRecord, "toString", stringGetterDesc); + .newMethod(stringComponentOverrideGetterRecord, "toString", stringGetterDesc); Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(stringField).tokenType()); Assertions.assertSame(TokenType.OBFUSCATED, project.getRemapper().getMapping(stringGetter).tokenType());