Skip to content

Commit

Permalink
fix drop invalid taking 2 runs to resolve a certain drop (#248)
Browse files Browse the repository at this point in the history 
grins
  • Loading branch information
@ix0rai
ix0rai committed Jan 9, 2025
1 parent f9cd1eb commit 7b3a900
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 38 deletions.
15 changes: 15 additions & 0 deletions enigma-cli/src/test/java/org/quiltmc/enigma/command/DropInvalidMappingsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
public class DropInvalidMappingsTest extends CommandTest {
private static final Path LONE_JAR = TestUtil.obfJar("lone_class");
private static final Path INNER_JAR = TestUtil.obfJar("inner_classes");
private static final Path ENUMS_JAR = TestUtil.obfJar("enums");
private static final Path INPUT_DIR = getResource("/drop_invalid_mappings/input/");
private static final Path EXPECTED_DIR = getResource("/drop_invalid_mappings/expected/");

Expand All @@ -21,6 +22,8 @@ public class DropInvalidMappingsTest extends CommandTest {
private static final Path MAPPING_SAVE_EXPECTED = EXPECTED_DIR.resolve("MappingSave.mapping");
private static final Path DISCARD_INNER_CLASS_INPUT = INPUT_DIR.resolve("DiscardInnerClass.mapping");
private static final Path DISCARD_INNER_CLASS_EXPECTED = EXPECTED_DIR.resolve("DiscardInnerClass.mapping");
private static final Path ENUMS_INPUT = INPUT_DIR.resolve("Enums.mapping");
private static final Path ENUMS_EXPECTED = EXPECTED_DIR.resolve("Enums.mapping");

@Test
public void testInvalidMappings() throws Exception {
Expand Down Expand Up @@ -69,4 +72,16 @@ public void testDiscardInnerClass() throws Exception {

Assertions.assertEquals(expectedLines, actualLines);
}

@Test
public void testEnums() throws Exception {
Path resultFile = Files.createTempFile("enums", ".mapping");

DropInvalidMappingsCommand.run(ENUMS_JAR, ENUMS_INPUT, resultFile);

String expectedLines = Files.readString(ENUMS_EXPECTED);
String actualLines = Files.readString(resultFile);

Assertions.assertEquals(expectedLines, actualLines);
}
}
5 changes: 5 additions & 0 deletions enigma-cli/src/test/resources/drop_invalid_mappings/expected/Enums.mapping
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
CLASS a Gaming
FIELD a wawa I
METHOD <init> (Ljava/lang/String;II)V
ARG 3 wawa
METHOD a wawa ()I
7 changes: 7 additions & 0 deletions enigma-cli/src/test/resources/drop_invalid_mappings/input/Enums.mapping
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CLASS a Gaming
FIELD a wawa I
METHOD <init> (Ljava/lang/String;II)V
ARG 3 wawa
METHOD a wawa ()I
METHOD valueOf (Ljava/lang/String;)La;
ARG 0 name
20 changes: 11 additions & 9 deletions enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -143,26 +142,29 @@ public Collection<Entry<?>> dropMappings(ProgressListener progress) {
}

private Collection<Entry<?>> dropMappings(EntryTree<EntryMapping> mappings, ProgressListener progress) {
MappingsChecker.Dropper dropper = new MappingsChecker.Dropper();

// drop mappings that don't match the jar
MappingsChecker checker = new MappingsChecker(this, this.jarIndex, mappings);
MappingsChecker.Dropped droppedBroken = checker.dropBrokenMappings(progress);

Map<Entry<?>, String> droppedBrokenMappings = droppedBroken.getDroppedMappings();
checker.collectBrokenMappings(progress, dropper);

Map<Entry<?>, String> droppedBrokenMappings = dropper.getPendingDroppedMappings();
for (Map.Entry<Entry<?>, String> mapping : droppedBrokenMappings.entrySet()) {
Logger.warn("Couldn't find {} ({}) in jar. Mapping was dropped.", mapping.getKey(), mapping.getValue());
}

MappingsChecker.Dropped droppedEmpty = checker.dropEmptyMappings(progress);
dropper.applyPendingDrops(mappings);
checker.collectEmptyMappings(progress, dropper);

Map<Entry<?>, String> droppedEmptyMappings = droppedEmpty.getDroppedMappings();
Map<Entry<?>, String> droppedEmptyMappings = dropper.getPendingDroppedMappings();
for (Map.Entry<Entry<?>, String> mapping : droppedEmptyMappings.entrySet()) {
Logger.warn("{} ({}) was empty. Mapping was dropped.", mapping.getKey(), mapping.getValue());
}

Collection<Entry<?>> droppedMappings = new HashSet<>();
droppedMappings.addAll(droppedBrokenMappings.keySet());
droppedMappings.addAll(droppedEmptyMappings.keySet());
return droppedMappings;
dropper.applyPendingDrops(mappings);

return dropper.getDroppedMappings().keySet();
}

public boolean isNavigable(Entry<?> obfEntry) {
Expand Down
89 changes: 60 additions & 29 deletions enigma/src/main/java/org/quiltmc/enigma/impl/translation/mapping/MappingsChecker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
import org.quiltmc.enigma.api.translation.mapping.ResolutionStrategy;
import org.quiltmc.enigma.api.translation.mapping.tree.EntryTree;
import org.quiltmc.enigma.api.translation.mapping.tree.EntryTreeNode;
import org.quiltmc.enigma.api.translation.representation.entry.ClassEntry;
import org.quiltmc.enigma.api.translation.representation.entry.Entry;
import org.quiltmc.enigma.api.translation.representation.entry.LocalVariableEntry;
import org.quiltmc.enigma.api.translation.representation.entry.MethodEntry;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
Expand All @@ -31,39 +33,50 @@ public MappingsChecker(EnigmaProject project, JarIndex index, EntryTree<EntryMap
this.mappings = mappings;
}

private Dropped dropMappings(ProgressListener progress, BiConsumer<Dropped, Entry<?>> dropper) {
Dropped dropped = new Dropped();

private Dropper collectMappings(ProgressListener progress, Dropper dropper, BiConsumer<Dropper, Entry<?>> dropFunction) {
// HashEntryTree#getAllEntries filters out empty classes
List<? extends Entry<?>> entries = StreamSupport.stream(this.mappings.spliterator(), false).map(EntryTreeNode::getEntry).toList();
List<? extends Entry<?>> entries = new ArrayList<>(StreamSupport.stream(this.mappings.spliterator(), false).map(EntryTreeNode::getEntry).toList());

// sort so that we begin with local variables and end with class entries. this is probably a terrible way of doing this
entries.sort((a, b) -> {
if (a instanceof LocalVariableEntry && !(b instanceof LocalVariableEntry)) {
return -1;
} else if (b instanceof LocalVariableEntry && !(a instanceof LocalVariableEntry)) {
return 1;
} else if (a instanceof ClassEntry && !(b instanceof ClassEntry)) {
return 1;
} else if (b instanceof ClassEntry && !(a instanceof ClassEntry)) {
return -1;
}

return 0;
});

progress.init(entries.size(), "Checking for dropped mappings");

int steps = 0;
for (Entry<?> entry : entries) {
progress.step(steps++, entry.toString());
dropper.accept(dropped, entry);
dropFunction.accept(dropper, entry);
}

dropped.apply(this.mappings);

return dropped;
return dropper;
}

public Dropped dropBrokenMappings(ProgressListener progress) {
return this.dropMappings(progress, this::tryDropBrokenEntry);
public Dropper collectBrokenMappings(ProgressListener progress, Dropper dropper) {
return this.collectMappings(progress, dropper, this::tryDropBrokenEntry);
}

private void tryDropBrokenEntry(Dropped dropped, Entry<?> entry) {
if (this.shouldDropBrokenEntry(dropped, entry)) {
private void tryDropBrokenEntry(Dropper dropper, Entry<?> entry) {
if (this.shouldDropBrokenEntry(dropper, entry)) {
EntryMapping mapping = this.mappings.get(entry);
if (mapping != null) {
dropped.drop(entry, mapping);
dropper.addPendingDrop(entry, mapping);
}
}
}

private boolean shouldDropBrokenEntry(Dropped dropped, Entry<?> entry) {
private boolean shouldDropBrokenEntry(Dropper dropper, Entry<?> entry) {
if (!this.index.getIndex(EntryIndex.class).hasEntry(entry)
|| (entry instanceof LocalVariableEntry parameter && !this.project.validateParameterIndex(parameter))) {
return true;
Expand All @@ -80,47 +93,49 @@ private boolean shouldDropBrokenEntry(Dropped dropped, Entry<?> entry) {
}

// Method entry has parameter names, keep it even though it's not the root.
return !(entry instanceof MethodEntry) || this.hasNoMappedChildren(entry, dropped);
return !(entry instanceof MethodEntry) || this.hasNoMappedChildren(entry, dropper);

// Entry is not the root, and is not a method with params
}

public Dropped dropEmptyMappings(ProgressListener progress) {
return this.dropMappings(progress, this::tryDropEmptyEntry);
public Dropper collectEmptyMappings(ProgressListener progress, Dropper dropperBroken) {
return this.collectMappings(progress, dropperBroken, this::tryDropEmptyEntry);
}

private void tryDropEmptyEntry(Dropped dropped, Entry<?> entry) {
if (this.shouldDropEmptyMapping(dropped, entry)) {
private void tryDropEmptyEntry(Dropper dropper, Entry<?> entry) {
if (this.shouldDropEmptyMapping(dropper, entry)) {
EntryMapping mapping = this.mappings.get(entry);
if (mapping != null) {
dropped.drop(entry, mapping);
dropper.addPendingDrop(entry, mapping);
}
}
}

private boolean shouldDropEmptyMapping(Dropped dropped, Entry<?> entry) {
private boolean shouldDropEmptyMapping(Dropper dropper, Entry<?> entry) {
EntryMapping mapping = this.mappings.get(entry);
if (mapping != null) {
boolean isEmpty = (mapping.targetName() == null && mapping.javadoc() == null) || !this.project.isRenamable(entry);

if (isEmpty) {
return this.hasNoMappedChildren(entry, dropped);
return this.hasNoMappedChildren(entry, dropper);
}
}

return false;
}

private boolean hasNoMappedChildren(Entry<?> entry, Dropped dropped) {
private boolean hasNoMappedChildren(Entry<?> entry, Dropper dropper) {
var children = this.mappings.getChildren(entry);

// account for child mappings that have been dropped already
if (!children.isEmpty()) {
var droppedMappings = dropper.getDroppedAndPending();

for (Entry<?> child : children) {
var mapping = this.mappings.get(child);
if ((!dropped.getDroppedMappings().containsKey(child)
if ((!droppedMappings.containsKey(child)
&& mapping != null && mapping.tokenType() != TokenType.OBFUSCATED)
|| !this.hasNoMappedChildren(child, dropped)) {
|| !this.hasNoMappedChildren(child, dropper)) {
return false;
}
}
Expand All @@ -129,14 +144,17 @@ private boolean hasNoMappedChildren(Entry<?> entry, Dropped dropped) {
return true;
}

public static class Dropped {
public static class Dropper {
private final Map<Entry<?>, String> droppedMappings = new HashMap<>();
private final Map<Entry<?>, String> pendingDroppedMappings = new HashMap<>();

public void drop(Entry<?> entry, EntryMapping mapping) {
this.droppedMappings.put(entry, mapping.targetName() != null ? mapping.targetName() : entry.getName());
public void addPendingDrop(Entry<?> entry, EntryMapping mapping) {
this.pendingDroppedMappings.put(entry, mapping.targetName() != null ? mapping.targetName() : entry.getName());
}

void apply(EntryTree<EntryMapping> mappings) {
public void applyPendingDrops(EntryTree<EntryMapping> mappings) {
this.droppedMappings.putAll(this.pendingDroppedMappings);

for (Entry<?> entry : this.droppedMappings.keySet()) {
EntryTreeNode<EntryMapping> node = mappings.findNode(entry);
if (node == null) {
Expand All @@ -147,10 +165,23 @@ void apply(EntryTree<EntryMapping> mappings) {
mappings.remove(childEntry);
}
}

this.pendingDroppedMappings.clear();
}

public Map<Entry<?>, String> getDroppedAndPending() {
var map = new HashMap<Entry<?>, String>();
map.putAll(this.droppedMappings);
map.putAll(this.pendingDroppedMappings);
return map;
}

public Map<Entry<?>, String> getDroppedMappings() {
return this.droppedMappings;
}

public Map<Entry<?>, String> getPendingDroppedMappings() {
return this.pendingDroppedMappings;
}
}
}

0 comments on commit 7b3a900

Please sign in to comment.