From 828953646ec2ad6fd6fa9c22a8a3769da40da085 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Thu, 7 Dec 2023 15:40:52 +0100 Subject: [PATCH] Support remapping destination descriptors, expand tests --- .../OuterClassNameInheritingVisitor.java | 35 ++- .../OuterClassNameInheritingVisitorTest.java | 254 +++++++++++++++--- 2 files changed, 247 insertions(+), 42 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/adapter/OuterClassNameInheritingVisitor.java b/src/main/java/net/fabricmc/mappingio/adapter/OuterClassNameInheritingVisitor.java index 8bd24791..5c987657 100644 --- a/src/main/java/net/fabricmc/mappingio/adapter/OuterClassNameInheritingVisitor.java +++ b/src/main/java/net/fabricmc/mappingio/adapter/OuterClassNameInheritingVisitor.java @@ -20,6 +20,7 @@ import java.util.Arrays; import java.util.EnumSet; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -29,6 +30,7 @@ import net.fabricmc.mappingio.MappedElementKind; import net.fabricmc.mappingio.MappingFlag; +import net.fabricmc.mappingio.MappingUtil; import net.fabricmc.mappingio.MappingVisitor; /** @@ -61,11 +63,13 @@ public boolean visitHeader() throws IOException { } @Override + @SuppressWarnings("unchecked") public void visitNamespaces(String srcNamespace, List dstNamespaces) throws IOException { dstNsCount = dstNamespaces.size(); if (pass == collectClassesPass) { visitedDstName = new boolean[dstNsCount]; + dstNameBySrcNameByNamespace = new HashMap[dstNsCount]; } else if (pass >= firstEmitPass) { super.visitNamespaces(srcNamespace, dstNamespaces); } @@ -90,7 +94,7 @@ public boolean visitClass(String srcName) throws IOException { this.srcName = srcName; if (pass == collectClassesPass) { - dstNameBySrcName.putIfAbsent(srcName, new String[dstNsCount]); + dstNamesBySrcName.putIfAbsent(srcName, new String[dstNsCount]); } else if (pass >= firstEmitPass) { super.visitClass(srcName); } @@ -103,11 +107,11 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam if (pass == collectClassesPass) { if (targetKind != MappedElementKind.CLASS) return; - dstNameBySrcName.get(srcName)[namespace] = name; + dstNamesBySrcName.get(srcName)[namespace] = name; } else if (pass >= firstEmitPass) { if (targetKind == MappedElementKind.CLASS) { visitedDstName[namespace] = true; - name = dstNameBySrcName.get(srcName)[namespace]; + name = dstNamesBySrcName.get(srcName)[namespace]; } super.visitDstName(targetKind, namespace, name); @@ -118,14 +122,26 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam public void visitDstDesc(MappedElementKind targetKind, int namespace, String desc) throws IOException { if (pass < firstEmitPass) return; - // TODO: Remap type descriptors of changed classes + if (modifiedClasses.contains(srcName)) { + Map nsDstNameBySrcName = dstNameBySrcNameByNamespace[namespace]; + + if (nsDstNameBySrcName == null) { + dstNameBySrcNameByNamespace[namespace] = nsDstNameBySrcName = dstNamesBySrcName.entrySet() + .stream() + .filter(entry -> entry.getValue()[namespace] != null) + .collect(HashMap::new, (map, entry) -> map.put(entry.getKey(), entry.getValue()[namespace]), HashMap::putAll); + } + + desc = MappingUtil.mapDesc(desc, nsDstNameBySrcName); + } + super.visitDstDesc(targetKind, namespace, desc); } @Override public boolean visitElementContent(MappedElementKind targetKind) throws IOException { if (targetKind == MappedElementKind.CLASS && pass > collectClassesPass) { - String[] dstNames = dstNameBySrcName.get(srcName); + String[] dstNames = dstNamesBySrcName.get(srcName); for (int ns = 0; ns < dstNames.length; ns++) { String dstName = dstNames[ns]; @@ -137,12 +153,13 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept for (int pos = parts.length - 2; pos >= 0; pos--) { String outerSrcName = String.join("$", Arrays.copyOfRange(parts, 0, pos + 1)); - String outerDstName = dstNameBySrcName.get(outerSrcName)[ns]; + String outerDstName = dstNamesBySrcName.get(outerSrcName)[ns]; if (outerDstName != null) { dstName = outerDstName + "$" + String.join("$", Arrays.copyOfRange(parts, pos + 1, parts.length)); dstNames[ns] = dstName; + modifiedClasses.add(srcName); break; } } @@ -171,16 +188,16 @@ public boolean visitEnd() throws IOException { } return super.visitEnd(); - } private static final int collectClassesPass = 1; private static final int fixOuterClassesPass = 2; private static final int firstEmitPass = 3; - private final boolean nextNeedsMultiplePasses = next.getFlags().contains(MappingFlag.NEEDS_MULTIPLE_PASSES); - private final Map dstNameBySrcName = new HashMap<>(); + private final Map dstNamesBySrcName = new HashMap<>(); + private final Set modifiedClasses = new HashSet<>(); private int pass = 1; private int dstNsCount = -1; private String srcName; private boolean[] visitedDstName; + private Map[] dstNameBySrcNameByNamespace; } diff --git a/src/test/java/net/fabricmc/mappingio/adapter/OuterClassNameInheritingVisitorTest.java b/src/test/java/net/fabricmc/mappingio/adapter/OuterClassNameInheritingVisitorTest.java index 76fcd9e4..7d351a19 100644 --- a/src/test/java/net/fabricmc/mappingio/adapter/OuterClassNameInheritingVisitorTest.java +++ b/src/test/java/net/fabricmc/mappingio/adapter/OuterClassNameInheritingVisitorTest.java @@ -20,60 +20,248 @@ import java.io.IOException; import java.util.Arrays; +import java.util.EnumSet; +import java.util.Set; import org.junit.jupiter.api.Test; import net.fabricmc.mappingio.MappedElementKind; +import net.fabricmc.mappingio.MappingFlag; +import net.fabricmc.mappingio.MappingVisitor; +import net.fabricmc.mappingio.NopMappingVisitor; import net.fabricmc.mappingio.tree.MemoryMappingTree; import net.fabricmc.mappingio.tree.VisitableMappingTree; -import net.fabricmc.mappingio.tree.MappingTree.ClassMapping; public class OuterClassNameInheritingVisitorTest { + private static void accept(MappingVisitor visitor) throws IOException { + do { + if (visitor.visitHeader()) { + visitor.visitNamespaces("source", Arrays.asList("dstNs0", "dstNs1", "dstNs2", "dstNs3", "dstNs4", "dstNs5", "dstNs6")); + } + + if (visitor.visitContent()) { + if (visitor.visitClass("class_1")) { + visitor.visitDstName(MappedElementKind.CLASS, 0, "class1Ns0Rename"); + visitor.visitDstName(MappedElementKind.CLASS, 1, "class1Ns1Rename"); + visitor.visitDstName(MappedElementKind.CLASS, 2, "class1Ns2Rename"); + visitor.visitDstName(MappedElementKind.CLASS, 4, "class1Ns4Rename"); + + if (visitor.visitElementContent(MappedElementKind.CLASS)) { + if (visitor.visitField("field_1", "Lclass_1;")) { + for (int i = 0; i <= 6; i++) { + visitor.visitDstDesc(MappedElementKind.FIELD, i, "Lclass_1;"); + visitor.visitElementContent(MappedElementKind.FIELD); + } + } + } + } + + if (visitor.visitClass("class_1$class_2")) { + visitor.visitDstName(MappedElementKind.CLASS, 2, "class1Ns2Rename$class2Ns2Rename"); + visitor.visitDstName(MappedElementKind.CLASS, 3, "class_1$class2Ns3Rename"); + visitor.visitDstName(MappedElementKind.CLASS, 4, "class_1$class_2"); + visitor.visitDstName(MappedElementKind.CLASS, 5, "class_1$class2Ns5Rename"); + + if (visitor.visitElementContent(MappedElementKind.CLASS)) { + if (visitor.visitField("field_2", "Lclass_1$class_2;")) { + for (int i = 0; i <= 6; i++) { + visitor.visitDstDesc(MappedElementKind.FIELD, i, "Lclass_1$class_2;"); + visitor.visitElementContent(MappedElementKind.FIELD); + } + } + } + } + + if (visitor.visitClass("class_1$class_2$class_3")) { + visitor.visitDstName(MappedElementKind.CLASS, 5, "class_1$class2Ns5Rename$class3Ns5Rename"); + visitor.visitDstName(MappedElementKind.CLASS, 6, "class_1$class_2$class3Ns6Rename"); + + if (visitor.visitElementContent(MappedElementKind.CLASS)) { + if (visitor.visitField("field_2", "Lclass_1$class_2$class_3;")) { + for (int i = 0; i <= 6; i++) { + visitor.visitDstDesc(MappedElementKind.FIELD, i, "Lclass_1$class_2$class_3;"); + visitor.visitElementContent(MappedElementKind.FIELD); + } + } + } + } + } + } while (!visitor.visitEnd()); + } + + @Test + public void directVisit() throws IOException { + accept(new OuterClassNameInheritingVisitor(new CheckingVisitor(false))); + } + @Test - public void testOuterClassFixing() throws IOException { + public void tree() throws IOException { VisitableMappingTree tree = new MemoryMappingTree(); - tree.visitNamespaces("source", Arrays.asList("dstNs0", "dstNs1", "dstNs2", "dstNs3", "dstNs4", "dstNs5", "dstNs6")); + accept(new OuterClassNameInheritingVisitor(tree)); + tree.accept(new CheckingVisitor(true)); + } + + private static class CheckingVisitor extends NopMappingVisitor { + CheckingVisitor(boolean tree) { + super(true); + this.tree = tree; + } - tree.visitClass("class_1"); - tree.visitDstName(MappedElementKind.CLASS, 0, "class1Ns0Rename"); - tree.visitDstName(MappedElementKind.CLASS, 1, "class1Ns1Rename"); - tree.visitDstName(MappedElementKind.CLASS, 2, "class1Ns2Rename"); - tree.visitDstName(MappedElementKind.CLASS, 4, "class1Ns4Rename"); + @Override + public Set getFlags() { + return EnumSet.of(MappingFlag.NEEDS_DST_FIELD_DESC, MappingFlag.NEEDS_DST_METHOD_DESC); + } - tree.visitClass("class_1$class_2"); - tree.visitDstName(MappedElementKind.CLASS, 2, "class1Ns2Rename$class2Ns2Rename"); - tree.visitDstName(MappedElementKind.CLASS, 3, "class_1$class2Ns3Rename"); - tree.visitDstName(MappedElementKind.CLASS, 4, "class_1$class_2"); - tree.visitDstName(MappedElementKind.CLASS, 5, "class_1$class2Ns5Rename"); + @Override + public boolean visitClass(String srcName) throws IOException { + clsSrcName = srcName; + return true; + } - tree.visitClass("class_1$class_2$class_3"); - tree.visitDstName(MappedElementKind.CLASS, 5, "class_1$class2Ns5Rename$class3Ns5Rename"); - tree.visitDstName(MappedElementKind.CLASS, 6, "class_1$class_2$class3Ns6Rename"); + @Override + public void visitDstDesc(MappedElementKind targetKind, int namespace, String desc) throws IOException { + if (tree) return; // trees handle destination descriptor remapping themselves - VisitableMappingTree processedTree = new MemoryMappingTree(); - tree.accept(new OuterClassNameInheritingVisitor(processedTree)); + switch (clsSrcName) { + case "class_1": + assertEquals("Lclass_1;", desc); + break; + case "class_1$class_2": + switch (namespace) { + case 0: + assertEquals("Lclass1Ns0Rename$class_2;", desc); + break; + case 1: + assertEquals("Lclass1Ns1Rename$class_2;", desc); + break; + case 2: + assertEquals("Lclass1Ns2Rename$class2Ns2Rename;", desc); + break; + case 3: + assertEquals("Lclass_1$class2Ns3Rename;", desc); + break; + case 4: + assertEquals("Lclass_1$class_2;", desc); + break; + case 5: + assertEquals("Lclass_1$class2Ns5Rename;", desc); + break; + case 6: + assertEquals("Lclass_1$class_2;", desc); + break; + default: + throw new IllegalStateException(); + } - ClassMapping class2 = processedTree.getClass("class_1$class_2"); - ClassMapping class3 = processedTree.getClass("class_1$class_2$class_3"); + break; + case "class_1$class_2$class_3": + switch (namespace) { + case 0: + assertEquals("Lclass1Ns0Rename$class_2$class_3;", desc); + break; + case 1: + assertEquals("Lclass1Ns1Rename$class_2$class_3;", desc); + break; + case 2: + assertEquals("Lclass1Ns2Rename$class2Ns2Rename$class_3;", desc); + break; + case 3: + assertEquals("Lclass_1$class2Ns3Rename$class_3;", desc); + break; + case 4: + assertEquals("Lclass_1$class_2$class_3;", desc); + break; + case 5: + assertEquals("Lclass_1$class2Ns5Rename$class3Ns5Rename;", desc); + break; + case 6: + assertEquals("Lclass_1$class_2$class3Ns6Rename;", desc); + break; + default: + throw new IllegalStateException(); + } - assertEquals("class1Ns0Rename$class_2", class2.getDstName(0)); - assertEquals("class1Ns0Rename$class_2$class_3", class3.getDstName(0)); + break; + default: + throw new IllegalStateException(); + } + } - assertEquals("class1Ns1Rename$class_2", class2.getDstName(1)); - assertEquals("class1Ns1Rename$class_2$class_3", class3.getDstName(1)); + @Override + public void visitDstName(MappedElementKind targetKind, int namespace, String name) throws IOException { + if (targetKind != MappedElementKind.CLASS) return; - assertEquals("class1Ns2Rename$class2Ns2Rename", class2.getDstName(2)); - assertEquals("class1Ns2Rename$class2Ns2Rename$class_3", class3.getDstName(2)); + switch (clsSrcName) { + case "class_1": + break; + case "class_1$class_2": + switch (namespace) { + case 0: + assertEquals("class1Ns0Rename$class_2", name); + break; + case 1: + assertEquals("class1Ns1Rename$class_2", name); + break; + case 2: + assertEquals("class1Ns2Rename$class2Ns2Rename", name); + break; + case 3: + assertEquals("class_1$class2Ns3Rename", name); + break; + case 4: + assertEquals("class_1$class_2", name); + break; + case 5: + assertEquals("class_1$class2Ns5Rename", name); + break; + case 6: + assertEquals("class_1$class_2", name); + break; + default: + throw new IllegalStateException(); + } - assertEquals("class_1$class2Ns3Rename", class2.getDstName(3)); - assertEquals("class_1$class2Ns3Rename$class_3", class3.getDstName(3)); + break; + case "class_1$class_2$class_3": + switch (namespace) { + case 0: + assertEquals("class1Ns0Rename$class_2$class_3", name); + break; + case 1: + assertEquals("class1Ns1Rename$class_2$class_3", name); + break; + case 2: + assertEquals("class1Ns2Rename$class2Ns2Rename$class_3", name); + break; + case 3: + assertEquals("class_1$class2Ns3Rename$class_3", name); + break; + case 4: + assertEquals("class_1$class_2$class_3", name); + break; + case 5: + assertEquals("class_1$class2Ns5Rename$class3Ns5Rename", name); + break; + case 6: + assertEquals("class_1$class_2$class3Ns6Rename", name); + break; + default: + throw new IllegalStateException(); + } - assertEquals("class_1$class_2", class2.getDstName(4)); - assertEquals("class_1$class_2$class_3", class3.getDstName(4)); + break; + default: + throw new IllegalStateException(); + } + } - assertEquals("class_1$class2Ns5Rename", class2.getDstName(5)); - assertEquals("class_1$class2Ns5Rename$class3Ns5Rename", class3.getDstName(5)); + @Override + public boolean visitEnd() throws IOException { + return ++passesDone == 2; + } - assertEquals("class_1$class_2$class3Ns6Rename", class3.getDstName(6)); + private final boolean tree; + private byte passesDone = 0; + private String clsSrcName; } }