Skip writing elements with no destination names where applicable (#111)

FabricMC · Aug 31, 2024 · 1f40679 · 1f40679
1 parent 3bc4d8e
commit 1f40679
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 103 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Overhauled the internal `ColumnFileReader` to behave more consistently
 - Made handling of the `NEEDS_MULTIPLE_PASSES` flag more consistent, reducing memory usage in a few cases
 - Made some internal methods in Enigma and TSRG readers actually private
+- Made all writers for formats which can't represent empty destination names skip such elements entirely, unless mapped child elements are present
 - Added missing `visitElementContent` calls to CSRG and Recaf Simple readers
 - Fixed member mapping merging via tree-API in `MemoryMappingTree`
 - Fixed duplicate mapping definitions not being handled correctly in multiple readers

diff --git a/src/main/java/net/fabricmc/mappingio/format/proguard/ProGuardFileWriter.java b/src/main/java/net/fabricmc/mappingio/format/proguard/ProGuardFileWriter.java
@@ -35,9 +35,11 @@ public final class ProGuardFileWriter implements MappingWriter {
 	private final Writer writer;
 	private final String dstNamespaceString;
 	private int dstNamespace = -1;
-	private String srcName;
-	private String srcDesc;
+	private String clsSrcName;
+	private String memberSrcName;
+	private String memberSrcDesc;
 	private String dstName;
+	private boolean classContentVisitPending;
 
 	/**
 	 * Constructs a ProGuard mapping writer that uses
@@ -101,23 +103,23 @@ public void visitNamespaces(String srcNamespace, List<String> dstNamespaces) thr
 
 	@Override
 	public boolean visitClass(String srcName) throws IOException {
-		this.srcName = srcName;
+		clsSrcName = srcName;
 
 		return true;
 	}
 
 	@Override
 	public boolean visitField(String srcName, @Nullable String srcDesc) throws IOException {
-		this.srcName = srcName;
-		this.srcDesc = srcDesc;
+		memberSrcName = srcName;
+		memberSrcDesc = srcDesc;
 
 		return true;
 	}
 
 	@Override
 	public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOException {
-		this.srcName = srcName;
-		this.srcDesc = srcDesc;
+		memberSrcName = srcName;
+		memberSrcDesc = srcDesc;
 
 		return true;
 	}
@@ -143,26 +145,41 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam
 
 	@Override
 	public boolean visitElementContent(MappedElementKind targetKind) throws IOException {
-		if (dstName == null) dstName = srcName;
+		if (targetKind == MappedElementKind.CLASS) {
+			if (dstName == null) {
+				classContentVisitPending = true;
+				return true;
+			}
+		} else {
+			if (dstName == null) {
+				return false;
+			} else if (classContentVisitPending) {
+				String memberDstName = dstName;
+				dstName = clsSrcName;
+				visitElementContent(MappedElementKind.CLASS);
+				classContentVisitPending = false;
+				dstName = memberDstName;
+			}
+		}
 
 		switch (targetKind) {
 		case CLASS:
-			writer.write(toJavaClassName(srcName));
+			writer.write(toJavaClassName(clsSrcName));
 			dstName = toJavaClassName(dstName) + ":";
 			break;
 		case FIELD:
 			writeIndent();
-			writer.write(toJavaType(srcDesc));
+			writer.write(toJavaType(memberSrcDesc));
 			writer.write(' ');
-			writer.write(srcName);
+			writer.write(memberSrcName);
 			break;
 		case METHOD:
 			writeIndent();
-			writer.write(toJavaType(srcDesc.substring(srcDesc.indexOf(')', 1) + 1)));
+			writer.write(toJavaType(memberSrcDesc.substring(memberSrcDesc.indexOf(')', 1) + 1)));
 			writer.write(' ');
-			writer.write(srcName);
+			writer.write(memberSrcName);
 			writer.write('(');
-			List<String> argTypes = extractArgumentTypes(srcDesc);
+			List<String> argTypes = extractArgumentTypes(memberSrcDesc);
 
 			for (int i = 0; i < argTypes.size(); i++) {
 				if (i > 0) {
@@ -182,8 +199,8 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept
 		writer.write(dstName);
 		writer.write('\n');
 
-		srcName = srcDesc = dstName = null;
-		return true;
+		dstName = null;
+		return targetKind == MappedElementKind.CLASS;
 	}
 
 	@Override

diff --git a/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileWriter.java b/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileWriter.java
@@ -74,36 +74,41 @@ public void visitMetadata(String key, @Nullable String value) throws IOException
 
 	@Override
 	public boolean visitClass(String srcName) throws IOException {
-		this.srcName = srcName;
+		clsSrcName = srcName;
+		hasAnyDstNames = false;
 
 		return true;
 	}
 
 	@Override
 	public boolean visitField(String srcName, @Nullable String srcDesc) throws IOException {
-		this.srcName = srcName;
-		this.srcDesc = srcDesc;
+		memberSrcName = srcName;
+		memberSrcDesc = srcDesc;
+		hasAnyDstNames = false;
 
 		return true;
 	}
 
 	@Override
 	public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOException {
-		this.srcName = srcName;
-		this.srcDesc = srcDesc;
+		memberSrcName = srcName;
+		memberSrcDesc = srcDesc;
+		hasAnyDstNames = false;
 
 		return true;
 	}
 
 	@Override
 	public boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String srcName) throws IOException {
-		if (tsrg2) {
-			this.srcName = srcName;
-			this.lvIndex = lvIndex;
-			return true;
+		if (!tsrg2) {
+			return false;
 		}
 
-		return false;
+		this.lvIndex = lvIndex;
+		argSrcName = srcName;
+		hasAnyDstNames = false;
+
+		return true;
 	}
 
 	@Override
@@ -116,19 +121,56 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam
 		if (!tsrg2 && namespace != 0) return;
 
 		dstNames[namespace] = name;
+		hasAnyDstNames |= name != null;
 	}
 
 	@Override
 	public boolean visitElementContent(MappedElementKind targetKind) throws IOException {
+		if (classContentVisitPending && targetKind != MappedElementKind.CLASS && hasAnyDstNames) {
+			String[] memberOrArgDstNames = dstNames.clone();
+			Arrays.fill(dstNames, clsSrcName);
+			visitElementContent(MappedElementKind.CLASS);
+			classContentVisitPending = false;
+			dstNames = memberOrArgDstNames;
+		}
+
+		if (methodContentVisitPending && targetKind == MappedElementKind.METHOD_ARG && hasAnyDstNames) {
+			String[] argDstNames = dstNames.clone();
+			Arrays.fill(dstNames, memberSrcName);
+			visitElementContent(MappedElementKind.METHOD);
+			methodContentVisitPending = false;
+			dstNames = argDstNames;
+		}
+
+		String srcName = null;
+
 		switch (targetKind) {
 		case CLASS:
+			if (!hasAnyDstNames) {
+				classContentVisitPending = true;
+				return true;
+			}
+
+			srcName = clsSrcName;
 			break;
 		case FIELD:
 		case METHOD:
+			if (!hasAnyDstNames) {
+				if (targetKind == MappedElementKind.METHOD) {
+					methodContentVisitPending = true;
+					return tsrg2;
+				}
+
+				return false;
+			}
+
+			srcName = memberSrcName;
 			writeTab();
 			break;
 		case METHOD_ARG:
 			assert tsrg2;
+			if (!hasAnyDstNames) return false;
+			srcName = argSrcName;
 			writeTab();
 			writeTab();
 			write(Integer.toString(lvIndex));
@@ -143,7 +185,7 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept
 		if (targetKind == MappedElementKind.METHOD
 				|| (targetKind == MappedElementKind.FIELD && tsrg2)) {
 			writeSpace();
-			write(srcDesc);
+			write(memberSrcDesc);
 		}
 
 		int dstNsCount = tsrg2 ? dstNames.length : 1;
@@ -156,9 +198,7 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept
 
 		writeLn();
 
-		srcName = srcDesc = null;
 		Arrays.fill(dstNames, null);
-		lvIndex = -1;
 
 		return targetKind == MappedElementKind.CLASS
 				|| (tsrg2 && targetKind == MappedElementKind.METHOD);
@@ -195,8 +235,13 @@ private void writeLn() throws IOException {
 
 	private final Writer writer;
 	private final boolean tsrg2;
-	private String srcName;
-	private String srcDesc;
+	private String clsSrcName;
+	private String memberSrcName;
+	private String memberSrcDesc;
+	private String argSrcName;
 	private String[] dstNames;
+	private boolean hasAnyDstNames;
 	private int lvIndex = -1;
+	private boolean classContentVisitPending;
+	private boolean methodContentVisitPending;
 }
diff --git a/src/test/resources/read/valid-with-holes/csrg.csrg b/src/test/resources/read/valid-with-holes/csrg.csrg
@@ -1,33 +1,10 @@
-class_1 class_1
 class_2 class2Ns0Rename
-class_3 class_3
-class_4 class_4
 class_5 class5Ns0Rename
-class_6 class_6
-class_7 class_7
 class_7$class_8 class_7$class8Ns0Rename
-class_9$class_10 class_9$class_10
-class_11$class_12 class_11$class_12
 class_13$class_14 class_13$class14Ns0Rename
-class_15$class_16 class_15$class_16
-class_17 class_17
 class_17$class_18$class_19 class_17$class_18$class19Ns0Rename
-class_20$class_21$class_22 class_20$class_21$class_22
-class_23$class_24$class_25 class_23$class_24$class_25
 class_26$class_27$class_28 class_26$class_27$class28Ns0Rename
-class_29$class_30$class_31 class_29$class_30$class_31
-class_32 class_32
-class_32 field_1 field_1
 class_32 field_2 field2Ns0Rename
-class_32 field_3 field_3
-class_32 field_4 field_4
 class_32 field_5 field5Ns0Rename
-class_32 field_6 field_6
-class_32 method_1 ()I method_1
 class_32 method_2 (I)V method2Ns0Rename
-class_32 method_3 (Lcls;)Lcls; method_3
-class_32 method_4 (ILcls;)Lpkg/cls; method_4
 class_32 method_5 (Lcls;[I)[[B method5Ns0Rename
-class_32 method_6 ()I method_6
-class_32 method_7 (I)V method_7
-class_32 method_8 (Lcls;)Lcls; method_8
diff --git a/src/test/resources/read/valid-with-holes/migration-map.xml b/src/test/resources/read/valid-with-holes/migration-map.xml
@@ -1,23 +1,9 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <migrationMap>
-	<entry oldName="class_1" newName="class_1" type="class" />
 	<entry oldName="class_2" newName="class2Ns0Rename" type="class" />
-	<entry oldName="class_3" newName="class_3" type="class" />
-	<entry oldName="class_4" newName="class_4" type="class" />
 	<entry oldName="class_5" newName="class5Ns0Rename" type="class" />
-	<entry oldName="class_6" newName="class_6" type="class" />
-	<entry oldName="class_7" newName="class_7" type="class" />
 	<entry oldName="class_7$class_8" newName="class_7$class8Ns0Rename" type="class" />
-	<entry oldName="class_9$class_10" newName="class_9$class_10" type="class" />
-	<entry oldName="class_11$class_12" newName="class_11$class_12" type="class" />
 	<entry oldName="class_13$class_14" newName="class_13$class14Ns0Rename" type="class" />
-	<entry oldName="class_15$class_16" newName="class_15$class_16" type="class" />
-	<entry oldName="class_17" newName="class_17" type="class" />
 	<entry oldName="class_17$class_18$class_19" newName="class_17$class_18$class19Ns0Rename" type="class" />
-	<entry oldName="class_20$class_21$class_22" newName="class_20$class_21$class_22" type="class" />
-	<entry oldName="class_23$class_24$class_25" newName="class_23$class_24$class_25" type="class" />
 	<entry oldName="class_26$class_27$class_28" newName="class_26$class_27$class28Ns0Rename" type="class" />
-	<entry oldName="class_29$class_30$class_31" newName="class_29$class_30$class_31" type="class" />
-	<entry oldName="class_32" newName="class_32" type="class" />
 </migrationMap>
-
diff --git a/src/test/resources/read/valid-with-holes/tsrg.tsrg b/src/test/resources/read/valid-with-holes/tsrg.tsrg
@@ -1,33 +1,11 @@
-class_1 class_1
 class_2 class2Ns0Rename
-class_3 class_3
-class_4 class_4
 class_5 class5Ns0Rename
-class_6 class_6
-class_7 class_7
 class_7$class_8 class_7$class8Ns0Rename
-class_9$class_10 class_9$class_10
-class_11$class_12 class_11$class_12
 class_13$class_14 class_13$class14Ns0Rename
-class_15$class_16 class_15$class_16
-class_17 class_17
 class_17$class_18$class_19 class_17$class_18$class19Ns0Rename
-class_20$class_21$class_22 class_20$class_21$class_22
-class_23$class_24$class_25 class_23$class_24$class_25
 class_26$class_27$class_28 class_26$class_27$class28Ns0Rename
-class_29$class_30$class_31 class_29$class_30$class_31
 class_32 class_32
-	field_1 field_1
 	field_2 field2Ns0Rename
-	field_3 field_3
-	field_4 field_4
 	field_5 field5Ns0Rename
-	field_6 field_6
-	method_1 ()I method_1
 	method_2 (I)V method2Ns0Rename
-	method_3 (Lcls;)Lcls; method_3
-	method_4 (ILcls;)Lpkg/cls; method_4
 	method_5 (Lcls;[I)[[B method5Ns0Rename
-	method_6 ()I method_6
-	method_7 (I)V method_7
-	method_8 (Lcls;)Lcls; method_8
diff --git a/src/test/resources/read/valid-with-holes/tsrgV2.tsrg b/src/test/resources/read/valid-with-holes/tsrgV2.tsrg
@@ -1,40 +1,27 @@
 tsrg2 source target target2
-class_1 class_1 class_1
 class_2 class2Ns0Rename class_2
 class_3 class_3 class3Ns1Rename
-class_4 class_4 class_4
 class_5 class5Ns0Rename class_5
 class_6 class_6 class6Ns1Rename
-class_7 class_7 class_7
 class_7$class_8 class_7$class8Ns0Rename class_7$class_8
 class_9$class_10 class_9$class_10 class_9$class10Ns1Rename
-class_11$class_12 class_11$class_12 class_11$class_12
 class_13$class_14 class_13$class14Ns0Rename class_13$class_14
 class_15$class_16 class_15$class_16 class_15$class16Ns1Rename
-class_17 class_17 class_17
 class_17$class_18$class_19 class_17$class_18$class19Ns0Rename class_17$class_18$class_19
 class_20$class_21$class_22 class_20$class_21$class_22 class_20$class_21$class22Ns1Rename
-class_23$class_24$class_25 class_23$class_24$class_25 class_23$class_24$class_25
 class_26$class_27$class_28 class_26$class_27$class28Ns0Rename class_26$class_27$class_28
 class_29$class_30$class_31 class_29$class_30$class_31 class_29$class_30$class31Ns1Rename
 class_32 class_32 class_32
-	field_1 I field_1 field_1
 	field_2 Lcls; field2Ns0Rename field_2
 	field_3 Lpkg/cls; field_3 field3Ns1Rename
-	field_4 [I field_4 field_4
 	field_5 I field5Ns0Rename field_5
 	field_6 Lcls; field_6 field6Ns1Rename
-	method_1 ()I method_1 method_1
 	method_2 (I)V method2Ns0Rename method_2
 	method_3 (Lcls;)Lcls; method_3 method3Ns1Rename
-	method_4 (ILcls;)Lpkg/cls; method_4 method_4
 	method_5 (Lcls;[I)[[B method5Ns0Rename method_5
 	method_6 ()I method_6 method6Ns1Rename
 	method_7 (I)V method_7 method_7
-		1 param_1 param_1 param_1
 		3 param_2 param_2 param2Ns1Rename
 		5 param_3 param3Ns0Rename param_3
-		7 param_4 param_4 param_4
 		9 param_5 param5Ns0Rename param_5
 		11 param_6 param_6 param6Ns1Rename
-	method_8 (Lcls;)Lcls; method_8 method_8