Resolve the correct amount of entries when renaming a method (#174)

* Interface union tests

* Resolve the correct amount of entries when renaming a method

* Add an extra test

* Fix checkstyle errors

* Add a comment

* Fix another checkstyle error

* Move #newVC to TestUtil

enigma/src/main/java/org/quiltmc/enigma/api/translation/mapping/EntryRemapper.java

@@ -1,6 +1,7 @@
 package org.quiltmc.enigma.api.translation.mapping;
 
 import org.quiltmc.enigma.api.analysis.index.jar.EntryIndex;
+import org.quiltmc.enigma.api.analysis.index.jar.InheritanceIndex;
 import org.quiltmc.enigma.api.analysis.index.jar.JarIndex;
 import org.quiltmc.enigma.api.analysis.index.mapping.MappingsIndex;
 import org.quiltmc.enigma.api.service.NameProposalService;
@@ -19,9 +20,12 @@
 import org.quiltmc.enigma.util.validation.Message;
 import org.quiltmc.enigma.util.validation.ValidationContext;
 
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.HashSet;
 import java.util.Objects;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Stream;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
@@ -83,7 +87,7 @@ private void doPutMapping(ValidationContext vc, Entry<?> obfuscatedEntry, @Nonnu
 		EntryMapping oldMapping = this.getMapping(obfuscatedEntry);
 		boolean renaming = !Objects.equals(oldMapping.targetName(), deobfMapping.targetName());
 
-		Collection<Entry<?>> resolvedEntries = this.obfResolver.resolveEntry(obfuscatedEntry, renaming ? ResolutionStrategy.RESOLVE_ROOT : ResolutionStrategy.RESOLVE_CLOSEST);
+		Collection<Entry<?>> resolvedEntries = renaming ? this.resolveAllRoots(obfuscatedEntry) : this.obfResolver.resolveEntry(obfuscatedEntry, ResolutionStrategy.RESOLVE_CLOSEST);
 
 		if (renaming && deobfMapping.targetName() != null) {
 			for (Entry<?> resolvedEntry : resolvedEntries) {
@@ -105,6 +109,41 @@ private void doPutMapping(ValidationContext vc, Entry<?> obfuscatedEntry, @Nonnu
 		this.mappingsIndex.reindexEntry(deobfMapping, obfuscatedEntry);
 	}
 
+	private Collection<Entry<?>> resolveAllRoots(Entry<?> obfuscatedEntry) {
+		if (!(obfuscatedEntry instanceof MethodEntry methodEntry)) {
+			return this.obfResolver.resolveEntry(obfuscatedEntry, ResolutionStrategy.RESOLVE_ROOT);
+		}
+
+		InheritanceIndex inheritanceIndex = this.jarIndex.getIndex(InheritanceIndex.class);
+		var owner = methodEntry.getParent();
+		var descendants = inheritanceIndex.getDescendants(owner);
+		var knownParents = new HashSet<>(inheritanceIndex.getParents(owner));
+
+		// Find all classes with an "unknown" parent, so we can also resolve the method from there and find other definitions
+		// If interfaces A and B define method `void foo()`, a class C may implement both interfaces, having a single method `void foo()`
+		// and effectively "joining" the two interface methods, so you have to keep both "in sync"
+		List<ClassEntry> classes = new ArrayList<>();
+		for (ClassEntry descendant : descendants) {
+			var parents = inheritanceIndex.getParents(descendant);
+			if (parents.size() > 1) { // one of them is one of the owner's descendants
+				Set<ClassEntry> otherParents = new HashSet<>(parents);
+				otherParents.removeAll(descendants);
+				otherParents.removeAll(knownParents);
+				if (!otherParents.isEmpty()) {
+					classes.add(descendant);
+					knownParents.addAll(otherParents);
+				}
+			}
+		}
+
+		Set<Entry<?>> resolution = new HashSet<>(this.obfResolver.resolveEntry(obfuscatedEntry, ResolutionStrategy.RESOLVE_ROOT));
+		for (ClassEntry clazz : classes) {
+			resolution.addAll(this.obfResolver.resolveEntry(methodEntry.withParent(clazz), ResolutionStrategy.RESOLVE_ROOT));
+		}
+
+		return resolution;
+	}
+
 	// todo this needs to be fixed for hashed mappings!
 	// note: just supressing warnings until it's fixed
 	@SuppressWarnings("all")

enigma/src/test/java/org/quiltmc/enigma/input/interface_union/AInterface.java

@@ -0,0 +1,11 @@
+package org.quiltmc.enigma.input.interface_union;
+
+public interface AInterface {
+	int methodA();
+
+	// BInterface -> Union1Class
+	void methodFoo();
+
+	// -> Union3Record
+	double baz();
+}

enigma/src/test/java/org/quiltmc/enigma/input/interface_union/BInterface.java

@@ -0,0 +1,11 @@
+package org.quiltmc.enigma.input.interface_union;
+
+public interface BInterface {
+	// AInterface -> Union1Class
+	void methodFoo();
+
+	double methodB();
+
+	// CClass -> Union2Class
+	boolean methodBar();
+}

enigma/src/test/java/org/quiltmc/enigma/input/interface_union/CClass.java

@@ -0,0 +1,15 @@
+package org.quiltmc.enigma.input.interface_union;
+
+import java.util.Random;
+
+public class CClass {
+	// BInterface -> Union2Class
+	public boolean methodBar() {
+		return true;
+	}
+
+	// DInterface -> Union4Class
+	public float factor() {
+		return (float) new Random().nextGaussian();
+	}
+}

enigma/src/test/java/org/quiltmc/enigma/input/interface_union/DInterface.java

@@ -0,0 +1,5 @@
+package org.quiltmc.enigma.input.interface_union;
+
+public interface DInterface {
+	float factor();
+}

enigma/src/test/java/org/quiltmc/enigma/input/interface_union/Union1Class.java

@@ -0,0 +1,31 @@
+package org.quiltmc.enigma.input.interface_union;
+
+import java.util.Random;
+
+public class Union1Class implements AInterface, BInterface {
+	@Override
+	public int methodA() {
+		return 32767;
+	}
+
+	// AInterface + BInterface
+	@Override
+	public void methodFoo() {
+		System.out.println("foo");
+	}
+
+	@Override
+	public double baz() {
+		return 200;
+	}
+
+	@Override
+	public double methodB() {
+		return new Random().nextGaussian();
+	}
+
+	@Override
+	public boolean methodBar() {
+		return false;
+	}
+}

enigma/src/test/java/org/quiltmc/enigma/input/interface_union/Union2Class.java

@@ -0,0 +1,20 @@
+package org.quiltmc.enigma.input.interface_union;
+
+import java.util.Random;
+
+public class Union2Class extends CClass implements BInterface {
+	@Override
+	public void methodFoo() {
+	}
+
+	@Override
+	public double methodB() {
+		return 6.02e23;
+	}
+
+	// BInterface + CClass
+	@Override
+	public boolean methodBar() {
+		return new Random().nextExponential() > 2.0;
+	}
+}

enigma/src/test/java/org/quiltmc/enigma/input/interface_union/Union3Record.java

@@ -0,0 +1,14 @@
+package org.quiltmc.enigma.input.interface_union;
+
+public record Union3Record(double baz) implements AInterface {
+	@Override
+	public int methodA() {
+		return -1;
+	}
+
+	@Override
+	public void methodFoo() {
+	}
+
+	// AInterface -> baz()
+}

enigma/src/test/java/org/quiltmc/enigma/input/interface_union/Union4Class.java

@@ -0,0 +1,5 @@
+package org.quiltmc.enigma.input.interface_union;
+
+public class Union4Class extends CClass implements DInterface {
+	// DInterface.factor() implemented in CClass
+}

enigma/src/test/java/org/quiltmc/enigma/translation/mapping/EntryRemapperTest.java

@@ -0,0 +1,103 @@
+package org.quiltmc.enigma.translation.mapping;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.quiltmc.enigma.TestEntryFactory;
+import org.quiltmc.enigma.TestUtil;
+import org.quiltmc.enigma.api.Enigma;
+import org.quiltmc.enigma.api.EnigmaProject;
+import org.quiltmc.enigma.api.ProgressListener;
+import org.quiltmc.enigma.api.class_provider.ClasspathClassProvider;
+import org.quiltmc.enigma.api.translation.mapping.EntryMapping;
+import org.quiltmc.enigma.api.translation.mapping.EntryRemapper;
+import org.quiltmc.enigma.api.translation.mapping.tree.EntryTree;
+import org.quiltmc.enigma.api.translation.mapping.tree.HashEntryTree;
+import org.quiltmc.enigma.api.translation.representation.entry.Entry;
+
+import java.nio.file.Path;
+
+public class EntryRemapperTest {
+	public static final Path JAR = TestUtil.obfJar("interface_union");
+	private static EnigmaProject project;
+	private static EntryRemapper remapper;
+
+	@BeforeAll
+	public static void beforeAll() throws Exception {
+		Enigma enigma = Enigma.create();
+		project = enigma.openJar(JAR, new ClasspathClassProvider(), ProgressListener.none());
+		remapper = project.getRemapper();
+	}
+
+	@BeforeEach
+	public void beforeEach() {
+		EntryTree<EntryMapping> mappings = new HashEntryTree<>();
+		project.setMappings(mappings, ProgressListener.none());
+		remapper = project.getRemapper();
+	}
+
+	private static void assertName(Entry<?> entry, String expected) {
+		Entry<?> deobf = remapper.deobfuscate(entry);
+		Assertions.assertNotNull(deobf);
+		Assertions.assertEquals(expected, deobf.getName());
+	}
+
+	@Test
+	public void testUnionRename() {
+		var name = "unionAB";
+		remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newMethod("e", "a", "()V"), new EntryMapping(name));
+
+		assertName(TestEntryFactory.newMethod("e", "a", "()V"), name);
+		assertName(TestEntryFactory.newMethod("a", "a", "()V"), name);
+		assertName(TestEntryFactory.newMethod("b", "a", "()V"), name);
+
+		name = "unionBC";
+		remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newMethod("f", "a", "()Z"), new EntryMapping(name));
+
+		assertName(TestEntryFactory.newMethod("f", "a", "()Z"), name);
+		assertName(TestEntryFactory.newMethod("b", "a", "()Z"), name);
+		assertName(TestEntryFactory.newMethod("c", "a", "()Z"), name);
+
+		name = "unionA3";
+		remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newMethod("g", "a", "()D"), new EntryMapping(name));
+
+		assertName(TestEntryFactory.newMethod("g", "a", "()D"), name);
+		assertName(TestEntryFactory.newMethod("a", "a", "()D"), name);
+	}
+
+	@Test
+	public void testElementRename() {
+		var name = "unionAB";
+		remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newMethod("e", "a", "()V"), new EntryMapping(name));
+
+		assertName(TestEntryFactory.newMethod("a", "a", "()V"), name);
+		assertName(TestEntryFactory.newMethod("e", "a", "()V"), name);
+		assertName(TestEntryFactory.newMethod("b", "a", "()V"), name);
+
+		name = "unionBC";
+		remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newMethod("b", "a", "()Z"), new EntryMapping(name));
+
+		assertName(TestEntryFactory.newMethod("b", "a", "()Z"), name);
+		assertName(TestEntryFactory.newMethod("f", "a", "()Z"), name);
+		assertName(TestEntryFactory.newMethod("c", "a", "()Z"), name);
+
+		name = "unionA3";
+		remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newMethod("a", "a", "()D"), new EntryMapping(name));
+
+		assertName(TestEntryFactory.newMethod("a", "a", "()D"), name);
+		assertName(TestEntryFactory.newMethod("g", "a", "()D"), name);
+
+		name = "unionCD";
+		remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newMethod("c", "a", "()F"), new EntryMapping(name));
+
+		assertName(TestEntryFactory.newMethod("c", "a", "()F"), name);
+		assertName(TestEntryFactory.newMethod("d", "a", "()F"), name);
+
+		name = "unionDC";
+		remapper.putMapping(TestUtil.newVC(), TestEntryFactory.newMethod("d", "a", "()F"), new EntryMapping(name));
+
+		assertName(TestEntryFactory.newMethod("d", "a", "()F"), name);
+		assertName(TestEntryFactory.newMethod("c", "a", "()F"), name);
+	}
+}