From 9fec03dece822fb6a771f20708eb8934758c56e5 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 18 Nov 2023 17:45:17 +0100 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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 From 848eacf4c5763da19c5cf7bdbe61248aae2a650c Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Sat, 18 Nov 2023 20:15:39 +0100 Subject: [PATCH 05/10] Overhaul `ColumnFileReader` --- .../mappingio/format/ColumnFileReader.java | 328 +++++++++++------- .../format/enigma/EnigmaFileReader.java | 2 +- .../mappingio/format/srg/SrgFileReader.java | 2 +- .../mappingio/format/srg/TsrgFileReader.java | 74 ++-- .../format/tiny/Tiny1FileReader.java | 4 +- .../format/tiny/Tiny2FileReader.java | 8 +- 6 files changed, 251 insertions(+), 167 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java b/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java index 17fb1077..6ab12578 100644 --- a/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java @@ -28,8 +28,9 @@ @ApiStatus.Internal public final class ColumnFileReader implements Closeable { - public ColumnFileReader(Reader reader, char columnSeparator) { + public ColumnFileReader(Reader reader, char indentationChar, char columnSeparator) { this.reader = reader; + this.indentationChar = indentationChar; this.columnSeparator = columnSeparator; } @@ -43,45 +44,20 @@ public void close() throws IOException { * *

The reader will point to the next column or end of line if successful, otherwise remains unchanged. * - * @param expect content to expect + * @param expected content to expect * @return true if the column was read and had the expected content, false otherwise * @throws IOException */ - public boolean nextCol(String expect) throws IOException { - if (eol) return false; - - int len = expect.length(); - if (!fillBuffer(len)) return false; - - for (int i = 0; i < len; i++) { - if (buffer[bufferPos + i] != expect.charAt(i)) return false; // read failed, not all of expect available - } - - char trailing = 0; - - if (fillBuffer(len + 1) // not eof - && (trailing = buffer[bufferPos + len]) != columnSeparator // not end of column - && trailing != '\n' // not end of line - && trailing != '\r') { - return false; // read failed, column contains data beyond expect - } - - // successful read - - bufferPos += expect.length(); - - // seek to the start of the next column - if (trailing == columnSeparator) { - bufferPos++; - } else { - eol = true; - } + public boolean nextCol(String expected) throws IOException { + if (read(false, false, true, expected) == noMatch) return false; return true; } /** * Read and consume a column without unescaping. + * + * @return null if already at eol or eof, otherwise the read string (may be empty). */ @Nullable public String nextCol() throws IOException { @@ -89,36 +65,75 @@ public String nextCol() throws IOException { } /** - * Read and consume a column with unescaping. + * Read and consume a column, and unescape it if requested. + * + * @return null if already at eol or eof, otherwise the read string. + * Empty if there was no content or eol was encountered while reading. */ @Nullable - public String nextEscapedCol() throws IOException { - return nextCol(true); + public String nextCol(boolean unescape) throws IOException { + return read(unescape, true, true, null); } /** - * Read and consume a column and unescape it if requested. + * Read a column without consuming, and unescape if requested. + * Since it doesn't consume, it won't (un)mark bof, eol or eof. + * + * @return null if already at eol or eof, otherwise the read string. + * Empty if there was no content or eol was encountered while reading. */ @Nullable - public String nextCol(boolean unescape) throws IOException { - if (eol) return null; + public String peekCol(boolean unescape) throws IOException { + return read(unescape, false, true, null); + } + + /** + * @param unescape Whether to unescape the read string. + * @param consume Whether to advance the bufferPos. + * @param stopAtNextCol Whether to only read one column. + * @param expected If not null, the read string must match this exactly, otherwise we early-exit with {@link #noMatch}. Always consumes if matched. + * + * @return null if already at eol or eof, otherwise the read string. + * Empty if there was no content or eol was encountered while reading. + * If {@code expected} is not null, it will be returned if matched, otherwise {@link #noMatch}. + */ + @Nullable + private String read(boolean unescape, boolean consume, boolean stopAtNextCol, @Nullable String expected) throws IOException { + if (eol) return expected == null ? null : noMatch; + + int expectedLength = expected != null ? expected.length() : -1; + + // Check if the buffer needs to be filled and if we hit eof while doing so + if (expectedLength > 0 && bufferPos + expectedLength >= bufferLimit) { + if (!fillBuffer(expectedLength, !consume, false)) return noMatch; + } int start; - int end = bufferPos; + int end = this.bufferPos; int firstEscaped = -1; + int charsRead = 0; + int modifiedBufferPos = -1; + boolean filled = true; readLoop: for (;;) { while (end < bufferLimit) { char c = buffer[end]; - if (c == columnSeparator || c == '\n' || c == '\r') { // end of the current column + if (expected != null) { + if ((charsRead < expectedLength && c != expected.charAt(charsRead)) + || charsRead > expectedLength) { + return noMatch; + } + } + + if (c == '\n' || c == '\r' || (stopAtNextCol && c == columnSeparator)) { // stop reading start = bufferPos; - bufferPos = end; + modifiedBufferPos = end; // seek to the start of the next column if (c == columnSeparator) { - bufferPos++; - } else { + modifiedBufferPos++; + } else if (consume) { eol = true; } @@ -127,13 +142,14 @@ public String nextCol(boolean unescape) throws IOException { firstEscaped = bufferPos; } + charsRead++; end++; } // buffer ran out, refill int oldStart = bufferPos; - boolean filled = fillBuffer(end - bufferPos + 1); + filled = fillBuffer(end - bufferPos + 1, !consume, consume); int posShift = bufferPos - oldStart; // fillBuffer may compact the data, shifting it to the buffer start assert posShift <= 0; end += posShift; @@ -141,70 +157,68 @@ public String nextCol(boolean unescape) throws IOException { if (!filled) { start = bufferPos; - bufferPos = end; - eol = true; break; } } - int len = end - start; + if (expected != null) { + consume = true; + } + + String ret; - if (len == 0) { - return ""; - } else if (firstEscaped >= 0) { - return Tiny2Util.unescape(String.valueOf(buffer, start, len)); + if (expected != null) { + ret = expected; } else { - return String.valueOf(buffer, start, len); + int len = end - start; + + if (len == 0) { + ret = ""; + } else if (firstEscaped >= 0) { + ret = Tiny2Util.unescape(String.valueOf(buffer, start, len)); + } else { + ret = String.valueOf(buffer, start, len); + } } - } - - /** - * Read and consume all column until eol and unescape if requested. - */ - @Nullable - public String nextCols(boolean unescape) throws IOException { - if (eol) return null; - int end = bufferPos; - int firstEscaped = -1; - boolean filled; + if (consume) { + if (charsRead > 0) bof = false; + if (!filled) eof = eol = true; + if (modifiedBufferPos != -1) bufferPos = modifiedBufferPos; - readLoop: do { - while (end < bufferLimit) { - char c = buffer[end]; + if (eol && !eof) { // manually check for eof + int charsToRead = buffer[bufferPos] == '\r' ? 2 : 1; // 2 for \r\n, 1 for just \n - if (c == '\n' || c == '\r') { // end of the current column - break readLoop; - } else if (unescape && c == '\\' && firstEscaped < 0) { - firstEscaped = bufferPos; + if (end >= bufferLimit - charsToRead) { + fillBuffer(charsToRead, false, true); } - - end++; } + } - // buffer ran out, refill - - int oldStart = bufferPos; - filled = fillBuffer(end - bufferPos + 1); - int posShift = bufferPos - oldStart; // fillBuffer may compact the data, shifting it to the buffer start - assert posShift <= 0; - end += posShift; - if (firstEscaped >= 0) firstEscaped += posShift; - } while (filled); - - int start = bufferPos; - bufferPos = end; - eol = true; + return ret; + } - int len = end - start; + /** + * Read and consume all columns until eol, and unescape if requested. + * + * @return null if already at eol or eof, otherwise the read string. + * Empty if there was no content or eol was encountered while reading. + */ + @Nullable + public String nextCols(boolean unescape) throws IOException { + return read(unescape, true, false, null); + } - if (len == 0) { - return ""; - } else if (firstEscaped >= 0) { - return Tiny2Util.unescape(String.valueOf(buffer, start, len)); - } else { - return String.valueOf(buffer, start, len); - } + /** + * Read all columns until eol without consuming, and unescape if requested. + * Since it doesn't consume, it won't (un)mark bof, eol or eof. + * + * @return null if already at eol or eof, otherwise the read string. + * Empty if there was no content or eol was encountered while reading. + */ + @Nullable + public String peekCols(boolean unescape) throws IOException { + return read(unescape, false, false, null); } /** @@ -221,87 +235,160 @@ public int nextIntCol() throws IOException { } public boolean nextLine(int indent) throws IOException { - fillLopo: do { + fillLoop: do { while (bufferPos < bufferLimit) { char c = buffer[bufferPos]; if (c == '\n') { if (indent == 0) { // skip empty lines if indent is 0 - if (!fillBuffer(2)) break fillLopo; + if (!fillBuffer(2, false, true)) break fillLoop; c = buffer[bufferPos + 1]; if (c == '\n' || c == '\r') { // 2+ consecutive new lines, consume first nl and retry bufferPos++; lineNumber++; + bof = false; continue; } } - if (!fillBuffer(indent + 1)) return false; + if (!fillBuffer(indent + 1, false, true)) return false; for (int i = 1; i <= indent; i++) { - if (buffer[bufferPos + i] != '\t') return false; + if (buffer[bufferPos + i] != indentationChar) return false; } bufferPos += indent + 1; lineNumber++; + bof = false; eol = false; return true; } bufferPos++; + bof = false; } - } while (fillBuffer(1)); + } while (fillBuffer(1, false, true)); return false; } public boolean hasExtraIndents() throws IOException { - return fillBuffer(1) && buffer[bufferPos] == '\t'; + return fillBuffer(1, false, false) && buffer[bufferPos] == indentationChar; } public int getLineNumber() { return lineNumber; } + public boolean isAtBof() { + return bof; + } + public boolean isAtEof() { return eof; } - public void mark() { - if (bufferPos > 0) { + /** + * Marks the present position in the stream. Subsequent calls to + * {@link #reset()} will reposition the stream to this point. + * In comparison to {@link java.io.Reader#mark(int)} this method stacks, + * so don't forget to call {@link #discardMark()} if you don't need the mark anymore. + * + * @return the mark index (starting at 1) + */ + public int mark() { + if (markIdx == 0 && bufferPos > 0) { // save memory int available = bufferLimit - bufferPos; System.arraycopy(buffer, bufferPos, buffer, 0, available); bufferPos = 0; bufferLimit = available; - markedLineNumber = lineNumber; - markedEol = eol; - markedEof = eof; } - mark = bufferPos; + if (markIdx == markedBufferPositions.length) { + markedBufferPositions = Arrays.copyOf(markedBufferPositions, markedBufferPositions.length * 2); + markedLineNumbers = Arrays.copyOf(markedLineNumbers, markedLineNumbers.length * 2); + markedBofs = Arrays.copyOf(markedBofs, markedBofs.length * 2); + markedEols = Arrays.copyOf(markedEols, markedEols.length * 2); + markedEofs = Arrays.copyOf(markedEofs, markedEofs.length * 2); + } + + markedBufferPositions[markIdx] = bufferPos; + markedLineNumbers[markIdx] = lineNumber; + markedBofs[markIdx] = bof; + markedEols[markIdx] = eol; + markedEofs[markIdx] = eof; + + return ++markIdx; + } + + /** + * Discard the last mark. + */ + public void discardMark() { + discardMark(markIdx); } - public void reset() { - if (mark < 0) throw new IllegalStateException("not marked"); + /** + * Discard the mark at specified index and all above, if present. + */ + private void discardMark(int index) { + if (markIdx == 0) throw new IllegalStateException("no mark to discard"); + if (index < 1 || index > markIdx) throw new IllegalStateException("index out of bounds"); + + for (int i = markIdx; i >= index; i--) { + markedBufferPositions[i-1] = 0; + markedLineNumbers[i-1] = 0; + } + + markIdx = index - 1; + } + + /** + * Reset to last mark. The marked data isn't discarded, so can be called multiple times. + * If you want to reset to an older mark, use {@link #reset(int)}. + * + * @return The index of the mark that was reset to. + */ + public int reset() { + reset(markIdx); + return markIdx; + } + + /** + * Reset to the mark with the specified index. + * Unless reset to {@code 0}, the marked data isn't discarded afterwards, + * so can be called multiple times. + * Use negative indices to reset to a mark relative to the current one. + */ + public void reset(int indexToResetTo) { + if (markIdx == 0) throw new IllegalStateException("no mark to reset to"); + if (indexToResetTo < -markIdx || indexToResetTo > markIdx) throw new IllegalStateException("index out of bounds"); + + if (indexToResetTo < 0) indexToResetTo += markIdx; + int arrayIdx = indexToResetTo == 0 ? indexToResetTo : indexToResetTo - 1; + + bufferPos = markedBufferPositions[arrayIdx]; + lineNumber = markedLineNumbers[arrayIdx]; + bof = markedBofs[arrayIdx]; + eol = markedEols[arrayIdx]; + eof = markedEofs[arrayIdx]; - bufferPos = mark; - lineNumber = markedLineNumber; - eol = markedEol; - eof = markedEof; + if (indexToResetTo == 0) discardMark(1); + markIdx = indexToResetTo; } - private boolean fillBuffer(int count) throws IOException { + private boolean fillBuffer(int count, boolean preventCompaction, boolean markEof) throws IOException { int available = bufferLimit - bufferPos; int req = count - available; if (req <= 0) return true; if (bufferPos + count > buffer.length) { // not enough remaining buffer space - if (mark >= 0) { // marked for rewind -> grow + if (markIdx > 0 || preventCompaction) { // can't compact -> grow buffer = Arrays.copyOf(buffer, Math.max(bufferPos + count, buffer.length * 2)); - } else { // not marked, compact and grow as needed + } else { // compact and grow as needed if (count > buffer.length) { // too small for compacting to suffice -> grow and compact char[] newBuffer = new char[Math.max(count, buffer.length * 2)]; System.arraycopy(buffer, bufferPos, newBuffer, 0, available); @@ -321,7 +408,7 @@ private boolean fillBuffer(int count) throws IOException { int read = reader.read(buffer, bufferLimit, buffer.length - bufferLimit); if (read < 0) { - eof = eol = true; + if (markEof) eof = eol = true; return false; } @@ -331,16 +418,21 @@ private boolean fillBuffer(int count) throws IOException { return true; } + private static final String noMatch = new String(); private final Reader reader; + private final char indentationChar; private final char columnSeparator; private char[] buffer = new char[4096 * 4]; private int bufferPos; private int bufferLimit; - private int mark = -1; private int lineNumber = 1; + private boolean bof = true; private boolean eol; // tracks whether the last column has been read, otherwise ambiguous if the last col is empty private boolean eof; - private int markedLineNumber; - private boolean markedEol; - private boolean markedEof; + private int markIdx = 0; // 0 means no mark + private int[] markedBufferPositions = new int[3]; + private int[] markedLineNumbers = new int[3]; + private boolean[] markedBofs = new boolean[3]; + private boolean[] markedEols = new boolean[3]; + private boolean[] markedEofs = new boolean[3]; } diff --git a/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java b/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java index cafb1811..409bebac 100644 --- a/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java @@ -38,7 +38,7 @@ public static void read(Reader reader, MappingVisitor visitor) throws IOExceptio } public static void read(Reader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException { - read(new ColumnFileReader(reader, ' '), sourceNs, targetNs, visitor); + read(new ColumnFileReader(reader, '\t', ' '), sourceNs, targetNs, visitor); } public static void read(ColumnFileReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException { diff --git a/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java b/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java index 8fe35146..d79cf4a9 100644 --- a/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java @@ -39,7 +39,7 @@ public static void read(Reader reader, MappingVisitor visitor) throws IOExceptio } public static void read(Reader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException { - read(new ColumnFileReader(reader, ' '), sourceNs, targetNs, visitor); + read(new ColumnFileReader(reader, '\t', ' '), sourceNs, targetNs, visitor); } private static void read(ColumnFileReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException { 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 59582b07..b25ad7ae 100644 --- a/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileReader.java @@ -16,7 +16,6 @@ package net.fabricmc.mappingio.format.srg; -import java.io.CharArrayReader; import java.io.IOException; import java.io.Reader; import java.util.ArrayList; @@ -36,7 +35,7 @@ private TsrgFileReader() { } public static List getNamespaces(Reader reader) throws IOException { - return getNamespaces(new ColumnFileReader(reader, ' ')); + return getNamespaces(new ColumnFileReader(reader, '\t', ' ')); } private static List getNamespaces(ColumnFileReader reader) throws IOException { @@ -58,47 +57,38 @@ public static void read(Reader reader, MappingVisitor visitor) throws IOExceptio read(reader, MappingUtil.NS_SOURCE_FALLBACK, MappingUtil.NS_TARGET_FALLBACK, visitor); } - public static void read(Reader r, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException { - ColumnFileReader reader; - CharArrayReader parentReader = null; + public static void read(Reader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException { + read(new ColumnFileReader(reader, '\t', ' '), sourceNs, targetNs, visitor); + } - if (visitor.getFlags().contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) { - char[] buffer = new char[100_000]; - int pos = 0; - int len; + public static void read(ColumnFileReader reader, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException { + MappingFormat format = reader.nextCol("tsrg2") ? format = MappingFormat.TSRG_2_FILE : MappingFormat.TSRG_FILE; + String srcNamespace; + List dstNamespaces; - while ((len = r.read(buffer, pos, buffer.length - pos)) >= 0) { - pos += len; - if (pos == buffer.length) buffer = Arrays.copyOf(buffer, buffer.length * 2); + if (format == MappingFormat.TSRG_2_FILE) { + srcNamespace = reader.nextCol(); + dstNamespaces = new ArrayList<>(); + String dstNamespace; + + while ((dstNamespace = reader.nextCol()) != null) { + dstNamespaces.add(dstNamespace); } - parentReader = new CharArrayReader(buffer, 0, pos); - reader = new ColumnFileReader(parentReader, ' '); + reader.nextLine(0); } else { - reader = new ColumnFileReader(r, ' '); + srcNamespace = sourceNs; + dstNamespaces = Collections.singletonList(targetNs); } - MappingFormat format = MappingFormat.TSRG_FILE; - String srcNamespace = sourceNs; - List dstNamespaces = Collections.singletonList(targetNs); - - 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); - } - - reader.nextLine(0); - } + if (visitor.getFlags().contains(MappingFlag.NEEDS_MULTIPLE_PASSES)) { + reader.mark(); + } - int dstNsCount = dstNamespaces.size(); - List nameTmp = dstNamespaces.size() > 1 ? new ArrayList<>(dstNamespaces.size() - 1) : null; + int dstNsCount = dstNamespaces.size(); + List nameTmp = dstNamespaces.size() > 1 ? new ArrayList<>(dstNamespaces.size() - 1) : null; + for (;;) { if (visitor.visitHeader()) { visitor.visitNamespaces(srcNamespace, dstNamespaces); } @@ -111,8 +101,14 @@ public static void read(Reader r, String sourceNs, String targetNs, MappingVisit if (reader.hasExtraIndents()) continue; reader.mark(); String line = reader.nextCols(false); - if (line == null && reader.isAtEof()) continue; + + if ((line == null || line.isEmpty()) && reader.isAtEof()) { + reader.discardMark(); + continue; + } + reader.reset(); + reader.discardMark(); String[] parts = line.split("((?<= )|(?= ))"); // Split on spaces, but keep them if (format != MappingFormat.TSRG_2_FILE && parts.length >= 4 && !parts[3].startsWith("#")) { // CSRG @@ -174,12 +170,8 @@ public static void read(Reader r, String sourceNs, String targetNs, MappingVisit if (visitor.visitEnd()) break; - if (parentReader == null) { - throw new IllegalStateException("repeated visitation requested without NEEDS_MULTIPLE_PASSES"); - } else { - parentReader.reset(); - reader = new ColumnFileReader(parentReader, ' '); - } + int markIdx = reader.reset(); + assert markIdx == 1; } } diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java index d0ddcc36..f6addc8f 100644 --- a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java @@ -34,7 +34,7 @@ private Tiny1FileReader() { } public static List getNamespaces(Reader reader) throws IOException { - return getNamespaces(new ColumnFileReader(reader, '\t')); + return getNamespaces(new ColumnFileReader(reader, '\t', '\t')); } private static List getNamespaces(ColumnFileReader reader) throws IOException { @@ -53,7 +53,7 @@ private static List getNamespaces(ColumnFileReader reader) throws IOExce } public static void read(Reader reader, MappingVisitor visitor) throws IOException { - read(new ColumnFileReader(reader, '\t'), visitor); + read(new ColumnFileReader(reader, '\t', '\t'), visitor); } private static void read(ColumnFileReader reader, MappingVisitor visitor) throws IOException { diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java index 6381d80f..0f6bfcf1 100644 --- a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java @@ -31,7 +31,7 @@ private Tiny2FileReader() { } public static List getNamespaces(Reader reader) throws IOException { - return getNamespaces(new ColumnFileReader(reader, '\t')); + return getNamespaces(new ColumnFileReader(reader, '\t', '\t')); } private static List getNamespaces(ColumnFileReader reader) throws IOException { @@ -52,7 +52,7 @@ private static List getNamespaces(ColumnFileReader reader) throws IOExce } public static void read(Reader reader, MappingVisitor visitor) throws IOException { - read(new ColumnFileReader(reader, '\t'), visitor); + read(new ColumnFileReader(reader, '\t', '\t'), visitor); } private static void read(ColumnFileReader reader, MappingVisitor visitor) throws IOException { @@ -95,7 +95,7 @@ private static void read(ColumnFileReader reader, MappingVisitor visitor) throws } else { String key = reader.nextCol(); if (key == null) throw new IOException("missing property key in line "+reader.getLineNumber()); - String value = reader.nextEscapedCol(); // may be missing -> null + String value = reader.nextCol(true); // may be missing -> null if (key.equals(Tiny2Util.escapedNamesProperty)) { escapeNames = true; @@ -201,7 +201,7 @@ private static void readElement(ColumnFileReader reader, MappedElementKind kind, } private static void readComment(ColumnFileReader reader, MappedElementKind subjectKind, MappingVisitor visitor) throws IOException { - String comment = reader.nextEscapedCol(); + String comment = reader.nextCol(true); if (comment == null) throw new IOException("missing comment in line "+reader.getLineNumber()); visitor.visitComment(subjectKind, comment); From 22a4f206fed8f9c72e9d907d593d96e5e7d99660 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Tue, 21 Nov 2023 15:30:55 +0100 Subject: [PATCH 06/10] Return null also when freshly encountering EOL as first char Achieved in part by not always seeking to the start of the next column --- .../mappingio/format/ColumnFileReader.java | 60 +++++++++++-------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java b/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java index 6ab12578..d3151e0a 100644 --- a/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java @@ -57,7 +57,7 @@ public boolean nextCol(String expected) throws IOException { /** * Read and consume a column without unescaping. * - * @return null if already at eol or eof, otherwise the read string (may be empty). + * @return null if nothing has been read (first char was eol), otherwise the read string (may be empty). */ @Nullable public String nextCol() throws IOException { @@ -67,8 +67,7 @@ public String nextCol() throws IOException { /** * Read and consume a column, and unescape it if requested. * - * @return null if already at eol or eof, otherwise the read string. - * Empty if there was no content or eol was encountered while reading. + * @return null if nothing has been read (first char was eol), otherwise the read string (may be empty). */ @Nullable public String nextCol(boolean unescape) throws IOException { @@ -79,8 +78,7 @@ public String nextCol(boolean unescape) throws IOException { * Read a column without consuming, and unescape if requested. * Since it doesn't consume, it won't (un)mark bof, eol or eof. * - * @return null if already at eol or eof, otherwise the read string. - * Empty if there was no content or eol was encountered while reading. + * @return null if nothing has been read (first char was eol), otherwise the read string (may be empty). */ @Nullable public String peekCol(boolean unescape) throws IOException { @@ -93,8 +91,7 @@ public String peekCol(boolean unescape) throws IOException { * @param stopAtNextCol Whether to only read one column. * @param expected If not null, the read string must match this exactly, otherwise we early-exit with {@link #noMatch}. Always consumes if matched. * - * @return null if already at eol or eof, otherwise the read string. - * Empty if there was no content or eol was encountered while reading. + * @return null if nothing has been read (first char was eol), otherwise the read string (may be empty). * If {@code expected} is not null, it will be returned if matched, otherwise {@link #noMatch}. */ @Nullable @@ -111,29 +108,37 @@ private String read(boolean unescape, boolean consume, boolean stopAtNextCol, @N int start; int end = this.bufferPos; int firstEscaped = -1; - int charsRead = 0; + int contentCharsRead = 0; int modifiedBufferPos = -1; + int startOffset = 0; + boolean readAnything = false; boolean filled = true; readLoop: for (;;) { while (end < bufferLimit) { char c = buffer[end]; + boolean isColumnSeparator = (c == columnSeparator); - if (expected != null) { - if ((charsRead < expectedLength && c != expected.charAt(charsRead)) - || charsRead > expectedLength) { + // skip leading column separator + if (isColumnSeparator && !readAnything) { + startOffset = 1; + contentCharsRead = -1; + } + + readAnything = true; + + if (expected != null && contentCharsRead > -1) { + if ((contentCharsRead < expectedLength && c != expected.charAt(contentCharsRead)) + || contentCharsRead > expectedLength) { return noMatch; } } - if (c == '\n' || c == '\r' || (stopAtNextCol && c == columnSeparator)) { // stop reading - start = bufferPos; + if (c == '\n' || c == '\r' || (isColumnSeparator && stopAtNextCol && contentCharsRead > -1)) { // stop reading + start = bufferPos + startOffset; modifiedBufferPos = end; - // seek to the start of the next column - if (c == columnSeparator) { - modifiedBufferPos++; - } else if (consume) { + if (!isColumnSeparator && consume) { eol = true; } @@ -142,7 +147,7 @@ private String read(boolean unescape, boolean consume, boolean stopAtNextCol, @N firstEscaped = bufferPos; } - charsRead++; + contentCharsRead++; end++; } @@ -173,7 +178,7 @@ private String read(boolean unescape, boolean consume, boolean stopAtNextCol, @N int len = end - start; if (len == 0) { - ret = ""; + ret = readAnything ? "" : null; } else if (firstEscaped >= 0) { ret = Tiny2Util.unescape(String.valueOf(buffer, start, len)); } else { @@ -182,7 +187,7 @@ private String read(boolean unescape, boolean consume, boolean stopAtNextCol, @N } if (consume) { - if (charsRead > 0) bof = false; + if (readAnything) bof = false; if (!filled) eof = eol = true; if (modifiedBufferPos != -1) bufferPos = modifiedBufferPos; @@ -201,8 +206,7 @@ private String read(boolean unescape, boolean consume, boolean stopAtNextCol, @N /** * Read and consume all columns until eol, and unescape if requested. * - * @return null if already at eol or eof, otherwise the read string. - * Empty if there was no content or eol was encountered while reading. + * @return null if nothing has been read (first char was eol), otherwise the read string (may be empty). */ @Nullable public String nextCols(boolean unescape) throws IOException { @@ -213,8 +217,7 @@ public String nextCols(boolean unescape) throws IOException { * Read all columns until eol without consuming, and unescape if requested. * Since it doesn't consume, it won't (un)mark bof, eol or eof. * - * @return null if already at eol or eof, otherwise the read string. - * Empty if there was no content or eol was encountered while reading. + * @return null if nothing has been read (first char was eol), otherwise the read string (may be empty). */ @Nullable public String peekCols(boolean unescape) throws IOException { @@ -223,6 +226,8 @@ public String peekCols(boolean unescape) throws IOException { /** * Read and consume a column and convert it to integer. + * + * @return -1 if nothing has been read (first char was eol), otherwise the number present. */ public int nextIntCol() throws IOException { String str = nextCol(false); @@ -283,6 +288,13 @@ public int getLineNumber() { return lineNumber; } + /** + * Whether or not EOL has been encountered in the current line yet. + */ + public boolean isAtEol() { + return eol; + } + public boolean isAtBof() { return bof; } From 47dd16f948806507f207e7a75157d421cec22619 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Tue, 21 Nov 2023 16:12:45 +0100 Subject: [PATCH 07/10] Fix issue with start offset --- .../fabricmc/mappingio/format/ColumnFileReader.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java b/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java index d3151e0a..3c4c44e0 100644 --- a/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/ColumnFileReader.java @@ -135,10 +135,10 @@ private String read(boolean unescape, boolean consume, boolean stopAtNextCol, @N } if (c == '\n' || c == '\r' || (isColumnSeparator && stopAtNextCol && contentCharsRead > -1)) { // stop reading - start = bufferPos + startOffset; + start = bufferPos; modifiedBufferPos = end; - if (!isColumnSeparator && consume) { + if (!isColumnSeparator && (consume || expected != null)) { eol = true; } @@ -166,13 +166,11 @@ private String read(boolean unescape, boolean consume, boolean stopAtNextCol, @N } } - if (expected != null) { - consume = true; - } - + start += startOffset; String ret; if (expected != null) { + consume = true; ret = expected; } else { int len = end - start; From c81258d4b9e71cab81f78b8f346338fead72e555 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Tue, 21 Nov 2023 21:18:23 +0100 Subject: [PATCH 08/10] Enhance Enigma and Tiny v1 reader validation --- .../format/enigma/EnigmaFileReader.java | 50 ++++++- .../format/tiny/Tiny1FileReader.java | 52 ++++--- .../read/InvalidContentReadTest.java | 138 ++++++++++++++++++ 3 files changed, 218 insertions(+), 22 deletions(-) create mode 100644 src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java diff --git a/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java b/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java index 409bebac..f6d5180b 100644 --- a/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaFileReader.java @@ -61,6 +61,8 @@ public static void read(ColumnFileReader reader, String sourceNs, String targetN do { if (reader.nextCol("CLASS")) { // class: CLASS [] readClass(reader, 0, null, null, commentSb, finalVisitor); + } else { + invalidLine(reader, "CLASS"); } } while (reader.nextLine(0)); } @@ -89,6 +91,8 @@ private static void readClass(ColumnFileReader reader, int indent, String outerS String dstInnerName = reader.nextCol(); String dstName = dstInnerName; + if (dstName != null && dstName.isEmpty()) throw new IOException("empty class-name-b in line "+reader.getLineNumber()); // null is allowed, empty is not + // merge with outer name if available if (outerDstClass != null || dstInnerName != null && outerSrcClass != null) { @@ -98,6 +102,7 @@ private static void readClass(ColumnFileReader reader, int indent, String outerS dstName = String.format("%s$%s", outerDstClass, dstInnerName); } + checkEol(reader); readClassBody(reader, indent, srcName, dstName, commentSb, visitor); } @@ -136,16 +141,25 @@ private static void readClassBody(ColumnFileReader reader, int indent, String sr dstName = null; srcDesc = dstNameOrSrcDesc; } else { + if (srcDesc.isEmpty()) throw new IOException("missing member-desc-a in line "+reader.getLineNumber()); dstName = dstNameOrSrcDesc; } - if (isMethod && visitor.visitMethod(srcName, srcDesc)) { - if (dstName != null && !dstName.isEmpty()) visitor.visitDstName(MappedElementKind.METHOD, 0, dstName); - readMethod(reader, indent, commentSb, visitor); - } else if (!isMethod && visitor.visitField(srcName, srcDesc)) { + if (isMethod) { + if (!srcDesc.startsWith("(")) throw new IOException("invalid method-desc-a in line "+reader.getLineNumber()); + + if (visitor.visitMethod(srcName, srcDesc)) { + if (dstName != null && !dstName.isEmpty()) visitor.visitDstName(MappedElementKind.METHOD, 0, dstName); + checkEol(reader); + readMethod(reader, indent, commentSb, visitor); + } + } else if (visitor.visitField(srcName, srcDesc)) { if (dstName != null && !dstName.isEmpty()) visitor.visitDstName(MappedElementKind.FIELD, 0, dstName); + checkEol(reader); readElement(reader, MappedElementKind.FIELD, indent, commentSb, visitor); } + } else { + invalidLine(reader, "CLASS, METHOD, FIELD or COMMENT"); } } @@ -199,8 +213,12 @@ private static void readMethod(ColumnFileReader reader, int indent, StringBuilde readElement(reader, MappedElementKind.METHOD_ARG, indent, commentSb, visitor); } + } else { + invalidLine(reader, "COMMENT or ARG"); } } + + checkEol(reader); } submitComment(MappedElementKind.METHOD, commentSb, visitor); @@ -212,7 +230,11 @@ private static void readElement(ColumnFileReader reader, MappedElementKind kind, while (reader.nextLine(indent + kind.level + 1)) { if (reader.nextCol("COMMENT")) { // comment: COMMENT readComment(reader, commentSb); + } else { + invalidLine(reader, "COMMENT"); } + + checkEol(reader); } submitComment(kind, commentSb, visitor); @@ -234,4 +256,24 @@ private static void submitComment(MappedElementKind kind, StringBuilder commentS visitor.visitComment(kind, commentSb.toString()); commentSb.setLength(0); } + + private static void invalidLine(ColumnFileReader reader, String expected) throws IOException { + String line = reader.nextCol(); + + if (line != null && line.startsWith(" ")) { + throw new IOException("Found indentation using spaces in line "+reader.getLineNumber()+", expected tab"); + } else if (!reader.isAtBof()) { // empty files are allowed + throw new IOException("invalid line "+reader.getLineNumber()+", expected "+expected); + } + } + + private static void checkEol(ColumnFileReader reader) throws IOException { + if (!reader.isAtEol()) { + String rest = reader.nextCols(false); + + if (rest != null && !rest.trim().startsWith("# ")) { + throw new IOException("line ending expected in line "+reader.getLineNumber()+", found: '"+rest+"'"); + } + } + } } diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java index f6addc8f..1c0b9b37 100644 --- a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java @@ -137,25 +137,35 @@ private static void read(ColumnFileReader reader, MappingVisitor visitor) throws final String prefix = "# INTERMEDIARY-COUNTER "; String[] parts; - if (line.startsWith(prefix) - && (parts = line.substring(prefix.length()).split(" ")).length == 2) { - String property = null; - - switch (parts[0]) { - case "class": - property = nextIntermediaryClassProperty; - break; - case "field": - property = nextIntermediaryFieldProperty; - break; - case "method": - property = nextIntermediaryMethodProperty; - break; - } - - if (property != null) { - visitor.visitMetadata(property, parts[1]); + if (line.startsWith("# ")) { // metadata + if (line.startsWith(prefix.substring(2)) + && (parts = line.substring(prefix.length()).split(" ")).length == 2) { + String property = null; + + switch (parts[0]) { + case "class": + property = nextIntermediaryClassProperty; + break; + case "field": + property = nextIntermediaryFieldProperty; + break; + case "method": + property = nextIntermediaryMethodProperty; + break; + } + + if (property != null) { + visitor.visitMetadata(property, parts[1]); + } } + } else if (line.isEmpty()) { + throw new IOException("Found indentation without content in line "+reader.getLineNumber()); + } else if (line.startsWith(" ")) { + throw new IOException("Found indentation using spaces in line "+reader.getLineNumber()+", expected tab"); + } else if (line.startsWith("CLASS") + || line.startsWith("FIELD") + || line.startsWith("METHOD")) { + throw new IOException("Found invalid character after element kind declaration in line "+reader.getLineNumber()); } } } @@ -178,6 +188,12 @@ private static void readDstNames(ColumnFileReader reader, MappedElementKind subj if (!name.isEmpty()) visitor.visitDstName(subjectKind, dstNs, name); } + + String col; + + if (!reader.isAtEol() && (col = reader.nextCol()) != null && !col.startsWith("#")) { + throw new IOException("Found invalid additional column in line "+reader.getLineNumber()); + } } static final String nextIntermediaryClassProperty = "next-intermediary-class"; diff --git a/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java b/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java new file mode 100644 index 00000000..4ae5ba1e --- /dev/null +++ b/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java @@ -0,0 +1,138 @@ +/* + * 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.read; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.StringReader; + +import org.junit.jupiter.api.Test; + +import net.fabricmc.mappingio.MappedElementKind; +import net.fabricmc.mappingio.MappingReader; +import net.fabricmc.mappingio.NopMappingVisitor; +import net.fabricmc.mappingio.format.MappingFormat; + +public class InvalidContentReadTest { + String tinyHeader = "v1 source target\n"; + + @Test + public void enigmaFile() throws Exception { + MappingFormat format = MappingFormat.ENIGMA_FILE; + + checkThrows(" ", format); + checkThrows(" ", format); + checkThrows(" CLASS", format); + + checkEnigmaLine(MappedElementKind.CLASS); + checkEnigmaLine(MappedElementKind.FIELD); + checkEnigmaLine(MappedElementKind.METHOD); + } + + private void checkEnigmaLine(MappedElementKind kind) throws Exception { + MappingFormat format = MappingFormat.ENIGMA_FILE; + String prefix = (kind == MappedElementKind.CLASS ? "" : " ") + kind.name(); + + // Tabs for separation + checkThrows(prefix + " ", format); + checkThrows(prefix + " src", format); + + // Spaces for separation + prefix += " src"; + String suffix = ""; + + if (kind != MappedElementKind.CLASS) { + prefix = "CLASS src\n" + prefix; + suffix += kind == MappedElementKind.FIELD ? " I" : " ()V"; + } + + check(prefix, format, kind != MappedElementKind.CLASS); + checkWorks(prefix + suffix, format); + + checkThrows(prefix + " ", format); + checkThrows(prefix + " " + suffix, format); + + check(prefix + " dst", format, kind == MappedElementKind.METHOD); // field normally too, but doesn't have descriptor validation yet + checkWorks(prefix + " dst" + suffix, format); + + checkThrows(prefix + " dst ", format); + checkThrows(prefix + " dst " + suffix, format); + + check(prefix + " dst dst2", format, kind != MappedElementKind.FIELD); + checkThrows(prefix + " dst dst2" + suffix, format); + } + + @Test + public void tinyFile() throws Exception { + MappingFormat format = MappingFormat.TINY_FILE; + + checkThrows(" ", format); + checkWorks(tinyHeader, format); + checkThrows(tinyHeader + " ", format); + checkThrows(tinyHeader + " ", format); + + checkTinyLine(MappedElementKind.CLASS); + checkTinyLine(MappedElementKind.FIELD); + checkTinyLine(MappedElementKind.METHOD); + } + + private void checkTinyLine(MappedElementKind kind) throws Exception { + MappingFormat format = MappingFormat.TINY_FILE; + String prefix = tinyHeader + kind.name(); + + // No source/target + checkThrows(prefix, format); + + // Spaces for separation + checkThrows(prefix + " ", format); + checkThrows(prefix + " src", format); + + // Tabs for separation + prefix += " src"; + + if (kind != MappedElementKind.CLASS) { + checkThrows(prefix, format); + + prefix += kind == MappedElementKind.FIELD ? " I" : " ()V"; + checkThrows(prefix, format); + + prefix += " src"; + } + + checkThrows(prefix, format); + checkWorks(prefix + " ", format); + checkWorks(prefix + " dst", format); + checkThrows(prefix + " dst ", format); + checkThrows(prefix + " dst dst2", format); + } + + private void check(String fileContent, MappingFormat format, boolean shouldThrow) throws Exception { + if (shouldThrow) { + checkThrows(fileContent, format); + } else { + checkWorks(fileContent, format); + } + } + + private void checkWorks(String fileContent, MappingFormat format) throws Exception { + MappingReader.read(new StringReader(fileContent), format, new NopMappingVisitor(true)); + } + + private void checkThrows(String fileContent, MappingFormat format) { + assertThrows(Exception.class, () -> checkWorks(fileContent, format)); + } +} From f931df7e2d8ba0b52ec9173f0136b04e50e8a99b Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Wed, 22 Nov 2023 10:13:13 +0100 Subject: [PATCH 09/10] Add Tiny v2 line validation --- .../format/tiny/Tiny2FileReader.java | 22 ++++++++ .../read/InvalidContentReadTest.java | 52 +++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java index 0f6bfcf1..21f70ae4 100644 --- a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java @@ -115,6 +115,8 @@ private static void read(ColumnFileReader reader, MappingVisitor visitor) throws if (visitor.visitClass(srcName)) { readClass(reader, dstNsCount, escapeNames, visitor); } + } else { + unrecognizedLine(reader); } } } @@ -151,6 +153,8 @@ private static void readClass(ColumnFileReader reader, int dstNsCount, boolean e } } else if (reader.nextCol("c")) { // comment: c readComment(reader, MappedElementKind.CLASS, visitor); + } else { + unrecognizedLine(reader); } } } @@ -185,6 +189,8 @@ private static void readMethod(ColumnFileReader reader, int dstNsCount, boolean } } else if (reader.nextCol("c")) { // comment: c readComment(reader, MappedElementKind.METHOD, visitor); + } else { + unrecognizedLine(reader); } } } @@ -196,6 +202,8 @@ private static void readElement(ColumnFileReader reader, MappedElementKind kind, while (reader.nextLine(kind.level + 1)) { if (reader.nextCol("c")) { // comment: c readComment(reader, kind, visitor); + } else { + unrecognizedLine(reader); } } } @@ -214,5 +222,19 @@ private static void readDstNames(ColumnFileReader reader, MappedElementKind subj if (!name.isEmpty()) visitor.visitDstName(subjectKind, dstNs, name); } + + String col; + + if (!reader.isAtEol() && (col = reader.nextCol()) != null && !col.startsWith("#")) { + throw new IOException("Found invalid additional column in line "+reader.getLineNumber()); + } + } + + private static void unrecognizedLine(ColumnFileReader reader) throws IOException { + String col = reader.peekCol(false); + + if (col != null && (col.startsWith(" ") || col.substring(1).startsWith(" "))) { + throw new IOException("Found indentation using spaces in line "+reader.getLineNumber()+", expected tab"); + } } } diff --git a/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java b/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java index 4ae5ba1e..61000af4 100644 --- a/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java +++ b/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java @@ -29,6 +29,7 @@ public class InvalidContentReadTest { String tinyHeader = "v1 source target\n"; + String tiny2Header = "tiny 2 0 source target\n"; @Test public void enigmaFile() throws Exception { @@ -41,6 +42,7 @@ public void enigmaFile() throws Exception { checkEnigmaLine(MappedElementKind.CLASS); checkEnigmaLine(MappedElementKind.FIELD); checkEnigmaLine(MappedElementKind.METHOD); + // TODO: args } private void checkEnigmaLine(MappedElementKind kind) throws Exception { @@ -88,6 +90,7 @@ public void tinyFile() throws Exception { checkTinyLine(MappedElementKind.CLASS); checkTinyLine(MappedElementKind.FIELD); checkTinyLine(MappedElementKind.METHOD); + // TODO: args, vars } private void checkTinyLine(MappedElementKind kind) throws Exception { @@ -120,6 +123,55 @@ private void checkTinyLine(MappedElementKind kind) throws Exception { checkThrows(prefix + " dst dst2", format); } + @Test + public void tinyV2File() throws Exception { + MappingFormat format = MappingFormat.TINY_2_FILE; + + checkThrows(" ", format); + checkWorks(tiny2Header, format); + checkThrows(tiny2Header + " ", format); + checkThrows(tiny2Header + " ", format); + + checkTiny2Line(MappedElementKind.CLASS); + checkTiny2Line(MappedElementKind.FIELD); + checkTiny2Line(MappedElementKind.METHOD); + // TODO: args, vars + } + + private void checkTiny2Line(MappedElementKind kind) throws Exception { + MappingFormat format = MappingFormat.TINY_2_FILE; + String prefix = tiny2Header; + + if (kind == MappedElementKind.CLASS) { + prefix += "c"; + } else { + prefix += "c src \n " + (kind == MappedElementKind.FIELD ? "f" : "m"); + } + + // No source/target + checkThrows(prefix, format); + + // Spaces for separation + checkThrows(prefix + " ", format); + checkThrows(prefix + " src", format); + + // Tabs for separation + if (kind != MappedElementKind.CLASS) { + checkThrows(prefix, format); + + prefix += kind == MappedElementKind.FIELD ? " I" : " ()V"; + checkThrows(prefix, format); + } + + prefix += " src"; + + checkThrows(prefix, format); + checkWorks(prefix + " ", format); + checkWorks(prefix + " dst", format); + checkThrows(prefix + " dst ", format); + checkThrows(prefix + " dst dst2", format); + } + private void check(String fileContent, MappingFormat format, boolean shouldThrow) throws Exception { if (shouldThrow) { checkThrows(fileContent, format); From 158c5cbeb86dec7a3c429ec37414e30b563cdc19 Mon Sep 17 00:00:00 2001 From: NebelNidas Date: Wed, 22 Nov 2023 13:35:12 +0100 Subject: [PATCH 10/10] Add SRG and XSRG line validation --- .../mappingio/format/srg/SrgFileReader.java | 30 +++++- .../read/InvalidContentReadTest.java | 101 +++++++++++++++++- 2 files changed, 125 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java b/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java index d79cf4a9..ea392140 100644 --- a/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/srg/SrgFileReader.java @@ -97,7 +97,15 @@ private static void read(ColumnFileReader reader, String sourceNs, String target cols[i] = reader.nextCol(); } - if (!isMethod && cols[1] != null && cols[2] != null) format = MappingFormat.XSRG_FILE; + if (!isMethod) { + if (cols[1] != null && cols[2] != null) { + format = MappingFormat.XSRG_FILE; + } else if (cols[1] != null || cols[2] != null) { + String line = cols[1] == null ? cols[2] : (cols[2] == null ? cols[1] : cols[1] + cols[2]); + throw new IOException("unexpected content at line ending in line "+reader.getLineNumber()+": '"+line+"'"); + } + } + String srcDesc; String dstName; String dstDesc; @@ -142,6 +150,16 @@ private static void read(ColumnFileReader reader, String sourceNs, String target visitor.visitElementContent(kind); } } + } else { + invalidLine(reader, "'CL:', 'MD:' or 'FD:'"); + } + + if (!reader.isAtEol()) { + String rest = reader.nextCols(false); + + if (rest != null && !rest.trim().startsWith("# ")) { + throw new IOException("line ending expected in line "+reader.getLineNumber()+", found: '"+rest+"'"); + } } } while (reader.nextLine(0)); } @@ -155,4 +173,14 @@ private static void read(ColumnFileReader reader, String sourceNs, String target ((MappingTree) visitor).accept(parentVisitor); } } + + private static void invalidLine(ColumnFileReader reader, String expected) throws IOException { + String line = reader.nextCol(false); + + if (line != null && line.startsWith(" ")) { + throw new IOException("Found indentation using spaces in line "+reader.getLineNumber()+", expected tab"); + } else if (!reader.isAtBof()) { // empty files are allowed + throw new IOException("invalid line "+reader.getLineNumber()+", expected "+expected); + } + } } diff --git a/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java b/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java index 61000af4..e2d250b8 100644 --- a/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java +++ b/src/test/java/net/fabricmc/mappingio/read/InvalidContentReadTest.java @@ -105,18 +105,22 @@ private void checkTinyLine(MappedElementKind kind) throws Exception { checkThrows(prefix + " src", format); // Tabs for separation - prefix += " src"; + prefix += " "; + checkThrows(prefix, format); + prefix += "src"; + checkThrows(prefix, format); if (kind != MappedElementKind.CLASS) { + prefix += " "; checkThrows(prefix, format); - prefix += kind == MappedElementKind.FIELD ? " I" : " ()V"; + prefix += kind == MappedElementKind.FIELD ? "I" : "()V"; checkThrows(prefix, format); prefix += " src"; + checkThrows(prefix, format); } - checkThrows(prefix, format); checkWorks(prefix + " ", format); checkWorks(prefix + " dst", format); checkThrows(prefix + " dst ", format); @@ -158,12 +162,16 @@ private void checkTiny2Line(MappedElementKind kind) throws Exception { // Tabs for separation if (kind != MappedElementKind.CLASS) { checkThrows(prefix, format); + prefix += " "; + checkThrows(prefix, format); - prefix += kind == MappedElementKind.FIELD ? " I" : " ()V"; + prefix += kind == MappedElementKind.FIELD ? "I" : "()V"; checkThrows(prefix, format); } - prefix += " src"; + prefix += " "; + checkThrows(prefix, format); + prefix += "src"; checkThrows(prefix, format); checkWorks(prefix + " ", format); @@ -172,6 +180,89 @@ private void checkTiny2Line(MappedElementKind kind) throws Exception { checkThrows(prefix + " dst dst2", format); } + @Test + public void srgFile() throws Exception { + MappingFormat format = MappingFormat.SRG_FILE; + + checkThrows(" ", format); + checkThrows(" ", format); + + // TODO: packages + checkSrgLine(MappedElementKind.CLASS, format); + checkSrgLine(MappedElementKind.FIELD, format); + checkSrgLine(MappedElementKind.METHOD, format); + } + + @Test + public void xsrgFile() throws Exception { + MappingFormat format = MappingFormat.XSRG_FILE; + + checkThrows(" ", format); + checkThrows(" ", format); + + // TODO: packages + checkSrgLine(MappedElementKind.CLASS, format); + checkSrgLine(MappedElementKind.FIELD, format); + checkSrgLine(MappedElementKind.METHOD, format); + } + + private void checkSrgLine(MappedElementKind kind, MappingFormat format) throws Exception { + String prefix; + + if (kind == MappedElementKind.CLASS) { + prefix = "CL:"; + } else { + prefix = (kind == MappedElementKind.FIELD ? "FD:" : "MD:"); + } + + // No source/target + checkThrows(prefix, format); + + // Tabs for separation + checkThrows(prefix + " ", format); + checkThrows(prefix + " src", format); + checkThrows(prefix + " src dst", format); + + // Spaces for separation + prefix += " "; + checkThrows(prefix, format); + prefix += "src"; + checkThrows(prefix, format); + String suffix = ""; + + if (kind != MappedElementKind.CLASS) { + prefix += "/"; + checkThrows(prefix, format); + + prefix += "src"; + checkThrows(prefix, format); + + if (kind == MappedElementKind.METHOD || format == MappingFormat.XSRG_FILE) { + prefix += " "; + checkThrows(prefix, format); + + prefix += kind == MappedElementKind.FIELD ? "I" : "()V"; + checkThrows(prefix, format); + } + + prefix += " dst/"; + checkThrows(prefix, format); + + if (kind == MappedElementKind.METHOD) { + suffix += " ()V"; + } else if (format == MappingFormat.XSRG_FILE) { + suffix += " I"; + } + } else { + prefix += " "; + } + + checkThrows(prefix + "" + suffix, format); + checkWorks(prefix + "dst" + suffix, format); + checkThrows(prefix + "dst" + suffix + " ", format); + checkThrows(prefix + "dst" + suffix + " dst2", format); + } + private void check(String fileContent, MappingFormat format, boolean shouldThrow) throws Exception { if (shouldThrow) { checkThrows(fileContent, format);