Fix CSRG and JAM writers skipping elements whose parents have incompl…

…ete destination names (#97) * Fix CSRG and JAM writer bugs * Add changes to changelog * Reword changelog entry
FabricMC · Apr 14, 2024 · a182551 · a182551
1 parent 3da92ab
commit a182551
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+- Fixed CSRG and JAM writers sometimes skipping elements whose parents have incomplete destination names
 
 ## [0.6.0] - 2024-4-12
 - Added CSRG writer

diff --git a/src/main/java/net/fabricmc/mappingio/format/srg/CsrgFileWriter.java b/src/main/java/net/fabricmc/mappingio/format/srg/CsrgFileWriter.java
@@ -93,27 +93,27 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam
 
 	@Override
 	public boolean visitElementContent(MappedElementKind targetKind) throws IOException {
-		if (dstName == null) return false;
+		if (dstName != null) {
+			write(classSrcName);
 
-		write(classSrcName);
+			if (targetKind != MappedElementKind.CLASS) {
+				writeSpace();
+				write(memberSrcName);
 
-		if (targetKind != MappedElementKind.CLASS) {
-			writeSpace();
-			write(memberSrcName);
+				if (targetKind == MappedElementKind.METHOD) {
+					writeSpace();
+					write(methodSrcDesc);
+				}
 
-			if (targetKind == MappedElementKind.METHOD) {
-				writeSpace();
-				write(methodSrcDesc);
+				memberSrcName = methodSrcDesc = null;
 			}
 
-			memberSrcName = methodSrcDesc = null;
-		}
-
-		writeSpace();
-		write(dstName);
-		writeLn();
+			writeSpace();
+			write(dstName);
+			writeLn();
 
-		dstName = null;
+			dstName = null;
+		}
 
 		return targetKind == MappedElementKind.CLASS; // only members are supported, skip anything but class contents
 	}

diff --git a/src/main/java/net/fabricmc/mappingio/format/srg/JamFileWriter.java b/src/main/java/net/fabricmc/mappingio/format/srg/JamFileWriter.java
@@ -125,7 +125,9 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept
 		} else if (targetKind == MappedElementKind.FIELD
 				|| (isMethod = targetKind == MappedElementKind.METHOD)
 				|| (isArg = targetKind == MappedElementKind.METHOD_ARG)) {
-			if (classOnlyPass || memberSrcDesc == null || memberDstName == null) {
+			if (classOnlyPass) {
+				return false;
+			} else if (memberSrcDesc == null || (!isArg && memberDstName == null)) {
 				return isMethod;
 			}
 

diff --git a/src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java b/src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java
@@ -80,7 +80,7 @@ public boolean visitClass(String srcName, String[] dstNames) throws IOException
 
 		if (supCls == null) {
 			String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]};
-			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false;
+			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return true;
 			throw new RuntimeException("SubTree class not contained in SupTree: " + srcName);
 		}
 
@@ -111,7 +111,7 @@ public boolean visitField(String srcClsName, String srcName, String srcDesc,
 
 		if (supFld == null) {
 			String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]};
-			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false;
+			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return true;
 			throw new RuntimeException("SubTree field not contained in SupTree: " + srcName);
 		}
 
@@ -149,7 +149,7 @@ public boolean visitMethod(String srcClsName, String srcName, String srcDesc,
 
 		if (supMth == null) {
 			String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]};
-			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false;
+			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return true;
 			throw new RuntimeException("SubTree method not contained in SupTree: " + srcName);
 		}
 
@@ -187,7 +187,7 @@ public boolean visitMethodArg(String srcClsName, String srcMethodName, String sr
 
 		if (supArg == null) {
 			String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]};
-			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false;
+			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return true;
 			throw new RuntimeException("SubTree arg not contained in SupTree: " + srcName);
 		}
 
@@ -221,7 +221,7 @@ public boolean visitMethodVar(String srcClsName, String srcMethodName, String sr
 
 		if (supVar == null) {
 			String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]};
-			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false;
+			if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return true;
 			throw new RuntimeException("SubTree var not contained in SupTree: " + srcName);
 		}
 

diff --git a/src/test/resources/read/valid-with-holes/jam.jam b/src/test/resources/read/valid-with-holes/jam.jam
@@ -8,3 +8,5 @@ FD class_32 field_2 I field2Ns0Rename
 FD class_32 field_5 I field5Ns0Rename
 MD class_32 method_2 ()I method2Ns0Rename
 MD class_32 method_5 ()I method5Ns0Rename
+MP class_32 method_7 ()I 4 param3Ns0Rename
+MP class_32 method_7 ()I 8 param5Ns0Rename