Skip writing elements with no destination names where applicable

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
 - Fixed member mapping merging via tree-API in `MemoryMappingTree`
 - Fixed duplicate mapping definitions not being handled correctly in multiple readers

build.gradle

@@ -44,6 +44,7 @@ allprojects {
 	spotless {
 		java {
 			licenseHeaderFile(rootProject.file("HEADER")).yearSeparator(", ")
+			targetExclude 'src/test/java/net/fabricmc/mappingio/lib/**/*.java'
 		}
 	}
 
@@ -183,6 +184,16 @@ jar {
 	}
 }
 
+task generateTestMappings(type: JavaExec, dependsOn: testClasses) {
+	classpath sourceSets.test.runtimeClasspath
+	mainClass = 'net.fabricmc.mappingio.TestFileUpdater'
+}
+
+task updateTestMappings(type: 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 {
 	doFirst {

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");
 	}
 

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<MappingFlag> flags = visitor.getFlags();
 		MappingVisitor parentVisitor = null;
 

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";
+}

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;

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<MappingFlag> 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<String> 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;
 }

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<String> dstNamespaces) thr
 
 	@Override
 	public boolean visitClass(String srcName) throws IOException {
-		this.srcName = srcName;
+		clsSrcName = srcName;
 
 		return true;
 	}
 
 	@Override
 	public boolean visitField(String srcName, @Nullable String srcDesc) throws IOException {
-		this.srcName = srcName;
-		this.srcDesc = srcDesc;
+		memberSrcName = srcName;
+		memberSrcDesc = srcDesc;
 
 		return true;
 	}
 
 	@Override
 	public boolean visitMethod(String srcName, @Nullable String srcDesc) throws IOException {
-		this.srcName = srcName;
-		this.srcDesc = srcDesc;
+		memberSrcName = srcName;
+		memberSrcDesc = srcDesc;
 
 		return true;
 	}
@@ -143,26 +145,41 @@ public void visitDstName(MappedElementKind targetKind, int namespace, String nam
 
 	@Override
 	public boolean visitElementContent(MappedElementKind targetKind) throws IOException {
-		if (dstName == null) dstName = srcName;
+		if (targetKind == MappedElementKind.CLASS) {
+			if (dstName == null) {
+				classContentVisitPending = true;
+				return true;
+			}
+		} else {
+			if (dstName == null) {
+				return false;
+			} else if (classContentVisitPending) {
+				String memberDstName = dstName;
+				dstName = clsSrcName;
+				visitElementContent(MappedElementKind.CLASS);
+				classContentVisitPending = false;
+				dstName = memberDstName;
+			}
+		}
 
 		switch (targetKind) {
 		case CLASS:
-			writer.write(toJavaClassName(srcName));
+			writer.write(toJavaClassName(clsSrcName));
 			dstName = toJavaClassName(dstName) + ":";
 			break;
 		case FIELD:
 			writeIndent();
-			writer.write(toJavaType(srcDesc));
+			writer.write(toJavaType(memberSrcDesc));
 			writer.write(' ');
-			writer.write(srcName);
+			writer.write(memberSrcName);
 			break;
 		case METHOD:
 			writeIndent();
-			writer.write(toJavaType(srcDesc.substring(srcDesc.indexOf(')', 1) + 1)));
+			writer.write(toJavaType(memberSrcDesc.substring(memberSrcDesc.indexOf(')', 1) + 1)));
 			writer.write(' ');
-			writer.write(srcName);
+			writer.write(memberSrcName);
 			writer.write('(');
-			List<String> argTypes = extractArgumentTypes(srcDesc);
+			List<String> argTypes = extractArgumentTypes(memberSrcDesc);
 
 			for (int i = 0; i < argTypes.size(); i++) {
 				if (i > 0) {
@@ -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