From 9fec03dece822fb6a771f20708eb8934758c56e5 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 18 Nov 2023 17:45:17 +0100 Subject: [PATCH 1/4] Expand `VisitEndTest`; fix test trees being inconsistent --- .../mappingio/SubsetAssertingVisitor.java | 259 ++++++++++++++++++ .../net/fabricmc/mappingio/TestHelper.java | 19 +- .../mappingio/read/ValidContentReadTest.java | 224 +-------------- .../mappingio/visiting/VisitEndTest.java | 177 ++++++++++-- .../enigma-dir/class_32.mapping | 10 +- .../read/valid-with-holes/enigma.mappings | 10 +- .../read/valid-with-holes/tinyV2.tiny | 24 +- .../read/valid-with-holes/tsrg2.tsrg | 12 +- 8 files changed, 452 insertions(+), 283 deletions(-) create mode 100644 src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java diff --git a/src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java b/src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java new file mode 100644 index 00000000..907bc033 --- /dev/null +++ b/src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java @@ -0,0 +1,259 @@ +/* + * Copyright (c) 2023 FabricMC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.fabricmc.mappingio; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +import org.jetbrains.annotations.Nullable; + +import net.fabricmc.mappingio.format.MappingFormat; +import net.fabricmc.mappingio.tree.MappingTreeView; +import net.fabricmc.mappingio.tree.MappingTreeView.ClassMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.FieldMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodArgMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodMappingView; +import net.fabricmc.mappingio.tree.MappingTreeView.MethodVarMappingView; + +public class SubsetAssertingVisitor implements FlatMappingVisitor { + public SubsetAssertingVisitor(MappingTreeView supTree, @Nullable MappingFormat supFormat, @Nullable MappingFormat subFormat) { + this.supTree = supTree; + this.supDstNsCount = supTree.getMaxNamespaceId(); + this.subHasNamespaces = subFormat == null ? true : subFormat.hasNamespaces; + this.supHasNamespaces = supFormat == null ? true : supFormat.hasNamespaces; + this.supHasFieldDesc = supFormat == null ? true : supFormat.hasFieldDescriptors; + this.supHasArgs = supFormat == null ? true : supFormat.supportsArgs; + this.supHasVars = supFormat == null ? true : supFormat.supportsLocals; + this.supHasComments = supFormat == null ? true : supFormat.supportsComments; + } + + @Override + public void visitNamespaces(String srcNamespace, List dstNamespaces) throws IOException { + assertTrue(srcNamespace.equals(subHasNamespaces ? supTree.getSrcNamespace() : MappingUtil.NS_SOURCE_FALLBACK)); + this.dstNamespaces = dstNamespaces; + + if (!subHasNamespaces) { + assertTrue(dstNamespaces.size() == 1); + assertTrue(dstNamespaces.get(0).equals(MappingUtil.NS_TARGET_FALLBACK)); + return; + } + + for (int i = 0; i < dstNamespaces.size(); i++) { + String dstNs = dstNamespaces.get(i); + boolean contained = supTree.getDstNamespaces().contains(dstNs); + + if (!supHasNamespaces) { + if (contained) return; + } else { + assertTrue(contained); + } + } + + if (!supHasNamespaces) throw new RuntimeException("SubTree namespace not contained in SupTree"); + } + + @Override + public boolean visitClass(String srcName, String[] dstNames) throws IOException { + ClassMappingView supCls = supTree.getClass(srcName); + Map supDstNamesByNsName = new HashMap<>(); + + if (supCls == null) { + String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; + if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; + throw new RuntimeException("SubTree class not contained in SupTree: " + srcName); + } + + for (int supNs = 0; supNs < supDstNsCount; supNs++) { + supDstNamesByNsName.put(supTree.getNamespaceName(supNs), supCls.getDstName(supNs)); + } + + for (int subNs = 0; subNs < dstNames.length; subNs++) { + String supDstName = supDstNamesByNsName.get(dstNamespaces.get(subNs)); + if (!supHasNamespaces && supDstName == null) continue; + assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); + } + + return true; + } + + @Override + public void visitClassComment(String srcName, String[] dstNames, String comment) throws IOException { + if (!supHasComments) return; + assertEquals(supTree.getClass(srcName).getComment(), comment); + } + + @Override + public boolean visitField(String srcClsName, String srcName, String srcDesc, + String[] dstClsNames, String[] dstNames, String[] dstDescs) throws IOException { + FieldMappingView supFld = supTree.getClass(srcClsName).getField(srcName, srcDesc); + Map supDstDataByNsName = new HashMap<>(); + + if (supFld == null) { + String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; + if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; + throw new RuntimeException("SubTree field not contained in SupTree: " + srcName); + } + + for (int supNs = 0; supNs < supDstNsCount; supNs++) { + supDstDataByNsName.put(supTree.getNamespaceName(supNs), new String[]{supFld.getDstName(supNs), supFld.getDstDesc(supNs)}); + } + + for (int subNs = 0; subNs < dstNames.length; subNs++) { + String[] supDstData = supDstDataByNsName.get(dstNamespaces.get(subNs)); + if (!supHasNamespaces && supDstData == null) continue; + + String supDstName = supDstData[0]; + assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); + + if (!supHasFieldDesc) continue; + String supDstDesc = supDstData[1]; + assertTrue(dstDescs == null || dstDescs[subNs] == null || dstDescs[subNs].equals(supDstDesc)); + } + + return true; + } + + @Override + public void visitFieldComment(String srcClsName, String srcName, String srcDesc, + String[] dstClsNames, String[] dstNames, String[] dstDescs, String comment) throws IOException { + if (!supHasComments) return; + assertEquals(supTree.getClass(srcClsName).getField(srcName, srcDesc).getComment(), comment); + } + + @Override + public boolean visitMethod(String srcClsName, String srcName, String srcDesc, + String[] dstClsNames, String[] dstNames, String[] dstDescs) throws IOException { + MethodMappingView supMth = supTree.getClass(srcClsName).getMethod(srcName, srcDesc); + Map supDstDataByNsName = new HashMap<>(); + + if (supMth == null) { + String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; + if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; + throw new RuntimeException("SubTree method not contained in SupTree: " + srcName); + } + + for (int supNs = 0; supNs < supDstNsCount; supNs++) { + supDstDataByNsName.put(supTree.getNamespaceName(supNs), new String[]{supMth.getDstName(supNs), supMth.getDstDesc(supNs)}); + } + + for (int subNs = 0; subNs < dstNames.length; subNs++) { + String[] supDstData = supDstDataByNsName.get(dstNamespaces.get(subNs)); + if (!supHasNamespaces && supDstData == null) continue; + + String supDstName = supDstData[0]; + assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); + + String supDstDesc = supDstData[1]; + assertTrue(dstDescs == null || dstDescs[subNs] == null || dstDescs[subNs].equals(supDstDesc)); + } + + return true; + } + + @Override + public void visitMethodComment(String srcClsName, String srcName, String srcDesc, + String[] dstClsNames, String[] dstNames, String[] dstDescs, String comment) throws IOException { + if (!supHasComments) return; + assertEquals(supTree.getClass(srcClsName).getMethod(srcName, srcDesc).getComment(), comment); + } + + @Override + public boolean visitMethodArg(String srcClsName, String srcMethodName, String srcMethodDesc, int argPosition, int lvIndex, String srcName, + String[] dstClsNames, String[] dstMethodNames, String[] dstMethodDescs, String[] dstNames) throws IOException { + if (!supHasArgs) return false; + MethodArgMappingView supArg = supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getArg(argPosition, lvIndex, srcName); + Map supDstNamesByNsName = new HashMap<>(); + + if (supArg == null) { + String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; + if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; + throw new RuntimeException("SubTree arg not contained in SupTree: " + srcName); + } + + for (int supNs = 0; supNs < supDstNsCount; supNs++) { + supDstNamesByNsName.put(supTree.getNamespaceName(supNs), supArg.getDstName(supNs)); + } + + for (int subNs = 0; subNs < dstNames.length; subNs++) { + String supDstName = supDstNamesByNsName.get(dstNamespaces.get(subNs)); + if (!supHasNamespaces && supDstName == null) continue; + assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); + } + + return true; + } + + @Override + public void visitMethodArgComment(String srcClsName, String srcMethodName, String srcMethodDesc, int argPosition, int lvIndex, String srcArgName, + String[] dstClsNames, String[] dstMethodNames, String[] dstMethodDescs, String[] dstNames, String comment) throws IOException { + if (!supHasComments) return; + assertEquals(supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getArg(argPosition, lvIndex, srcArgName).getComment(), comment); + } + + @Override + public boolean visitMethodVar(String srcClsName, String srcMethodName, String srcMethodDesc, + int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, String srcName, + String[] dstClsNames, String[] dstMethodNames, String[] dstMethodDescs, String[] dstNames) throws IOException { + if (!supHasVars) return false; + MethodVarMappingView supVar = supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName); + Map supDstNamesByNsName = new HashMap<>(); + + if (supVar == null) { + String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; + if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; + throw new RuntimeException("SubTree var not contained in SupTree: " + srcName); + } + + for (int supNs = 0; supNs < supDstNsCount; supNs++) { + supDstNamesByNsName.put(supTree.getNamespaceName(supNs), supVar.getDstName(supNs)); + } + + for (int subNs = 0; subNs < dstNames.length; subNs++) { + String supDstName = supDstNamesByNsName.get(dstNamespaces.get(subNs)); + if (!supHasNamespaces && supDstName == null) continue; + assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); + } + + return true; + } + + @Override + public void visitMethodVarComment(String srcClsName, String srcMethodName, String srcMethodDesc, + int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, String srcVarName, + String[] dstClsNames, String[] dstMethodNames, String[] dstMethodDescs, String[] dstNames, String comment) throws IOException { + if (!supHasComments) return; + assertEquals(supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcVarName).getComment(), comment); + } + + private final MappingTreeView supTree; + private final int supDstNsCount; + private final boolean subHasNamespaces; + private final boolean supHasNamespaces; + private final boolean supHasFieldDesc; + private final boolean supHasArgs; + private final boolean supHasVars; + private final boolean supHasComments; + private List dstNamespaces; +} + diff --git a/src/test/java/net/fabricmc/mappingio/TestHelper.java b/src/test/java/net/fabricmc/mappingio/TestHelper.java index c563d422..4b1dbabb 100644 --- a/src/test/java/net/fabricmc/mappingio/TestHelper.java +++ b/src/test/java/net/fabricmc/mappingio/TestHelper.java @@ -204,7 +204,7 @@ private static void visitMethod(MemoryMappingTree tree, int... dstNs) { } private static void visitMethodArg(MemoryMappingTree tree, int... dstNs) { - tree.visitMethodArg(counter.getAndIncrement(), counter.getAndIncrement(), nameGen.src(argKind)); + tree.visitMethodArg(nameGen.getCounter().getAndIncrement(), nameGen.getCounter().getAndIncrement(), nameGen.src(argKind)); for (int ns : dstNs) { tree.visitDstName(argKind, ns, nameGen.dst(argKind, ns)); @@ -212,7 +212,8 @@ private static void visitMethodArg(MemoryMappingTree tree, int... dstNs) { } private static void visitMethodVar(MemoryMappingTree tree, int... dstNs) { - tree.visitMethodVar(counter.get(), counter.get(), counter.getAndIncrement(), counter.getAndIncrement(), nameGen.src(varKind)); + tree.visitMethodVar(nameGen.getCounter().get(), nameGen.getCounter().get(), + nameGen.getCounter().getAndIncrement(), nameGen.getCounter().getAndIncrement(), nameGen.src(varKind)); for (int ns : dstNs) { tree.visitDstName(varKind, ns, nameGen.dst(varKind, ns)); @@ -233,6 +234,7 @@ public void reset() { argNum.get().set(0); varNum.get().set(0); nsNum.get().set(0); + counter.get().set(0); } private void resetNsNum() { @@ -300,6 +302,10 @@ public String dst(MappedElementKind kind, int ns) { return sb.toString(); } + public AtomicInteger getCounter() { + return counter.get(); + } + private AtomicInteger getCounter(MappedElementKind kind) { switch (kind) { case CLASS: @@ -348,9 +354,17 @@ private String getPrefix(MappedElementKind kind) { private ThreadLocal argNum = ThreadLocal.withInitial(() -> new AtomicInteger()); private ThreadLocal varNum = ThreadLocal.withInitial(() -> new AtomicInteger()); private ThreadLocal nsNum = ThreadLocal.withInitial(() -> new AtomicInteger()); + private ThreadLocal counter = ThreadLocal.withInitial(() -> new AtomicInteger()); } public static class MappingDirs { + @Nullable + public static MemoryMappingTree getCorrespondingTree(Path dir) { + if (dir.equals(VALID)) return createTestTree(); + if (dir.equals(VALID_WITH_HOLES)) return createTestTreeWithHoles(); + return null; + } + public static final Path DETECTION = getResource("/detection/"); public static final Path VALID = getResource("/read/valid/"); public static final Path VALID_WITH_HOLES = getResource("/read/valid-with-holes/"); @@ -360,7 +374,6 @@ public static class MappingDirs { private static final String mthDesc = "()I"; private static final String comment = "This is a comment"; private static final NameGen nameGen = new NameGen(); - private static final AtomicInteger counter = new AtomicInteger(); private static final MappedElementKind clsKind = MappedElementKind.CLASS; private static final MappedElementKind fldKind = MappedElementKind.FIELD; private static final MappedElementKind mthKind = MappedElementKind.METHOD; diff --git a/src/test/java/net/fabricmc/mappingio/read/ValidContentReadTest.java b/src/test/java/net/fabricmc/mappingio/read/ValidContentReadTest.java index ec8e6889..fe1a4279 100644 --- a/src/test/java/net/fabricmc/mappingio/read/ValidContentReadTest.java +++ b/src/test/java/net/fabricmc/mappingio/read/ValidContentReadTest.java @@ -16,32 +16,16 @@ package net.fabricmc.mappingio.read; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; - -import java.io.IOException; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Objects; - import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import net.fabricmc.mappingio.FlatMappingVisitor; import net.fabricmc.mappingio.MappingReader; -import net.fabricmc.mappingio.MappingUtil; +import net.fabricmc.mappingio.SubsetAssertingVisitor; import net.fabricmc.mappingio.TestHelper; import net.fabricmc.mappingio.adapter.FlatAsRegularMappingVisitor; import net.fabricmc.mappingio.format.MappingFormat; import net.fabricmc.mappingio.tree.MappingTree; -import net.fabricmc.mappingio.tree.MappingTree.ClassMapping; -import net.fabricmc.mappingio.tree.MappingTree.FieldMapping; -import net.fabricmc.mappingio.tree.MappingTree.MethodArgMapping; -import net.fabricmc.mappingio.tree.MappingTree.MethodMapping; -import net.fabricmc.mappingio.tree.MappingTree.MethodVarMapping; import net.fabricmc.mappingio.tree.MemoryMappingTree; import net.fabricmc.mappingio.tree.VisitableMappingTree; @@ -146,210 +130,6 @@ private VisitableMappingTree checkHoles(MappingFormat format) throws Exception { } private void assertSubset(MappingTree subTree, @Nullable MappingFormat subFormat, MappingTree supTree, @Nullable MappingFormat supFormat) throws Exception { - int supDstNsCount = supTree.getMaxNamespaceId(); - boolean subHasNamespaces = subFormat == null ? true : subFormat.hasNamespaces; - boolean supHasNamespaces = supFormat == null ? true : supFormat.hasNamespaces; - boolean supHasFieldDesc = supFormat == null ? true : supFormat.hasFieldDescriptors; - boolean supHasArgs = supFormat == null ? true : supFormat.supportsArgs; - boolean supHasVars = supFormat == null ? true : supFormat.supportsLocals; - boolean supHasComments = supFormat == null ? true : supFormat.supportsComments; - - subTree.accept(new FlatAsRegularMappingVisitor(new FlatMappingVisitor() { - @Override - public void visitNamespaces(String srcNamespace, List dstNamespaces) throws IOException { - assertTrue(srcNamespace.equals(subHasNamespaces ? supTree.getSrcNamespace() : MappingUtil.NS_SOURCE_FALLBACK)); - - if (!subHasNamespaces) { - assertTrue(dstNamespaces.size() == 1); - assertTrue(dstNamespaces.get(0).equals(MappingUtil.NS_TARGET_FALLBACK)); - return; - } - - for (String dstNs : dstNamespaces) { - boolean contained = supTree.getDstNamespaces().contains(dstNs); - - if (!supHasNamespaces) { - if (contained) return; - } else { - assertTrue(contained); - } - } - - if (!supHasNamespaces) throw new RuntimeException("SubTree namespace not contained in SupTree"); - } - - @Override - public boolean visitClass(String srcName, String[] dstNames) throws IOException { - ClassMapping supCls = supTree.getClass(srcName); - Map supDstNamesByNsName = new HashMap<>(); - - if (supCls == null) { - String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; - if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; - throw new RuntimeException("SubTree class not contained in SupTree: " + srcName); - } - - for (int supNs = 0; supNs < supDstNsCount; supNs++) { - supDstNamesByNsName.put(supTree.getNamespaceName(supNs), supCls.getDstName(supNs)); - } - - for (int subNs = 0; subNs < dstNames.length; subNs++) { - String supDstName = supDstNamesByNsName.get(subTree.getNamespaceName(subNs)); - if (!supHasNamespaces && supDstName == null) continue; - assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); - } - - return true; - } - - @Override - public void visitClassComment(String srcName, String[] dstNames, String comment) throws IOException { - if (!supHasComments) return; - assertEquals(supTree.getClass(srcName).getComment(), comment); - } - - @Override - public boolean visitField(String srcClsName, String srcName, String srcDesc, - String[] dstClsNames, String[] dstNames, String[] dstDescs) throws IOException { - FieldMapping supFld = supTree.getClass(srcClsName).getField(srcName, srcDesc); - Map supDstDataByNsName = new HashMap<>(); - - if (supFld == null) { - String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; - if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; - throw new RuntimeException("SubTree field not contained in SupTree: " + srcName); - } - - for (int supNs = 0; supNs < supDstNsCount; supNs++) { - supDstDataByNsName.put(supTree.getNamespaceName(supNs), new String[]{supFld.getDstName(supNs), supFld.getDstDesc(supNs)}); - } - - for (int subNs = 0; subNs < dstNames.length; subNs++) { - String[] supDstData = supDstDataByNsName.get(subTree.getNamespaceName(subNs)); - if (!supHasNamespaces && supDstData == null) continue; - - String supDstName = supDstData[0]; - assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); - - if (!supHasFieldDesc) continue; - String supDstDesc = supDstData[1]; - assertTrue(dstDescs == null || dstDescs[subNs] == null || dstDescs[subNs].equals(supDstDesc)); - } - - return true; - } - - @Override - public void visitFieldComment(String srcClsName, String srcName, String srcDesc, - String[] dstClsNames, String[] dstNames, String[] dstDescs, String comment) throws IOException { - if (!supHasComments) return; - assertEquals(supTree.getClass(srcClsName).getField(srcName, srcDesc).getComment(), comment); - } - - @Override - public boolean visitMethod(String srcClsName, String srcName, String srcDesc, - String[] dstClsNames, String[] dstNames, String[] dstDescs) throws IOException { - MethodMapping supMth = supTree.getClass(srcClsName).getMethod(srcName, srcDesc); - Map supDstDataByNsName = new HashMap<>(); - - if (supMth == null) { - String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; - if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; - throw new RuntimeException("SubTree method not contained in SupTree: " + srcName); - } - - for (int supNs = 0; supNs < supDstNsCount; supNs++) { - supDstDataByNsName.put(supTree.getNamespaceName(supNs), new String[]{supMth.getDstName(supNs), supMth.getDstDesc(supNs)}); - } - - for (int subNs = 0; subNs < dstNames.length; subNs++) { - String[] supDstData = supDstDataByNsName.get(subTree.getNamespaceName(subNs)); - if (!supHasNamespaces && supDstData == null) continue; - - String supDstName = supDstData[0]; - assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); - - String supDstDesc = supDstData[1]; - assertTrue(dstDescs == null || dstDescs[subNs] == null || dstDescs[subNs].equals(supDstDesc)); - } - - return true; - } - - @Override - public void visitMethodComment(String srcClsName, String srcName, String srcDesc, - String[] dstClsNames, String[] dstNames, String[] dstDescs, String comment) throws IOException { - if (!supHasComments) return; - assertEquals(supTree.getClass(srcClsName).getMethod(srcName, srcDesc).getComment(), comment); - } - - @Override - public boolean visitMethodArg(String srcClsName, String srcMethodName, String srcMethodDesc, int argPosition, int lvIndex, String srcName, - String[] dstClsNames, String[] dstMethodNames, String[] dstMethodDescs, String[] dstNames) throws IOException { - if (!supHasArgs) return false; - MethodArgMapping supArg = supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getArg(argPosition, lvIndex, srcName); - Map supDstNamesByNsName = new HashMap<>(); - - if (supArg == null) { - String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; - if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; - throw new RuntimeException("SubTree arg not contained in SupTree: " + srcName); - } - - for (int supNs = 0; supNs < supDstNsCount; supNs++) { - supDstNamesByNsName.put(supTree.getNamespaceName(supNs), supArg.getDstName(supNs)); - } - - for (int subNs = 0; subNs < dstNames.length; subNs++) { - String supDstName = supDstNamesByNsName.get(subTree.getNamespaceName(subNs)); - if (!supHasNamespaces && supDstName == null) continue; - assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); - } - - return true; - } - - @Override - public void visitMethodArgComment(String srcClsName, String srcMethodName, String srcMethodDesc, int argPosition, int lvIndex, String srcArgName, - String[] dstClsNames, String[] dstMethodNames, String[] dstMethodDescs, String[] dstNames, String comment) throws IOException { - if (!supHasComments) return; - assertEquals(supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getArg(argPosition, lvIndex, srcArgName).getComment(), comment); - } - - @Override - public boolean visitMethodVar(String srcClsName, String srcMethodName, String srcMethodDesc, - int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, String srcName, - String[] dstClsNames, String[] dstMethodNames, String[] dstMethodDescs, String[] dstNames) throws IOException { - if (!supHasVars) return false; - MethodVarMapping supVar = supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName); - Map supDstNamesByNsName = new HashMap<>(); - - if (supVar == null) { - String[] tmpDst = supHasNamespaces ? dstNames : new String[]{dstNames[0]}; - if (!Arrays.stream(tmpDst).anyMatch(Objects::nonNull)) return false; - throw new RuntimeException("SubTree var not contained in SupTree: " + srcName); - } - - for (int supNs = 0; supNs < supDstNsCount; supNs++) { - supDstNamesByNsName.put(supTree.getNamespaceName(supNs), supVar.getDstName(supNs)); - } - - for (int subNs = 0; subNs < dstNames.length; subNs++) { - String supDstName = supDstNamesByNsName.get(subTree.getNamespaceName(subNs)); - if (!supHasNamespaces && supDstName == null) continue; - assertTrue(dstNames[subNs] == null || dstNames[subNs].equals(supDstName) || (supDstName == null && dstNames[subNs].equals(srcName))); - } - - return true; - } - - @Override - public void visitMethodVarComment(String srcClsName, String srcMethodName, String srcMethodDesc, - int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, String srcVarName, - String[] dstClsNames, String[] dstMethodNames, String[] dstMethodDescs, String[] dstNames, String comment) throws IOException { - if (!supHasComments) return; - assertEquals(supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcVarName).getComment(), comment); - } - })); + subTree.accept(new FlatAsRegularMappingVisitor(new SubsetAssertingVisitor(supTree, supFormat, subFormat))); } } diff --git a/src/test/java/net/fabricmc/mappingio/visiting/VisitEndTest.java b/src/test/java/net/fabricmc/mappingio/visiting/VisitEndTest.java index e0266869..e2db8117 100644 --- a/src/test/java/net/fabricmc/mappingio/visiting/VisitEndTest.java +++ b/src/test/java/net/fabricmc/mappingio/visiting/VisitEndTest.java @@ -21,152 +21,269 @@ import java.io.IOException; import java.nio.file.Path; import java.util.EnumSet; -import java.util.HashSet; import java.util.List; import java.util.Set; import org.jetbrains.annotations.Nullable; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import net.fabricmc.mappingio.MappedElementKind; import net.fabricmc.mappingio.MappingFlag; import net.fabricmc.mappingio.MappingReader; import net.fabricmc.mappingio.MappingVisitor; +import net.fabricmc.mappingio.SubsetAssertingVisitor; import net.fabricmc.mappingio.TestHelper; +import net.fabricmc.mappingio.adapter.FlatAsRegularMappingVisitor; import net.fabricmc.mappingio.format.MappingFormat; +import net.fabricmc.mappingio.tree.MappingTree; +import net.fabricmc.mappingio.tree.MappingTreeView; +import net.fabricmc.mappingio.tree.MemoryMappingTree; public class VisitEndTest { - private static Set dirs = new HashSet<>(); + @Test + public void enigmaFile() throws Exception { + MappingFormat format = MappingFormat.ENIGMA_FILE; + check(format); + } + + @Test + public void enigmaDirectory() throws Exception { + MappingFormat format = MappingFormat.ENIGMA_DIR; + check(format); + } + + @Test + public void tinyFile() throws Exception { + MappingFormat format = MappingFormat.TINY_FILE; + check(format); + } + + @Test + public void tinyV2File() throws Exception { + MappingFormat format = MappingFormat.TINY_2_FILE; + check(format); + } - @BeforeAll - public static void setup() throws Exception { - dirs.add(TestHelper.MappingDirs.DETECTION); - dirs.add(TestHelper.MappingDirs.VALID); - dirs.add(TestHelper.MappingDirs.VALID_WITH_HOLES); + @Test + public void srgFile() throws Exception { + MappingFormat format = MappingFormat.SRG_FILE; + check(format); } @Test - public void testVisitEnd() throws Exception { - for (MappingFormat format : MappingFormat.values()) { - String filename = TestHelper.getFileName(format); - if (filename == null) continue; + public void xrgFile() throws Exception { + MappingFormat format = MappingFormat.XSRG_FILE; + check(format); + } - for (Path dir : dirs) { - MappingReader.read(dir.resolve(filename), format, new VisitEndTestVisitor(1, true)); - MappingReader.read(dir.resolve(filename), format, new VisitEndTestVisitor(1, false)); + @Test + public void csrgFile() throws Exception { + MappingFormat format = MappingFormat.CSRG_FILE; + check(format); + } - VisitEndTestVisitor threePassVisitor = new VisitEndTestVisitor(2, true); - MappingReader.read(dir.resolve(filename), format, threePassVisitor); - assertTrue(threePassVisitor.finishedVisitPassCount == threePassVisitor.visitPassCountToFinish); + @Test + public void tsrgFile() throws Exception { + MappingFormat format = MappingFormat.TSRG_FILE; + check(format); + } - threePassVisitor = new VisitEndTestVisitor(2, false); + @Test + public void tsrg2File() throws Exception { + MappingFormat format = MappingFormat.TSRG_2_FILE; + check(format); + } - try { - MappingReader.read(dir.resolve(filename), format, threePassVisitor); - } catch (Exception e) { - continue; // Reader doesn't support multiple passes without NEEDS_MULTIPLE_PASSES - } + @Test + public void proguardFile() throws Exception { + MappingFormat format = MappingFormat.PROGUARD_FILE; + check(format); + } - // Reader didn't throw an exception, make sure it actually behaved as expected - assertTrue(threePassVisitor.finishedVisitPassCount == threePassVisitor.visitPassCountToFinish); - } + private void check(MappingFormat format) throws Exception { + checkDir(TestHelper.MappingDirs.DETECTION, format); + checkDir(TestHelper.MappingDirs.VALID, format); + checkDir(TestHelper.MappingDirs.VALID_WITH_HOLES, format); + } + + private void checkDir(Path dir, MappingFormat format) throws Exception { + Path path = dir.resolve(TestHelper.getFileName(format)); + MappingTreeView supTree = TestHelper.MappingDirs.getCorrespondingTree(dir); + + checkCompliance(format, path, 1, true, supTree); + checkCompliance(format, path, 1, false, supTree); + + checkCompliance(format, path, 2, true, supTree); + checkCompliance(format, path, 3, true, supTree); + + VisitEndTestVisitor nonFlaggedVisitor; + + try { + nonFlaggedVisitor = checkCompliance(format, path, 2, false, supTree); + } catch (Exception e) { + return; // Reader doesn't support multiple passes without NEEDS_MULTIPLE_PASSES } + + // Reader didn't throw an exception, make sure it actually behaved as expected + assertTrue(nonFlaggedVisitor.finishedVisitPassCount == nonFlaggedVisitor.visitPassCountToFinish); + } + + private VisitEndTestVisitor checkCompliance(MappingFormat format, Path path, int visitPassCountToFinish, boolean setFlag, MappingTreeView supTree) throws Exception { + VisitEndTestVisitor visitor = new VisitEndTestVisitor(visitPassCountToFinish, setFlag, supTree, format); + MappingReader.read(path, format, visitor); + assertTrue(visitor.finishedVisitPassCount == visitPassCountToFinish); + return visitor; } private static class VisitEndTestVisitor implements MappingVisitor { - private VisitEndTestVisitor(int visitPassCountToFinish, boolean setFlag) { + private VisitEndTestVisitor(int visitPassCountToFinish, boolean setFlag, MappingTreeView supTree, MappingFormat subFormat) { this.visitPassCountToFinish = visitPassCountToFinish; this.setFlag = setFlag; + this.supTree = supTree; + this.subFormat = subFormat; + this.tree = new MemoryMappingTree(); + this.oldTrees = new MappingTree[visitPassCountToFinish - 1]; } @Override public Set getFlags() { return setFlag ? EnumSet.of(MappingFlag.NEEDS_MULTIPLE_PASSES) - : MappingVisitor.super.getFlags(); + : MappingFlag.NONE; } @Override public boolean visitHeader() throws IOException { check(); + tree.visitHeader(); return true; } @Override public void visitNamespaces(String srcNamespace, List dstNamespaces) throws IOException { check(); + tree.visitNamespaces(srcNamespace, dstNamespaces); + } + + @Override + public void visitMetadata(String key, @Nullable String value) throws IOException { + check(); + tree.visitMetadata(key, value); } @Override public boolean visitContent() throws IOException { check(); + tree.visitContent(); return true; } @Override public boolean visitClass(String srcName) throws IOException { check(); + tree.visitClass(srcName); return true; } @Override public boolean visitField(String srcName, @Nullable String srcDesc) throws IOException { check(); + tree.visitField(srcName, srcDesc); return true; } @Override public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOException { check(); + tree.visitMethod(srcName, srcDesc); return true; } @Override public boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String srcName) throws IOException { check(); + tree.visitMethodArg(argPosition, lvIndex, srcName); return true; } @Override public boolean visitMethodVar(int lvtRowIndex, int lvIndex, int startOpIdx, int endOpIdx, @Nullable String srcName) throws IOException { check(); + tree.visitMethodVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName); return true; } @Override public void visitDstName(MappedElementKind targetKind, int namespace, String name) throws IOException { check(); + tree.visitDstName(targetKind, namespace, name); } @Override public void visitDstDesc(MappedElementKind targetKind, int namespace, String desc) throws IOException { check(); + tree.visitDstDesc(targetKind, namespace, desc); } @Override public boolean visitElementContent(MappedElementKind targetKind) throws IOException { check(); + tree.visitElementContent(targetKind); return true; } @Override public void visitComment(MappedElementKind targetKind, String comment) throws IOException { check(); + tree.visitComment(targetKind, comment); } @Override public boolean visitEnd() throws IOException { + check(); + tree.visitEnd(); + checkContent(); finishedVisitPassCount++; - return finishedVisitPassCount == visitPassCountToFinish; + + if (finishedVisitPassCount == visitPassCountToFinish) { + return true; + } + + oldTrees[finishedVisitPassCount - 1] = new MemoryMappingTree(tree); + tree = new MemoryMappingTree(); + return false; } private void check() { assertTrue(finishedVisitPassCount < visitPassCountToFinish); } + /** + * Ensures every visit pass contains the same content. + */ + private void checkContent() throws IOException { + MappingTreeView subTree, supTree; + MappingFormat supFormat = null; + + if (finishedVisitPassCount == 0) { + if (this.supTree == null) return; + supTree = this.supTree; + } else { + supTree = oldTrees[finishedVisitPassCount - 1]; + if (finishedVisitPassCount > 1) supFormat = subFormat; + } + + subTree = tree; + subTree.accept(new FlatAsRegularMappingVisitor(new SubsetAssertingVisitor(supTree, supFormat, subFormat))); + supTree.accept(new FlatAsRegularMappingVisitor(new SubsetAssertingVisitor(subTree, subFormat, supFormat))); + } + private final int visitPassCountToFinish; private final boolean setFlag; + private final MappingTreeView supTree; + private final MappingFormat subFormat; + private final MappingTree[] oldTrees; private int finishedVisitPassCount; + private MemoryMappingTree tree; } } diff --git a/src/test/resources/read/valid-with-holes/enigma-dir/class_32.mapping b/src/test/resources/read/valid-with-holes/enigma-dir/class_32.mapping index 22751d60..6a269a1e 100644 --- a/src/test/resources/read/valid-with-holes/enigma-dir/class_32.mapping +++ b/src/test/resources/read/valid-with-holes/enigma-dir/class_32.mapping @@ -18,13 +18,13 @@ CLASS class_32 METHOD method_6 ()I COMMENT This is a comment METHOD method_7 ()I - ARG 5 + ARG 1 + ARG 3 + ARG 5 param3Ns0Rename ARG 7 - ARG 9 param3Ns0Rename - ARG 11 COMMENT This is a comment - ARG 13 param5Ns0Rename + ARG 9 param5Ns0Rename COMMENT This is a comment - ARG 15 + ARG 11 COMMENT This is a comment METHOD method_8 ()I diff --git a/src/test/resources/read/valid-with-holes/enigma.mappings b/src/test/resources/read/valid-with-holes/enigma.mappings index 059a030e..f40fb44f 100644 --- a/src/test/resources/read/valid-with-holes/enigma.mappings +++ b/src/test/resources/read/valid-with-holes/enigma.mappings @@ -58,13 +58,13 @@ CLASS class_32 METHOD method_6 ()I COMMENT This is a comment METHOD method_7 ()I - ARG 5 + ARG 1 + ARG 3 + ARG 5 param3Ns0Rename ARG 7 - ARG 9 param3Ns0Rename - ARG 11 COMMENT This is a comment - ARG 13 param5Ns0Rename + ARG 9 param5Ns0Rename COMMENT This is a comment - ARG 15 + ARG 11 COMMENT This is a comment METHOD method_8 ()I diff --git a/src/test/resources/read/valid-with-holes/tinyV2.tiny b/src/test/resources/read/valid-with-holes/tinyV2.tiny index 19c97d02..bc6c49fe 100644 --- a/src/test/resources/read/valid-with-holes/tinyV2.tiny +++ b/src/test/resources/read/valid-with-holes/tinyV2.tiny @@ -46,22 +46,22 @@ c class_32 m ()I method_6 method6Ns1Rename c This is a comment m ()I method_7 - p 5 param_1 - p 7 param_2 param2Ns1Rename - p 9 param_3 param3Ns0Rename - p 11 param_4 + p 1 param_1 + p 3 param_2 param2Ns1Rename + p 5 param_3 param3Ns0Rename + p 7 param_4 c This is a comment - p 13 param_5 param5Ns0Rename + p 9 param_5 param5Ns0Rename c This is a comment - p 15 param_6 param6Ns1Rename + p 11 param_6 param6Ns1Rename c This is a comment m ()I method_8 - v 16 16 16 var_1 - v 18 18 18 var_2 var2Ns1Rename - v 20 20 20 var_3 var3Ns0Rename - v 22 22 22 var_4 + v 12 12 12 var_1 + v 14 14 14 var_2 var2Ns1Rename + v 16 16 16 var_3 var3Ns0Rename + v 18 18 18 var_4 c This is a comment - v 24 24 24 var_5 var5Ns0Rename + v 20 20 20 var_5 var5Ns0Rename c This is a comment - v 26 26 26 var_6 var6Ns1Rename + v 22 22 22 var_6 var6Ns1Rename c This is a comment diff --git a/src/test/resources/read/valid-with-holes/tsrg2.tsrg b/src/test/resources/read/valid-with-holes/tsrg2.tsrg index c656ad3d..3fd2de0e 100644 --- a/src/test/resources/read/valid-with-holes/tsrg2.tsrg +++ b/src/test/resources/read/valid-with-holes/tsrg2.tsrg @@ -31,10 +31,10 @@ class_32 class_32 class_32 method_5 ()I method5Ns0Rename method_5 method_6 ()I method_6 method6Ns1Rename method_7 ()I method_7 method_7 - 5 param_1 param_1 param_1 - 7 param_2 param_2 param2Ns1Rename - 9 param_3 param3Ns0Rename param_3 - 11 param_4 param_4 param_4 - 13 param_5 param5Ns0Rename param_5 - 15 param_6 param_6 param6Ns1Rename + 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 ()I method_8 method_8 From cf20689412e02622ecb4957a7168d0d3a83e5998 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 18 Nov 2023 17:48:30 +0100 Subject: [PATCH 2/4] Fix NPE in `MemoryMappingTree` --- .../net/fabricmc/mappingio/tree/MemoryMappingTree.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java b/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java index a3515874..2836c320 100644 --- a/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java +++ b/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java @@ -709,6 +709,7 @@ public void visitComment(MappedElementKind targetKind, String comment) { abstract static class Entry> implements ElementMapping { protected Entry(MemoryMappingTree tree, String srcName) { + this.tree = tree; this.srcName = srcName; this.dstNames = new String[tree.dstNamespaces.size()]; } @@ -814,6 +815,7 @@ protected void copyFrom(T o, boolean replace) { // TODO: copy args+vars } + protected final MemoryMappingTree tree; protected String srcName; protected String[] dstNames; protected String comment; @@ -822,15 +824,11 @@ protected void copyFrom(T o, boolean replace) { static final class ClassEntry extends Entry implements ClassMapping { ClassEntry(MemoryMappingTree tree, String srcName) { super(tree, srcName); - - this.tree = tree; } ClassEntry(MemoryMappingTree tree, ClassMapping src, int srcNsEquivalent) { super(tree, src, srcNsEquivalent); - this.tree = tree; - for (FieldMapping field : src.getFields()) { addField(field); } @@ -1125,7 +1123,6 @@ public String toString() { private static final byte FLAG_HAS_ANY_METHOD_DESC = 4; private static final byte FLAG_MISSES_ANY_METHOD_DESC = 8; - protected final MemoryMappingTree tree; private Map fields = null; private Map methods = null; private byte flags; From d0c308aff4e121b6816996b1ec88dfee50ed6791 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 18 Nov 2023 19:27:04 +0100 Subject: [PATCH 3/4] Fix TSRG reader not handling multiple passes correctly --- .../mappingio/format/srg/TsrgFileReader.java | 37 ++++++++----------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java b/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java index f0e32c28..59582b07 100644 --- a/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java @@ -79,32 +79,27 @@ public static void read(Reader r, String sourceNs, String targetNs, MappingVisit } MappingFormat format = MappingFormat.TSRG_FILE; - if (reader.nextCol("tsrg2")) format = MappingFormat.TSRG_2_FILE; - String srcNamespace; - List dstNamespaces; + String srcNamespace = sourceNs; + List dstNamespaces = Collections.singletonList(targetNs); - if (format == MappingFormat.TSRG_2_FILE) { // tsrg2 magic - srcNamespace = reader.nextCol(); - dstNamespaces = new ArrayList<>(); - String dstNamespace; + for (;;) { + if (reader.nextCol("tsrg2")) { // tsrg2 magic + format = MappingFormat.TSRG_2_FILE; + srcNamespace = reader.nextCol(); + dstNamespaces = new ArrayList<>(); + String dstNamespace; + + while ((dstNamespace = reader.nextCol()) != null) { + dstNamespaces.add(dstNamespace); + } - while ((dstNamespace = reader.nextCol()) != null) { - dstNamespaces.add(dstNamespace); + reader.nextLine(0); } - reader.nextLine(0); - } else { - srcNamespace = sourceNs; - dstNamespaces = Collections.singletonList(targetNs); - } - - int dstNsCount = dstNamespaces.size(); - List nameTmp = dstNamespaces.size() > 1 ? new ArrayList<>(dstNamespaces.size() - 1) : null; - - for (;;) { - boolean visitHeader = visitor.visitHeader(); + int dstNsCount = dstNamespaces.size(); + List nameTmp = dstNamespaces.size() > 1 ? new ArrayList<>(dstNamespaces.size() - 1) : null; - if (visitHeader) { + if (visitor.visitHeader()) { visitor.visitNamespaces(srcNamespace, dstNamespaces); } From b720406901d6eea27ba3b602974bbfbf88b48618 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 18 Nov 2023 19:34:04 +0100 Subject: [PATCH 4/4] Add changes to changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35a5ded2..4c18ffd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ All notable changes to this project will be documented in this file.
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 NPE in `MemoryMappingTree` +- Fixed TSRG2 reader not handling multiple passes correctly + ## [0.5.0] - 2023-11-15 - Actually marked `HierarchyInfoProvider` as experimental - Added changelog