Fix up SrgMerger, hopefully for the last time

src/main/java/net/fabricmc/loom/configuration/providers/forge/FieldMigratedMappingsProvider.java

@@ -1,7 +1,7 @@
 /*
  * This file is part of fabric-loom, licensed under the MIT License (MIT).
  *
- * Copyright (c) 2021 FabricMC
+ * Copyright (c) 2021-2022 FabricMC
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -116,7 +116,7 @@ public class FieldMigratedMappingsProvider extends MappingsProviderImpl {
 		if (extension.shouldGenerateSrgTiny()) {
 			if (Files.notExists(rawTinyMappingsWithSrg) || isRefreshDeps()) {
 				// Merge tiny mappings with srg
-				SrgMerger.mergeSrg(project.getLogger(), getRawSrgFile(project), rawTinyMappings, rawTinyMappingsWithSrg, true);
+				SrgMerger.mergeSrg(getRawSrgFile(project), rawTinyMappings, rawTinyMappingsWithSrg, getMojmapSrgFileIfPossible(project), true);
 			}
 		}
 

src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingsProviderImpl.java

@@ -43,6 +43,7 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.function.Supplier;
 
+import com.google.common.base.Stopwatch;
 import com.google.common.base.Suppliers;
 import com.google.gson.JsonObject;
 import org.apache.tools.ant.util.StringUtils;
@@ -198,7 +199,9 @@ public class MappingsProviderImpl implements MappingsProvider, SharedService {
 		if (extension.shouldGenerateSrgTiny()) {
 			if (Files.notExists(tinyMappingsWithSrg) || isRefreshDeps()) {
 				// Merge tiny mappings with srg
-				SrgMerger.mergeSrg(project.getLogger(), getRawSrgFile(project), tinyMappings, tinyMappingsWithSrg, true);
+				Stopwatch stopwatch = Stopwatch.createStarted();
+				SrgMerger.mergeSrg(getRawSrgFile(project), tinyMappings, tinyMappingsWithSrg, getMojmapSrgFileIfPossible(project), true);
+				project.getLogger().info(":merged srg mappings in " + stopwatch.stop());
 			}
 
 			mappingTreeWithSrg = Suppliers.memoize(() -> readMappings(tinyMappingsWithSrg));
@@ -249,6 +252,15 @@ public class MappingsProviderImpl implements MappingsProvider, SharedService {
 		return extension.getSrgProvider().getSrg();
 	}
 
+	public Path getMojmapSrgFileIfPossible(Project project) {
+		try {
+			LoomGradleExtension extension = LoomGradleExtension.get(project);
+			return SrgProvider.getMojmapTsrg2(project, extension);
+		} catch (IOException e) {
+			throw new UncheckedIOException(e);
+		}
+	}
+
 	public void manipulateMappings(Project project, Path mappingsJar) throws IOException {
 	}
 

src/main/java/net/fabricmc/loom/util/srg/SrgMerger.java

@@ -1,7 +1,7 @@
 /*
  * This file is part of fabric-loom, licensed under the MIT License (MIT).
  *
- * Copyright (c) 2020-2021 FabricMC
+ * Copyright (c) 2020-2022 FabricMC
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -32,8 +32,6 @@ import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import com.google.common.base.Stopwatch;
-import org.gradle.api.logging.Logger;
 import org.jetbrains.annotations.Nullable;
 
 import net.fabricmc.loom.api.mappings.layered.MappingsNamespace;
@@ -41,14 +39,15 @@ import net.fabricmc.loom.util.function.CollectionUtil;
 import net.fabricmc.mappingio.FlatMappingVisitor;
 import net.fabricmc.mappingio.MappingReader;
 import net.fabricmc.mappingio.adapter.MappingNsRenamer;
+import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch;
 import net.fabricmc.mappingio.adapter.RegularAsFlatMappingVisitor;
+import net.fabricmc.mappingio.format.ProGuardReader;
 import net.fabricmc.mappingio.format.Tiny2Writer;
 import net.fabricmc.mappingio.format.TsrgReader;
 import net.fabricmc.mappingio.tree.MappingTree;
 import net.fabricmc.mappingio.tree.MappingTreeView;
 import net.fabricmc.mappingio.tree.MemoryMappingTree;
 
-// TODO: Check if #49 came back with the rewrite
 /**
  * Utilities for merging SRG mappings.
  *
@@ -60,14 +59,26 @@ public final class SrgMerger {
 	private final MemoryMappingTree output;
 	private final FlatMappingVisitor flatOutput;
 	private final boolean lenient;
+	private final @Nullable MemoryMappingTree extra;
 
-	private SrgMerger(Path srg, Path tiny, boolean lenient) throws IOException {
+	private SrgMerger(Path srg, Path tiny, @Nullable Path extraProguard, boolean lenient) throws IOException {
 		this.srg = readSrg(srg);
 		this.src = new MemoryMappingTree();
 		this.output = new MemoryMappingTree();
 		this.flatOutput = new RegularAsFlatMappingVisitor(output);
 		this.lenient = lenient;
 
+		if (extraProguard != null) {
+			this.extra = new MemoryMappingTree();
+
+			try (BufferedReader reader = Files.newBufferedReader(extraProguard)) {
+				MappingSourceNsSwitch nsSwitch = new MappingSourceNsSwitch(extra, "official");
+				ProGuardReader.read(reader, "named", "official", nsSwitch);
+			}
+		} else {
+			this.extra = null;
+		}
+
 		MappingReader.read(tiny, this.src);
 
 		if (!"official".equals(this.src.getSrcNamespace())) {
@@ -175,11 +186,30 @@ public final class SrgMerger {
 		if (tinyMethod != null) {
 			copyDstNames(dstNames, tinyMethod);
 		} else {
-			// Do not allow missing methods as these are typically subclass methods and cause issues where
-			// class B extends A, and overrides a method from the superclass. Then the subclass method
-			// DOES get a srg name but not a yarn/intermediary name.
-			// In the best case, they are only methods like <init> or toString which have the same name in every NS.
-			return;
+			if (srgMethod.getSrcName().equals(srgMethod.getDstName(0))) {
+				// These are only methods like <init> or toString which have the same name in every NS.
+				// We can safely ignore those.
+				return;
+			}
+
+			@Nullable MappingTree.MethodMapping fillMethod = null;
+
+			if (extra != null) {
+				MappingTree.MethodMapping extraMethod = extra.getMethod(srgClass.getSrcName(), srgMethod.getSrcName(), srgMethod.getSrcDesc());
+
+				if (extraMethod != null && extraMethod.getSrcName().equals(extraMethod.getDstName(0))) {
+					fillMethod = extraMethod;
+				}
+			}
+
+			if (fillMethod != null) {
+				fillMappings(dstNames, fillMethod);
+			} else {
+				// Do not allow missing methods as these are typically subclass methods and cause issues where
+				// class B extends A, and overrides a method from the superclass. Then the subclass method
+				// DOES get a srg name but not a yarn/intermediary name.
+				return;
+			}
 		}
 
 		flatOutput.visitMethod(srgClass.getSrcName(), srgMethod.getSrcName(), srgMethod.getSrcDesc(), dstNames);
@@ -194,26 +224,25 @@ public final class SrgMerger {
 	 * <p>If {@code lenient} is true, the merger will not error when encountering names not present
 	 * in the tiny mappings. Instead, the names will be filled from the {@code official} namespace.
 	 *
-	 * @param srg     the SRG file in .tsrg format
-	 * @param tiny    the tiny file
-	 * @param out     the output file, will be in tiny v2
-	 * @param lenient whether lenient mode is enabled
+	 * @param srg           the SRG file in .tsrg format
+	 * @param tiny          the tiny file
+	 * @param out           the output file, will be in tiny v2
+	 * @param extraProguard an extra Proguard obfuscation mappings file that will be used to determine
+	 *                      whether an unobfuscated name is needed
+	 * @param lenient       whether lenient mode is enabled
 	 * @throws IOException      if an IO error occurs while reading or writing the mappings
 	 * @throws MappingException if the input tiny tree's default namespace is not 'official'
 	 *                          or if an element mentioned in the SRG file does not have tiny mappings in non-lenient mode
 	 */
-	public static void mergeSrg(Logger logger, Path srg, Path tiny, Path out, boolean lenient)
+	public static void mergeSrg(Path srg, Path tiny, Path out, @Nullable Path extraProguard, boolean lenient)
 			throws IOException, MappingException {
-		Stopwatch stopwatch = Stopwatch.createStarted();
-		MemoryMappingTree tree = new SrgMerger(srg, tiny, lenient).merge();
+		MemoryMappingTree tree = new SrgMerger(srg, tiny, extraProguard, lenient).merge();
 
 		try (Tiny2Writer writer = new Tiny2Writer(Files.newBufferedWriter(out), false)) {
 			tree.accept(writer);
 		} catch (IOException e) {
 			e.printStackTrace();
 		}
-
-		logger.info(":merged srg mappings in " + stopwatch.stop());
 	}
 
 	private MemoryMappingTree readSrg(Path srg) throws IOException {

src/test/groovy/net/fabricmc/loom/test/unit/forge/SrgMergerTest.groovy

@@ -0,0 +1,73 @@
+/*
+ * This file is part of fabric-loom, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) 2022 FabricMC
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+package net.fabricmc.loom.test.unit.forge
+
+import net.fabricmc.loom.util.srg.SrgMerger
+import spock.lang.Specification
+import spock.lang.TempDir
+
+import java.nio.file.Files
+import java.nio.file.Path
+
+class SrgMergerTest extends Specification {
+    @TempDir
+    Path mappingsDir
+
+    def "test SrgMerger"() {
+        def srgInput = extractTempFile("srgInput.tsrg")
+        def tinyInput = extractTempFile("tinyInput.tiny")
+        def proguardInput = extractTempFile("proguard.txt")
+        def output = mappingsDir.resolve("output.tiny")
+        def expected = readTestData("expectedOutput.tiny")
+
+        when:
+            SrgMerger.mergeSrg(srgInput, tinyInput, output, proguardInput, true)
+
+        then:
+            Files.readAllLines(output) == expected
+    }
+
+    private InputStream openTestDataStream(String path) {
+        return getClass().getResourceAsStream("/forge/testSrg/$path")
+    }
+
+    private List<String> readTestData(String path) {
+        try (def input = openTestDataStream(path)) {
+            assert input != null
+            return input.readLines()
+        }
+    }
+
+    private Path extractTempFile(String path) {
+        def output = mappingsDir.resolve(path)
+
+        try (def input = openTestDataStream(path)) {
+            assert input != null
+            Files.copy(input, output)
+        }
+
+        return output
+    }
+}

src/test/resources/forge/testSrg/expectedOutput.tiny

@@ -0,0 +1,7 @@
+tiny	2	0	official	srg	intermediary	named
+c	a	test/FakeClass	class_1	test/SomeClass
+	f	I	a	f_1_	field_1	myInt
+	m	()I	a	m_1_	method_1	getMyInt
+c	b	test/FakeSubclass	class_2	test/SomeSubclass
+c	test/DeObfClass	test/DeObfClass	test/DeObfClass	test/DeObfClass
+	m	()V	iAmNotObfuscated	m_2_	iAmNotObfuscated	iAmNotObfuscated

src/test/resources/forge/testSrg/proguard.txt

@@ -0,0 +1,9 @@
+test.FakeClass -> a:
+    void <init>() -> <init>
+    int fakeField -> a
+    int getFakeField() -> a
+test.FakeSubclass -> b:
+    void <init>() -> <init>
+test.DeObfClass -> test.DeObfClass:
+    void <init>() -> <init>
+    void iAmNotObfuscated() -> iAmNotObfuscated

src/test/resources/forge/testSrg/srgInput.tsrg

@@ -0,0 +1,11 @@
+tsrg2 left right
+a test/FakeClass
+	<init> ()V <init>
+	a I f_1_
+	a ()I m_1_
+b test/FakeSubclass
+	<init> ()V <init>
+	a ()I m_1_
+test/DeObfClass test/DeObfClass
+	<init> ()V <init>
+	iAmNotObfuscated ()V m_2_

src/test/resources/forge/testSrg/tinyInput.tiny

@@ -0,0 +1,5 @@
+tiny	2	0	official	intermediary	named
+c	a	class_1	test/SomeClass
+	f	I	a	field_1	myInt
+	m	()I	a	method_1	getMyInt
+c	b	class_2	test/SomeSubclass