diff --git a/enigma-cli/src/test/java/org/quiltmc/enigma/command/DropInvalidMappingsTest.java b/enigma-cli/src/test/java/org/quiltmc/enigma/command/DropInvalidMappingsTest.java index 0ad911a7..8a56b7c1 100644 --- a/enigma-cli/src/test/java/org/quiltmc/enigma/command/DropInvalidMappingsTest.java +++ b/enigma-cli/src/test/java/org/quiltmc/enigma/command/DropInvalidMappingsTest.java @@ -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/"); @@ -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 { @@ -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); + } } diff --git a/enigma-cli/src/test/resources/drop_invalid_mappings/expected/Enums.mapping b/enigma-cli/src/test/resources/drop_invalid_mappings/expected/Enums.mapping new file mode 100644 index 00000000..a758f1fe --- /dev/null +++ b/enigma-cli/src/test/resources/drop_invalid_mappings/expected/Enums.mapping @@ -0,0 +1,5 @@ +CLASS a Gaming + FIELD a wawa I + METHOD (Ljava/lang/String;II)V + ARG 3 wawa + METHOD a wawa ()I diff --git a/enigma-cli/src/test/resources/drop_invalid_mappings/input/Enums.mapping b/enigma-cli/src/test/resources/drop_invalid_mappings/input/Enums.mapping new file mode 100644 index 00000000..70cb73c6 --- /dev/null +++ b/enigma-cli/src/test/resources/drop_invalid_mappings/input/Enums.mapping @@ -0,0 +1,7 @@ +CLASS a Gaming + FIELD a wawa I + METHOD (Ljava/lang/String;II)V + ARG 3 wawa + METHOD a wawa ()I + METHOD valueOf (Ljava/lang/String;)La; + ARG 0 name diff --git a/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java b/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java index 00e1e2c6..8cc423f3 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java +++ b/enigma/src/main/java/org/quiltmc/enigma/api/EnigmaProject.java @@ -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; @@ -143,26 +142,29 @@ public Collection> dropMappings(ProgressListener progress) { } private Collection> dropMappings(EntryTree 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, String> droppedBrokenMappings = droppedBroken.getDroppedMappings(); + checker.collectBrokenMappings(progress, dropper); + + Map, String> droppedBrokenMappings = dropper.getPendingDroppedMappings(); for (Map.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, String> droppedEmptyMappings = droppedEmpty.getDroppedMappings(); + Map, String> droppedEmptyMappings = dropper.getPendingDroppedMappings(); for (Map.Entry, String> mapping : droppedEmptyMappings.entrySet()) { Logger.warn("{} ({}) was empty. Mapping was dropped.", mapping.getKey(), mapping.getValue()); } - Collection> 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) { diff --git a/enigma/src/main/java/org/quiltmc/enigma/impl/translation/mapping/MappingsChecker.java b/enigma/src/main/java/org/quiltmc/enigma/impl/translation/mapping/MappingsChecker.java index 6660261e..a2e77c0f 100644 --- a/enigma/src/main/java/org/quiltmc/enigma/impl/translation/mapping/MappingsChecker.java +++ b/enigma/src/main/java/org/quiltmc/enigma/impl/translation/mapping/MappingsChecker.java @@ -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; @@ -31,39 +33,50 @@ public MappingsChecker(EnigmaProject project, JarIndex index, EntryTree> dropper) { - Dropped dropped = new Dropped(); - + private Dropper collectMappings(ProgressListener progress, Dropper dropper, BiConsumer> dropFunction) { // HashEntryTree#getAllEntries filters out empty classes - List> entries = StreamSupport.stream(this.mappings.spliterator(), false).map(EntryTreeNode::getEntry).toList(); + List> 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; @@ -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; } } @@ -129,14 +144,17 @@ private boolean hasNoMappedChildren(Entry entry, Dropped dropped) { return true; } - public static class Dropped { + public static class Dropper { private final Map, String> droppedMappings = new HashMap<>(); + private final Map, 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 mappings) { + public void applyPendingDrops(EntryTree mappings) { + this.droppedMappings.putAll(this.pendingDroppedMappings); + for (Entry entry : this.droppedMappings.keySet()) { EntryTreeNode node = mappings.findNode(entry); if (node == null) { @@ -147,10 +165,23 @@ void apply(EntryTree mappings) { mappings.remove(childEntry); } } + + this.pendingDroppedMappings.clear(); + } + + public Map, String> getDroppedAndPending() { + var map = new HashMap, String>(); + map.putAll(this.droppedMappings); + map.putAll(this.pendingDroppedMappings); + return map; } public Map, String> getDroppedMappings() { return this.droppedMappings; } + + public Map, String> getPendingDroppedMappings() { + return this.pendingDroppedMappings; + } } }