From 1f40679b0496fd0b17c049c8075772337a775f4e Mon Sep 17 00:00:00 2001 From: NebelNidas <48808497+NebelNidas@users.noreply.github.com> Date: Sat, 31 Aug 2024 15:35:30 +0200 Subject: [PATCH] Skip writing elements with no destination names where applicable (#111) --- CHANGELOG.md | 1 + .../format/proguard/ProGuardFileWriter.java | 49 ++++++++---- .../mappingio/format/srg/TsrgFileWriter.java | 75 +++++++++++++++---- .../resources/read/valid-with-holes/csrg.csrg | 23 ------ .../read/valid-with-holes/migration-map.xml | 14 ---- .../resources/read/valid-with-holes/tsrg.tsrg | 22 ------ .../read/valid-with-holes/tsrgV2.tsrg | 13 ---- 7 files changed, 94 insertions(+), 103 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23a6c6f3..bc08c27d 100644 --- 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 index ef024410..cd771bcc 100644 --- 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 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 argTypes = extractArgumentTypes(srcDesc); + List 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 index 41ac2c79..5acd1884 100644 --- 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 index ef91ca84..2b811438 100644 --- 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 index 29d91399..dc805b2f 100644 --- 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 @@ - - - - - - - - - - - - - - diff --git a/src/test/resources/read/valid-with-holes/tsrg.tsrg b/src/test/resources/read/valid-with-holes/tsrg.tsrg index 09667011..89fbadd8 100644 --- 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 index 3749147c..b9409ad8 100644 --- 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