diff --git a/src/main/java/net/fabricmc/loom/util/aw2at/Aw2At.java b/src/main/java/net/fabricmc/loom/util/aw2at/Aw2At.java index 06fd7e86..e18f5efd 100644 --- a/src/main/java/net/fabricmc/loom/util/aw2at/Aw2At.java +++ b/src/main/java/net/fabricmc/loom/util/aw2at/Aw2At.java @@ -37,6 +37,7 @@ import org.cadixdev.bombe.type.signature.MethodSignature; import org.gradle.api.Project; import org.gradle.api.tasks.SourceSet; import org.gradle.api.tasks.SourceSetContainer; +import org.jetbrains.annotations.VisibleForTesting; import net.fabricmc.accesswidener.AccessWidenerReader; import net.fabricmc.accesswidener.AccessWidenerVisitor; @@ -80,22 +81,28 @@ public final class Aw2At { remapJar.getAtAccessWideners().addAll(extension.getForge().getExtraAccessWideners()); } + /** + * Converts an access widener file to an access transform set. + * + * @param reader the reader that is used to read the AW + * @return the access transform set + */ public static AccessTransformSet toAccessTransformSet(BufferedReader reader) throws IOException { AccessTransformSet atSet = AccessTransformSet.create(); new AccessWidenerReader(new AccessWidenerVisitor() { @Override - public void visitClass(String name, AccessWidenerReader.AccessType access, boolean transitive) { + public void visitClass(String name, AccessWidenerReader.AccessType access, boolean transitive) { atSet.getOrCreateClass(name).merge(toAt(access)); } @Override - public void visitMethod(String owner, String name, String descriptor, AccessWidenerReader.AccessType access, boolean transitive) { + public void visitMethod(String owner, String name, String descriptor, AccessWidenerReader.AccessType access, boolean transitive) { atSet.getOrCreateClass(owner).mergeMethod(MethodSignature.of(name, descriptor), toAt(access)); } @Override - public void visitField(String owner, String name, String descriptor, AccessWidenerReader.AccessType access, boolean transitive) { + public void visitField(String owner, String name, String descriptor, AccessWidenerReader.AccessType access, boolean transitive) { atSet.getOrCreateClass(owner).mergeField(name, toAt(access)); } }).read(reader); @@ -103,16 +110,17 @@ public final class Aw2At { return atSet; } - private static AccessTransform toAt(AccessWidenerReader.AccessType access) { + @VisibleForTesting + public static AccessTransform toAt(AccessWidenerReader.AccessType access) { return switch (access) { - // FIXME: This behaviour doesn't match what the actual AW does for methods. + // This behaviour doesn't match what the actual AW does for methods. // - accessible makes the method final if it was private // - extendable makes the method protected if it was (package-)private - // Neither of these can be achieved with Forge ATs without using bytecode analysis. - // However, this might be good enough for us. (The effects only apply in prod.) + // It also ignores that mutable doesn't change the visibility (but AT disallows bare "-f" modifier). + // None of these can be achieved with Forge ATs without using bytecode analysis. + // However, this is good enough for us. (The "unwanted" effects to visibility only apply in prod.) case ACCESSIBLE -> AccessTransform.of(AccessChange.PUBLIC); - case EXTENDABLE -> AccessTransform.of(AccessChange.PUBLIC, ModifierChange.REMOVE); - case MUTABLE -> AccessTransform.of(ModifierChange.REMOVE); + case EXTENDABLE, MUTABLE -> AccessTransform.of(AccessChange.PUBLIC, ModifierChange.REMOVE); }; } } diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/forge/Aw2AtTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/forge/Aw2AtTest.groovy new file mode 100644 index 00000000..12d7707c --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/forge/Aw2AtTest.groovy @@ -0,0 +1,67 @@ +/* + * 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.accesswidener.AccessWidenerReader +import net.fabricmc.loom.util.aw2at.Aw2At +import org.cadixdev.at.AccessChange +import org.cadixdev.at.ModifierChange +import spock.lang.Specification + +class Aw2AtTest extends Specification { + def "test accessible"() { + when: + def at = Aw2At.toAt(AccessWidenerReader.AccessType.ACCESSIBLE) + + then: + at.access == AccessChange.PUBLIC + // AW makes previously private methods also final, but this cannot be replicated + // using Forge AT. + at.final == ModifierChange.NONE + } + + def "test extendable"() { + when: + def at = Aw2At.toAt(AccessWidenerReader.AccessType.EXTENDABLE) + + then: + // AW makes previously private methods protected and does not change the visibility + // of protected methods, but this cannot be replicated using Forge AT. + at.access == AccessChange.PUBLIC + at.final == ModifierChange.REMOVE + } + + def "test mutable"() { + when: + def at = Aw2At.toAt(AccessWidenerReader.AccessType.MUTABLE) + + then: + // The access change to public is needed because the Forge AT format cannot + // have a bare "-f" modifier. Reusing the previous visibility needs bytecode analysis, + // so this is good enough. + at.access == AccessChange.PUBLIC + at.final == ModifierChange.REMOVE + } +} diff --git a/src/test/resources/projects/forge/aw2At/expected.accesstransformer.cfg b/src/test/resources/projects/forge/aw2At/expected.accesstransformer.cfg index 493714c9..ed4649a7 100644 --- a/src/test/resources/projects/forge/aw2At/expected.accesstransformer.cfg +++ b/src/test/resources/projects/forge/aw2At/expected.accesstransformer.cfg @@ -1,2 +1,3 @@ public net.minecraft.world.level.GameRules$BooleanValue m_46250_(Z)Lnet/minecraft/world/level/GameRules$Type; +public-f net.minecraft.world.level.storage.loot.LootTable f_79108_ public-f net.minecraft.world.level.block.IronBarsBlock m_54217_(Lnet/minecraft/world/level/block/state/BlockState;Z)Z diff --git a/src/test/resources/projects/forge/aw2At/src/main/resources/my.accesswidener b/src/test/resources/projects/forge/aw2At/src/main/resources/my.accesswidener index a9babdd7..2f7216be 100644 --- a/src/test/resources/projects/forge/aw2At/src/main/resources/my.accesswidener +++ b/src/test/resources/projects/forge/aw2At/src/main/resources/my.accesswidener @@ -1,3 +1,4 @@ accessWidener v1 named accessible method net/minecraft/world/level/GameRules$BooleanValue create (Z)Lnet/minecraft/world/level/GameRules$Type; extendable method net/minecraft/world/level/block/IronBarsBlock attachsTo (Lnet/minecraft/world/level/block/state/BlockState;Z)Z +mutable field net/minecraft/world/level/storage/loot/LootTable paramSet Lnet/minecraft/world/level/storage/loot/parameters/LootContextParamSet;