fix a couple possible crashes in package renamer

QuiltMC · Oct 29, 2024 · 8247fe0 · 8247fe0
1 parent 01e2d66
commit 8247fe0
Show file tree

Hide file tree

Showing 20 changed files with 110 additions and 52 deletions.
diff --git a/enigma-swing/src/main/java/org/quiltmc/enigma/gui/util/PackageRenamer.java b/enigma-swing/src/main/java/org/quiltmc/enigma/gui/util/PackageRenamer.java
@@ -181,7 +181,7 @@ public CompletableFuture<Void> renamePackage(String path, String input) {
 	private void handleNode(int divergenceIndex, boolean rename, String[] oldPackageNames, String[] newPackageNames, Map<String, Runnable> renameStack, TreeNode node) {
 		if (node instanceof ClassSelectorClassNode classNode && rename) {
 			String oldName = classNode.getDeobfEntry().getFullName();
-			int finalPackageIndex = divergenceIndex - 1;
+			int finalPackageIndex = divergenceIndex == 0 ? 0 : divergenceIndex - 1;
 
 			// skips all classes that do not match the exact package being renamed
 			if (this.mode == Mode.MOVE) {
@@ -268,9 +268,9 @@ private void handleNode(int divergenceIndex, boolean rename, String[] oldPackage
 				}
 			}
 
-			if (packageName.equals(newPackageNames[index])) {
+			if (newPackageNames.length - 1 >= index && packageName.equals(newPackageNames[index])) {
 				this.handlePackage(index, false, oldPackageNames, newPackageNames, renameStack, packageNode);
-			} else if (packageName.equals(oldPackageNames[index])) {
+			} else if (oldPackageNames.length - 1 >= index && packageName.equals(oldPackageNames[index])) {
 				this.handlePackage(index, true, oldPackageNames, newPackageNames, renameStack, packageNode);
 			}
 		}

diff --git a/enigma-swing/src/test/java/org/quiltmc/enigma/TestPackageRename.java b/enigma-swing/src/test/java/org/quiltmc/enigma/TestPackageRename.java
@@ -1,6 +1,7 @@
 package org.quiltmc.enigma;
 
 import org.quiltmc.enigma.api.EnigmaProfile;
+import org.quiltmc.enigma.api.EnigmaProject;
 import org.quiltmc.enigma.gui.EditableType;
 import org.quiltmc.enigma.gui.Gui;
 import org.quiltmc.enigma.gui.docker.AllClassesDocker;
@@ -26,83 +27,96 @@
 public class TestPackageRename {
 	public static final Path JAR = TestUtil.obfJar("complete");
 	public static final Path MAPPINGS = Path.of("src/test/resources/test_mappings");
-	private static Translator deobfuscator;
+	private static EnigmaProject project;
 
 	@Test
 	void testRemoveOnePackage() throws InterruptedException {
 		renamePackage("a/b/c", "a/c", PackageRenamer.Mode.REFACTOR);
 
-		assertMapping(TestEntryFactory.newClass("A"), TestEntryFactory.newClass("a/c/A"));
-		assertMapping(TestEntryFactory.newClass("B"), TestEntryFactory.newClass("a/c/B"));
-		assertMapping(TestEntryFactory.newClass("C"), TestEntryFactory.newClass("a/c/C"));
-		assertMapping(TestEntryFactory.newClass("D"), TestEntryFactory.newClass("a/D"));
-		assertMapping(TestEntryFactory.newClass("E"), TestEntryFactory.newClass("E"));
+		assertMapping("a","a/c/A");
+		assertMapping("b", "a/c/B");
+		assertMapping("c", "a/c/C");
+		assertMapping("d", "a/D");
+		assertMapping("e", "E");
 	}
 
 	@Test
 	void testRemoveTwoPackages() throws InterruptedException {
 		renamePackage("a/b/c", "a", PackageRenamer.Mode.REFACTOR);
 
-		assertMapping(TestEntryFactory.newClass("A"), TestEntryFactory.newClass("a/A"));
-		assertMapping(TestEntryFactory.newClass("B"), TestEntryFactory.newClass("a/B"));
-		assertMapping(TestEntryFactory.newClass("C"), TestEntryFactory.newClass("a/C"));
-		assertMapping(TestEntryFactory.newClass("D"), TestEntryFactory.newClass("a/D"));
-		assertMapping(TestEntryFactory.newClass("E"), TestEntryFactory.newClass("E"));
+		assertMapping("a", "a/A");
+		assertMapping("b", "a/B");
+		assertMapping("c", "a/C");
+		assertMapping("d", "a/D");
+		assertMapping("e", "E");
 	}
 
 	@Test
 	void testPackageConservation() throws InterruptedException {
 		renamePackage("a/b", "a", PackageRenamer.Mode.REFACTOR);
 
-		assertMapping(TestEntryFactory.newClass("A"), TestEntryFactory.newClass("a/c/A"));
-		assertMapping(TestEntryFactory.newClass("B"), TestEntryFactory.newClass("a/c/B"));
-		assertMapping(TestEntryFactory.newClass("C"), TestEntryFactory.newClass("a/C"));
-		assertMapping(TestEntryFactory.newClass("D"), TestEntryFactory.newClass("a/D"));
-		assertMapping(TestEntryFactory.newClass("E"), TestEntryFactory.newClass("E"));
+		assertMapping("a", "a/c/A");
+		assertMapping("b", "a/c/B");
+		assertMapping("c", "a/C");
+		assertMapping("d", "a/D");
+		assertMapping("e", "E");
 	}
 
 	@Test
 	void testAppendOnePackage() throws InterruptedException {
 		renamePackage("a/b/c", "a/b/c/d", PackageRenamer.Mode.REFACTOR);
 
-		assertMapping(TestEntryFactory.newClass("A"), TestEntryFactory.newClass("a/b/c/d/A"));
-		assertMapping(TestEntryFactory.newClass("B"), TestEntryFactory.newClass("a/b/c/d/B"));
-		assertMapping(TestEntryFactory.newClass("C"), TestEntryFactory.newClass("a/b/C"));
-		assertMapping(TestEntryFactory.newClass("D"), TestEntryFactory.newClass("a/D"));
-		assertMapping(TestEntryFactory.newClass("E"), TestEntryFactory.newClass("E"));
+		assertMapping("a", "a/b/c/d/A");
+		assertMapping("b", "a/b/c/d/B");
+		assertMapping("c", "a/b/C");
+		assertMapping("d", "a/D");
+		assertMapping("e", "E");
 	}
 
 	@Test
 	void testSimpleRename() throws InterruptedException {
 		renamePackage("a/b/c", "a/b/d", PackageRenamer.Mode.REFACTOR);
 
-		assertMapping(TestEntryFactory.newClass("A"), TestEntryFactory.newClass("a/b/d/A"));
-		assertMapping(TestEntryFactory.newClass("B"), TestEntryFactory.newClass("a/b/d/B"));
-		assertMapping(TestEntryFactory.newClass("C"), TestEntryFactory.newClass("a/b/C"));
-		assertMapping(TestEntryFactory.newClass("D"), TestEntryFactory.newClass("a/D"));
-		assertMapping(TestEntryFactory.newClass("E"), TestEntryFactory.newClass("E"));
+		assertMapping("a", "a/b/d/A");
+		assertMapping("b", "a/b/d/B");
+		assertMapping("c", "a/b/C");
+		assertMapping("d", "a/D");
+		assertMapping("e", "E");
 	}
 
 	@Test
 	void testFirstPackageRename() throws InterruptedException {
 		renamePackage("a", "b", PackageRenamer.Mode.REFACTOR);
 
-		assertMapping(TestEntryFactory.newClass("A"), TestEntryFactory.newClass("b/b/c/A"));
-		assertMapping(TestEntryFactory.newClass("B"), TestEntryFactory.newClass("b/b/c/B"));
-		assertMapping(TestEntryFactory.newClass("C"), TestEntryFactory.newClass("b/b/C"));
-		assertMapping(TestEntryFactory.newClass("D"), TestEntryFactory.newClass("b/D"));
-		assertMapping(TestEntryFactory.newClass("E"), TestEntryFactory.newClass("E"));
+		assertMapping("a", "b/b/c/A");
+		assertMapping("b", "b/b/c/B");
+		assertMapping("c", "b/b/C");
+		assertMapping("d", "b/D");
+		assertMapping("e", "E");
 	}
 
 	@Test
 	void testPackageMove() throws InterruptedException {
 		renamePackage("a/b/c", "a/c", PackageRenamer.Mode.MOVE);
 
-		assertMapping(TestEntryFactory.newClass("A"), TestEntryFactory.newClass("a/c/A"));
-		assertMapping(TestEntryFactory.newClass("B"), TestEntryFactory.newClass("a/c/B"));
-		assertMapping(TestEntryFactory.newClass("C"), TestEntryFactory.newClass("a/b/C"));
-		assertMapping(TestEntryFactory.newClass("D"), TestEntryFactory.newClass("a/D"));
-		assertMapping(TestEntryFactory.newClass("E"), TestEntryFactory.newClass("E"));
+		assertMapping("a", "a/c/A");
+		assertMapping("b", "a/c/B");
+		assertMapping("c", "a/b/C");
+		assertMapping("d", "a/D");
+		assertMapping("e", "E");
+	}
+
+	@Test
+	void testPackageMerge() throws InterruptedException {
+		renamePackage("b", "a", PackageRenamer.Mode.REFACTOR);
+
+		assertMapping("a", "a/b/c/A");
+		assertMapping("b", "a/b/c/B");
+		assertMapping("c", "a/b/C");
+		assertMapping("d", "a/D");
+		assertMapping("e", "E");
+		assertMapping("f", "a/F");
+		assertMapping("g", "a/c/G");
 	}
 
 	private static void renamePackage(String packageName, String newName, PackageRenamer.Mode mode) throws InterruptedException {
@@ -124,21 +138,35 @@ private static ClassSelectorPopupMenu setupMenu() throws InterruptedException {
 		gui.getController().openJar(JAR).thenRun(() -> gui.getController().openMappings(MAPPINGS).thenRun(latch::countDown));
 		latch.await();
 
-		deobfuscator = gui.getController().getProject().getRemapper().getDeobfuscator();
+		project = gui.getController().getProject();
 		return gui.getDockerManager().getDocker(AllClassesDocker.class).getPopupMenu();
 	}
 
 	private static void assertBaseMappings() {
-		// assert starting mappings
-		assertMapping(TestEntryFactory.newClass("A"), TestEntryFactory.newClass("a/b/c/A"));
-		assertMapping(TestEntryFactory.newClass("B"), TestEntryFactory.newClass("a/b/c/B"));
-		assertMapping(TestEntryFactory.newClass("C"), TestEntryFactory.newClass("a/b/C"));
-		assertMapping(TestEntryFactory.newClass("D"), TestEntryFactory.newClass("a/D"));
-		assertMapping(TestEntryFactory.newClass("E"), TestEntryFactory.newClass("E"));
+		// a
+		assertMapping("a", "a/b/c/A");
+		assertMapping("b", "a/b/c/B");
+		assertMapping("c", "a/b/C");
+		assertMapping("d", "a/D");
+
+		// lone
+		assertMapping("e", "E");
+
+		// b
+		assertMapping("f", "b/F");
+		assertMapping("g", "b/c/G");
+	}
+
+	private static void assertMapping(String obf, String deobf) {
+		assertMapping(TestEntryFactory.newClass(obf), TestEntryFactory.newClass(deobf));
 	}
 
 	private static void assertMapping(Entry<?> obf, Entry<?> deobf) {
-		TranslateResult<? extends Entry<?>> result = deobfuscator.extendedTranslate(obf);
+		if (!project.getRemapper().getMappings().contains(obf)) {
+			throw new RuntimeException("no mapping for " + obf.getFullName() + " !");
+		}
+
+		TranslateResult<? extends Entry<?>> result = project.getRemapper().getDeobfuscator().extendedTranslate(obf);
 		assertThat(result, is(notNullValue()));
 		assertThat(result.getValue(), is(deobf));
 

diff --git a/enigma-swing/src/test/resources/test_mappings/a/D.mapping b/enigma-swing/src/test/resources/test_mappings/a/D.mapping
diff --git a/enigma-swing/src/test/resources/test_mappings/a/b/C.mapping b/enigma-swing/src/test/resources/test_mappings/a/b/C.mapping
diff --git a/enigma-swing/src/test/resources/test_mappings/a/b/c.mapping b/enigma-swing/src/test/resources/test_mappings/a/b/c.mapping
@@ -0,0 +1 @@
+CLASS c a/b/C
diff --git a/enigma-swing/src/test/resources/test_mappings/a/b/c/A.mapping b/enigma-swing/src/test/resources/test_mappings/a/b/c/A.mapping
diff --git a/enigma-swing/src/test/resources/test_mappings/a/b/c/B.mapping b/enigma-swing/src/test/resources/test_mappings/a/b/c/B.mapping
diff --git a/enigma-swing/src/test/resources/test_mappings/a/b/c/a.mapping b/enigma-swing/src/test/resources/test_mappings/a/b/c/a.mapping
@@ -0,0 +1 @@
+CLASS a a/b/c/A
diff --git a/enigma-swing/src/test/resources/test_mappings/a/b/c/b.mapping b/enigma-swing/src/test/resources/test_mappings/a/b/c/b.mapping
@@ -0,0 +1 @@
+CLASS b a/b/c/B
diff --git a/enigma-swing/src/test/resources/test_mappings/a/d.mapping b/enigma-swing/src/test/resources/test_mappings/a/d.mapping
@@ -0,0 +1 @@
+CLASS d a/D
diff --git a/enigma-swing/src/test/resources/test_mappings/b/c/g.mapping b/enigma-swing/src/test/resources/test_mappings/b/c/g.mapping
@@ -0,0 +1 @@
+CLASS g b/c/G
diff --git a/enigma-swing/src/test/resources/test_mappings/b/f.mapping b/enigma-swing/src/test/resources/test_mappings/b/f.mapping
@@ -0,0 +1 @@
+CLASS f b/F
diff --git a/...rc/test/resources/test_mappings/E.mapping → ...rc/test/resources/test_mappings/e.mapping b/...rc/test/resources/test_mappings/E.mapping → ...rc/test/resources/test_mappings/e.mapping
@@ -1,2 +1,2 @@
-CLASS E E
+CLASS e E
 	COMMENT a
diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/packages/E.java b/enigma/src/test/java/org/quiltmc/enigma/input/packages/E.java
@@ -0,0 +1,4 @@
+package org.quiltmc.enigma.input.packages;
+
+public class E {
+}
diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/packages/a/D.java b/enigma/src/test/java/org/quiltmc/enigma/input/packages/a/D.java
@@ -0,0 +1,4 @@
+package org.quiltmc.enigma.input.packages.a;
+
+public class D {
+}
diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/packages/a/b/C.java b/enigma/src/test/java/org/quiltmc/enigma/input/packages/a/b/C.java
@@ -0,0 +1,4 @@
+package org.quiltmc.enigma.input.packages.a.b;
+
+public class C {
+}
diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/packages/a/b/c/A.java b/enigma/src/test/java/org/quiltmc/enigma/input/packages/a/b/c/A.java
@@ -0,0 +1,4 @@
+package org.quiltmc.enigma.input.packages.a.b.c;
+
+public class A {
+}
diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/packages/a/b/c/B.java b/enigma/src/test/java/org/quiltmc/enigma/input/packages/a/b/c/B.java
@@ -0,0 +1,4 @@
+package org.quiltmc.enigma.input.packages.a.b.c;
+
+public class B {
+}
diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/packages/b/F.java b/enigma/src/test/java/org/quiltmc/enigma/input/packages/b/F.java
@@ -0,0 +1,4 @@
+package org.quiltmc.enigma.input.packages.b;
+
+public class F {
+}
diff --git a/enigma/src/test/java/org/quiltmc/enigma/input/packages/b/c/G.java b/enigma/src/test/java/org/quiltmc/enigma/input/packages/b/c/G.java
@@ -0,0 +1,4 @@
+package org.quiltmc.enigma.input.packages.b.c;
+
+public class G {
+}