diff --git a/CHANGELOG.md b/CHANGELOG.md index 33ecfcf9..ff8e5db5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Overhauled the internal `ColumnFileReader` to behave more consistently - Made handling of the `NEEDS_MULTIPLE_PASSES` flag more consistent, reducing memory usage in a few cases - Made some internal methods in Enigma and TSRG readers actually private +- Made all writers for formats which can't represent empty destination names skip such elements entirely, unless mapped child elements are present - Added missing `visitElementContent` calls to CSRG and Recaf Simple readers - Added protection to `MemoryMappingTree` guarding against external data modification while a visitation pass is in progress - Clearly defined Tree-API contracts regarding returned collections' mutability diff --git a/build.gradle b/build.gradle index 5930f9c8..cab681b6 100644 --- a/build.gradle +++ b/build.gradle @@ -44,6 +44,7 @@ allprojects { spotless { java { licenseHeaderFile(rootProject.file("HEADER")).yearSeparator(", ") + targetExclude 'src/test/java/net/fabricmc/mappingio/lib/**/*.java' } } @@ -183,8 +184,20 @@ jar { } } +tasks.register("generateTestMappings", JavaExec) { + dependsOn("testClasses") + classpath = sourceSets.test.runtimeClasspath + mainClass = 'net.fabricmc.mappingio.TestFileUpdater' +} + +tasks.register("updateTestMappings", Copy) { + dependsOn("generateTestMappings") + from 'build/resources/test/' + into 'src/test/resources/' +} + // A task to ensure that the version being released has not already been released. -task checkVersion { +tasks.register("checkVersion") { doFirst { def xml = new URL("https://maven.fabricmc.net/net/fabricmc/mapping-io/maven-metadata.xml").text def metadata = new XmlSlurper().parseText(xml) diff --git a/mapping-io-extras/src/test/java/net/fabricmc/mappingio/extras/MappingTreeRemapperTest.java b/mapping-io-extras/src/test/java/net/fabricmc/mappingio/extras/MappingTreeRemapperTest.java index af5107fb..d93b96b4 100644 --- a/mapping-io-extras/src/test/java/net/fabricmc/mappingio/extras/MappingTreeRemapperTest.java +++ b/mapping-io-extras/src/test/java/net/fabricmc/mappingio/extras/MappingTreeRemapperTest.java @@ -19,20 +19,23 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.IOException; + import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.objectweb.asm.Type; import net.fabricmc.mappingio.TestHelper; import net.fabricmc.mappingio.tree.MappingTree; +import net.fabricmc.mappingio.tree.MemoryMappingTree; public class MappingTreeRemapperTest { private static MappingTree mappingTree; private static MappingTreeRemapper remapper; @BeforeAll - public static void setup() { - mappingTree = TestHelper.createTestTree(); + public static void setup() throws IOException { + mappingTree = TestHelper.acceptTestMappings(new MemoryMappingTree()); remapper = new MappingTreeRemapper(mappingTree, "source", "target"); } diff --git a/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaDirReader.java b/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaDirReader.java index a22682ec..ef42bf20 100644 --- a/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaDirReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/enigma/EnigmaDirReader.java @@ -48,6 +48,9 @@ public static void read(Path dir, MappingVisitor visitor) throws IOException { } public static void read(Path dir, String sourceNs, String targetNs, MappingVisitor visitor) throws IOException { + if (!Files.exists(dir)) throw new IOException("Directory does not exist: " + dir); + if (!Files.isDirectory(dir)) throw new IOException("Not a directory: " + dir); + Set flags = visitor.getFlags(); MappingVisitor parentVisitor = null; diff --git a/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapConstants.java b/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapConstants.java new file mode 100644 index 00000000..b9da53eb --- /dev/null +++ b/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapConstants.java @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2024 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.format.intellij; + +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public final class MigrationMapConstants { + private MigrationMapConstants() { + } + + public static final String ORDER_KEY = "migrationmap:order"; + public static final String MISSING_NAME = "Unnamed migration map"; +} diff --git a/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapFileReader.java b/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapFileReader.java index 4968e945..1148c177 100644 --- a/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapFileReader.java +++ b/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapFileReader.java @@ -108,9 +108,15 @@ private static void read0(BufferedReader reader, String sourceNs, String targetN switch (name) { case "name": + case "order": case "description": if (visitHeader) { - // TODO: visit as metadata once https://github.com/FabricMC/mapping-io/pull/29 is merged + String value = xmlReader.getAttributeValue(null, "value"); + + if (name.equals("order")) name = MigrationMapConstants.ORDER_KEY; + if (name.equals("name") && value.equals(MigrationMapConstants.MISSING_NAME)) break; + + visitor.visitMetadata(name, value); } break; diff --git a/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapFileWriter.java b/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapFileWriter.java index 98d91c9d..45ed47ae 100644 --- a/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapFileWriter.java +++ b/src/main/java/net/fabricmc/mappingio/format/intellij/MigrationMapFileWriter.java @@ -46,7 +46,21 @@ public MigrationMapFileWriter(Writer writer) { public void close() throws IOException { try { if (xmlWriter != null) { + if (!wroteName) { + xmlWriter.writeCharacters("\n\t"); + xmlWriter.writeEmptyElement("name"); + xmlWriter.writeAttribute("value", MigrationMapConstants.MISSING_NAME); + } + + if (!wroteOrder) { + xmlWriter.writeCharacters("\n\t"); + xmlWriter.writeEmptyElement("order"); + xmlWriter.writeAttribute("value", "0"); + } + + xmlWriter.writeCharacters("\n"); xmlWriter.writeEndDocument(); + xmlWriter.writeCharacters("\n"); xmlWriter.close(); } } catch (XMLStreamException e) { @@ -61,13 +75,46 @@ public Set getFlags() { return flags; } + @Override + public boolean visitHeader() throws IOException { + assert xmlWriter == null; + + try { + xmlWriter = XMLOutputFactory.newInstance().createXMLStreamWriter(writer); + + xmlWriter.writeStartDocument("UTF-8", "1.0"); + xmlWriter.writeCharacters("\n"); + xmlWriter.writeStartElement("migrationMap"); + } catch (FactoryConfigurationError | XMLStreamException e) { + throw new IOException(e); + } + + return true; + } + @Override public void visitNamespaces(String srcNamespace, List dstNamespaces) throws IOException { } @Override public void visitMetadata(String key, @Nullable String value) throws IOException { - // TODO: Support once https://github.com/FabricMC/mapping-io/pull/29 is merged + try { + switch (key) { + case "name": + wroteName = true; + break; + case MigrationMapConstants.ORDER_KEY: + wroteOrder = true; + key = "order"; + break; + } + + xmlWriter.writeCharacters("\n\t"); + xmlWriter.writeEmptyElement(key); + xmlWriter.writeAttribute("value", value); + } catch (XMLStreamException e) { + throw new IOException(e); + } } @Override @@ -110,23 +157,16 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept if (dstName == null) return false; try { - if (xmlWriter == null) { - xmlWriter = XMLOutputFactory.newInstance().createXMLStreamWriter(writer); - - xmlWriter.writeStartDocument("UTF-8", "1.0"); - xmlWriter.writeStartElement("migrationMap"); - } - - xmlWriter.writeStartElement("entry"); + xmlWriter.writeCharacters("\n\t"); + xmlWriter.writeEmptyElement("entry"); xmlWriter.writeAttribute("oldName", srcName.replace('/', '.')); xmlWriter.writeAttribute("newName", dstName.replace('/', '.')); xmlWriter.writeAttribute("type", "class"); - xmlWriter.writeEndElement(); - - return false; - } catch (XMLStreamException | FactoryConfigurationError e) { + } catch (XMLStreamException e) { throw new IOException(e); } + + return false; } @Override @@ -138,6 +178,8 @@ public void visitComment(MappedElementKind targetKind, String comment) throws IO private final Writer writer; private XMLStreamWriter xmlWriter; + private boolean wroteName; + private boolean wroteOrder; private String srcName; private String dstName; } diff --git a/src/main/java/net/fabricmc/mappingio/format/proguard/ProGuardFileWriter.java b/src/main/java/net/fabricmc/mappingio/format/proguard/ProGuardFileWriter.java index ef024410..cd771bcc 100644 --- a/src/main/java/net/fabricmc/mappingio/format/proguard/ProGuardFileWriter.java +++ b/src/main/java/net/fabricmc/mappingio/format/proguard/ProGuardFileWriter.java @@ -35,9 +35,11 @@ public final class ProGuardFileWriter implements MappingWriter { private final Writer writer; private final String dstNamespaceString; private int dstNamespace = -1; - private String srcName; - private String srcDesc; + private String clsSrcName; + private String memberSrcName; + private String memberSrcDesc; private String dstName; + private boolean classContentVisitPending; /** * Constructs a ProGuard mapping writer that uses @@ -101,23 +103,23 @@ public void visitNamespaces(String srcNamespace, List dstNamespaces) thr @Override public boolean visitClass(String srcName) throws IOException { - this.srcName = srcName; + clsSrcName = srcName; return true; } @Override public boolean visitField(String srcName, @Nullable String srcDesc) throws IOException { - this.srcName = srcName; - this.srcDesc = srcDesc; + memberSrcName = srcName; + memberSrcDesc = srcDesc; return true; } @Override public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOException { - this.srcName = srcName; - this.srcDesc = srcDesc; + memberSrcName = srcName; + memberSrcDesc = srcDesc; return true; } @@ -143,26 +145,41 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam @Override public boolean visitElementContent(MappedElementKind targetKind) throws IOException { - if (dstName == null) dstName = srcName; + if (targetKind == MappedElementKind.CLASS) { + if (dstName == null) { + classContentVisitPending = true; + return true; + } + } else { + if (dstName == null) { + return false; + } else if (classContentVisitPending) { + String memberDstName = dstName; + dstName = clsSrcName; + visitElementContent(MappedElementKind.CLASS); + classContentVisitPending = false; + dstName = memberDstName; + } + } switch (targetKind) { case CLASS: - writer.write(toJavaClassName(srcName)); + writer.write(toJavaClassName(clsSrcName)); dstName = toJavaClassName(dstName) + ":"; break; case FIELD: writeIndent(); - writer.write(toJavaType(srcDesc)); + writer.write(toJavaType(memberSrcDesc)); writer.write(' '); - writer.write(srcName); + writer.write(memberSrcName); break; case METHOD: writeIndent(); - writer.write(toJavaType(srcDesc.substring(srcDesc.indexOf(')', 1) + 1))); + writer.write(toJavaType(memberSrcDesc.substring(memberSrcDesc.indexOf(')', 1) + 1))); writer.write(' '); - writer.write(srcName); + writer.write(memberSrcName); writer.write('('); - List argTypes = extractArgumentTypes(srcDesc); + List argTypes = extractArgumentTypes(memberSrcDesc); for (int i = 0; i < argTypes.size(); i++) { if (i > 0) { @@ -182,8 +199,8 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept writer.write(dstName); writer.write('\n'); - srcName = srcDesc = dstName = null; - return true; + dstName = null; + return targetKind == MappedElementKind.CLASS; } @Override diff --git a/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileWriter.java b/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileWriter.java index 41ac2c79..5acd1884 100644 --- a/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileWriter.java +++ b/src/main/java/net/fabricmc/mappingio/format/srg/TsrgFileWriter.java @@ -74,36 +74,41 @@ public void visitMetadata(String key, @Nullable String value) throws IOException @Override public boolean visitClass(String srcName) throws IOException { - this.srcName = srcName; + clsSrcName = srcName; + hasAnyDstNames = false; return true; } @Override public boolean visitField(String srcName, @Nullable String srcDesc) throws IOException { - this.srcName = srcName; - this.srcDesc = srcDesc; + memberSrcName = srcName; + memberSrcDesc = srcDesc; + hasAnyDstNames = false; return true; } @Override public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOException { - this.srcName = srcName; - this.srcDesc = srcDesc; + memberSrcName = srcName; + memberSrcDesc = srcDesc; + hasAnyDstNames = false; return true; } @Override public boolean visitMethodArg(int argPosition, int lvIndex, @Nullable String srcName) throws IOException { - if (tsrg2) { - this.srcName = srcName; - this.lvIndex = lvIndex; - return true; + if (!tsrg2) { + return false; } - return false; + this.lvIndex = lvIndex; + argSrcName = srcName; + hasAnyDstNames = false; + + return true; } @Override @@ -116,19 +121,56 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam if (!tsrg2 && namespace != 0) return; dstNames[namespace] = name; + hasAnyDstNames |= name != null; } @Override public boolean visitElementContent(MappedElementKind targetKind) throws IOException { + if (classContentVisitPending && targetKind != MappedElementKind.CLASS && hasAnyDstNames) { + String[] memberOrArgDstNames = dstNames.clone(); + Arrays.fill(dstNames, clsSrcName); + visitElementContent(MappedElementKind.CLASS); + classContentVisitPending = false; + dstNames = memberOrArgDstNames; + } + + if (methodContentVisitPending && targetKind == MappedElementKind.METHOD_ARG && hasAnyDstNames) { + String[] argDstNames = dstNames.clone(); + Arrays.fill(dstNames, memberSrcName); + visitElementContent(MappedElementKind.METHOD); + methodContentVisitPending = false; + dstNames = argDstNames; + } + + String srcName = null; + switch (targetKind) { case CLASS: + if (!hasAnyDstNames) { + classContentVisitPending = true; + return true; + } + + srcName = clsSrcName; break; case FIELD: case METHOD: + if (!hasAnyDstNames) { + if (targetKind == MappedElementKind.METHOD) { + methodContentVisitPending = true; + return tsrg2; + } + + return false; + } + + srcName = memberSrcName; writeTab(); break; case METHOD_ARG: assert tsrg2; + if (!hasAnyDstNames) return false; + srcName = argSrcName; writeTab(); writeTab(); write(Integer.toString(lvIndex)); @@ -143,7 +185,7 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept if (targetKind == MappedElementKind.METHOD || (targetKind == MappedElementKind.FIELD && tsrg2)) { writeSpace(); - write(srcDesc); + write(memberSrcDesc); } int dstNsCount = tsrg2 ? dstNames.length : 1; @@ -156,9 +198,7 @@ public boolean visitElementContent(MappedElementKind targetKind) throws IOExcept writeLn(); - srcName = srcDesc = null; Arrays.fill(dstNames, null); - lvIndex = -1; return targetKind == MappedElementKind.CLASS || (tsrg2 && targetKind == MappedElementKind.METHOD); @@ -195,8 +235,13 @@ private void writeLn() throws IOException { private final Writer writer; private final boolean tsrg2; - private String srcName; - private String srcDesc; + private String clsSrcName; + private String memberSrcName; + private String memberSrcDesc; + private String argSrcName; private String[] dstNames; + private boolean hasAnyDstNames; private int lvIndex = -1; + private boolean classContentVisitPending; + private boolean methodContentVisitPending; } diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileWriter.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileWriter.java index 6b049925..2931845c 100644 --- a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileWriter.java +++ b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileWriter.java @@ -29,6 +29,7 @@ import net.fabricmc.mappingio.MappingFlag; import net.fabricmc.mappingio.MappingWriter; import net.fabricmc.mappingio.format.MappingFormat; +import net.fabricmc.mappingio.format.intellij.MigrationMapConstants; /** * {@linkplain MappingFormat#TINY_2_FILE Tiny v2 file} writer. @@ -66,9 +67,13 @@ public void visitNamespaces(String srcNamespace, List dstNamespaces) thr @Override public void visitMetadata(String key, @Nullable String value) throws IOException { - if (key.equals(Tiny2Util.escapedNamesProperty)) { + switch (key) { + case Tiny2Util.escapedNamesProperty: escapeNames = true; wroteEscapedNamesProperty = true; + break; + case MigrationMapConstants.ORDER_KEY: + return; } writeTab(); diff --git a/src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java b/src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java index 878a03e6..daf63d3c 100644 --- a/src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java +++ b/src/test/java/net/fabricmc/mappingio/SubsetAssertingVisitor.java @@ -39,6 +39,9 @@ import net.fabricmc.mappingio.tree.MappingTreeView.MethodMappingView; import net.fabricmc.mappingio.tree.MappingTreeView.MethodVarMappingView; +/** + * A visitor which asserts that the visited mappings are a subset of a superset tree. + */ public class SubsetAssertingVisitor implements FlatMappingVisitor { /** * @param supTree The superset tree. @@ -47,6 +50,7 @@ public class SubsetAssertingVisitor implements FlatMappingVisitor { */ public SubsetAssertingVisitor(MappingTreeView supTree, @Nullable MappingFormat supFormat, @Nullable MappingFormat subFormat) { this.supTree = supTree; + this.subFormat = subFormat; this.supDstNsCount = supTree.getMaxNamespaceId(); this.supFeatures = supFormat == null ? FeatureSetInstantiator.withFullSupport() : supFormat.features(); this.subFeatures = subFormat == null ? FeatureSetInstantiator.withFullSupport() : subFormat.features(); @@ -59,8 +63,12 @@ public void visitNamespaces(String srcNamespace, List dstNamespaces) thr this.subDstNamespaces = dstNamespaces; if (!subFeatures.hasNamespaces()) { - assertEquals(1, dstNamespaces.size(), "Incoming mappings have multiple namespaces despite declaring not to support them"); - assertEquals(MappingUtil.NS_TARGET_FALLBACK, dstNamespaces.get(0), "Incoming mappings don't have default namespace name of non-namespaced formats"); + assertEquals(1, dstNamespaces.size(), "Incoming mappings have multiple namespaces (" + + String.join(", ", dstNamespaces) + + ") despite their supposed originating format (" + + subFormat + + ") declaring not to support them"); + assertEquals(MappingUtil.NS_TARGET_FALLBACK, dstNamespaces.get(0), "Incoming mappings don't have default destination namespace name of non-namespaced formats"); return; } @@ -90,10 +98,10 @@ public boolean visitClass(String srcName, @Nullable String[] dstNames) throws IO boolean supHasDstNames = supFeatures.classes().dstNames() != FeaturePresence.ABSENT; boolean subHasDstNames = subFeatures.classes().dstNames() != FeaturePresence.ABSENT; - if (supCls == null) { // SupTree doesn't have this class, ensure the incoming mappings don't have any data for it + if (supCls == null) { // supTree doesn't have this class, ensure the incoming mappings don't have any data for it if (supHasDstNames && subHasDstNames) { String[] subDstNames = supFeatures.hasNamespaces() || dstNames == null ? dstNames : new String[]{dstNames[subNsIfSupNotNamespaced]}; - assertTrue(isEmpty(subDstNames), "Incoming class not contained in SupTree: " + srcName); + assertTrue(isEmpty(subDstNames), "Incoming class not contained in supTree: " + srcName); } return true; @@ -124,7 +132,9 @@ public boolean visitClass(String srcName, @Nullable String[] dstNames) throws IO public void visitClassComment(String srcName, @Nullable String[] dstNames, String comment) throws IOException { if (!supFeatures.supportsClasses() || supFeatures.elementComments() == ElementCommentSupport.NONE) return; - assertEquals(supTree.getClass(srcName).getComment(), comment, "Incoming class comment not contained in supTree: " + srcName); + ClassMappingView supCls = Objects.requireNonNull(supTree.getClass(srcName), "Incoming class comment's parent class not contained in supTree: " + srcName); + + assertEquals(supCls.getComment(), comment, "Incoming class comment not contained in supTree: " + srcName); } @Override @@ -132,7 +142,10 @@ public boolean visitField(String srcClsName, String srcName, @Nullable String sr @Nullable String[] dstClsNames, @Nullable String[] dstNames, @Nullable String[] dstDescs) throws IOException { if (!supFeatures.supportsFields()) return true; - FieldMappingView supFld = supTree.getClass(srcClsName).getField(srcName, srcDesc); + String subFldId = srcClsName + "#" + srcName + ":" + srcDesc; + ClassMappingView supCls = Objects.requireNonNull(supTree.getClass(srcClsName), "Incoming field's parent class not contained in supTree: " + subFldId); + FieldMappingView supFld = supCls.getField(srcName, srcDesc); + boolean supHasSrcDescs = supFeatures.fields().srcDescs() != FeaturePresence.ABSENT; boolean subHasSrcDescs = subFeatures.fields().srcDescs() != FeaturePresence.ABSENT; boolean supHasDstNames = supFeatures.fields().dstNames() != FeaturePresence.ABSENT; @@ -140,17 +153,18 @@ public boolean visitField(String srcClsName, String srcName, @Nullable String sr boolean supHasDstDescs = supFeatures.fields().dstDescs() != FeaturePresence.ABSENT; boolean subHasDstDescs = subFeatures.fields().dstDescs() != FeaturePresence.ABSENT; - if (supFld == null) { // SupTree doesn't have this field, ensure the incoming mappings don't have any data for it + if (supFld == null) { // supTree doesn't have this field, ensure the incoming mappings don't have any data for it String[] subDstNames = null; String[] subDstDescs = null; if (supHasDstNames && subHasDstNames) subDstNames = supFeatures.hasNamespaces() || dstNames == null ? dstNames : new String[]{dstNames[subNsIfSupNotNamespaced]}; if (supHasDstDescs && subHasDstDescs) subDstDescs = supFeatures.hasNamespaces() || dstDescs == null ? dstDescs : new String[]{dstDescs[subNsIfSupNotNamespaced]}; - assertTrue(isEmpty(subDstNames) && isEmpty(subDstDescs), "Incoming field not contained in SupTree: " + srcName + ", descriptor: " + srcDesc); + assertTrue(isEmpty(subDstNames) && isEmpty(subDstDescs), "Incoming field not contained in supTree: " + subFldId); return true; } + String supFldId = srcClsName + "#" + srcName + ":" + supFld.getSrcDesc(); Map supDstDataByNsName = new HashMap<>(); for (int supNs = 0; supNs < supDstNsCount; supNs++) { @@ -159,7 +173,7 @@ public boolean visitField(String srcClsName, String srcName, @Nullable String sr for (int subNs = 0; subNs < subDstNamespaces.size(); subNs++) { if (supHasSrcDescs && subHasSrcDescs && srcDesc != null) { - assertEquals(supFld.getSrcDesc(), srcDesc, "Incoming field source descriptor differs from supTree"); + assertEquals(supFldId, subFldId, "Incoming field source descriptor differs from supTree"); } String[] supDstData = supDstDataByNsName.get(subDstNamespaces.get(subNs)); @@ -171,7 +185,7 @@ public boolean visitField(String srcClsName, String srcName, @Nullable String sr boolean uncompletedDst = subDstName == null && (supDstName == null || Objects.equals(supDstName, srcName)); if (!uncompletedDst) { - assertEquals(supDstName != null ? supDstName : srcName, subDstName, "Incoming field destination name differs from supTree"); + assertEquals(supDstName != null ? supDstName : srcName, subDstName, "Incoming field (" + subFldId + ") destination name differs from supTree"); } } @@ -181,7 +195,9 @@ public boolean visitField(String srcClsName, String srcName, @Nullable String sr boolean uncompletedDst = subDstDesc == null && (supDstDesc == null || Objects.equals(supDstDesc, srcDesc)); if (!uncompletedDst) { - assertEquals(supDstDesc != null ? supDstDesc : srcDesc, subDstDesc, "Incoming field destination descriptor differs from supTree"); + String subFldDestId = srcClsName + "#" + srcName + ":" + subDstDesc; + String supFldDestId = srcClsName + "#" + srcName + ":" + (supDstDesc != null ? supDstDesc : srcDesc); + assertEquals(supFldDestId, subFldDestId, "Incoming field destination descriptor differs from supTree"); } } } @@ -194,7 +210,11 @@ public void visitFieldComment(String srcClsName, String srcName, @Nullable Strin @Nullable String[] dstClsNames, @Nullable String[] dstNames, @Nullable String[] dstDescs, String comment) throws IOException { if (!supFeatures.supportsFields() || supFeatures.elementComments() == ElementCommentSupport.NONE) return; - assertEquals(supTree.getClass(srcClsName).getField(srcName, srcDesc).getComment(), comment); + String subFldId = srcClsName + "#" + srcName + ":" + srcDesc; + ClassMappingView supCls = Objects.requireNonNull(supTree.getClass(srcClsName), "Incoming field comment's parent class not contained in supTree: " + subFldId); + FieldMappingView supFld = Objects.requireNonNull(supCls.getField(srcName, srcDesc), "Incoming field comment's parent field not contained in supTree: " + subFldId); + + assertEquals(supFld.getComment(), comment); } @Override @@ -202,7 +222,10 @@ public boolean visitMethod(String srcClsName, String srcName, @Nullable String s @Nullable String[] dstClsNames, @Nullable String[] dstNames, @Nullable String[] dstDescs) throws IOException { if (!supFeatures.supportsMethods()) return true; - MethodMappingView supMth = supTree.getClass(srcClsName).getMethod(srcName, srcDesc); + String subMthId = srcClsName + "#" + srcName + srcDesc; + ClassMappingView supCls = Objects.requireNonNull(supTree.getClass(srcClsName), "Incoming method's parent class not contained in supTree: " + subMthId); + MethodMappingView supMth = supCls.getMethod(srcName, srcDesc); + boolean supHasSrcDescs = supFeatures.methods().srcDescs() != FeaturePresence.ABSENT; boolean subHasSrcDescs = subFeatures.methods().srcDescs() != FeaturePresence.ABSENT; boolean supHasDstNames = supFeatures.methods().dstNames() != FeaturePresence.ABSENT; @@ -210,17 +233,18 @@ public boolean visitMethod(String srcClsName, String srcName, @Nullable String s boolean supHasDstDescs = supFeatures.methods().dstDescs() != FeaturePresence.ABSENT; boolean subHasDstDescs = subFeatures.methods().dstDescs() != FeaturePresence.ABSENT; - if (supMth == null) { // SupTree doesn't have this method, ensure the incoming mappings don't have any data for it + if (supMth == null) { // supTree doesn't have this method, ensure the incoming mappings don't have any data for it String[] subDstNames = null; String[] subDstDescs = null; if (supHasDstNames && subHasDstNames) subDstNames = supFeatures.hasNamespaces() || dstNames == null ? dstNames : new String[]{dstNames[subNsIfSupNotNamespaced]}; if (supHasDstDescs && subHasDstDescs) subDstDescs = supFeatures.hasNamespaces() || dstDescs == null ? dstDescs : new String[]{dstDescs[subNsIfSupNotNamespaced]}; - assertTrue(isEmpty(subDstNames) && isEmpty(subDstDescs), "Incoming method not contained in SupTree: " + srcName + ", descriptor: " + srcDesc); + assertTrue(isEmpty(subDstNames) && isEmpty(subDstDescs), "Incoming method not contained in supTree: " + subMthId); return true; } + String supMthId = srcClsName + "#" + srcName + supMth.getSrcDesc(); Map supDstDataByNsName = new HashMap<>(); for (int supNs = 0; supNs < supDstNsCount; supNs++) { @@ -229,7 +253,7 @@ public boolean visitMethod(String srcClsName, String srcName, @Nullable String s for (int subNs = 0; subNs < subDstNamespaces.size(); subNs++) { if (supHasSrcDescs && subHasSrcDescs && srcDesc != null) { - assertEquals(supMth.getSrcDesc(), srcDesc, "Incoming method source descriptor differs from supTree"); + assertEquals(supMthId, subMthId, "Incoming method source descriptor differs from supTree"); } String[] supDstData = supDstDataByNsName.get(subDstNamespaces.get(subNs)); @@ -241,7 +265,7 @@ public boolean visitMethod(String srcClsName, String srcName, @Nullable String s boolean uncompletedDst = subDstName == null && (supDstName == null || Objects.equals(supDstName, srcName)); if (!uncompletedDst) { - assertEquals(supDstName != null ? supDstName : srcName, subDstName, "Incoming method destination name differs from supTree"); + assertEquals(supDstName != null ? supDstName : srcName, subDstName, "Incoming method (" + subMthId + ") destination name differs from supTree"); } } @@ -251,7 +275,10 @@ public boolean visitMethod(String srcClsName, String srcName, @Nullable String s boolean uncompletedDst = subDstDesc == null && (supDstDesc == null || Objects.equals(supDstDesc, srcDesc)); if (!uncompletedDst) { - assertEquals(supDstDesc != null ? supDstDesc : srcDesc, subDstDesc, "Incoming method destination descriptor differs from supTree"); + String subMthDestId = srcClsName + "#" + srcName + subDstDesc; + String supMthDestId = srcClsName + "#" + srcName + (supDstDesc != null ? supDstDesc : srcDesc); + + assertEquals(supMthDestId, subMthDestId, "Incoming method destination descriptor differs from supTree"); } } } @@ -264,7 +291,11 @@ public void visitMethodComment(String srcClsName, String srcName, @Nullable Stri @Nullable String[] dstClsNames, @Nullable String[] dstNames, @Nullable String[] dstDescs, String comment) throws IOException { if (!supFeatures.supportsMethods() || supFeatures.elementComments() == ElementCommentSupport.NONE) return; - assertEquals(supTree.getClass(srcClsName).getMethod(srcName, srcDesc).getComment(), comment); + String subMthId = srcClsName + "#" + srcName + srcDesc; + ClassMappingView supCls = Objects.requireNonNull(supTree.getClass(srcClsName), "Incoming method comment's parent class not contained in supTree: " + subMthId); + MethodMappingView supMth = Objects.requireNonNull(supCls.getMethod(srcName, srcDesc), "Incoming method comment's parent method not contained in supTree: " + subMthId); + + assertEquals(supMth.getComment(), comment); } @Override @@ -272,18 +303,25 @@ public boolean visitMethodArg(String srcClsName, String srcMethodName, @Nullable @Nullable String[] dstClsNames, @Nullable String[] dstMethodNames, @Nullable String[] dstMethodDescs, @Nullable String[] dstNames) throws IOException { if (!supFeatures.supportsArgs()) return true; - MethodArgMappingView supArg = supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getArg(argPosition, lvIndex, srcName); + String subArgId = srcClsName + "#" + srcMethodName + srcMethodDesc + ":" + argPosition + ":" + lvIndex + ":" + srcName; + ClassMappingView supCls = Objects.requireNonNull(supTree.getClass(srcClsName), "Incoming arg's parent class not contained in supTree: " + subArgId); + MethodMappingView supMth = Objects.requireNonNull(supCls.getMethod(srcMethodName, srcMethodDesc), "Incoming arg's parent method not contained in supTree: " + subArgId); + MethodArgMappingView supArg = supMth.getArg(argPosition, lvIndex, srcName); + boolean supHasPositions = supFeatures.args().positions() != FeaturePresence.ABSENT; boolean subHasPositions = subFeatures.args().positions() != FeaturePresence.ABSENT; boolean supHasLvIndices = supFeatures.args().lvIndices() != FeaturePresence.ABSENT; boolean subHasLvIndices = subFeatures.args().lvIndices() != FeaturePresence.ABSENT; + boolean supHasSrcNames = supFeatures.args().srcNames() != FeaturePresence.ABSENT; + boolean subHasSrcNames = subFeatures.args().srcNames() != FeaturePresence.ABSENT; boolean supHasDstNames = supFeatures.args().dstNames() != FeaturePresence.ABSENT; boolean subHasDstNames = subFeatures.args().dstNames() != FeaturePresence.ABSENT; - if (supArg == null) { // SupTree doesn't have this arg, ensure the incoming mappings don't have any data for it + if (supArg == null) { // supTree doesn't have this arg, ensure the incoming mappings don't have any data for it if (supHasDstNames && subHasDstNames) { String[] subDstNames = supFeatures.hasNamespaces() || dstNames == null ? dstNames : new String[]{dstNames[subNsIfSupNotNamespaced]}; - assertTrue(isEmpty(subDstNames), "Incoming arg not contained in SupTree: " + "position: " + argPosition + ", lvIndex: " + lvIndex + ", name: " + srcName); + + assertTrue(isEmpty(subDstNames), "Incoming arg not contained in supTree: " + subArgId); } return true; @@ -297,21 +335,25 @@ public boolean visitMethodArg(String srcClsName, String srcMethodName, @Nullable for (int subNs = 0; subNs < subDstNamespaces.size(); subNs++) { if (supHasPositions && subHasPositions) { - assertEquals(supArg.getArgPosition(), argPosition, "Incoming arg position differs from supTree"); + assertEquals(supArg.getArgPosition(), argPosition, "Incoming arg (" + subArgId + ") argPosition differs from supTree"); } if (supHasLvIndices && subHasLvIndices) { - assertEquals(supArg.getLvIndex(), lvIndex, "Incoming arg local variable index differs from supTree"); + assertEquals(supArg.getLvIndex(), lvIndex, "Incoming arg (" + subArgId + ") lvIndex differs from supTree"); + } + + if (supHasSrcNames && subHasSrcNames) { + assertEquals(supArg.getSrcName(), srcName, "Incoming arg (" + subArgId + ") srcName differs from supTree"); } if (supHasDstNames && subHasDstNames && dstNames != null) { String supDstName = supDstNamesByNsName.get(subDstNamespaces.get(subNs)); - if (supDstName == null && !supFeatures.hasNamespaces()) continue; - String subDstName = dstNames[subNs]; - if (subDstName == null && (supDstName == null || Objects.equals(supDstName, srcName))) continue; // uncompleted dst name + boolean uncompletedDst = subDstName == null && (supDstName == null || Objects.equals(supDstName, srcName)); - assertEquals(supDstName != null ? supDstName : srcName, subDstName, "Incoming arg destination name differs from supTree"); + if (!uncompletedDst) { + assertEquals(supDstName != null ? supDstName : srcName, subDstName, "Incoming arg (" + subArgId + ") destination name differs from supTree"); + } } } @@ -323,7 +365,13 @@ public void visitMethodArgComment(String srcClsName, String srcMethodName, @Null @Nullable String[] dstClsNames, @Nullable String[] dstMethodNames, @Nullable String[] dstMethodDescs, @Nullable String[] dstNames, String comment) throws IOException { if (!supFeatures.supportsArgs() || supFeatures.elementComments() == ElementCommentSupport.NONE) return; - assertEquals(supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getArg(argPosition, lvIndex, srcArgName).getComment(), comment); + String subArgId = srcClsName + "#" + srcMethodName + srcMethodDesc + ":" + argPosition + ":" + lvIndex + ":" + srcArgName; + + ClassMappingView supCls = Objects.requireNonNull(supTree.getClass(srcClsName), "Incoming arg comment's parent class not contained in supTree: " + subArgId); + MethodMappingView supMth = Objects.requireNonNull(supCls.getMethod(srcMethodName, srcMethodDesc), "Incoming arg comment's parent method not contained in supTree: " + subArgId); + MethodArgMappingView supArg = Objects.requireNonNull(supMth.getArg(argPosition, lvIndex, srcArgName), "Incoming arg comment's parent arg not contained in supTree: " + subArgId); + + assertEquals(supArg.getComment(), comment); } @Override @@ -331,7 +379,11 @@ public boolean visitMethodVar(String srcClsName, String srcMethodName, @Nullable @Nullable String[] dstClsNames, @Nullable String[] dstMethodNames, @Nullable String[] dstMethodDescs, @Nullable String[] dstNames) throws IOException { if (!supFeatures.supportsVars()) return true; - MethodVarMappingView supVar = supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName); + String subVarId = srcClsName + "#" + srcMethodName + srcMethodDesc + ":" + lvtRowIndex + ":" + lvIndex + ":" + startOpIdx + ":" + endOpIdx + ":" + srcName; + ClassMappingView supCls = Objects.requireNonNull(supTree.getClass(srcClsName), "Incoming var's parent class not contained in supTree: " + subVarId); + MethodMappingView supMth = Objects.requireNonNull(supCls.getMethod(srcMethodName, srcMethodDesc), "Incoming var's parent method not contained in supTree: " + subVarId); + MethodVarMappingView supVar = Objects.requireNonNull(supMth.getVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcName), "Incoming var not contained in supTree: " + subVarId); + boolean supHasLvIndices = supFeatures.vars().lvIndices() != FeaturePresence.ABSENT; boolean subHasLvIndices = subFeatures.vars().lvIndices() != FeaturePresence.ABSENT; boolean supHasLvtIndices = supFeatures.vars().lvtRowIndices() != FeaturePresence.ABSENT; @@ -340,13 +392,16 @@ public boolean visitMethodVar(String srcClsName, String srcMethodName, @Nullable boolean subHasStartOpIndices = subFeatures.vars().startOpIndices() != FeaturePresence.ABSENT; boolean supHasEndOpIndices = supFeatures.vars().endOpIndices() != FeaturePresence.ABSENT; boolean subHasEndOpIndices = subFeatures.vars().endOpIndices() != FeaturePresence.ABSENT; + boolean supHasSrcNames = supFeatures.vars().srcNames() != FeaturePresence.ABSENT; + boolean subHasSrcNames = subFeatures.vars().srcNames() != FeaturePresence.ABSENT; boolean supHasDstNames = supFeatures.vars().dstNames() != FeaturePresence.ABSENT; boolean subHasDstNames = subFeatures.vars().dstNames() != FeaturePresence.ABSENT; - if (supVar == null) { // SupTree doesn't have this var, ensure the incoming mappings don't have any data for it + if (supVar == null) { // supTree doesn't have this var, ensure the incoming mappings don't have any data for it if (supHasDstNames && subHasDstNames) { String[] subDstNames = supFeatures.hasNamespaces() || dstNames == null ? dstNames : new String[]{dstNames[subNsIfSupNotNamespaced]}; - assertTrue(isEmpty(subDstNames), "Incoming var not contained in SupTree: " + "lvtRowIndex: " + lvtRowIndex + ", lvIndex: " + lvIndex + ", startOpIdx: " + startOpIdx + ", endOpIdx: " + endOpIdx + ", name: " + srcName); + + assertTrue(isEmpty(subDstNames), "Incoming var not contained in supTree: " + subVarId); } return true; @@ -360,29 +415,33 @@ public boolean visitMethodVar(String srcClsName, String srcMethodName, @Nullable for (int subNs = 0; subNs < subDstNamespaces.size(); subNs++) { if (supHasLvIndices && subHasLvIndices) { - assertEquals(supVar.getLvIndex(), lvIndex, "Incoming var lvIndex differs from supTree"); + assertEquals(supVar.getLvIndex(), lvIndex, "Incoming var (" + subVarId + ") lvIndex differs from supTree"); } if (supHasLvtIndices && subHasLvtIndices) { - assertEquals(supVar.getLvtRowIndex(), lvtRowIndex, "Incoming var lvtIndex differs from supTree"); + assertEquals(supVar.getLvtRowIndex(), lvtRowIndex, "Incoming var (" + subVarId + ") lvtRowIndex differs from supTree"); } if (supHasStartOpIndices && subHasStartOpIndices) { - assertEquals(supVar.getStartOpIdx(), startOpIdx, "Incoming var startOpIndex differs from supTree"); + assertEquals(supVar.getStartOpIdx(), startOpIdx, "Incoming var (" + subVarId + ") startOpIndex differs from supTree"); } if (supHasEndOpIndices && subHasEndOpIndices) { - assertEquals(supVar.getEndOpIdx(), endOpIdx, "Incoming var endOpIndex differs from supTree"); + assertEquals(supVar.getEndOpIdx(), endOpIdx, "Incoming var (" + subVarId + ") endOpIndex differs from supTree"); } - String supDstName = supDstNamesByNsName.get(subDstNamespaces.get(subNs)); - if (supDstName == null && !supFeatures.hasNamespaces()) continue; + if (supHasSrcNames && subHasSrcNames) { + assertEquals(supVar.getSrcName(), srcName, "Incoming var (" + subVarId + ") srcName differs from supTree"); + } if (supHasDstNames && subHasDstNames && dstNames != null) { + String supDstName = supDstNamesByNsName.get(subDstNamespaces.get(subNs)); String subDstName = dstNames[subNs]; - if (subDstName == null && (supDstName == null || Objects.equals(supDstName, srcName))) continue; // uncompleted dst name + boolean uncompletedDst = subDstName == null && (supDstName == null || Objects.equals(supDstName, srcName)); - assertEquals(supDstName != null ? supDstName : srcName, subDstName, "Incoming var destination name differs from supTree"); + if (!uncompletedDst) { + assertEquals(supDstName != null ? supDstName : srcName, subDstName, "Incoming var (" + subVarId + ") destination name differs from supTree"); + } } } @@ -394,7 +453,13 @@ public void visitMethodVarComment(String srcClsName, String srcMethodName, @Null @Nullable String[] dstClsNames, @Nullable String[] dstMethodNames, @Nullable String[] dstMethodDescs, @Nullable String[] dstNames, String comment) throws IOException { if (!supFeatures.supportsVars() || supFeatures.elementComments() == ElementCommentSupport.NONE) return; - assertEquals(supTree.getClass(srcClsName).getMethod(srcMethodName, srcMethodDesc).getVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcVarName).getComment(), comment); + String subVarId = srcClsName + "#" + srcMethodName + srcMethodDesc + ":" + lvtRowIndex + ":" + lvIndex + ":" + startOpIdx + ":" + endOpIdx + ":" + srcVarName; + + ClassMappingView supCls = Objects.requireNonNull(supTree.getClass(srcClsName), "Incoming var comment's parent class not contained in supTree: " + subVarId); + MethodMappingView supMth = Objects.requireNonNull(supCls.getMethod(srcMethodName, srcMethodDesc), "Incoming var comment's parent method not contained in supTree: " + subVarId); + MethodVarMappingView supVar = Objects.requireNonNull(supMth.getVar(lvtRowIndex, lvIndex, startOpIdx, endOpIdx, srcVarName), "Incoming var comment's parent var not contained in supTree: " + subVarId); + + assertEquals(supVar.getComment(), comment); } private boolean isEmpty(String[] arr) { @@ -409,9 +474,9 @@ private boolean isEmpty(String[] arr) { private final MappingTreeView supTree; private final int supDstNsCount; + private final MappingFormat subFormat; private final FeatureSet supFeatures; private final FeatureSet subFeatures; private int subNsIfSupNotNamespaced; private List subDstNamespaces; } - diff --git a/src/test/java/net/fabricmc/mappingio/TestFileUpdater.java b/src/test/java/net/fabricmc/mappingio/TestFileUpdater.java new file mode 100644 index 00000000..dc55b91a --- /dev/null +++ b/src/test/java/net/fabricmc/mappingio/TestFileUpdater.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2024 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 java.io.IOException; +import java.nio.file.Path; + +import net.fabricmc.mappingio.format.MappingFormat; + +public class TestFileUpdater { + public static void main(String[] args) throws IOException { + for (MappingFormat format : MappingFormat.values()) { + if (!format.hasWriter) { + continue; + } + + Path defaultPath = TestHelper.MappingDirs.VALID.resolve(TestHelper.getFileName(format)); + Path holesPath = TestHelper.MappingDirs.VALID_WITH_HOLES.resolve(TestHelper.getFileName(format)); + Path repeatPath = TestHelper.MappingDirs.REPEATED_ELEMENTS.resolve(TestHelper.getFileName(format)); + + TestHelper.acceptTestMappings(MappingWriter.create(defaultPath, format)); + TestHelper.acceptTestMappingsWithHoles(MappingWriter.create(holesPath, format)); + + if (format != MappingFormat.ENIGMA_DIR) { + TestHelper.acceptTestMappingsWithRepeats( + MappingWriter.create(repeatPath, format), + format != MappingFormat.ENIGMA_FILE, + format != MappingFormat.ENIGMA_FILE); + } + } + } +} diff --git a/src/test/java/net/fabricmc/mappingio/TestHelper.java b/src/test/java/net/fabricmc/mappingio/TestHelper.java index 50603594..b2cbc567 100644 --- a/src/test/java/net/fabricmc/mappingio/TestHelper.java +++ b/src/test/java/net/fabricmc/mappingio/TestHelper.java @@ -20,6 +20,7 @@ import java.net.URISyntaxException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -28,7 +29,10 @@ import org.cadixdev.lorenz.io.MappingFormats; import org.jetbrains.annotations.Nullable; +import net.fabricmc.mappingio.adapter.ForwardingMappingVisitor; import net.fabricmc.mappingio.format.MappingFormat; +import net.fabricmc.mappingio.format.intellij.MigrationMapConstants; +import net.fabricmc.mappingio.lib.jool.Unchecked; import net.fabricmc.mappingio.tree.MappingTreeView; import net.fabricmc.mappingio.tree.MemoryMappingTree; @@ -143,157 +147,325 @@ public static Path writeToDir(MappingTreeView tree, Path dir, MappingFormat form return path; } - // Has to be kept in sync with /resources/read/valid/* test mappings! - public static MemoryMappingTree createTestTree() { - MemoryMappingTree tree = new MemoryMappingTree(); - tree.visitNamespaces(MappingUtil.NS_SOURCE_FALLBACK, Arrays.asList(MappingUtil.NS_TARGET_FALLBACK, MappingUtil.NS_TARGET_FALLBACK + "2")); - int[] dstNs = new int[] { 0, 1 }; - nameGen.reset(); - - visitClass(tree, dstNs); - visitField(tree, dstNs); - visitMethod(tree, dstNs); - visitMethodArg(tree, dstNs); - visitMethodVar(tree, dstNs); - visitInnerClass(tree, 1, dstNs); - visitComment(tree); - visitField(tree, dstNs); - visitClass(tree, dstNs); - - return tree; - } + // After any changes, run "./gradlew updateTestMappings" to update the mapping files in the resources folder accordingly + public static T acceptTestMappings(T target) throws IOException { + MappingVisitor delegate = target instanceof VisitOrderVerifyingVisitor ? target : new VisitOrderVerifyingVisitor(target); + + if (delegate.visitHeader()) { + delegate.visitNamespaces(MappingUtil.NS_SOURCE_FALLBACK, Arrays.asList(MappingUtil.NS_TARGET_FALLBACK, MappingUtil.NS_TARGET_FALLBACK + "2")); + delegate.visitMetadata("name", "valid"); + delegate.visitMetadata(MigrationMapConstants.ORDER_KEY, "0"); + } - // Has to be kept in sync with /resources/read/valid-with-holes/* test mappings! - public static MemoryMappingTree createTestTreeWithHoles() { - MemoryMappingTree tree = new MemoryMappingTree(); - tree.visitNamespaces(MappingUtil.NS_SOURCE_FALLBACK, Arrays.asList(MappingUtil.NS_TARGET_FALLBACK, MappingUtil.NS_TARGET_FALLBACK + "2")); - nameGen.reset(); + if (delegate.visitContent()) { + int[] dstNs = new int[] { 0, 1 }; + nameGen.reset(); - // (Inner) Classes - for (int nestLevel = 0; nestLevel <= 2; nestLevel++) { - visitClass(tree); - visitInnerClass(tree, nestLevel, 0); - visitInnerClass(tree, nestLevel, 1); + if (visitClass(delegate, dstNs)) { + visitField(delegate, dstNs); - visitInnerClass(tree, nestLevel); - visitComment(tree); + if (visitMethod(delegate, dstNs)) { + visitMethodArg(delegate, dstNs); + visitMethodVar(delegate, dstNs); + } + } - visitInnerClass(tree, nestLevel, 0); - visitComment(tree); + if (visitInnerClass(delegate, 1, dstNs)) { + visitComment(delegate); + visitField(delegate, dstNs); + } + + visitClass(delegate, dstNs); + } - visitInnerClass(tree, nestLevel, 1); - visitComment(tree); + if (!delegate.visitEnd()) { + acceptTestMappings(delegate); } - // Fields - visitClass(tree); - visitField(tree); - visitField(tree, 0); - visitField(tree, 1); + return target; + } + + // After any changes, run "./gradlew updateTestMappings" to update the mapping files in the resources folder accordingly. + // Make sure to keep the few manual changes in the files. + public static T acceptTestMappingsWithRepeats(T target, boolean repeatComments, boolean repeatClasses) throws IOException { + acceptTestMappings(new ForwardingMappingVisitor(new VisitOrderVerifyingVisitor(target, true)) { + private List replayQueue = new ArrayList<>(); + + @Override + public void visitMetadata(String key, @Nullable String value) throws IOException { + super.visitMetadata(key, key.equals("name") ? "repeated-elements" : value); + } + + @Override + public boolean visitClass(String srcName) throws IOException { + replayQueue.clear(); + + if (repeatClasses) { + replayQueue.add(Unchecked.runnable(() -> super.visitClass(srcName))); + } + + return super.visitClass(srcName); + } + + @Override + public boolean visitField(String srcName, @Nullable String srcDesc) throws IOException { + replayQueue.clear(); + replayQueue.add(Unchecked.runnable(() -> super.visitField(srcName, srcDesc))); + return super.visitField(srcName, srcDesc); + } + + @Override + public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOException { + replayQueue.clear(); + replayQueue.add(Unchecked.runnable(() -> super.visitMethod(srcName, srcDesc))); + return super.visitMethod(srcName, srcDesc); + } + + @Override + public boolean visitMethodArg(int lvIndex, int argIndex, String srcName) throws IOException { + replayQueue.clear(); + replayQueue.add(Unchecked.runnable(() -> super.visitMethodArg(lvIndex, argIndex, srcName))); + return super.visitMethodArg(lvIndex, argIndex, srcName); + } + + @Override + public boolean visitMethodVar(int lvIndex, int varIndex, int startOpIdx, int endOpIdx, String srcName) throws IOException { + replayQueue.clear(); + replayQueue.add(Unchecked.runnable(() -> super.visitMethodVar(lvIndex, varIndex, startOpIdx, endOpIdx, srcName))); + return super.visitMethodVar(lvIndex, varIndex, startOpIdx, endOpIdx, srcName); + } + + @Override + public void visitDstName(MappedElementKind targetKind, int namespace, String name) throws IOException { + if (targetKind == MappedElementKind.CLASS && !repeatClasses) { + super.visitDstName(targetKind, namespace, name); + } else { + replayQueue.add(Unchecked.runnable(() -> super.visitDstName(targetKind, namespace, name))); + super.visitDstName(targetKind, namespace, name + "0"); + } + } + + @Override + public void visitDstDesc(MappedElementKind targetKind, int namespace, String desc) throws IOException { + replayQueue.add(Unchecked.runnable(() -> super.visitDstDesc(targetKind, namespace, desc))); + super.visitDstDesc(targetKind, namespace, desc); + } - visitField(tree); - visitComment(tree); + @Override + public boolean visitElementContent(MappedElementKind targetKind) throws IOException { + boolean ret = super.visitElementContent(targetKind); - visitField(tree, 0); - visitComment(tree); + if (!replayQueue.isEmpty()) { + replayQueue.forEach(Runnable::run); - visitField(tree, 1); - visitComment(tree); + ret = super.visitElementContent(targetKind); + } - // Methods - visitMethod(tree); - visitMethod(tree, 0); - visitMethod(tree, 1); + return ret; + } - visitMethod(tree); - visitComment(tree); + @Override + public void visitComment(MappedElementKind targetKind, String comment) throws IOException { + if (repeatComments) { + super.visitComment(targetKind, comment + "."); + } - visitMethod(tree, 0); - visitComment(tree); + super.visitComment(targetKind, comment); + } + }); - visitMethod(tree, 1); - visitComment(tree); + return target; + } - // Method args - visitMethod(tree); - visitMethodArg(tree); - visitMethodArg(tree, 1); - visitMethodArg(tree, 0); + // After any changes, run "./gradlew updateTestMappings" to update the mapping files in the resources folder accordingly + public static T acceptTestMappingsWithHoles(T target) throws IOException { + MappingVisitor delegate = target instanceof VisitOrderVerifyingVisitor ? target : new VisitOrderVerifyingVisitor(target); - visitMethodArg(tree); - visitComment(tree); + if (delegate.visitHeader()) { + delegate.visitNamespaces(MappingUtil.NS_SOURCE_FALLBACK, Arrays.asList(MappingUtil.NS_TARGET_FALLBACK, MappingUtil.NS_TARGET_FALLBACK + "2")); + } - visitMethodArg(tree, 0); - visitComment(tree); + if (delegate.visitContent()) { + nameGen.reset(); - visitMethodArg(tree, 1); - visitComment(tree); + // (Inner) Classes + for (int nestLevel = 0; nestLevel <= 2; nestLevel++) { + visitClass(delegate); + visitInnerClass(delegate, nestLevel, 0); + visitInnerClass(delegate, nestLevel, 1); - // Method vars - visitMethod(tree); - visitMethodVar(tree); - visitMethodVar(tree, 1); - visitMethodVar(tree, 0); + if (visitInnerClass(delegate, nestLevel)) { + visitComment(delegate); + } - visitMethodVar(tree); - visitComment(tree); + if (visitInnerClass(delegate, nestLevel, 0)) { + visitComment(delegate); + } - visitMethodVar(tree, 0); - visitComment(tree); + if (visitInnerClass(delegate, nestLevel, 1)) { + visitComment(delegate); + } + } - visitMethodVar(tree, 1); - visitComment(tree); + if (visitClass(delegate)) { + // Fields + visitField(delegate); + visitField(delegate, 0); + visitField(delegate, 1); + + if (visitField(delegate)) { + visitComment(delegate); + } + + if (visitField(delegate, 0)) { + visitComment(delegate); + } + + if (visitField(delegate, 1)) { + visitComment(delegate); + } + + // Methods + visitMethod(delegate); + visitMethod(delegate, 0); + visitMethod(delegate, 1); + + if (visitMethod(delegate)) { + visitComment(delegate); + } + + if (visitMethod(delegate, 0)) { + visitComment(delegate); + } + + if (visitMethod(delegate, 1)) { + visitComment(delegate); + } + + // Method args + if (visitMethod(delegate)) { + visitMethodArg(delegate); + visitMethodArg(delegate, 1); + visitMethodArg(delegate, 0); + + if (visitMethodArg(delegate)) { + visitComment(delegate); + } + + if (visitMethodArg(delegate, 0)) { + visitComment(delegate); + } + + if (visitMethodArg(delegate, 1)) { + visitComment(delegate); + } + } + + // Method vars + if (visitMethod(delegate)) { + visitMethodVar(delegate); + visitMethodVar(delegate, 1); + visitMethodVar(delegate, 0); + + if (visitMethodVar(delegate)) { + visitComment(delegate); + } + + if (visitMethodVar(delegate, 0)) { + visitComment(delegate); + } + + if (visitMethodVar(delegate, 1)) { + visitComment(delegate); + } + } + } + } + + if (!delegate.visitEnd()) { + acceptTestMappingsWithHoles(delegate); + } - return tree; + return target; } - private static void visitClass(MemoryMappingTree tree, int... dstNs) { - visitInnerClass(tree, 0, dstNs); + private static boolean visitClass(MappingVisitor target, int... dstNs) throws IOException { + return visitInnerClass(target, 0, dstNs); } - private static void visitInnerClass(MemoryMappingTree tree, int nestLevel, int... dstNs) { - tree.visitClass(nestLevel <= 0 ? nameGen.src(clsKind) : nameGen.srcInnerCls(nestLevel)); + private static boolean visitInnerClass(MappingVisitor target, int nestLevel, int... dstNs) throws IOException { + if (!target.visitClass(nestLevel <= 0 ? nameGen.src(clsKind) : nameGen.srcInnerCls(nestLevel))) { + return false; + } for (int ns : dstNs) { - tree.visitDstName(clsKind, ns, nameGen.dst(clsKind, ns)); + target.visitDstName(clsKind, ns, nameGen.dst(clsKind, ns)); } + + return target.visitElementContent(clsKind); } - private static void visitField(MemoryMappingTree tree, int... dstNs) { - tree.visitField(nameGen.src(fldKind), nameGen.desc(fldKind)); + private static boolean visitField(MappingVisitor target, int... dstNs) throws IOException { + String desc; + + if (!target.visitField(nameGen.src(fldKind), desc = nameGen.desc(fldKind))) { + return false; + } for (int ns : dstNs) { - tree.visitDstName(fldKind, ns, nameGen.dst(fldKind, ns)); + target.visitDstName(fldKind, ns, nameGen.dst(fldKind, ns)); + target.visitDstDesc(fldKind, ns, desc); } + + return target.visitElementContent(fldKind); } - private static void visitMethod(MemoryMappingTree tree, int... dstNs) { - tree.visitMethod(nameGen.src(mthKind), nameGen.desc(mthKind)); + private static boolean visitMethod(MappingVisitor target, int... dstNs) throws IOException { + String desc; + + if (!target.visitMethod(nameGen.src(mthKind), desc = nameGen.desc(mthKind))) { + return false; + } for (int ns : dstNs) { - tree.visitDstName(mthKind, ns, nameGen.dst(mthKind, ns)); + target.visitDstName(mthKind, ns, nameGen.dst(mthKind, ns)); + target.visitDstDesc(mthKind, ns, desc); } + + return target.visitElementContent(mthKind); } - private static void visitMethodArg(MemoryMappingTree tree, int... dstNs) { - tree.visitMethodArg(nameGen.getCounter().getAndIncrement(), nameGen.getCounter().getAndIncrement(), nameGen.src(argKind)); + private static boolean visitMethodArg(MappingVisitor target, int... dstNs) throws IOException { + if (!target.visitMethodArg(nameGen.getCounter().getAndIncrement(), nameGen.getCounter().getAndIncrement(), nameGen.src(argKind))) { + return false; + } for (int ns : dstNs) { - tree.visitDstName(argKind, ns, nameGen.dst(argKind, ns)); + target.visitDstName(argKind, ns, nameGen.dst(argKind, ns)); } + + return target.visitElementContent(argKind); } - private static void visitMethodVar(MemoryMappingTree tree, int... dstNs) { - tree.visitMethodVar(nameGen.getCounter().get(), nameGen.getCounter().get(), - nameGen.getCounter().getAndIncrement(), nameGen.getCounter().getAndIncrement(), nameGen.src(varKind)); + private static boolean visitMethodVar(MappingVisitor target, int... dstNs) throws IOException { + if (!target.visitMethodVar( + nameGen.getCounter().get(), + nameGen.getCounter().get(), + nameGen.getCounter().getAndIncrement(), + nameGen.getCounter().getAndIncrement(), + nameGen.src(varKind))) { + return false; + } for (int ns : dstNs) { - tree.visitDstName(varKind, ns, nameGen.dst(varKind, ns)); + target.visitDstName(varKind, ns, nameGen.dst(varKind, ns)); } + + return target.visitElementContent(varKind); } - private static void visitComment(MemoryMappingTree tree) { - tree.visitComment(nameGen.lastKind.get(), comment); + private static void visitComment(MappingVisitor target) throws IOException { + target.visitComment(nameGen.lastKind.get(), comment); } private static class NameGen { @@ -442,16 +614,17 @@ private String getPrefix(MappedElementKind kind) { 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(); + public static MemoryMappingTree getCorrespondingTree(Path dir) throws IOException { + if (dir.equals(VALID)) return acceptTestMappings(new MemoryMappingTree()); + if (dir.equals(REPEATED_ELEMENTS)) return acceptTestMappingsWithRepeats(new MemoryMappingTree(), true, true); + if (dir.equals(VALID_WITH_HOLES)) return acceptTestMappingsWithHoles(new MemoryMappingTree()); 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/"); public static final Path REPEATED_ELEMENTS = getResource("/read/repeated-elements/"); + public static final Path VALID_WITH_HOLES = getResource("/read/valid-with-holes/"); public static final Path MERGING = getResource("/merging/"); } diff --git a/src/test/java/net/fabricmc/mappingio/VisitOrderVerifyingVisitor.java b/src/test/java/net/fabricmc/mappingio/VisitOrderVerifyingVisitor.java index 0c2a69d8..02732191 100644 --- a/src/test/java/net/fabricmc/mappingio/VisitOrderVerifyingVisitor.java +++ b/src/test/java/net/fabricmc/mappingio/VisitOrderVerifyingVisitor.java @@ -111,7 +111,7 @@ public void visitMetadata(String key, @Nullable String value) throws IOException @Override public boolean visitContent() throws IOException { - assertNamespacesVisited(); + assertHeaderVisited(); assertContentNotVisited(); visitedContent = true; diff --git a/src/test/java/net/fabricmc/mappingio/lib/jool/Unchecked.java b/src/test/java/net/fabricmc/mappingio/lib/jool/Unchecked.java new file mode 100644 index 00000000..0e5901bd --- /dev/null +++ b/src/test/java/net/fabricmc/mappingio/lib/jool/Unchecked.java @@ -0,0 +1,104 @@ +/* + * Copyright (c), Data Geekery GmbH, contact@datageekery.com + * + * 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.lib.jool; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.function.Consumer; + +import net.fabricmc.mappingio.lib.jool.fi.lang.CheckedRunnable; + +/** + * Improved interoperability between checked exceptions and Java 8. + * + *

Checked exceptions are one of Java's biggest flaws. Due to backwards-compatibility, we're inheriting all the checked + * exception trouble back from JDK 1.0. This becomes even more obvious when using lambda expressions, most of which are + * not allowed to throw checked exceptions. + * + *

This library tries to ease some pain and wraps / unwraps a variety of API elements from the JDK 8 to improve + * interoperability with checked exceptions. + * + * @author Lukas Eder + */ +public class Unchecked { + private Unchecked() { } + + /** + * A {@link Consumer} that wraps any {@link Throwable} in a {@link RuntimeException}. + */ + public static final Consumer THROWABLE_TO_RUNTIME_EXCEPTION = t -> { + if (t instanceof Error) { + throw (Error) t; + } + + if (t instanceof RuntimeException) { + throw (RuntimeException) t; + } + + if (t instanceof IOException) { + throw new UncheckedIOException((IOException) t); + } + + // [#230] Clients will not expect needing to handle this. + if (t instanceof InterruptedException) { + Thread.currentThread().interrupt(); + } + + throw new RuntimeException(t); + }; + + /** + * Wrap a {@link CheckedRunnable} in a {@link Runnable}. + * + *

Example: + * 


+	 * new Thread(Unchecked.runnable(() -> {
+	 *     throw new Exception("Cannot run this thread");
+	 * })).start();
+	 *
+ */ + public static Runnable runnable(CheckedRunnable runnable) { + return runnable(runnable, THROWABLE_TO_RUNTIME_EXCEPTION); + } + + /** + * Wrap a {@link CheckedRunnable} in a {@link Runnable} with a custom handler for checked exceptions. + * + *

Example: + * 


+	 * new Thread(Unchecked.runnable(
+	 *     () -> {
+	 *         throw new Exception("Cannot run this thread");
+	 *     },
+	 *     e -> {
+	 *         throw new IllegalStateException(e);
+	 *     }
+	 * )).start();
+	 *
+ */ + public static Runnable runnable(CheckedRunnable runnable, Consumer handler) { + return () -> { + try { + runnable.run(); + } catch (Throwable e) { + handler.accept(e); + + throw new IllegalStateException("Exception handler must throw a RuntimeException", e); + } + }; + } +} diff --git a/src/test/java/net/fabricmc/mappingio/lib/jool/fi/lang/CheckedRunnable.java b/src/test/java/net/fabricmc/mappingio/lib/jool/fi/lang/CheckedRunnable.java new file mode 100644 index 00000000..5ebd7726 --- /dev/null +++ b/src/test/java/net/fabricmc/mappingio/lib/jool/fi/lang/CheckedRunnable.java @@ -0,0 +1,30 @@ +/* + * Copyright (c), Data Geekery GmbH, contact@datageekery.com + * + * 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.lib.jool.fi.lang; + +/** + * A {@link Runnable} that allows for checked exceptions. + * + * @author Lukas Eder + */ +@FunctionalInterface +public interface CheckedRunnable { + /** + * Run this runnable. + */ + void run() throws Throwable; +} diff --git a/src/test/java/net/fabricmc/mappingio/read/ValidContentReadTest.java b/src/test/java/net/fabricmc/mappingio/read/ValidContentReadTest.java index 577f0d02..27b1121f 100644 --- a/src/test/java/net/fabricmc/mappingio/read/ValidContentReadTest.java +++ b/src/test/java/net/fabricmc/mappingio/read/ValidContentReadTest.java @@ -19,7 +19,6 @@ import java.nio.file.Path; import org.jetbrains.annotations.Nullable; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import net.fabricmc.mappingio.MappingReader; @@ -29,23 +28,11 @@ import net.fabricmc.mappingio.adapter.FlatAsRegularMappingVisitor; import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch; import net.fabricmc.mappingio.format.MappingFormat; -import net.fabricmc.mappingio.tree.MappingTree; import net.fabricmc.mappingio.tree.MappingTreeView; import net.fabricmc.mappingio.tree.MemoryMappingTree; import net.fabricmc.mappingio.tree.VisitableMappingTree; public class ValidContentReadTest { - private static MappingTree testTree; - private static MappingTree testTreeWithHoles; - private static MappingTree testTreeWithRepeatedElements; - - @BeforeAll - public static void setup() throws Exception { - testTree = TestHelper.createTestTree(); - testTreeWithHoles = TestHelper.createTestTreeWithHoles(); - testTreeWithRepeatedElements = testTree; - } - @Test public void enigmaFile() throws Exception { MappingFormat format = MappingFormat.ENIGMA_FILE; @@ -160,11 +147,12 @@ public void jobfFile() throws Exception { private void checkDefault(MappingFormat format) throws Exception { Path path = TestHelper.MappingDirs.VALID.resolve(TestHelper.getFileName(format)); + VisitableMappingTree referenceTree = TestHelper.acceptTestMappings(new MemoryMappingTree()); VisitableMappingTree tree = new MemoryMappingTree(); boolean allowConsecutiveDuplicateElementVisits = false; MappingReader.read(path, format, new VisitOrderVerifyingVisitor(tree, allowConsecutiveDuplicateElementVisits)); - assertEqual(tree, format, testTree, allowConsecutiveDuplicateElementVisits); + assertEqual(tree, format, referenceTree, allowConsecutiveDuplicateElementVisits); tree = new MemoryMappingTree(); MappingReader.read(path, format, @@ -172,28 +160,20 @@ private void checkDefault(MappingFormat format) throws Exception { new VisitOrderVerifyingVisitor( new MappingSourceNsSwitch( new VisitOrderVerifyingVisitor(tree, allowConsecutiveDuplicateElementVisits), - testTree.getSrcNamespace()), + referenceTree.getSrcNamespace()), allowConsecutiveDuplicateElementVisits), - testTree.getDstNamespaces().get(0))); - assertEqual(tree, format, testTree, allowConsecutiveDuplicateElementVisits); - } - - private void checkHoles(MappingFormat format) throws Exception { - Path path = TestHelper.MappingDirs.VALID_WITH_HOLES.resolve(TestHelper.getFileName(format)); - - VisitableMappingTree tree = new MemoryMappingTree(); - boolean allowConsecutiveDuplicateElementVisits = false; - - MappingReader.read(path, format, new VisitOrderVerifyingVisitor(tree, allowConsecutiveDuplicateElementVisits)); - assertEqual(tree, format, testTreeWithHoles, allowConsecutiveDuplicateElementVisits); + referenceTree.getDstNamespaces().get(0))); + assertEqual(tree, format, referenceTree, allowConsecutiveDuplicateElementVisits); } private void checkRepeated(MappingFormat format, boolean allowConsecutiveDuplicateElementVisits) throws Exception { Path path = TestHelper.MappingDirs.REPEATED_ELEMENTS.resolve(TestHelper.getFileName(format)); + VisitableMappingTree referenceTree = TestHelper.acceptTestMappingsWithRepeats(new MemoryMappingTree(), true, true); VisitableMappingTree tree = new MemoryMappingTree(); + MappingReader.read(path, format, new VisitOrderVerifyingVisitor(tree, allowConsecutiveDuplicateElementVisits)); - assertEqual(tree, format, testTreeWithRepeatedElements, allowConsecutiveDuplicateElementVisits); + assertEqual(tree, format, referenceTree, allowConsecutiveDuplicateElementVisits); tree = new MemoryMappingTree(); MappingReader.read(path, format, @@ -201,10 +181,21 @@ private void checkRepeated(MappingFormat format, boolean allowConsecutiveDuplica new VisitOrderVerifyingVisitor( new MappingSourceNsSwitch( new VisitOrderVerifyingVisitor(tree, allowConsecutiveDuplicateElementVisits), - testTreeWithRepeatedElements.getSrcNamespace()), + referenceTree.getSrcNamespace()), allowConsecutiveDuplicateElementVisits), - testTreeWithRepeatedElements.getDstNamespaces().get(0))); - assertEqual(tree, format, testTreeWithRepeatedElements, allowConsecutiveDuplicateElementVisits); + referenceTree.getDstNamespaces().get(0))); + assertEqual(tree, format, referenceTree, allowConsecutiveDuplicateElementVisits); + } + + private void checkHoles(MappingFormat format) throws Exception { + Path path = TestHelper.MappingDirs.VALID_WITH_HOLES.resolve(TestHelper.getFileName(format)); + + VisitableMappingTree referenceTree = TestHelper.acceptTestMappingsWithHoles(new MemoryMappingTree()); + VisitableMappingTree tree = new MemoryMappingTree(); + boolean allowConsecutiveDuplicateElementVisits = false; + + MappingReader.read(path, format, new VisitOrderVerifyingVisitor(tree, allowConsecutiveDuplicateElementVisits)); + assertEqual(tree, format, referenceTree, allowConsecutiveDuplicateElementVisits); } private void assertEqual(MappingTreeView tree, MappingFormat format, MappingTreeView referenceTree, boolean allowConsecutiveDuplicateElementVisits) throws Exception { diff --git a/src/test/java/net/fabricmc/mappingio/tree/MetadataTest.java b/src/test/java/net/fabricmc/mappingio/tree/MetadataTest.java index 1e29d5ce..8965a20a 100644 --- a/src/test/java/net/fabricmc/mappingio/tree/MetadataTest.java +++ b/src/test/java/net/fabricmc/mappingio/tree/MetadataTest.java @@ -42,7 +42,8 @@ public class MetadataTest { @BeforeAll public static void setup() throws Exception { - tree = TestHelper.createTestTree(); + tree = TestHelper.acceptTestMappings(new MemoryMappingTree()); + tree.getMetadata().clear(); for (int i = 0; i < 40; i++) { String key = "key" + random.nextInt(3); @@ -59,8 +60,8 @@ public void testOrder() throws Exception { tree.accept(new NopMappingVisitor(true) { @Override public void visitMetadata(String key, @Nullable String value) { - assertEquals(key, keys.get(visitCount)); - assertEquals(value, values.get(visitCount)); + assertEquals(keys.get(visitCount), key); + assertEquals(values.get(visitCount), value); visitCount++; } diff --git a/src/test/java/net/fabricmc/mappingio/visiting/VisitEndTest.java b/src/test/java/net/fabricmc/mappingio/visiting/VisitEndTest.java index d301f236..3574368c 100644 --- a/src/test/java/net/fabricmc/mappingio/visiting/VisitEndTest.java +++ b/src/test/java/net/fabricmc/mappingio/visiting/VisitEndTest.java @@ -19,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; import java.util.EnumSet; import java.util.List; @@ -128,11 +129,14 @@ public void jobfFile() throws Exception { private void check(MappingFormat format) throws Exception { checkDir(TestHelper.MappingDirs.DETECTION, format); checkDir(TestHelper.MappingDirs.VALID, format); + checkDir(TestHelper.MappingDirs.REPEATED_ELEMENTS, format); checkDir(TestHelper.MappingDirs.VALID_WITH_HOLES, format); } private void checkDir(Path dir, MappingFormat format) throws Exception { Path path = dir.resolve(TestHelper.getFileName(format)); + if (!Files.exists(path)) return; + MappingTreeView supTree = TestHelper.MappingDirs.getCorrespondingTree(dir); checkCompliance(format, path, 1, true, supTree); diff --git a/src/test/java/net/fabricmc/mappingio/write/WriteTest.java b/src/test/java/net/fabricmc/mappingio/write/WriteTest.java index 7a0c1e7f..1da5f300 100644 --- a/src/test/java/net/fabricmc/mappingio/write/WriteTest.java +++ b/src/test/java/net/fabricmc/mappingio/write/WriteTest.java @@ -29,6 +29,7 @@ import net.fabricmc.mappingio.MappedElementKind; import net.fabricmc.mappingio.MappingReader; +import net.fabricmc.mappingio.MappingWriter; import net.fabricmc.mappingio.SubsetAssertingVisitor; import net.fabricmc.mappingio.TestHelper; import net.fabricmc.mappingio.adapter.FlatAsRegularMappingVisitor; @@ -44,16 +45,22 @@ public class WriteTest { private static Path dir; private static MappingTreeView validTree; private static Map treeNsAltMap = new HashMap<>(); + private static MappingTreeView validWithRepeatsTree; + private static Map treeWithRepeatsNsAltMap = new HashMap<>(); private static MappingTreeView validWithHolesTree; private static Map treeWithHolesNsAltMap = new HashMap<>(); @BeforeAll public static void setup() throws Exception { - validTree = TestHelper.createTestTree(); + validTree = TestHelper.acceptTestMappings(new MemoryMappingTree()); treeNsAltMap.put(validTree.getDstNamespaces().get(0), validTree.getSrcNamespace()); treeNsAltMap.put(validTree.getDstNamespaces().get(1), validTree.getSrcNamespace()); - validWithHolesTree = TestHelper.createTestTreeWithHoles(); + validWithRepeatsTree = TestHelper.acceptTestMappingsWithRepeats(new MemoryMappingTree(), true, true); + treeWithRepeatsNsAltMap.put(validWithRepeatsTree.getDstNamespaces().get(0), validWithRepeatsTree.getSrcNamespace()); + treeWithRepeatsNsAltMap.put(validWithRepeatsTree.getDstNamespaces().get(1), validWithRepeatsTree.getSrcNamespace()); + + validWithHolesTree = TestHelper.acceptTestMappingsWithHoles(new MemoryMappingTree()); treeWithHolesNsAltMap.put(validWithHolesTree.getDstNamespaces().get(0), validWithHolesTree.getSrcNamespace()); treeWithHolesNsAltMap.put(validWithHolesTree.getDstNamespaces().get(1), validWithHolesTree.getSrcNamespace()); } @@ -129,12 +136,19 @@ public void jobfFile() throws Exception { } private void check(MappingFormat format) throws Exception { - Path path = TestHelper.writeToDir(validTree, dir, format); + Path path = dir.resolve(TestHelper.getFileName(format)); + TestHelper.acceptTestMappings(MappingWriter.create(path, format)); readWithMio(validTree, path, format); readWithLorenz(path, format); readWithSrgUtils(validTree, format, treeNsAltMap); - path = TestHelper.writeToDir(validWithHolesTree, dir, format); + boolean isEnigma = format == MappingFormat.ENIGMA_FILE || format == MappingFormat.ENIGMA_DIR; + TestHelper.acceptTestMappingsWithRepeats(MappingWriter.create(path, format), !isEnigma, !isEnigma); + readWithMio(validWithRepeatsTree, path, format); + readWithLorenz(path, format); + readWithSrgUtils(validWithRepeatsTree, format, treeWithRepeatsNsAltMap); + + TestHelper.acceptTestMappingsWithHoles(MappingWriter.create(path, format)); readWithMio(validWithHolesTree, path, format); readWithLorenz(path, format); readWithSrgUtils(validWithHolesTree, format, treeWithHolesNsAltMap); diff --git a/src/test/resources/read/repeated-elements/enigma.mappings b/src/test/resources/read/repeated-elements/enigma.mappings index 5c888fb3..fb966451 100644 --- a/src/test/resources/read/repeated-elements/enigma.mappings +++ b/src/test/resources/read/repeated-elements/enigma.mappings @@ -9,6 +9,7 @@ CLASS class_1 class1Ns0Rename CLASS class_2 class2Ns0Rename0 CLASS class_2 class2Ns0Rename1 COMMENT This is a comment. + CLASS class_2 class2Ns0Rename2 CLASS class_2 class2Ns0Rename COMMENT This is a comment FIELD field_2 field2Ns0Rename0 Lcls; diff --git a/src/test/resources/read/repeated-elements/migration-map.xml b/src/test/resources/read/repeated-elements/migration-map.xml index dc689ab6..4a49c0ce 100644 --- a/src/test/resources/read/repeated-elements/migration-map.xml +++ b/src/test/resources/read/repeated-elements/migration-map.xml @@ -1,10 +1,11 @@ - - - - - - + + + + + + + + - diff --git a/src/test/resources/read/repeated-elements/tinyV2.tiny b/src/test/resources/read/repeated-elements/tinyV2.tiny index a8636c70..184c44e7 100644 --- a/src/test/resources/read/repeated-elements/tinyV2.tiny +++ b/src/test/resources/read/repeated-elements/tinyV2.tiny @@ -1,4 +1,5 @@ tiny 2 0 source target target2 + name repeated-elements c class_1 class1Ns0Rename0 class1Ns1Rename0 c class_1 class1Ns0Rename class1Ns1Rename f I field_1 field1Ns0Rename0 field1Ns1Rename0 diff --git a/src/test/resources/read/valid-with-holes/csrg.csrg b/src/test/resources/read/valid-with-holes/csrg.csrg index ef91ca84..2b811438 100644 --- a/src/test/resources/read/valid-with-holes/csrg.csrg +++ b/src/test/resources/read/valid-with-holes/csrg.csrg @@ -1,33 +1,10 @@ -class_1 class_1 class_2 class2Ns0Rename -class_3 class_3 -class_4 class_4 class_5 class5Ns0Rename -class_6 class_6 -class_7 class_7 class_7$class_8 class_7$class8Ns0Rename -class_9$class_10 class_9$class_10 -class_11$class_12 class_11$class_12 class_13$class_14 class_13$class14Ns0Rename -class_15$class_16 class_15$class_16 -class_17 class_17 class_17$class_18$class_19 class_17$class_18$class19Ns0Rename -class_20$class_21$class_22 class_20$class_21$class_22 -class_23$class_24$class_25 class_23$class_24$class_25 class_26$class_27$class_28 class_26$class_27$class28Ns0Rename -class_29$class_30$class_31 class_29$class_30$class_31 -class_32 class_32 -class_32 field_1 field_1 class_32 field_2 field2Ns0Rename -class_32 field_3 field_3 -class_32 field_4 field_4 class_32 field_5 field5Ns0Rename -class_32 field_6 field_6 -class_32 method_1 ()I method_1 class_32 method_2 (I)V method2Ns0Rename -class_32 method_3 (Lcls;)Lcls; method_3 -class_32 method_4 (ILcls;)Lpkg/cls; method_4 class_32 method_5 (Lcls;[I)[[B method5Ns0Rename -class_32 method_6 ()I method_6 -class_32 method_7 (I)V method_7 -class_32 method_8 (Lcls;)Lcls; method_8 diff --git a/src/test/resources/read/valid-with-holes/migration-map.xml b/src/test/resources/read/valid-with-holes/migration-map.xml index 29d91399..7beec0cf 100644 --- a/src/test/resources/read/valid-with-holes/migration-map.xml +++ b/src/test/resources/read/valid-with-holes/migration-map.xml @@ -1,23 +1,11 @@ - - - - - - - - - - - - - - - - - - - + + + + + + + + - diff --git a/src/test/resources/read/valid-with-holes/tsrg.tsrg b/src/test/resources/read/valid-with-holes/tsrg.tsrg index 09667011..89fbadd8 100644 --- a/src/test/resources/read/valid-with-holes/tsrg.tsrg +++ b/src/test/resources/read/valid-with-holes/tsrg.tsrg @@ -1,33 +1,11 @@ -class_1 class_1 class_2 class2Ns0Rename -class_3 class_3 -class_4 class_4 class_5 class5Ns0Rename -class_6 class_6 -class_7 class_7 class_7$class_8 class_7$class8Ns0Rename -class_9$class_10 class_9$class_10 -class_11$class_12 class_11$class_12 class_13$class_14 class_13$class14Ns0Rename -class_15$class_16 class_15$class_16 -class_17 class_17 class_17$class_18$class_19 class_17$class_18$class19Ns0Rename -class_20$class_21$class_22 class_20$class_21$class_22 -class_23$class_24$class_25 class_23$class_24$class_25 class_26$class_27$class_28 class_26$class_27$class28Ns0Rename -class_29$class_30$class_31 class_29$class_30$class_31 class_32 class_32 - field_1 field_1 field_2 field2Ns0Rename - field_3 field_3 - field_4 field_4 field_5 field5Ns0Rename - field_6 field_6 - method_1 ()I method_1 method_2 (I)V method2Ns0Rename - method_3 (Lcls;)Lcls; method_3 - method_4 (ILcls;)Lpkg/cls; method_4 method_5 (Lcls;[I)[[B method5Ns0Rename - method_6 ()I method_6 - method_7 (I)V method_7 - method_8 (Lcls;)Lcls; method_8 diff --git a/src/test/resources/read/valid-with-holes/tsrgV2.tsrg b/src/test/resources/read/valid-with-holes/tsrgV2.tsrg index 3749147c..b9409ad8 100644 --- a/src/test/resources/read/valid-with-holes/tsrgV2.tsrg +++ b/src/test/resources/read/valid-with-holes/tsrgV2.tsrg @@ -1,40 +1,27 @@ tsrg2 source target target2 -class_1 class_1 class_1 class_2 class2Ns0Rename class_2 class_3 class_3 class3Ns1Rename -class_4 class_4 class_4 class_5 class5Ns0Rename class_5 class_6 class_6 class6Ns1Rename -class_7 class_7 class_7 class_7$class_8 class_7$class8Ns0Rename class_7$class_8 class_9$class_10 class_9$class_10 class_9$class10Ns1Rename -class_11$class_12 class_11$class_12 class_11$class_12 class_13$class_14 class_13$class14Ns0Rename class_13$class_14 class_15$class_16 class_15$class_16 class_15$class16Ns1Rename -class_17 class_17 class_17 class_17$class_18$class_19 class_17$class_18$class19Ns0Rename class_17$class_18$class_19 class_20$class_21$class_22 class_20$class_21$class_22 class_20$class_21$class22Ns1Rename -class_23$class_24$class_25 class_23$class_24$class_25 class_23$class_24$class_25 class_26$class_27$class_28 class_26$class_27$class28Ns0Rename class_26$class_27$class_28 class_29$class_30$class_31 class_29$class_30$class_31 class_29$class_30$class31Ns1Rename class_32 class_32 class_32 - field_1 I field_1 field_1 field_2 Lcls; field2Ns0Rename field_2 field_3 Lpkg/cls; field_3 field3Ns1Rename - field_4 [I field_4 field_4 field_5 I field5Ns0Rename field_5 field_6 Lcls; field_6 field6Ns1Rename - method_1 ()I method_1 method_1 method_2 (I)V method2Ns0Rename method_2 method_3 (Lcls;)Lcls; method_3 method3Ns1Rename - method_4 (ILcls;)Lpkg/cls; method_4 method_4 method_5 (Lcls;[I)[[B method5Ns0Rename method_5 method_6 ()I method_6 method6Ns1Rename method_7 (I)V method_7 method_7 - 1 param_1 param_1 param_1 3 param_2 param_2 param2Ns1Rename 5 param_3 param3Ns0Rename param_3 - 7 param_4 param_4 param_4 9 param_5 param5Ns0Rename param_5 11 param_6 param_6 param6Ns1Rename - method_8 (Lcls;)Lcls; method_8 method_8 diff --git a/src/test/resources/read/valid-with-holes/xsrg.xsrg b/src/test/resources/read/valid-with-holes/xsrg.xsrg index 0cb4a99f..6145d47e 100644 --- a/src/test/resources/read/valid-with-holes/xsrg.xsrg +++ b/src/test/resources/read/valid-with-holes/xsrg.xsrg @@ -4,7 +4,7 @@ CL: class_7$class_8 class_7$class8Ns0Rename CL: class_13$class_14 class_13$class14Ns0Rename CL: class_17$class_18$class_19 class_17$class_18$class19Ns0Rename CL: class_26$class_27$class_28 class_26$class_27$class28Ns0Rename -FD: class_32/field_2 Lcls; class_32/field2Ns0Rename I +FD: class_32/field_2 Lcls; class_32/field2Ns0Rename Lcls; FD: class_32/field_5 I class_32/field5Ns0Rename I -MD: class_32/method_2 (I)V class_32/method2Ns0Rename ()I -MD: class_32/method_5 (Lcls;[I)[[B class_32/method5Ns0Rename ()I +MD: class_32/method_2 (I)V class_32/method2Ns0Rename (I)V +MD: class_32/method_5 (Lcls;[I)[[B class_32/method5Ns0Rename (Lcls;[I)[[B diff --git a/src/test/resources/read/valid/migration-map.xml b/src/test/resources/read/valid/migration-map.xml index 161a8c9d..4266c8a6 100644 --- a/src/test/resources/read/valid/migration-map.xml +++ b/src/test/resources/read/valid/migration-map.xml @@ -1,7 +1,8 @@ - - - + + + + + - diff --git a/src/test/resources/read/valid/tinyV2.tiny b/src/test/resources/read/valid/tinyV2.tiny index c681c7d2..347970de 100644 --- a/src/test/resources/read/valid/tinyV2.tiny +++ b/src/test/resources/read/valid/tinyV2.tiny @@ -1,4 +1,5 @@ tiny 2 0 source target target2 + name valid c class_1 class1Ns0Rename class1Ns1Rename f I field_1 field1Ns0Rename field1Ns1Rename m ()I method_1 method1Ns0Rename method1Ns1Rename