Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip writing elements with no destination names where applicable #111

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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) {
Expand All @@ -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
Expand Down
75 changes: 60 additions & 15 deletions src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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));
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}
23 changes: 0 additions & 23 deletions src/test/resources/read/valid-with-holes/csrg.csrg
Original file line number Diff line number Diff line change
@@ -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
14 changes: 0 additions & 14 deletions src/test/resources/read/valid-with-holes/migration-map.xml
Original file line number Diff line number Diff line change
@@ -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>

22 changes: 0 additions & 22 deletions src/test/resources/read/valid-with-holes/tsrg.tsrg
Original file line number Diff line number Diff line change
@@ -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
13 changes: 0 additions & 13 deletions src/test/resources/read/valid-with-holes/tsrgV2.tsrg
Original file line number Diff line number Diff line change
@@ -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
Loading