Add an option (enabled by default) to map synthetic field and method names from the official mojang mappings. (#538)

src/main/java/net/fabricmc/loom/api/mappings/layered/spec/LayeredMappingSpecBuilder.java

@@ -24,7 +24,10 @@
 
 package net.fabricmc.loom.api.mappings.layered.spec;
 
+import groovy.lang.Closure;
+import groovy.lang.DelegatesTo;
 import org.gradle.api.Action;
+import org.gradle.util.ConfigureUtil;
 import org.jetbrains.annotations.ApiStatus;
 
 /**
@@ -38,14 +41,34 @@ public interface LayeredMappingSpecBuilder {
 	LayeredMappingSpecBuilder addLayer(MappingsSpec<?> mappingSpec);
 
 	/**
-	 * Add a layer that uses the official mappings provided by Mojang.
+	 * Add a layer that uses the official mappings provided by Mojang with the default options.
 	 */
-	LayeredMappingSpecBuilder officialMojangMappings();
+	default LayeredMappingSpecBuilder officialMojangMappings() {
+		return officialMojangMappings(builder -> { });
+	}
+
+	/**
+	 * Configure and add a layer that uses the official mappings provided by Mojang.
+	 */
+	@SuppressWarnings("rawtypes")
+	default LayeredMappingSpecBuilder officialMojangMappings(@DelegatesTo(value = MojangMappingsSpecBuilder.class, strategy = Closure.DELEGATE_FIRST) Closure closure) {
+		return officialMojangMappings(mojangMappingsSpecBuilder -> ConfigureUtil.configure(closure, mojangMappingsSpecBuilder));
+	}
+
+	/**
+	 * Configure and add a layer that uses the official mappings provided by Mojang.
+	 */
+	LayeredMappingSpecBuilder officialMojangMappings(Action<MojangMappingsSpecBuilder> action);
 
 	default LayeredMappingSpecBuilder parchment(Object object) {
 		return parchment(object, parchmentMappingsSpecBuilder -> parchmentMappingsSpecBuilder.setRemovePrefix(true));
 	}
 
+	@SuppressWarnings("rawtypes")
+	default LayeredMappingSpecBuilder parchment(Object object, @DelegatesTo(value = ParchmentMappingsSpecBuilder.class, strategy = Closure.DELEGATE_FIRST) Closure closure) {
+		return parchment(object, parchmentMappingsSpecBuilder -> ConfigureUtil.configure(closure, parchmentMappingsSpecBuilder));
+	}
+
 	LayeredMappingSpecBuilder parchment(Object object, Action<ParchmentMappingsSpecBuilder> action);
 
 	/**

src/main/java/net/fabricmc/loom/api/mappings/layered/spec/MojangMappingsSpecBuilder.java

@@ -0,0 +1,36 @@
+/*
+ * This file is part of fabric-loom, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) 2021 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.api.mappings.layered.spec;
+
+public interface MojangMappingsSpecBuilder {
+	/**
+	 * When enabled synthetic fields and methods will be mapped to name specified in the official mojang mappings.
+	 *
+	 * <p>When disabled synthetic fields and methods will not be mapped leaving them with their intermediary name.
+	 */
+	MojangMappingsSpecBuilder setNameSyntheticMembers(boolean value);
+
+	boolean getNameSyntheticMembers();
+}

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

@@ -33,10 +33,11 @@ import org.gradle.api.Action;
 import net.fabricmc.loom.api.mappings.layered.spec.FileSpec;
 import net.fabricmc.loom.api.mappings.layered.spec.LayeredMappingSpecBuilder;
 import net.fabricmc.loom.api.mappings.layered.spec.MappingsSpec;
+import net.fabricmc.loom.api.mappings.layered.spec.MojangMappingsSpecBuilder;
 import net.fabricmc.loom.api.mappings.layered.spec.ParchmentMappingsSpecBuilder;
 import net.fabricmc.loom.configuration.providers.mappings.extras.signatures.SignatureFixesSpec;
 import net.fabricmc.loom.configuration.providers.mappings.intermediary.IntermediaryMappingsSpec;
-import net.fabricmc.loom.configuration.providers.mappings.mojmap.MojangMappingsSpec;
+import net.fabricmc.loom.configuration.providers.mappings.mojmap.MojangMappingsSpecBuilderImpl;
 import net.fabricmc.loom.configuration.providers.mappings.parchment.ParchmentMappingsSpecBuilderImpl;
 
 public class LayeredMappingSpecBuilderImpl implements LayeredMappingSpecBuilder {
@@ -49,8 +50,10 @@ public class LayeredMappingSpecBuilderImpl implements LayeredMappingSpecBuilder
 	}
 
 	@Override
-	public LayeredMappingSpecBuilder officialMojangMappings() {
+	public LayeredMappingSpecBuilder officialMojangMappings(Action<MojangMappingsSpecBuilder> action) {
-		return addLayer(new MojangMappingsSpec());
+		MojangMappingsSpecBuilderImpl builder = MojangMappingsSpecBuilderImpl.builder();
+		action.execute(builder);
+		return addLayer(builder.build());
 	}
 
 	@Override

src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingLayer.java

@@ -31,13 +31,15 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
+import java.util.regex.Pattern;
 
 import org.gradle.api.logging.Logger;
 
 import net.fabricmc.loom.api.mappings.layered.MappingLayer;
 import net.fabricmc.loom.api.mappings.layered.MappingsNamespace;
-import net.fabricmc.loom.configuration.providers.minecraft.MinecraftVersionMeta;
 import net.fabricmc.loom.configuration.providers.mappings.intermediary.IntermediaryMappingLayer;
+import net.fabricmc.loom.configuration.providers.mappings.utils.DstNameFilterMappingVisitor;
+import net.fabricmc.loom.configuration.providers.minecraft.MinecraftVersionMeta;
 import net.fabricmc.loom.util.HashedDownloadUtil;
 import net.fabricmc.mappingio.MappingVisitor;
 import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch;
@@ -45,8 +47,10 @@ import net.fabricmc.mappingio.format.ProGuardReader;
 
 public record MojangMappingLayer(MinecraftVersionMeta.Download clientDownload,
 									MinecraftVersionMeta.Download serverDownload,
-									Path workingDir,
+									Path workingDir, boolean nameSyntheticMembers,
 									Logger logger) implements MappingLayer {
+	private static final Pattern SYNTHETIC_NAME_PATTERN = Pattern.compile("^(access|this|val\\$this|lambda\\$.*)\\$[0-9]+$");
+
 	@Override
 	public void visit(MappingVisitor mappingVisitor) throws IOException {
 		Path clientMappings = workingDir().resolve("client.txt");
@@ -56,8 +60,11 @@ public record MojangMappingLayer(MinecraftVersionMeta.Download clientDownload,
 
 		printMappingsLicense(clientMappings);
 
+		// Filter out field names matching the pattern
+		DstNameFilterMappingVisitor nameFilter = new DstNameFilterMappingVisitor(mappingVisitor, SYNTHETIC_NAME_PATTERN);
+
 		// Make official the source namespace
-		MappingSourceNsSwitch nsSwitch = new MappingSourceNsSwitch(mappingVisitor, MappingsNamespace.OFFICIAL.toString());
+		MappingSourceNsSwitch nsSwitch = new MappingSourceNsSwitch(nameSyntheticMembers() ? mappingVisitor : nameFilter, MappingsNamespace.OFFICIAL.toString());
 
 		try (BufferedReader clientBufferedReader = Files.newBufferedReader(clientMappings, StandardCharsets.UTF_8);
 				BufferedReader serverBufferedReader = Files.newBufferedReader(serverMappings, StandardCharsets.UTF_8)) {

src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingsSpec.java

@@ -28,7 +28,7 @@ import net.fabricmc.loom.api.mappings.layered.MappingContext;
 import net.fabricmc.loom.api.mappings.layered.spec.MappingsSpec;
 import net.fabricmc.loom.configuration.providers.minecraft.MinecraftVersionMeta;
 
-public record MojangMappingsSpec() implements MappingsSpec<MojangMappingLayer> {
+public record MojangMappingsSpec(boolean nameSyntheticMembers) implements MappingsSpec<MojangMappingLayer> {
 	// Keys in dependency manifest
 	private static final String MANIFEST_CLIENT_MAPPINGS = "client_mappings";
 	private static final String MANIFEST_SERVER_MAPPINGS = "server_mappings";
@@ -45,6 +45,7 @@ public record MojangMappingsSpec() implements MappingsSpec<MojangMappingLayer> {
 				versionInfo.download(MANIFEST_CLIENT_MAPPINGS),
 				versionInfo.download(MANIFEST_SERVER_MAPPINGS),
 				context.workingDirectory("mojang"),
+				nameSyntheticMembers(),
 				context.getLogger()
 		);
 	}

src/main/java/net/fabricmc/loom/configuration/providers/mappings/mojmap/MojangMappingsSpecBuilderImpl.java

@@ -0,0 +1,54 @@
+/*
+ * This file is part of fabric-loom, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) 2021 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.configuration.providers.mappings.mojmap;
+
+import net.fabricmc.loom.api.mappings.layered.spec.MojangMappingsSpecBuilder;
+
+public class MojangMappingsSpecBuilderImpl implements MojangMappingsSpecBuilder {
+	// TODO 0.11 loom change default to false
+	private boolean nameSyntheticMembers = true;
+
+	private MojangMappingsSpecBuilderImpl() {
+	}
+
+	public static MojangMappingsSpecBuilderImpl builder() {
+		return new MojangMappingsSpecBuilderImpl();
+	}
+
+	@Override
+	public MojangMappingsSpecBuilder setNameSyntheticMembers(boolean value) {
+		nameSyntheticMembers = value;
+		return this;
+	}
+
+	@Override
+	public boolean getNameSyntheticMembers() {
+		return nameSyntheticMembers;
+	}
+
+	public MojangMappingsSpec build() {
+		return new MojangMappingsSpec(nameSyntheticMembers);
+	}
+}

src/main/java/net/fabricmc/loom/configuration/providers/mappings/utils/DstNameFilterMappingVisitor.java

@@ -0,0 +1,54 @@
+/*
+ * This file is part of fabric-loom, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) 2021 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.configuration.providers.mappings.utils;
+
+import java.io.IOException;
+import java.util.regex.Pattern;
+
+import net.fabricmc.mappingio.MappedElementKind;
+import net.fabricmc.mappingio.MappingVisitor;
+import net.fabricmc.mappingio.adapter.ForwardingMappingVisitor;
+
+/**
+ * Filters out method and field names based on the provided regex pattern.
+ */
+public class DstNameFilterMappingVisitor extends ForwardingMappingVisitor {
+	private final Pattern pattern;
+
+	public DstNameFilterMappingVisitor(MappingVisitor next, Pattern pattern) {
+		super(next);
+
+		this.pattern = pattern;
+	}
+
+	@Override
+	public void visitDstName(MappedElementKind targetKind, int namespace, String name) throws IOException {
+		if ((targetKind == MappedElementKind.FIELD || targetKind == MappedElementKind.METHOD) && pattern.matcher(name).matches()) {
+			return;
+		}
+
+		super.visitDstName(targetKind, namespace, name);
+	}
+}

src/test/groovy/net/fabricmc/loom/test/integration/MojangMappingsProjectTest.groovy

@@ -48,4 +48,30 @@ class MojangMappingsProjectTest extends Specification implements GradleProjectTe
 		where:
 			version << STANDARD_TEST_VERSIONS
 	}
+
+	@Unroll
+	def "mojang mappings without synthetic field names (gradle #version)"() {
+		setup:
+			def gradle = gradleProject(project: "minimalBase", version: version)
+
+			gradle.buildGradle << '''
+                dependencies {
+                    minecraft "com.mojang:minecraft:1.18-pre5"
+                    mappings loom.layered {
+						officialMojangMappings {
+							nameSyntheticMembers = false
+						}
+					}
+                }
+            '''
+
+		when:
+			def result = gradle.run(task: "build")
+
+		then:
+			result.task(":build").outcome == SUCCESS
+
+		where:
+			version << STANDARD_TEST_VERSIONS
+	}
 }

src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/LayeredMappingSpecBuilderTest.groovy

@@ -42,7 +42,7 @@ class LayeredMappingSpecBuilderTest extends Specification {
             }
             def layers = spec.layers()
         then:
-            spec.version == "layered+hash.961"
+            spec.version == "layered+hash.2192"
             layers.size() == 2
             layers[0].class == IntermediaryMappingsSpec
             layers[1].class == MojangMappingsSpec
@@ -57,7 +57,7 @@ class LayeredMappingSpecBuilderTest extends Specification {
             def layers = spec.layers()
             def parchment = layers[2] as ParchmentMappingsSpec
         then:
-            spec.version == "layered+hash.863714404"
+            spec.version == "layered+hash.863752565"
             layers.size() == 3
             layers[0].class == IntermediaryMappingsSpec
             layers[1].class == MojangMappingsSpec
@@ -77,7 +77,7 @@ class LayeredMappingSpecBuilderTest extends Specification {
             def layers = spec.layers()
             def parchment = layers[2] as ParchmentMappingsSpec
         then:
-            spec.version == "layered+hash.863714410"
+            spec.version == "layered+hash.863752571"
             layers.size() == 3
             layers[0].class == IntermediaryMappingsSpec
             layers[1].class == MojangMappingsSpec
@@ -97,7 +97,7 @@ class LayeredMappingSpecBuilderTest extends Specification {
             def layers = spec.layers()
             def parchment = layers[2] as ParchmentMappingsSpec
         then:
-            spec.version == "layered+hash.1144465487"
+            spec.version == "layered+hash.1144427326"
             layers.size() == 3
             layers[0].class == IntermediaryMappingsSpec
             layers[1].class == MojangMappingsSpec

src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/MojangMappingLayerTest.groovy

@@ -25,17 +25,17 @@
 package net.fabricmc.loom.test.unit.layeredmappings
 
 import net.fabricmc.loom.configuration.providers.mappings.intermediary.IntermediaryMappingsSpec
-import net.fabricmc.loom.configuration.providers.mappings.mojmap.MojangMappingsSpec
+import net.fabricmc.loom.configuration.providers.mappings.mojmap.MojangMappingsSpecBuilderImpl
 
 class MojangMappingLayerTest extends LayeredMappingsSpecification {
-    def "Read mojang mappings" () {
+    def "Read mojang mappings with synthetic field names" () {
         setup:
             mockMappingsProvider.intermediaryTinyFile() >> extractFileFromZip(downloadFile(INTERMEDIARY_1_17_URL, "intermediary.jar"), "mappings/mappings.tiny")
             mockMinecraftProvider.getVersionInfo() >> VERSION_META_1_17
         when:
             def mappings = getLayeredMappings(
                     new IntermediaryMappingsSpec(),
-                    new MojangMappingsSpec()
+                    buildMojangMappingsSpec(true)
             )
             def tiny = getTiny(mappings)
         then:
@@ -45,5 +45,32 @@ class MojangMappingLayerTest extends LayeredMappingsSpecification {
             mappings.classes[0].srcName.hashCode() == 1869546970 // MojMap name, just check the hash
             mappings.classes[0].getDstName(0) == "net/minecraft/class_2354"
             mappings.classes[0].methods[0].args.size() == 0 // No Args
+            tiny.contains('this$0')
+    }
+
+    def "Read mojang mappings without synthetic field names" () {
+        setup:
+            mockMappingsProvider.intermediaryTinyFile() >> extractFileFromZip(downloadFile(INTERMEDIARY_1_17_URL, "intermediary.jar"), "mappings/mappings.tiny")
+            mockMinecraftProvider.getVersionInfo() >> VERSION_META_1_17
+        when:
+            def mappings = getLayeredMappings(
+                    new IntermediaryMappingsSpec(),
+                    buildMojangMappingsSpec(false)
+            )
+            def tiny = getTiny(mappings)
+        then:
+            mappings.srcNamespace == "named"
+            mappings.dstNamespaces == ["intermediary", "official"]
+            mappings.classes.size() == 6113
+            mappings.classes[0].srcName.hashCode() == 1869546970 // MojMap name, just check the hash
+            mappings.classes[0].getDstName(0) == "net/minecraft/class_2354"
+            mappings.classes[0].methods[0].args.size() == 0 // No Args
+            !tiny.contains('this$0')
+    }
+
+    static def buildMojangMappingsSpec(boolean nameSyntheticFields) {
+        def builder = MojangMappingsSpecBuilderImpl.builder()
+        builder.setNameSyntheticMembers(nameSyntheticFields)
+        return builder.build()
     }
 }

src/test/groovy/net/fabricmc/loom/test/unit/layeredmappings/ParchmentMappingLayerTest.groovy

@@ -38,7 +38,7 @@ class ParchmentMappingLayerTest extends LayeredMappingsSpecification {
             withMavenFile(PARCHMENT_NOTATION, downloadFile(PARCHMENT_URL, "parchment.zip"))
             def mappings = getLayeredMappings(
                     new IntermediaryMappingsSpec(),
-                    new MojangMappingsSpec(),
+                    new MojangMappingsSpec(true),
                     new ParchmentMappingsSpec(FileSpec.create(PARCHMENT_NOTATION), false)
             )
             def tiny = getTiny(mappings)
@@ -61,7 +61,7 @@ class ParchmentMappingLayerTest extends LayeredMappingsSpecification {
             withMavenFile(PARCHMENT_NOTATION, downloadFile(PARCHMENT_URL, "parchment.zip"))
             def mappings = getLayeredMappings(
                     new IntermediaryMappingsSpec(),
-                    new MojangMappingsSpec(),
+                    new MojangMappingsSpec(true),
                     new ParchmentMappingsSpec(FileSpec.create(PARCHMENT_NOTATION), true)
             )
             def tiny = getTiny(mappings)