Handle methods with inherited mapped names correctly

sfPlayer1 · Apr 20, 2023 · 740460d · 740460d
1 parent 8ce6570
commit 740460d
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 61 deletions.
diff --git a/src/matcher/gui/MatchPaneSrc.java b/src/matcher/gui/MatchPaneSrc.java
@@ -369,7 +369,7 @@ public void onClassSelect(ClassInstance cls) {
 
 		if (cls != null) {
 			for (MethodInstance mth : cls.getMethods()) {
-				if (!mth.isReal() || (gui.isHideUnmappedA() && !mth.hasMappedName() && !mth.hasMappedChildren())) {
+				if (!mth.isReal() || (gui.isHideUnmappedA() && !mth.hasNonInheritedMappedName() && !mth.hasMappedChildren())) {
 					continue;
 				}
 

diff --git a/src/matcher/mapping/Mappings.java b/src/matcher/mapping/Mappings.java
@@ -3,17 +3,13 @@
 import java.io.Closeable;
 import java.io.IOException;
 import java.io.UncheckedIOException;
-import java.net.URI;
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
-import org.objectweb.asm.tree.MethodNode;
-
 import net.fabricmc.mappingio.FlatMappingVisitor;
 import net.fabricmc.mappingio.MappedElementKind;
 import net.fabricmc.mappingio.MappingReader;
@@ -835,62 +831,10 @@ private static boolean shouldExportAny(FieldInstance[] fields, MappingFormat for
 	private static boolean shouldExportName(MethodInstance method, MappingsExportVerbosity verbosity, boolean forAnyInput, Set<Set<MethodInstance>> exportedHierarchies) {
 		return verbosity == MappingsExportVerbosity.FULL
 				|| method.getAllHierarchyMembers().size() == 1
-				|| (method.getParents().isEmpty() || forAnyInput && isAnyInputRoot(method))
+				|| (method.getParents().isEmpty() || forAnyInput && method.isAnyInputRoot())
 				&& (verbosity == MappingsExportVerbosity.ROOTS || !exportedHierarchies.contains(method.getAllHierarchyMembers())); // FIXME: forAnyInput + minimal needs to use an exportedHierarchies set per origin
 	}
 
-	private static boolean isAnyInputRoot(MethodInstance method) {
-		ClassInstance cls = method.getCls();
-		String name = method.getName();
-		String desc = method.getDesc();
-
-		// check if each origin that supplies this method has a parent within the same origin
-
-		for (int i = 0; i < cls.getAsmNodes().length; i++) {
-			for (MethodNode m : cls.getAsmNodes()[i].methods) {
-				if (m.name.equals(method.getName())
-						&& m.desc.equals(method.getDesc())) {
-					if (!hasParentMethod(name, desc, method.getParents(), cls.getAsmNodeOrigin(i))) {
-						return true;
-					} else {
-						break;
-					}
-				}
-			}
-		}
-
-		return false;
-	}
-
-	private static boolean hasParentMethod(String name, String desc, Collection<MethodInstance> parents, URI reqOrigin) {
-		// check direct parents (must supply the method from the required origin)
-
-		for (MethodInstance parent : parents) {
-			ClassInstance parentCls = parent.getCls();
-
-			for (int i = 0; i < parentCls.getAsmNodes().length; i++) {
-				if (parentCls.getAsmNodeOrigin(i).equals(reqOrigin)) {
-					for (MethodNode m : parentCls.getAsmNodes()[i].methods) {
-						if (m.name.equals(name)
-								&& m.desc.equals(desc)) {
-							return true;
-						}
-					}
-				}
-			}
-		}
-
-		// check indirect parents recursively
-
-		for (MethodInstance parent : parents) {
-			if (!parent.getParents().isEmpty() && hasParentMethod(name, desc, parent.getParents(), reqOrigin)) {
-				return true;
-			}
-		}
-
-		return false;
-	}
-
 	public static void clear(ClassEnv env) {
 		for (ClassInstance cls : env.getClasses()) {
 			if (!cls.isReal()) continue;

diff --git a/src/matcher/serdes/MatchesIo.java b/src/matcher/serdes/MatchesIo.java
@@ -441,7 +441,7 @@ private static void writeClass(ClassInstance cls, char side, Writer out) throws
 
 			for (MethodInstance method : cls.getMethods()) {
 				if ((method.hasMatch() || !method.isMatchable())
-						&& (saveUnmapped || method.hasMappedName() || method.hasMappedChildren())) {
+						&& (saveUnmapped || method.hasNonInheritedMappedName() || method.hasMappedChildren())) {
 					writeMethod(method, 'a', out);
 				}
 			}
@@ -455,7 +455,7 @@ private static void writeClass(ClassInstance cls, char side, Writer out) throws
 
 			for (MethodInstance method : cls.getMatch().getMethods()) {
 				if (!method.isMatchable()
-						&& (saveUnmapped || method.hasMappedName() || method.hasMappedChildren())) {
+						&& (saveUnmapped || method.hasNonInheritedMappedName() || method.hasMappedChildren())) {
 					writeMethod(method, 'b', out);
 				}
 			}

diff --git a/src/matcher/type/ClassInstance.java b/src/matcher/type/ClassInstance.java
@@ -920,7 +920,7 @@ public void setMappedName(String mappedName) {
 	@Override
 	public boolean hasMappedChildren() {
 		for (MethodInstance mth : methods) {
-			if (mth.hasMappedName() || mth.hasMappedChildren()) return true;
+			if (mth.hasNonInheritedMappedName() || mth.hasMappedChildren()) return true;
 		}
 
 		for (FieldInstance fld : fields) {

diff --git a/src/matcher/type/MethodInstance.java b/src/matcher/type/MethodInstance.java
@@ -1,6 +1,8 @@
 package matcher.type;
 
+import java.net.URI;
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
@@ -354,6 +356,81 @@ public boolean hasMappedChildren() {
 		return false;
 	}
 
+	public boolean hasNonInheritedMappedName() {
+		return !hasParentMethod() && hasMappedName();
+	}
+
+	public boolean hasParentMethod() {
+		if (hasParentMethod != null) return hasParentMethod;
+
+		return hasParentMethod = checkAncestry(AncestryCheck.HAS_PARENT_METHOD);
+	}
+
+	public boolean isAnyInputRoot() {
+		if (anyInputRoot != null) return anyInputRoot;
+
+		return anyInputRoot = checkAncestry(AncestryCheck.IS_ANY_INPUT_ROOT);
+	}
+
+	enum AncestryCheck {
+		HAS_PARENT_METHOD,
+		IS_ANY_INPUT_ROOT
+	}
+
+	private boolean checkAncestry(AncestryCheck checkType) {
+		// check if each origin that supplies this method has a parent within the same origin
+
+		for (int i = 0; i < cls.getAsmNodes().length; i++) {
+			for (MethodNode m : cls.getAsmNodes()[i].methods) {
+				if (m.name.equals(getName()) && m.desc.equals(getDesc())) {
+					boolean parentFound = hasParentMethod(getParents(), cls.getAsmNodeOrigin(i));
+
+					if (parentFound && checkType == AncestryCheck.HAS_PARENT_METHOD) {
+						return true;
+					} else if (parentFound && checkType == AncestryCheck.IS_ANY_INPUT_ROOT) {
+						break;
+					} else if (!parentFound && checkType == AncestryCheck.IS_ANY_INPUT_ROOT) {
+						return true;
+					}
+				}
+			}
+		}
+
+		return false;
+	}
+
+	private boolean hasParentMethod(Collection<MethodInstance> parents, URI reqOrigin) {
+		// check direct parents (must supply the method from the required origin)
+
+		for (MethodInstance parent : parents) {
+			ClassInstance parentCls = parent.getCls();
+
+			if (parentCls.getAsmNodes() == null) {
+				continue;
+			}
+
+			for (int i = 0; i < parentCls.getAsmNodes().length; i++) {
+				if (parentCls.getAsmNodeOrigin(i).equals(reqOrigin)) {
+					for (MethodNode m : parentCls.getAsmNodes()[i].methods) {
+						if (m.name.equals(getName()) && m.desc.equals(getDesc())) {
+							return true;
+						}
+					}
+				}
+			}
+		}
+
+		// check indirect parents recursively
+
+		for (MethodInstance parent : parents) {
+			if (!parent.getParents().isEmpty() && hasParentMethod(parent.getParents(), reqOrigin)) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
 	public ClassInstance getRetType() {
 		return retType;
 	}
@@ -503,6 +580,8 @@ public static String getId(String name, String desc) {
 	final MethodSignature signature;
 	private final MethodNode asmNode;
 
+	Boolean hasParentMethod;
+	Boolean anyInputRoot;
 	MethodType type = MethodType.UNKNOWN;
 
 	final Set<MethodInstance> refsIn = Util.newIdentityHashSet();