From 98d8f3767253a1a3308542c2e896cbf5f4382033 Mon Sep 17 00:00:00 2001 From: shartte Date: Sat, 18 Sep 2021 22:15:32 +0200 Subject: [PATCH 1/2] Fixes merging of mappings for cases like: class_1234 is mapped, but class_1234$1 is not. (#498) --- .../mappings/MappingsProviderImpl.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingsProviderImpl.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingsProviderImpl.java index 47165bfc..d2cc3d03 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingsProviderImpl.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/MappingsProviderImpl.java @@ -34,9 +34,11 @@ import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.util.Arrays; import java.util.Collections; import java.util.Objects; import java.util.function.Consumer; +import java.util.regex.Pattern; import com.google.common.base.Stopwatch; import com.google.common.net.UrlEscapers; @@ -66,6 +68,7 @@ import net.fabricmc.mappingio.adapter.MappingSourceNsSwitch; import net.fabricmc.mappingio.format.MappingFormat; import net.fabricmc.mappingio.format.Tiny2Reader; import net.fabricmc.mappingio.format.Tiny2Writer; +import net.fabricmc.mappingio.tree.MappingTree; import net.fabricmc.mappingio.tree.MemoryMappingTree; import net.fabricmc.stitch.Command; import net.fabricmc.stitch.commands.CommandProposeFieldNames; @@ -296,6 +299,8 @@ public class MappingsProviderImpl extends DependencyProvider implements Mappings Tiny2Reader.read(reader, tree); } + inheritMappedNamesOfEnclosingClasses(tree); + try (Tiny2Writer writer = new Tiny2Writer(Files.newBufferedWriter(out, StandardCharsets.UTF_8), false)) { tree.accept(writer); } @@ -303,6 +308,40 @@ public class MappingsProviderImpl extends DependencyProvider implements Mappings project.getLogger().info(":merged mappings in " + stopwatch.stop()); } + /** + * Searches the mapping tree for inner classes with no mapped name, whose enclosing classes have mapped names. + * Currently, Yarn does not export mappings for these inner classes. + */ + private void inheritMappedNamesOfEnclosingClasses(MemoryMappingTree tree) { + int intermediaryIdx = tree.getNamespaceId("intermediary"); + int namedIdx = tree.getNamespaceId("named"); + + // The tree does not have an index by intermediary names by default + tree.setIndexByDstNames(true); + + for (MappingTree.ClassMapping classEntry : tree.getClasses()) { + String intermediaryName = classEntry.getDstName(intermediaryIdx); + String namedName = classEntry.getDstName(namedIdx); + + if (intermediaryName.equals(namedName) && intermediaryName.contains("$")) { + String[] path = intermediaryName.split(Pattern.quote("$")); + int parts = path.length; + + for (int i = parts - 2; i >= 0; i--) { + String currentPath = String.join("$", Arrays.copyOfRange(path, 0, i + 1)); + String namedParentClass = tree.mapClassName(currentPath, intermediaryIdx, namedIdx); + + if (!namedParentClass.equals(currentPath)) { + classEntry.setDstName(namedParentClass + + "$" + String.join("$", Arrays.copyOfRange(path, i + 1, path.length)), + namedIdx); + break; + } + } + } + } + } + private MemoryMappingTree readIntermediaryTree() throws IOException { MemoryMappingTree tree = new MemoryMappingTree(); MappingNsCompleter nsCompleter = new MappingNsCompleter(tree, Collections.singletonMap(MappingsNamespace.NAMED.toString(), MappingsNamespace.INTERMEDIARY.toString()), true); From 256e61ce3c9cbebabb3cadb7f77e62e29adff09d Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sun, 19 Sep 2021 08:53:13 +0100 Subject: [PATCH 2/2] Rebuild record components from field names, improves decompile (#497) --- .../minecraft/MinecraftMappedProvider.java | 2 +- .../loom/util/RecordComponentFixVisitor.java | 71 +++++++++++++++++++ .../loom/util/TinyRemapperHelper.java | 22 +++++- .../resources/projects/java16/build.gradle | 2 +- .../projects/java16/gradle.properties | 6 +- 5 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/util/RecordComponentFixVisitor.java diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java index b5f175fa..70f344d0 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/MinecraftMappedProvider.java +++ b/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); diff --git a/src/main/java/net/fabricmc/loom/util/RecordComponentFixVisitor.java b/src/main/java/net/fabricmc/loom/util/RecordComponentFixVisitor.java new file mode 100644 index 00000000..14b26e20 --- /dev/null +++ b/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); + } +} diff --git a/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java b/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java index 77f710d1..7f593db5 100644 --- a/src/main/java/net/fabricmc/loom/util/TinyRemapperHelper.java +++ b/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(); } diff --git a/src/test/resources/projects/java16/build.gradle b/src/test/resources/projects/java16/build.gradle index dbf1f7e6..7372d08b 100644 --- a/src/test/resources/projects/java16/build.gradle +++ b/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 { diff --git a/src/test/resources/projects/java16/gradle.properties b/src/test/resources/projects/java16/gradle.properties index 52d27567..5c21b1cb 100644 --- a/src/test/resources/projects/java16/gradle.properties +++ b/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