Rebuild record components from field names, improves decompile (#497)

src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java

@@ -107,7 +107,7 @@ public class MinecraftMappedProvider extends DependencyProvider {
 
 			Files.deleteIfExists(output);
 
-			TinyRemapper remapper = TinyRemapperHelper.getTinyRemapper(getProject(), fromM, toM);
+			TinyRemapper remapper = TinyRemapperHelper.getTinyRemapper(getProject(), fromM, toM, true);
 
 			try (OutputConsumerPath outputConsumer = new OutputConsumerPath.Builder(output).build()) {
 				outputConsumer.addNonClassFiles(input);

src/main/java/net/fabricmc/loom/util/RecordComponentFixVisitor.java

@@ -0,0 +1,71 @@
+/*
+ * 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.util;
+
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.FieldVisitor;
+import org.objectweb.asm.RecordComponentVisitor;
+
+import net.fabricmc.mappingio.tree.MemoryMappingTree;
+
+public class RecordComponentFixVisitor extends ClassVisitor {
+	private final MemoryMappingTree mappings;
+	private final int intermediaryNsId;
+
+	private String owner;
+	private boolean hasExistingComponents = false;
+
+	public RecordComponentFixVisitor(ClassVisitor classVisitor, MemoryMappingTree mappings, int intermediaryNsId) {
+		super(Constants.ASM_VERSION, classVisitor);
+		this.mappings = mappings;
+		this.intermediaryNsId = intermediaryNsId;
+	}
+
+	@Override
+	public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) {
+		this.owner = name;
+
+		super.visit(version, access, name, signature, superName, interfaces);
+	}
+
+	@Override
+	public RecordComponentVisitor visitRecordComponent(String name, String descriptor, String signature) {
+		// Should never happen, but let's be safe
+		hasExistingComponents = true;
+
+		return super.visitRecordComponent(name, descriptor, signature);
+	}
+
+	@Override
+	public FieldVisitor visitField(int access, String name, String descriptor, String signature, Object value) {
+		String intermediaryName = mappings.getField(owner, name, descriptor).getName(intermediaryNsId);
+
+		if (!hasExistingComponents && intermediaryName != null && intermediaryName.startsWith("comp_")) {
+			super.visitRecordComponent(name, descriptor, signature);
+		}
+
+		return super.visitField(access, name, descriptor, signature, value);
+	}
+}

src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java

@@ -34,7 +34,9 @@ import com.google.common.collect.ImmutableMap;
 import org.gradle.api.Project;
 
 import net.fabricmc.loom.LoomGradleExtension;
+import net.fabricmc.loom.api.mappings.layered.MappingsNamespace;
 import net.fabricmc.mappingio.tree.MappingTree;
+import net.fabricmc.mappingio.tree.MemoryMappingTree;
 import net.fabricmc.tinyremapper.IMappingProvider;
 import net.fabricmc.tinyremapper.TinyRemapper;
 
@@ -57,14 +59,32 @@ public final class TinyRemapperHelper {
 	}
 
 	public static TinyRemapper getTinyRemapper(Project project, String fromM, String toM) throws IOException {
+		return getTinyRemapper(project, fromM, toM, false);
+	}
+
+	public static TinyRemapper getTinyRemapper(Project project, String fromM, String toM, boolean fixRecords) throws IOException {
 		LoomGradleExtension extension = LoomGradleExtension.get(project);
+		MemoryMappingTree mappingTree = extension.getMappingsProvider().getMappings();
+
+		if (fixRecords && !mappingTree.getSrcNamespace().equals(fromM)) {
+			throw new IllegalStateException("Mappings src namespace must match remap src namespace");
+		}
+
+		int intermediaryNsId = mappingTree.getNamespaceId(MappingsNamespace.INTERMEDIARY.toString());
 
 		return TinyRemapper.newRemapper()
-				.withMappings(create(extension.getMappingsProvider().getMappings(), fromM, toM, true))
+				.withMappings(create(mappingTree, fromM, toM, true))
 				.withMappings(out -> JSR_TO_JETBRAINS.forEach(out::acceptClass))
 				.renameInvalidLocals(true)
 				.rebuildSourceFilenames(true)
 				.invalidLvNamePattern(MC_LV_PATTERN)
+				.extraPreApplyVisitor((cls, next) -> {
+					if (fixRecords && !cls.isRecord() && "java/lang/Record".equals(cls.getSuperName())) {
+						return new RecordComponentFixVisitor(next, mappingTree, intermediaryNsId);
+					}
+
+					return next;
+				})
 				.build();
 	}
 

src/test/resources/projects/java16/build.gradle

@@ -15,7 +15,7 @@ dependencies {
 	minecraft "com.mojang:minecraft:${project.minecraft_version}"
 	mappings "net.fabricmc:yarn:${project.yarn_mappings}:v2"
 	modImplementation "net.fabricmc:fabric-loader:${project.loader_version}"
-	modImplementation "net.fabricmc.fabric-api:fabric-api:${project.fabric_version}"
+	//modImplementation "net.fabricmc.fabric-api:fabric-api:${project.fabric_version}"
 }
 
 tasks.withType(JavaCompile).configureEach {

src/test/resources/projects/java16/gradle.properties

@@ -1,8 +1,8 @@
 org.gradle.jvmargs=-Xmx1G
 
-minecraft_version=1.17.1
-yarn_mappings=1.17.1+build.29
-loader_version=0.11.6
+minecraft_version=21w37a
+yarn_mappings=21w37a+build.5
+loader_version=0.11.7
 fabric_version=0.37.1+1.17
 
 mod_version = 1.0.0