From 5f513b0efc785e957616269b564bd7a9bc79b6af Mon Sep 17 00:00:00 2001 From: modmuss Date: Wed, 1 Oct 2025 11:27:14 +0100 Subject: [PATCH 01/25] Add basic unit tests for SpecContext (#1377) --- .../processors/SpecContextImpl.java | 107 +++----- .../processors/SpecContextProjectView.java | 94 +++++++ .../loom/test/unit/SpecContextTest.groovy | 239 ++++++++++++++++++ .../loom/test/util/GradleTestUtil.groovy | 16 +- 4 files changed, 384 insertions(+), 72 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/configuration/processors/SpecContextProjectView.java create mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/SpecContextTest.groovy diff --git a/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java index 057758a7..421b37c4 100644 --- a/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java +++ b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java @@ -38,22 +38,18 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import org.gradle.api.Project; -import org.gradle.api.artifacts.Configuration; -import org.gradle.api.artifacts.ProjectDependency; -import org.gradle.api.attributes.Usage; import org.gradle.api.plugins.JavaPlugin; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.api.RemapConfigurationSettings; import net.fabricmc.loom.api.processor.SpecContext; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftSourceSets; import net.fabricmc.loom.util.AsyncCache; -import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.fmj.FabricModJson; import net.fabricmc.loom.util.fmj.FabricModJsonFactory; import net.fabricmc.loom.util.fmj.FabricModJsonHelpers; -import net.fabricmc.loom.util.gradle.GradleUtils; /** * @param modDependencies External mods that are depended on @@ -65,20 +61,24 @@ public record SpecContextImpl( List localMods, List compileRuntimeMods) implements SpecContext { public static SpecContextImpl create(Project project) { - AsyncCache> fmjCache = new AsyncCache>(); + return create(new SpecContextProjectView.Impl(project, LoomGradleExtension.get(project))); + } + + @VisibleForTesting + public static SpecContextImpl create(SpecContextProjectView projectView) { + AsyncCache> fmjCache = new AsyncCache<>(); return new SpecContextImpl( - getDependentMods(project, fmjCache), - FabricModJsonHelpers.getModsInProject(project), - getCompileRuntimeMods(project, fmjCache) + getDependentMods(projectView, fmjCache), + projectView.getMods(), + getCompileRuntimeMods(projectView, fmjCache) ); } // Reruns a list of mods found on both the compile and/or runtime classpaths - private static List getDependentMods(Project project, AsyncCache> fmjCache) { - final LoomGradleExtension extension = LoomGradleExtension.get(project); + private static List getDependentMods(SpecContextProjectView projectView, AsyncCache> fmjCache) { var futures = new ArrayList>>(); - for (RemapConfigurationSettings entry : extension.getRemapConfigurations()) { + for (RemapConfigurationSettings entry : projectView.extension().getRemapConfigurations()) { final Set artifacts = entry.getSourceConfiguration().get().resolve(); for (File artifact : artifacts) { @@ -90,10 +90,9 @@ public record SpecContextImpl( } } - // TODO provide a project isolated way of doing this. - if (!extension.isProjectIsolationActive() && !GradleUtils.getBooleanProperty(project, Constants.Properties.DISABLE_PROJECT_DEPENDENT_MODS)) { + if (!projectView.disableProjectDependantMods()) { // Add all the dependent projects - for (Project dependentProject : getDependentProjects(project).toList()) { + for (Project dependentProject : getDependentProjects(projectView).toList()) { futures.add(fmjCache.get(dependentProject.getPath(), () -> FabricModJsonHelpers.getModsInProject(dependentProject))); } } @@ -101,19 +100,19 @@ public record SpecContextImpl( return sorted(AsyncCache.joinList(futures)); } - private static Stream getDependentProjects(Project project) { - final Stream runtimeProjects = getLoomProjectDependencies(project, project.getConfigurations().getByName(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME)); - final Stream compileProjects = getLoomProjectDependencies(project, project.getConfigurations().getByName(JavaPlugin.COMPILE_CLASSPATH_CONFIGURATION_NAME)); + private static Stream getDependentProjects(SpecContextProjectView projectView) { + final Stream runtimeProjects = projectView.getLoomProjectDependencies(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME); + final Stream compileProjects = projectView.getLoomProjectDependencies(JavaPlugin.COMPILE_CLASSPATH_CONFIGURATION_NAME); return Stream.concat(runtimeProjects, compileProjects) .distinct(); } // Returns a list of mods that are on both to compile and runtime classpath - private static List getCompileRuntimeMods(Project project, AsyncCache> fmjCache) { - var mods = new ArrayList<>(getCompileRuntimeModsFromRemapConfigs(project, fmjCache)); + private static List getCompileRuntimeMods(SpecContextProjectView projectView, AsyncCache> fmjCache) { + var mods = new ArrayList<>(getCompileRuntimeModsFromRemapConfigs(projectView, fmjCache)); - for (Project dependentProject : getCompileRuntimeProjectDependencies(project).toList()) { + for (Project dependentProject : getCompileRuntimeProjectDependencies(projectView).toList()) { List projectMods = fmjCache.getBlocking(dependentProject.getPath(), () -> { return FabricModJsonHelpers.getModsInProject(dependentProject); }); @@ -127,49 +126,49 @@ public record SpecContextImpl( } // Returns a list of jar mods that are found on the compile and runtime remapping configurations - private static List getCompileRuntimeModsFromRemapConfigs(Project project, AsyncCache> fmjCache) { - final LoomGradleExtension extension = LoomGradleExtension.get(project); - + private static List getCompileRuntimeModsFromRemapConfigs(SpecContextProjectView projectView, AsyncCache> fmjCache) { // A set of mod ids from all remap configurations that are considered for dependency transforms. final Set runtimeModIds = getModIds( - project, + projectView, fmjCache, - extension.getRuntimeRemapConfigurations().stream() + projectView.extension().getRuntimeRemapConfigurations().stream() .filter(settings -> settings.getApplyDependencyTransforms().get()) ); // A set of mod ids that are found on one or more remap configurations that target the common source set. // Null when split source sets are not enabled, meaning all mods are common. - final Set commonModIds = extension.areEnvironmentSourceSetsSplit() ? getModIds( - project, + final Set commonRuntimeModIds = projectView.extension().areEnvironmentSourceSetsSplit() ? getModIds( + projectView, fmjCache, - extension.getRuntimeRemapConfigurations().stream() + projectView.extension().getRuntimeRemapConfigurations().stream() .filter(settings -> settings.getSourceSet().map(sourceSet -> !sourceSet.getName().equals(MinecraftSourceSets.Split.CLIENT_ONLY_SOURCE_SET_NAME)).get()) .filter(settings -> settings.getApplyDependencyTransforms().get())) : null; - return getMods( - project, + Stream compileMods = getMods( + projectView, fmjCache, - extension.getCompileRemapConfigurations().stream() - .filter(settings -> settings.getApplyDependencyTransforms().get())) + projectView.extension().getCompileRemapConfigurations().stream() + .filter(settings -> settings.getApplyDependencyTransforms().get())); + + return compileMods // Only check based on the modid, as there may be differing versions used between the compile and runtime classpath. // We assume that the version used at runtime will be binary compatible with the version used to compile against. // It's not perfect but better than silently not supplying the mod, and this could happen with regular API that you compile against anyway. .filter(fabricModJson -> runtimeModIds.contains(fabricModJson.getId())) .sorted(Comparator.comparing(FabricModJson::getId)) - .map(fabricModJson -> new ModHolder(fabricModJson, commonModIds == null || commonModIds.contains(fabricModJson.getId()))) + .map(fabricModJson -> new ModHolder(fabricModJson, commonRuntimeModIds == null || commonRuntimeModIds.contains(fabricModJson.getId()))) .toList(); } - private static Stream getMods(Project project, AsyncCache> fmjCache, Stream stream) { - return stream.flatMap(resolveArtifacts(project, true)) + private static Stream getMods(SpecContextProjectView projectView, AsyncCache> fmjCache, Stream stream) { + return stream.flatMap(projectView.resolveArtifacts(true)) .map(modFromZip(fmjCache)) .filter(Objects::nonNull); } - private static Set getModIds(Project project, AsyncCache> fmjCache, Stream stream) { - return getMods(project, fmjCache, stream) + private static Set getModIds(SpecContextProjectView projectView, AsyncCache> fmjCache, Stream stream) { + return getMods(projectView, fmjCache, stream) .map(FabricModJson::getId) .collect(Collectors.toSet()); } @@ -185,43 +184,19 @@ public record SpecContextImpl( }; } - private static Function> resolveArtifacts(Project project, boolean runtime) { - final Usage usage = project.getObjects().named(Usage.class, runtime ? Usage.JAVA_RUNTIME : Usage.JAVA_API); - - return settings -> { - final Configuration configuration = settings.getSourceConfiguration().get().copyRecursive(); - configuration.setCanBeConsumed(false); - configuration.attributes(attributes -> attributes.attribute(Usage.USAGE_ATTRIBUTE, usage)); - return configuration.resolve().stream().map(File::toPath); - }; - } - // Returns a list of Loom Projects found in both the runtime and compile classpath - private static Stream getCompileRuntimeProjectDependencies(Project project) { - final LoomGradleExtension extension = LoomGradleExtension.get(project); - - // TODO provide a project isolated way of doing this. - if (extension.isProjectIsolationActive() - || GradleUtils.getBooleanProperty(project, Constants.Properties.DISABLE_PROJECT_DEPENDENT_MODS)) { + private static Stream getCompileRuntimeProjectDependencies(SpecContextProjectView projectView) { + if (projectView.disableProjectDependantMods()) { return Stream.empty(); } - final Stream runtimeProjects = getLoomProjectDependencies(project, project.getConfigurations().getByName(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME)); - final List compileProjects = getLoomProjectDependencies(project, project.getConfigurations().getByName(JavaPlugin.COMPILE_CLASSPATH_CONFIGURATION_NAME)).toList(); + final Stream runtimeProjects = projectView.getLoomProjectDependencies(JavaPlugin.RUNTIME_CLASSPATH_CONFIGURATION_NAME); + final List compileProjects = projectView.getLoomProjectDependencies(JavaPlugin.COMPILE_CLASSPATH_CONFIGURATION_NAME).toList(); return runtimeProjects .filter(compileProjects::contains); // Use the intersection of the two configurations. } - // Returns a list of Loom Projects found in the provided Configuration - private static Stream getLoomProjectDependencies(Project project, Configuration configuration) { - return configuration.getAllDependencies() - .withType(ProjectDependency.class) - .stream() - .map((d) -> project.project(d.getPath())) - .filter(GradleUtils::isLoomProject); - } - // Sort to ensure stable caching private static List sorted(List mods) { return mods.stream().sorted(Comparator.comparing(FabricModJson::getId)).toList(); diff --git a/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextProjectView.java b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextProjectView.java new file mode 100644 index 00000000..c8686a20 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextProjectView.java @@ -0,0 +1,94 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processors; + +import java.io.File; +import java.nio.file.Path; +import java.util.List; +import java.util.function.Function; +import java.util.stream.Stream; + +import org.gradle.api.Project; +import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.ProjectDependency; +import org.gradle.api.attributes.Usage; + +import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.api.RemapConfigurationSettings; +import net.fabricmc.loom.util.Constants; +import net.fabricmc.loom.util.fmj.FabricModJson; +import net.fabricmc.loom.util.fmj.FabricModJsonHelpers; +import net.fabricmc.loom.util.gradle.GradleUtils; + +// Used to abstract out the Gradle API usage to ease unit testing. +public interface SpecContextProjectView { + LoomGradleExtension extension(); + + // Returns a list of Loom Projects found in the specified Configuration + Stream getLoomProjectDependencies(String name); + + Function> resolveArtifacts(boolean runtime); + + List getMods(); + + boolean disableProjectDependantMods(); + + record Impl(Project project, LoomGradleExtension extension) implements SpecContextProjectView { + @Override + public Stream getLoomProjectDependencies(String name) { + final Configuration configuration = project.getConfigurations().getByName(name); + return configuration.getAllDependencies() + .withType(ProjectDependency.class) + .stream() + .map((d) -> project.project(d.getPath())) + .filter(GradleUtils::isLoomProject); + } + + @Override + public Function> resolveArtifacts(boolean runtime) { + final Usage usage = project.getObjects().named(Usage.class, runtime ? Usage.JAVA_RUNTIME : Usage.JAVA_API); + + return settings -> { + final Configuration configuration = settings.getSourceConfiguration().get().copyRecursive(); + configuration.setCanBeConsumed(false); + configuration.attributes(attributes -> attributes.attribute(Usage.USAGE_ATTRIBUTE, usage)); + return configuration.resolve().stream().map(File::toPath); + }; + } + + @Override + public List getMods() { + return FabricModJsonHelpers.getModsInProject(project); + } + + @Override + public boolean disableProjectDependantMods() { + final LoomGradleExtension extension = LoomGradleExtension.get(project); + // TODO provide a project isolated way of doing this. + return extension.isProjectIsolationActive() + || GradleUtils.getBooleanProperty(project, Constants.Properties.DISABLE_PROJECT_DEPENDENT_MODS); + } + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/SpecContextTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/SpecContextTest.groovy new file mode 100644 index 00000000..3a07991d --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/SpecContextTest.groovy @@ -0,0 +1,239 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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 + +import java.nio.file.Path +import java.util.function.Function +import java.util.stream.Stream + +import groovy.transform.CompileStatic +import org.gradle.api.NamedDomainObjectList +import org.gradle.api.NamedDomainObjectProvider +import org.gradle.api.Project +import org.gradle.api.artifacts.Configuration +import spock.lang.Specification +import spock.lang.TempDir + +import net.fabricmc.loom.LoomGradleExtension +import net.fabricmc.loom.api.RemapConfigurationSettings +import net.fabricmc.loom.api.fmj.FabricModJsonV1Spec +import net.fabricmc.loom.configuration.processors.SpecContextImpl +import net.fabricmc.loom.configuration.processors.SpecContextProjectView +import net.fabricmc.loom.test.util.GradleTestUtil +import net.fabricmc.loom.util.ZipUtils +import net.fabricmc.loom.util.fmj.gen.FabricModJsonV1Generator + +import static org.mockito.Mockito.mock +import static org.mockito.Mockito.when + +@SuppressWarnings('ExplicitCallToModMethod') +class SpecContextTest extends Specification { + @TempDir + Path tempDir + + Project project + LoomGradleExtension extension + SpecContextProjectView projectView + NamedDomainObjectList remapConfigurations + + RemapConfigurationSettings implementation + RemapConfigurationSettings runtimeOnly + RemapConfigurationSettings compileOnly + + Map> runtimeArtifacts = [:] + Map> apiArtifacts = [:] + + void setup() { + project = GradleTestUtil.mockProject() + extension = LoomGradleExtension.get(project) + projectView = mock(SpecContextProjectView.class) + remapConfigurations = project.getObjects().namedDomainObjectList(RemapConfigurationSettings.class) + + when(projectView.extension()).thenReturn(extension) + when(extension.getRemapConfigurations()).thenReturn(remapConfigurations) + when(projectView.resolveArtifacts(true)).thenReturn(resolve(runtimeArtifacts)) + when(projectView.resolveArtifacts(false)).thenReturn(resolve(apiArtifacts)) + + implementation = createConfigurationSettings("implementation") + runtimeOnly = createConfigurationSettings("runtimeOnly") + compileOnly = createConfigurationSettings("compileOnly") + remapConfigurations.addAll([ + implementation, + runtimeOnly, + compileOnly + ]) + + when(extension.getCompileRemapConfigurations()).thenReturn([implementation, compileOnly]) + when(extension.getRuntimeRemapConfigurations()).thenReturn([implementation, runtimeOnly]) + } + + def "Empty"() { + setup: + dependencies( + implementation: [], + runtimeOnly: [], + compileOnly: [] + ) + + when: + def specContext = SpecContextImpl.create(projectView) + + then: + specContext.modDependencies().size() == 0 + specContext.localMods().size() == 0 + specContext.modDependenciesCompileRuntime().size() == 0 + specContext.modDependenciesCompileRuntimeClient().size() == 0 + specContext.allMods().size() == 0 + } + + def "implementation dependency"() { + setup: + dependencies( + implementation: [mod("test1")], + runtimeOnly: [], + compileOnly: [] + ) + + when: + def specContext = SpecContextImpl.create(projectView) + + then: + specContext.modDependencies().size() == 1 + specContext.localMods().size() == 0 + specContext.modDependenciesCompileRuntime().size() == 1 + specContext.modDependenciesCompileRuntimeClient().size() == 0 + specContext.allMods().size() == 1 + } + + def "runtime only dependency"() { + setup: + dependencies( + implementation: [], + runtimeOnly: [mod("test1")], + compileOnly: [] + ) + + when: + def specContext = SpecContextImpl.create(projectView) + + then: + specContext.modDependencies().size() == 1 + specContext.localMods().size() == 0 + specContext.modDependenciesCompileRuntime().size() == 0 + specContext.modDependenciesCompileRuntimeClient().size() == 0 + specContext.allMods().size() == 1 + } + + def "compile only dependency"() { + setup: + dependencies( + implementation: [], + runtimeOnly: [], + compileOnly: [mod("test1")] + ) + + when: + def specContext = SpecContextImpl.create(projectView) + + then: + specContext.modDependencies().size() == 1 + specContext.localMods().size() == 0 + specContext.modDependenciesCompileRuntime().size() == 0 + specContext.modDependenciesCompileRuntimeClient().size() == 0 + specContext.allMods().size() == 1 + } + + // TODO I believe this test is testing broken behaviour + def "compile only runtime only dependency"() { + setup: + def test1 = mod("test1") + dependencies( + implementation: [], + runtimeOnly: [test1], + compileOnly: [test1] + ) + + when: + def specContext = SpecContextImpl.create(projectView) + + then: + specContext.modDependencies().size() == 2 + specContext.localMods().size() == 0 + specContext.modDependenciesCompileRuntime().size() == 0 + specContext.modDependenciesCompileRuntimeClient().size() == 0 + specContext.allMods().size() == 2 + } + + private void dependencies(Map> files) { + configureDependencies(files.implementation, this.implementation) + configureDependencies(files.runtimeOnly, this.runtimeOnly) + configureDependencies(files.compileOnly, this.compileOnly) + + runtimeArtifacts[this.implementation].addAll(files.implementation) + runtimeArtifacts[this.runtimeOnly].addAll(files.runtimeOnly) + apiArtifacts[this.implementation].addAll(files.implementation) + apiArtifacts[this.compileOnly].addAll(files.compileOnly) + } + + private static void configureDependencies(List files, RemapConfigurationSettings settings) { + def configuration = mock(Configuration.class) + when(configuration.resolve()).thenReturn(files*.toFile() as Set) + + def provider = mock(NamedDomainObjectProvider.class) + when(provider.get()).thenReturn(configuration) + + when(settings.getSourceConfiguration()).thenReturn(provider) + } + + private Path mod(String modId) { + def zip = tempDir.resolve("${modId}.zip") + + def spec = project.objects.newInstance(FabricModJsonV1Spec.class) + spec.modId.set(modId) + spec.version.set("1.0.0") + def json = FabricModJsonV1Generator.INSTANCE.generate(spec) + ZipUtils.add(zip, "fabric.mod.json", json) + + return zip + } + + private RemapConfigurationSettings createConfigurationSettings(String name) { + def settings = project.getObjects().newInstance(RemapConfigurationSettings.class, name) + settings.applyDependencyTransforms.set(true) + + runtimeArtifacts.put(settings, []) + apiArtifacts.put(settings, []) + + return settings + } + + @CompileStatic + private static Function> resolve(Map> artifacts) { + return { settings -> + def paths = artifacts.get(settings) + return paths != null ? paths.stream() : Stream.empty() + } + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/util/GradleTestUtil.groovy b/src/test/groovy/net/fabricmc/loom/test/util/GradleTestUtil.groovy index 264d3cd8..c1faaad2 100644 --- a/src/test/groovy/net/fabricmc/loom/test/util/GradleTestUtil.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/util/GradleTestUtil.groovy @@ -25,6 +25,7 @@ package net.fabricmc.loom.test.util import org.gradle.api.Project +import org.gradle.api.artifacts.ConfigurationContainer import org.gradle.api.artifacts.dsl.RepositoryHandler import org.gradle.api.file.RegularFile import org.gradle.api.file.RegularFileProperty @@ -47,6 +48,7 @@ import net.fabricmc.loom.util.download.Download import static org.mockito.ArgumentMatchers.any import static org.mockito.Mockito.* +import static org.mockito.Mockito.mock class GradleTestUtil { static Property mockProperty(T value) { @@ -64,17 +66,19 @@ class GradleTestUtil { } static Project mockProject() { - def mock = mock(Project.class) - def serviceRegistry = TestServiceFactory.createServiceRegistry(mock) + def project = mock(Project.class) + def serviceRegistry = TestServiceFactory.createServiceRegistry(project) def objectFactory = serviceRegistry.get(ObjectFactory) def providerFactory = serviceRegistry.get(ProviderFactory) def extensions = mockExtensionContainer() - when(mock.getExtensions()).thenReturn(extensions) - when(mock.getObjects()).thenReturn(objectFactory) - when(mock.provider(any())).thenAnswer { + def configurationContainer = mock(ConfigurationContainer.class) + when(project.getExtensions()).thenReturn(extensions) + when(project.getObjects()).thenReturn(objectFactory) + when(project.provider(any())).thenAnswer { providerFactory.provider(it.getArgument(0)) } - return mock + when(project.getConfigurations()).thenReturn(configurationContainer) + return project } static ExtensionContainer mockExtensionContainer() { From 103db759f617673553749d0e250da432b9c72bb6 Mon Sep 17 00:00:00 2001 From: modmuss Date: Wed, 1 Oct 2025 14:23:05 +0100 Subject: [PATCH 02/25] Fix SpecContext always resolving mod dependencies with the runtime attribute. (#1378) Likely fixes #1334 --- .../processors/SpecContextImpl.java | 20 ++++++++++++------- .../processors/SpecContextProjectView.java | 17 +++++++++++++--- .../loom/test/unit/SpecContextTest.groovy | 11 +++++----- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java index 421b37c4..0585746c 100644 --- a/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java +++ b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextImpl.java @@ -97,7 +97,7 @@ public record SpecContextImpl( } } - return sorted(AsyncCache.joinList(futures)); + return distinctSorted(AsyncCache.joinList(futures)); } private static Stream getDependentProjects(SpecContextProjectView projectView) { @@ -129,6 +129,7 @@ public record SpecContextImpl( private static List getCompileRuntimeModsFromRemapConfigs(SpecContextProjectView projectView, AsyncCache> fmjCache) { // A set of mod ids from all remap configurations that are considered for dependency transforms. final Set runtimeModIds = getModIds( + SpecContextProjectView.ArtifactUsage.RUNTIME, projectView, fmjCache, projectView.extension().getRuntimeRemapConfigurations().stream() @@ -138,6 +139,7 @@ public record SpecContextImpl( // A set of mod ids that are found on one or more remap configurations that target the common source set. // Null when split source sets are not enabled, meaning all mods are common. final Set commonRuntimeModIds = projectView.extension().areEnvironmentSourceSetsSplit() ? getModIds( + SpecContextProjectView.ArtifactUsage.RUNTIME, projectView, fmjCache, projectView.extension().getRuntimeRemapConfigurations().stream() @@ -146,6 +148,7 @@ public record SpecContextImpl( : null; Stream compileMods = getMods( + SpecContextProjectView.ArtifactUsage.COMPILE, projectView, fmjCache, projectView.extension().getCompileRemapConfigurations().stream() @@ -161,14 +164,14 @@ public record SpecContextImpl( .toList(); } - private static Stream getMods(SpecContextProjectView projectView, AsyncCache> fmjCache, Stream stream) { - return stream.flatMap(projectView.resolveArtifacts(true)) + private static Stream getMods(SpecContextProjectView.ArtifactUsage artifactUsage, SpecContextProjectView projectView, AsyncCache> fmjCache, Stream stream) { + return stream.flatMap(projectView.resolveArtifacts(artifactUsage)) .map(modFromZip(fmjCache)) .filter(Objects::nonNull); } - private static Set getModIds(SpecContextProjectView projectView, AsyncCache> fmjCache, Stream stream) { - return getMods(projectView, fmjCache, stream) + private static Set getModIds(SpecContextProjectView.ArtifactUsage artifactUsage, SpecContextProjectView projectView, AsyncCache> fmjCache, Stream stream) { + return getMods(artifactUsage, projectView, fmjCache, stream) .map(FabricModJson::getId) .collect(Collectors.toSet()); } @@ -198,8 +201,11 @@ public record SpecContextImpl( } // Sort to ensure stable caching - private static List sorted(List mods) { - return mods.stream().sorted(Comparator.comparing(FabricModJson::getId)).toList(); + private static List distinctSorted(List mods) { + return mods.stream() + .distinct() + .sorted(Comparator.comparing(FabricModJson::getId)) + .toList(); } @Override diff --git a/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextProjectView.java b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextProjectView.java index c8686a20..ee12e40f 100644 --- a/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextProjectView.java +++ b/src/main/java/net/fabricmc/loom/configuration/processors/SpecContextProjectView.java @@ -49,12 +49,23 @@ public interface SpecContextProjectView { // Returns a list of Loom Projects found in the specified Configuration Stream getLoomProjectDependencies(String name); - Function> resolveArtifacts(boolean runtime); + Function> resolveArtifacts(ArtifactUsage artifactUsage); List getMods(); boolean disableProjectDependantMods(); + enum ArtifactUsage { + RUNTIME(Usage.JAVA_RUNTIME), + COMPILE(Usage.JAVA_API); + + private final String gradleUsage; + + ArtifactUsage(String gradleUsage) { + this.gradleUsage = gradleUsage; + } + } + record Impl(Project project, LoomGradleExtension extension) implements SpecContextProjectView { @Override public Stream getLoomProjectDependencies(String name) { @@ -67,8 +78,8 @@ public interface SpecContextProjectView { } @Override - public Function> resolveArtifacts(boolean runtime) { - final Usage usage = project.getObjects().named(Usage.class, runtime ? Usage.JAVA_RUNTIME : Usage.JAVA_API); + public Function> resolveArtifacts(ArtifactUsage artifactUsage) { + final Usage usage = project.getObjects().named(Usage.class, artifactUsage.gradleUsage); return settings -> { final Configuration configuration = settings.getSourceConfiguration().get().copyRecursive(); diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/SpecContextTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/SpecContextTest.groovy index 3a07991d..ac966241 100644 --- a/src/test/groovy/net/fabricmc/loom/test/unit/SpecContextTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/unit/SpecContextTest.groovy @@ -73,8 +73,8 @@ class SpecContextTest extends Specification { when(projectView.extension()).thenReturn(extension) when(extension.getRemapConfigurations()).thenReturn(remapConfigurations) - when(projectView.resolveArtifacts(true)).thenReturn(resolve(runtimeArtifacts)) - when(projectView.resolveArtifacts(false)).thenReturn(resolve(apiArtifacts)) + when(projectView.resolveArtifacts(SpecContextProjectView.ArtifactUsage.RUNTIME)).thenReturn(resolve(runtimeArtifacts)) + when(projectView.resolveArtifacts(SpecContextProjectView.ArtifactUsage.COMPILE)).thenReturn(resolve(apiArtifacts)) implementation = createConfigurationSettings("implementation") runtimeOnly = createConfigurationSettings("runtimeOnly") @@ -165,7 +165,6 @@ class SpecContextTest extends Specification { specContext.allMods().size() == 1 } - // TODO I believe this test is testing broken behaviour def "compile only runtime only dependency"() { setup: def test1 = mod("test1") @@ -179,11 +178,11 @@ class SpecContextTest extends Specification { def specContext = SpecContextImpl.create(projectView) then: - specContext.modDependencies().size() == 2 + specContext.modDependencies().size() == 1 specContext.localMods().size() == 0 - specContext.modDependenciesCompileRuntime().size() == 0 + specContext.modDependenciesCompileRuntime().size() == 1 specContext.modDependenciesCompileRuntimeClient().size() == 0 - specContext.allMods().size() == 2 + specContext.allMods().size() == 1 } private void dependencies(Map> files) { From 7f95c3c60f3d570e4fc65b78416cfcbc6762e754 Mon Sep 17 00:00:00 2001 From: KikuGie Date: Fri, 3 Oct 2025 11:16:17 +0200 Subject: [PATCH 03/25] Make `fabric.mod.json` path configurable at configuration stage. (#1364) * Add `fabric.mod.json` path property. * Add path provider integration tests. * Separate fabric.mod.json reading methods and move selection to the helper method. * Separate FMJ getter methods and add configuration to datagen and testmod settings. * Remove the `fabricModJsonPath` property from Fabric API source sets * Address review requests by fixing formatting and removing redundant changes * Fix build * Move tests to existing file, and remove var usage. * Fix build :) --------- Co-authored-by: modmuss50 --- .../loom/api/LoomGradleExtensionAPI.java | 16 ++++++++ .../api/fabricapi/DataGenerationSettings.java | 11 ++++++ .../loom/api/fabricapi/GameTestSettings.java | 11 ++++++ .../fabricapi/FabricApiAbstractSourceSet.java | 16 +++----- .../extension/LoomGradleExtensionApiImpl.java | 26 +++++++------ .../loom/util/fmj/FabricModJsonFactory.java | 25 +++++++++---- .../loom/util/fmj/FabricModJsonHelpers.java | 28 ++++++++------ .../integration/InterfaceInjectionTest.groovy | 31 ++++++++++++++++ .../projects/fmjPathConfig/build.gradle | 37 +++++++++++++++++++ .../src/custom/resources/fabric.mod.json | 11 ++++++ .../src/main/java/ExampleMod.java | 10 +++++ .../src/main/java/InjectedInterface.java | 4 ++ 12 files changed, 185 insertions(+), 41 deletions(-) create mode 100644 src/test/resources/projects/fmjPathConfig/build.gradle create mode 100644 src/test/resources/projects/fmjPathConfig/src/custom/resources/fabric.mod.json create mode 100644 src/test/resources/projects/fmjPathConfig/src/main/java/ExampleMod.java create mode 100644 src/test/resources/projects/fmjPathConfig/src/main/java/InjectedInterface.java diff --git a/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java b/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java index 8c7ae953..4da936ba 100644 --- a/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java +++ b/src/main/java/net/fabricmc/loom/api/LoomGradleExtensionAPI.java @@ -66,6 +66,22 @@ public interface LoomGradleExtensionAPI { RegularFileProperty getAccessWidenerPath(); + /** + * Specifies the {@code fabric.mod.json} file location used in injected interface processing. + * + *

+ * By default, either {@code src/main/resources/fabric.mod.json} + * or {@code src/client/resources/fabric.mod.json} in the project directory is used. + *

+ * + *

+ * Providing a path to a different location allows using a shared or preprocessed + * file. However, at the end it must be put into the root of the processed + * resources directory (usually {@code build/resources/main}). + *

+ */ + RegularFileProperty getFabricModJsonPath(); + NamedDomainObjectContainer getDecompilerOptions(); void decompilers(Action> action); diff --git a/src/main/java/net/fabricmc/loom/api/fabricapi/DataGenerationSettings.java b/src/main/java/net/fabricmc/loom/api/fabricapi/DataGenerationSettings.java index 4b86632c..0b47ba3c 100644 --- a/src/main/java/net/fabricmc/loom/api/fabricapi/DataGenerationSettings.java +++ b/src/main/java/net/fabricmc/loom/api/fabricapi/DataGenerationSettings.java @@ -24,9 +24,13 @@ package net.fabricmc.loom.api.fabricapi; +import java.io.File; + import org.gradle.api.file.RegularFileProperty; import org.gradle.api.provider.Property; +import net.fabricmc.loom.util.fmj.FabricModJsonFactory; + /** * Represents the settings for data generation. */ @@ -67,4 +71,11 @@ public interface DataGenerationSettings { * Contains a boolean property indicating whether data generation will be compiled and ran with the client. */ Property getClient(); + + /** + * Sets {@link #getModId()} property based on the {@code id} field defined in the provided file. + */ + default void modId(File fabricModJsonFile) { + getModId().set(FabricModJsonFactory.createFromFile(fabricModJsonFile).getId()); + } } diff --git a/src/main/java/net/fabricmc/loom/api/fabricapi/GameTestSettings.java b/src/main/java/net/fabricmc/loom/api/fabricapi/GameTestSettings.java index 1e556191..de6a7251 100644 --- a/src/main/java/net/fabricmc/loom/api/fabricapi/GameTestSettings.java +++ b/src/main/java/net/fabricmc/loom/api/fabricapi/GameTestSettings.java @@ -24,10 +24,14 @@ package net.fabricmc.loom.api.fabricapi; +import java.io.File; + import org.gradle.api.provider.Property; import org.gradle.api.tasks.Optional; import org.jetbrains.annotations.ApiStatus; +import net.fabricmc.loom.util.fmj.FabricModJsonFactory; + /** * Represents the settings for game and/or client tests. */ @@ -89,4 +93,11 @@ public interface GameTestSettings { */ @Optional Property getUsername(); + + /** + * Sets {@link #getModId()} property based on the {@code id} field defined in the provided file. + */ + default void modId(File fabricModJsonFile) { + getModId().set(FabricModJsonFactory.createFromFile(fabricModJsonFile).getId()); + } } diff --git a/src/main/java/net/fabricmc/loom/configuration/fabricapi/FabricApiAbstractSourceSet.java b/src/main/java/net/fabricmc/loom/configuration/fabricapi/FabricApiAbstractSourceSet.java index 22bec596..9be2291d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/fabricapi/FabricApiAbstractSourceSet.java +++ b/src/main/java/net/fabricmc/loom/configuration/fabricapi/FabricApiAbstractSourceSet.java @@ -24,8 +24,6 @@ package net.fabricmc.loom.configuration.fabricapi; -import java.io.IOException; - import javax.inject.Inject; import org.gradle.api.Project; @@ -64,17 +62,13 @@ abstract class FabricApiAbstractSourceSet { }); modId.convention(getProject().provider(() -> { - try { - final FabricModJson fabricModJson = FabricModJsonFactory.createFromSourceSetsNullable(getProject(), sourceSet); + final FabricModJson fabricModJson = FabricModJsonFactory.createFromSourceSetsNullable(getProject(), sourceSet); - if (fabricModJson == null) { - throw new RuntimeException("Could not find a fabric.mod.json file in the data source set or a value for DataGenerationSettings.getModId()"); - } - - return fabricModJson.getId(); - } catch (IOException e) { - throw new org.gradle.api.UncheckedIOException("Failed to read mod id from the datagen source set.", e); + if (fabricModJson == null) { + throw new RuntimeException("Could not find a fabric.mod.json file in the data source set or a value for DataGenerationSettings.getModId()"); } + + return fabricModJson.getId(); })); extension.getMods().create(modId.get(), mod -> { diff --git a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java index e547ac61..b7ccb43f 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionApiImpl.java @@ -25,8 +25,8 @@ package net.fabricmc.loom.extension; import java.io.File; -import java.io.IOException; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; @@ -34,7 +34,6 @@ import org.gradle.api.Action; import org.gradle.api.NamedDomainObjectContainer; import org.gradle.api.NamedDomainObjectList; import org.gradle.api.Project; -import org.gradle.api.UncheckedIOException; import org.gradle.api.artifacts.Dependency; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.file.FileCollection; @@ -74,7 +73,7 @@ import net.fabricmc.loom.task.GenerateSourcesTask; import net.fabricmc.loom.util.DeprecationHelper; import net.fabricmc.loom.util.MirrorUtil; import net.fabricmc.loom.util.fmj.FabricModJson; -import net.fabricmc.loom.util.fmj.FabricModJsonFactory; +import net.fabricmc.loom.util.fmj.FabricModJsonHelpers; import net.fabricmc.loom.util.gradle.SourceSetHelper; /** @@ -86,6 +85,7 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA protected final ListProperty jarProcessors; protected final ConfigurableFileCollection log4jConfigs; protected final RegularFileProperty accessWidener; + protected final RegularFileProperty fabricModJsonPath; protected final ManifestLocations versionsManifests; protected final Property customMetadata; protected final SetProperty knownIndyBsms; @@ -118,6 +118,7 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA .empty(); this.log4jConfigs = project.files(directories.getDefaultLog4jConfigFile()); this.accessWidener = project.getObjects().fileProperty(); + this.fabricModJsonPath = project.getObjects().fileProperty(); this.versionsManifests = new ManifestLocations(); this.versionsManifests.add("mojang", MirrorUtil.getVersionManifests(project), -2); this.versionsManifests.add("fabric_experimental", MirrorUtil.getExperimentalVersions(project), -1); @@ -205,6 +206,11 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA return accessWidener; } + @Override + public RegularFileProperty getFabricModJsonPath() { + return fabricModJsonPath; + } + @Override public NamedDomainObjectContainer getDecompilerOptions() { return decompilers; @@ -293,17 +299,13 @@ public abstract class LoomGradleExtensionApiImpl implements LoomGradleExtensionA @Override public String getModVersion() { - try { - final FabricModJson fabricModJson = FabricModJsonFactory.createFromSourceSetsNullable(getProject(), SourceSetHelper.getMainSourceSet(getProject())); + List fabricModJsons = FabricModJsonHelpers.getModsInProject(getProject()); - if (fabricModJson == null) { - throw new RuntimeException("Could not find a fabric.mod.json file in the main sourceset"); - } - - return fabricModJson.getModVersion(); - } catch (IOException e) { - throw new UncheckedIOException("Failed to read mod version from main sourceset.", e); + if (fabricModJsons.isEmpty()) { + throw new RuntimeException("Could not find a fabric.mod.json file in the main sourceset"); } + + return fabricModJsons.getFirst().getModVersion(); } @Override diff --git a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonFactory.java b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonFactory.java index 0cfec9c6..bb0d2a31 100644 --- a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonFactory.java +++ b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonFactory.java @@ -107,27 +107,38 @@ public final class FabricModJsonFactory { return Optional.ofNullable(createFromZipNullable(zipPath)); } + public static FabricModJson createFromFile(File file) { + JsonObject modJson = readFmjJsonObject(file); + return create(modJson, new FabricModJsonSource.DirectorySource(file.toPath().getParent())); + } + @Nullable - public static FabricModJson createFromSourceSetsNullable(Project project, SourceSet... sourceSets) throws IOException { - final File file = SourceSetHelper.findFirstFileInResource(FABRIC_MOD_JSON, project, sourceSets); + public static FabricModJson createFromSourceSetsNullable(Project project, SourceSet... sourceSets) { + File file = SourceSetHelper.findFirstFileInResource(FABRIC_MOD_JSON, project, sourceSets); if (file == null) { return null; } + try { + JsonObject modJson = readFmjJsonObject(file); + return create(modJson, new FabricModJsonSource.SourceSetSource(project, sourceSets)); + } catch (JsonSyntaxException e) { + LOGGER.warn("Failed to parse fabric.mod.json: {}", file.getAbsolutePath()); + return null; + } + } + + private static JsonObject readFmjJsonObject(File file) { try (Reader reader = Files.newBufferedReader(file.toPath(), StandardCharsets.UTF_8)) { final JsonObject modJson = LoomGradlePlugin.GSON.fromJson(reader, JsonObject.class); if (modJson == null) { // fromJson returns null if the file is empty LOGGER.warn("Failed to parse empty fabric.mod.json: {}", file.getAbsolutePath()); - return null; } - return create(modJson, new FabricModJsonSource.SourceSetSource(project, sourceSets)); - } catch (JsonSyntaxException e) { - LOGGER.warn("Failed to parse fabric.mod.json: {}", file.getAbsolutePath()); - return null; + return modJson; } catch (IOException e) { throw new UncheckedIOException("Failed to read " + file.getAbsolutePath(), e); } diff --git a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonHelpers.java b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonHelpers.java index 43e0e474..4481283a 100644 --- a/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonHelpers.java +++ b/src/main/java/net/fabricmc/loom/util/fmj/FabricModJsonHelpers.java @@ -24,22 +24,32 @@ package net.fabricmc.loom.util.fmj; -import java.io.IOException; -import java.io.UncheckedIOException; +import java.io.File; import java.util.ArrayList; import java.util.Collections; import java.util.List; import org.gradle.api.Project; +import org.gradle.api.provider.Provider; import org.gradle.api.tasks.SourceSet; -import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.api.LoomGradleExtensionAPI; import net.fabricmc.loom.util.gradle.SourceSetHelper; +import net.fabricmc.loom.LoomGradleExtension; public class FabricModJsonHelpers { - // Returns a list of Mods found in the provided project's main or client sourcesets + /** + * Returns the list of mods provided by either {@link LoomGradleExtensionAPI#getFabricModJsonPath()} + * or {@code fabric.mod.json} in main or client resources. + */ public static List getModsInProject(Project project) { final LoomGradleExtension extension = LoomGradleExtension.get(project); + Provider overrideFile = extension.getFabricModJsonPath().getAsFile(); + + if (overrideFile.isPresent()) { + return List.of(FabricModJsonFactory.createFromFile(overrideFile.get())); + } + var sourceSets = new ArrayList(); sourceSets.add(SourceSetHelper.getMainSourceSet(project)); @@ -47,14 +57,10 @@ public class FabricModJsonHelpers { sourceSets.add(SourceSetHelper.getSourceSetByName("client", project)); } - try { - final FabricModJson fabricModJson = FabricModJsonFactory.createFromSourceSetsNullable(project, sourceSets.toArray(SourceSet[]::new)); + final FabricModJson fabricModJson = FabricModJsonFactory.createFromSourceSetsNullable(project, sourceSets.toArray(SourceSet[]::new)); - if (fabricModJson != null) { - return List.of(fabricModJson); - } - } catch (IOException e) { - throw new UncheckedIOException(e); + if (fabricModJson != null) { + return List.of(fabricModJson); } return Collections.emptyList(); diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/InterfaceInjectionTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/InterfaceInjectionTest.groovy index 947bb826..52946286 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/InterfaceInjectionTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/InterfaceInjectionTest.groovy @@ -24,6 +24,7 @@ package net.fabricmc.loom.test.integration +import org.gradle.testkit.runner.BuildResult import spock.lang.Specification import spock.lang.Unroll @@ -49,4 +50,34 @@ class InterfaceInjectionTest extends Specification implements GradleProjectTestT where: version << STANDARD_TEST_VERSIONS } + + @Unroll + def "Resolve custom FMJ"() { + setup: + GradleProject gradle = gradleProject(project: "fmjPathConfig", version: version) + + when: + BuildResult result = gradle.run(task: "build", args: ["-PoverrideFMJ=true"]) + + then: + result.task(":build").outcome == SUCCESS + + where: + version << STANDARD_TEST_VERSIONS + } + + @Unroll + def "Fail to find FMJ"() { + setup: + GradleProject gradle = gradleProject(project: "fmjPathConfig", version: version) + + when: + BuildResult result = gradle.run(task: "build", expectFailure: true) + + then: + result.task(":build") == null + + where: + version << STANDARD_TEST_VERSIONS + } } diff --git a/src/test/resources/projects/fmjPathConfig/build.gradle b/src/test/resources/projects/fmjPathConfig/build.gradle new file mode 100644 index 00000000..287825cf --- /dev/null +++ b/src/test/resources/projects/fmjPathConfig/build.gradle @@ -0,0 +1,37 @@ +// This is used by a range of tests that append to this file before running the gradle tasks. +// Can be used for tests that require minimal custom setup +plugins { + id 'fabric-loom' + id 'maven-publish' +} + +version = "1.0.0" +group = "com.example" + +// In multi-version setup this would be a separate project, +// but a source set will suffice for a test. +sourceSets { + custom { + } + main { + compileClasspath += sourceSets.custom.compileClasspath + runtimeClasspath += sourceSets.custom.runtimeClasspath + } +} + + +dependencies { + minecraft "com.mojang:minecraft:1.17.1" + mappings "net.fabricmc:yarn:1.17.1+build.59:v2" + modImplementation "net.fabricmc:fabric-loader:0.11.6" +} + +base { + archivesName = "fabric-example-mod" +} + +if (project.hasProperty("overrideFMJ")) { + loom { + fabricModJsonPath = file("src/custom/resources/fabric.mod.json") + } +} \ No newline at end of file diff --git a/src/test/resources/projects/fmjPathConfig/src/custom/resources/fabric.mod.json b/src/test/resources/projects/fmjPathConfig/src/custom/resources/fabric.mod.json new file mode 100644 index 00000000..64b2e239 --- /dev/null +++ b/src/test/resources/projects/fmjPathConfig/src/custom/resources/fabric.mod.json @@ -0,0 +1,11 @@ +{ + "schemaVersion": 1, + "id": "testmod", + "version": "1", + "name": "Test Mod", + "custom": { + "loom:injected_interfaces": { + "net/minecraft/class_310": ["InjectedInterface"] + } + } +} diff --git a/src/test/resources/projects/fmjPathConfig/src/main/java/ExampleMod.java b/src/test/resources/projects/fmjPathConfig/src/main/java/ExampleMod.java new file mode 100644 index 00000000..cd72e67d --- /dev/null +++ b/src/test/resources/projects/fmjPathConfig/src/main/java/ExampleMod.java @@ -0,0 +1,10 @@ +import net.minecraft.client.MinecraftClient; + +import net.fabricmc.api.ModInitializer; + +public class ExampleMod implements ModInitializer { + @Override + public void onInitialize() { + MinecraftClient.getInstance().newMethodThatDidNotExist(); + } +} diff --git a/src/test/resources/projects/fmjPathConfig/src/main/java/InjectedInterface.java b/src/test/resources/projects/fmjPathConfig/src/main/java/InjectedInterface.java new file mode 100644 index 00000000..a8225d0c --- /dev/null +++ b/src/test/resources/projects/fmjPathConfig/src/main/java/InjectedInterface.java @@ -0,0 +1,4 @@ +public interface InjectedInterface { + default void newMethodThatDidNotExist() { + } +} From dd90d7bd29f2cf4c2b5e58ed5be39bade5d06ad2 Mon Sep 17 00:00:00 2001 From: Joseph Burton Date: Fri, 3 Oct 2025 18:08:09 +0100 Subject: [PATCH 04/25] Add AnnotationsDataValidator (#1379) * Add AnnotationsDataValidator * Use Constants.ASM_VERSION --- .../TypeAnnotationNodeSerializer.java | 10 +- .../validate/AnnotationsDataValidator.java | 792 +++++++++++++++ .../validate/TypePathCheckerVisitor.java | 248 +++++ .../AnnotationsDataValidatorTest.groovy | 951 ++++++++++++++++++ .../TypePathCheckerVisitorTest.groovy | 94 ++ .../test/unit/processor/classes/AnnAdd.java | 34 + .../unit/processor/classes/AnnNotPresent.java | 34 + .../unit/processor/classes/AnnPresent.java | 34 + .../unit/processor/classes/AnnWithAttr.java | 37 + .../classes/ClassWithAnnotations.java | 34 + .../classes/ClassWithFieldTypes.java | 33 + .../classes/ClassWithGenericParams.java | 30 + .../classes/ClassWithImplements.java | 28 + .../classes/ClassWithReceiverAndParams.java | 33 + .../classes/ClassWithReturnType.java | 37 + .../classes/ClassWithoutAnnotations.java | 35 + 16 files changed, 2462 insertions(+), 2 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/validate/AnnotationsDataValidator.java create mode 100644 src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/validate/TypePathCheckerVisitor.java create mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/processor/AnnotationsDataValidatorTest.groovy create mode 100644 src/test/groovy/net/fabricmc/loom/test/unit/processor/TypePathCheckerVisitorTest.groovy create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnAdd.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnNotPresent.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnPresent.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnWithAttr.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithAnnotations.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithFieldTypes.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithGenericParams.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithImplements.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithReceiverAndParams.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithReturnType.java create mode 100644 src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithoutAnnotations.java diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/TypeAnnotationNodeSerializer.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/TypeAnnotationNodeSerializer.java index 92890268..3eda04a2 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/TypeAnnotationNodeSerializer.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/TypeAnnotationNodeSerializer.java @@ -31,6 +31,7 @@ import com.google.gson.JsonDeserializer; import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.google.gson.JsonParseException; +import com.google.gson.JsonPrimitive; import com.google.gson.JsonSerializationContext; import com.google.gson.JsonSerializer; import org.objectweb.asm.TypePath; @@ -43,7 +44,8 @@ class TypeAnnotationNodeSerializer implements JsonSerializer AnnotationNode annotation = context.deserialize(json, AnnotationNode.class); JsonObject jsonObject = json.getAsJsonObject(); int typeRef = jsonObject.getAsJsonPrimitive("type_ref").getAsInt(); - String typePath = jsonObject.getAsJsonPrimitive("type_path").getAsString(); + JsonPrimitive typePathPrimitive = jsonObject.getAsJsonPrimitive("type_path"); + String typePath = typePathPrimitive == null ? "" : typePathPrimitive.getAsString(); TypeAnnotationNode typeAnnotation = new TypeAnnotationNode(typeRef, TypePath.fromString(typePath), annotation.desc); annotation.accept(typeAnnotation); return typeAnnotation; @@ -53,7 +55,11 @@ class TypeAnnotationNodeSerializer implements JsonSerializer public JsonElement serialize(TypeAnnotationNode src, Type typeOfSrc, JsonSerializationContext context) { JsonObject json = context.serialize(src, AnnotationNode.class).getAsJsonObject(); json.addProperty("type_ref", src.typeRef); - json.addProperty("type_path", src.typePath.toString()); + + if (src.typePath != null) { + json.addProperty("type_path", src.typePath.toString()); + } + return json; } } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/validate/AnnotationsDataValidator.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/validate/AnnotationsDataValidator.java new file mode 100644 index 00000000..5450cc34 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/validate/AnnotationsDataValidator.java @@ -0,0 +1,792 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.extras.annotations.validate; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.function.Supplier; + +import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; +import org.objectweb.asm.Opcodes; +import org.objectweb.asm.Type; +import org.objectweb.asm.TypePath; +import org.objectweb.asm.TypeReference; +import org.objectweb.asm.signature.SignatureReader; +import org.objectweb.asm.signature.SignatureVisitor; +import org.objectweb.asm.tree.AnnotationNode; +import org.objectweb.asm.tree.ClassNode; +import org.objectweb.asm.tree.FieldNode; +import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.ParameterNode; +import org.objectweb.asm.tree.TypeAnnotationNode; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.AnnotationsData; +import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.ClassAnnotationData; +import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.GenericAnnotationData; +import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.MethodAnnotationData; +import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.TypeAnnotationKey; +import net.fabricmc.loom.util.Constants; + +public abstract class AnnotationsDataValidator { + private static final Logger LOGGER = LoggerFactory.getLogger(AnnotationsDataValidator.class); + + @Nullable + protected abstract ClassNode getClass(String name, boolean includeLibraries); + + @VisibleForTesting + protected void error(String message, Object... args) { + LOGGER.error(message, args); + } + + public boolean checkData(AnnotationsData data) { + boolean result = true; + + for (var classEntry : data.classes().entrySet()) { + result &= checkClassData(classEntry.getKey(), classEntry.getValue()); + } + + return result; + } + + private boolean checkClassData(String className, ClassAnnotationData classData) { + ClassNode clazz = getClass(className, false); + + if (clazz == null) { + error("No such target class: {}", className); + return false; + } + + boolean result = true; + + List annotations = concatLists(clazz.visibleAnnotations, clazz.invisibleAnnotations); + result &= checkAnnotationsToRemove(() -> className, classData.annotationsToRemove(), annotations); + result &= checkAnnotationsToAdd(() -> className, classData.annotationsToAdd(), annotations); + List typeAnnotations = concatLists(clazz.visibleTypeAnnotations, clazz.invisibleTypeAnnotations); + result &= checkTypeAnnotationsToRemove(() -> className, classData.typeAnnotationsToRemove(), typeAnnotations); + result &= checkTypeAnnotationsToAdd(() -> className, classData.typeAnnotationsToAdd(), typeAnnotations, typeAnnotation -> checkClassTypeAnnotation(typeAnnotation, clazz)); + + for (var fieldEntry : classData.fields().entrySet()) { + FieldNode field = findField(clazz, fieldEntry.getKey()); + + if (field == null) { + error("No such target field: {}.{}", className, fieldEntry.getKey()); + result = false; + continue; + } + + result &= checkGenericData( + () -> className + "." + fieldEntry.getKey(), + fieldEntry.getValue(), + concatLists(field.visibleAnnotations, field.invisibleAnnotations), + concatLists(field.visibleTypeAnnotations, field.invisibleTypeAnnotations), + typeAnnotation -> checkFieldTypeAnnotation(typeAnnotation, field) + ); + } + + for (var methodEntry : classData.methods().entrySet()) { + MethodNode method = findMethod(clazz, methodEntry.getKey()); + + if (method == null) { + error("No such target method: {}.{}", className, methodEntry.getKey()); + result = false; + continue; + } + + result &= checkMethodData(() -> className + "." + methodEntry.getKey(), methodEntry.getValue(), className, method); + } + + return result; + } + + private boolean checkGenericData(Supplier errorKey, GenericAnnotationData data, List annotations, List typeAnnotations, TypeAnnotationChecker typeAnnotationChecker) { + boolean result = true; + result &= checkAnnotationsToRemove(errorKey, data.annotationsToRemove(), annotations); + result &= checkAnnotationsToAdd(errorKey, data.annotationsToAdd(), annotations); + result &= checkTypeAnnotationsToRemove(errorKey, data.typeAnnotationsToRemove(), typeAnnotations); + result &= checkTypeAnnotationsToAdd(errorKey, data.typeAnnotationsToAdd(), typeAnnotations, typeAnnotationChecker); + return result; + } + + private boolean checkMethodData(Supplier errorKey, MethodAnnotationData data, String className, MethodNode method) { + boolean result = true; + List annotations = concatLists(method.visibleAnnotations, method.invisibleAnnotations); + result &= checkAnnotationsToRemove(errorKey, data.annotationsToRemove(), annotations); + result &= checkAnnotationsToAdd(errorKey, data.annotationsToAdd(), annotations); + List typeAnnotations = concatLists(method.visibleTypeAnnotations, method.invisibleTypeAnnotations); + result &= checkTypeAnnotationsToRemove(errorKey, data.typeAnnotationsToRemove(), typeAnnotations); + result &= checkTypeAnnotationsToAdd(errorKey, data.typeAnnotationsToAdd(), typeAnnotations, typeAnnotation -> checkMethodTypeAnnotation(typeAnnotation, className, method)); + + int syntheticParamCount = 0; + + if (method.parameters != null) { + for (ParameterNode param : method.parameters) { + if ((param.access & Opcodes.ACC_SYNTHETIC) != 0) { + syntheticParamCount++; + } + } + } + + for (var paramEntry : data.parameters().entrySet()) { + int paramIndex = paramEntry.getKey(); + + if (paramIndex < 0 || paramIndex >= Type.getArgumentCount(method.desc) - syntheticParamCount) { + error("Invalid parameter index: {} for method: {}", paramIndex, errorKey.get()); + result = false; + continue; + } + + List paramAnnotations = concatLists( + method.visibleParameterAnnotations == null || paramIndex >= method.visibleParameterAnnotations.length ? null : method.visibleParameterAnnotations[paramIndex], + method.invisibleParameterAnnotations == null || paramIndex >= method.invisibleParameterAnnotations.length ? null : method.invisibleParameterAnnotations[paramIndex] + ); + result &= checkGenericData(() -> errorKey.get() + ":" + paramIndex, paramEntry.getValue(), paramAnnotations, List.of(), typeAnnotation -> true); + + if (!paramEntry.getValue().typeAnnotationsToRemove().isEmpty() || !paramEntry.getValue().typeAnnotationsToAdd().isEmpty()) { + error("Type annotations cannot be added directly to method parameters: {}", errorKey.get()); + result = false; + } + } + + return result; + } + + private boolean checkAnnotationsToRemove(Supplier errorKey, Set annotationsToRemove, List annotations) { + Set annotationsNotRemoved = new LinkedHashSet<>(annotationsToRemove); + + for (AnnotationNode annotation : annotations) { + annotationsNotRemoved.remove(Type.getType(annotation.desc).getInternalName()); + } + + if (annotationsNotRemoved.isEmpty()) { + return true; + } + + for (String annotation : annotationsNotRemoved) { + error("Trying to remove annotation {} from {} but it's not present", annotation, errorKey.get()); + } + + return false; + } + + private boolean checkAnnotationsToAdd(Supplier errorKey, List annotationsToAdd, List annotations) { + Set existingAnnotations = new HashSet<>(); + + for (AnnotationNode annotation : annotations) { + existingAnnotations.add(annotation.desc); + } + + boolean result = true; + + for (AnnotationNode annotation : annotationsToAdd) { + if (!existingAnnotations.add(annotation.desc)) { + error("Trying to add annotation {} to {} but it's already present", annotation.desc, errorKey.get()); + result = false; + } + + result &= checkAnnotation(errorKey, annotation); + } + + return result; + } + + private boolean checkTypeAnnotationsToRemove(Supplier errorKey, Set typeAnnotationsToRemove, List typeAnnotations) { + Set typeAnnotationsNotRemoved = new LinkedHashSet<>(typeAnnotationsToRemove); + + for (TypeAnnotationNode typeAnnotation : typeAnnotations) { + typeAnnotationsNotRemoved.remove(new TypeAnnotationKey(typeAnnotation.typeRef, typePathToString(typeAnnotation.typePath), Type.getType(typeAnnotation.desc).getInternalName())); + } + + if (typeAnnotationsNotRemoved.isEmpty()) { + return true; + } + + for (TypeAnnotationKey typeAnnotation : typeAnnotationsNotRemoved) { + error("Trying to remove type annotation {} (typeRef={}, typePath={}) from {} but it's not present", typeAnnotation.name(), typeAnnotation.typeRef(), typeAnnotation.typePath(), errorKey.get()); + } + + return false; + } + + private boolean checkTypeAnnotationsToAdd(Supplier errorKey, List typeAnnotationsToAdd, List typeAnnotations, TypeAnnotationChecker checker) { + Set existingTypeAnnotations = new HashSet<>(); + + for (TypeAnnotationNode typeAnnotation : typeAnnotations) { + existingTypeAnnotations.add(new TypeAnnotationKey(typeAnnotation.typeRef, typePathToString(typeAnnotation.typePath), typeAnnotation.desc)); + } + + boolean result = true; + + for (TypeAnnotationNode typeAnnotation : typeAnnotationsToAdd) { + if (!existingTypeAnnotations.add(new TypeAnnotationKey(typeAnnotation.typeRef, typePathToString(typeAnnotation.typePath), typeAnnotation.desc))) { + error("Trying to add annotation {} (typeRef={}, typePath={}) to {} but it's already present", typeAnnotation.desc, typeAnnotation.typeRef, typeAnnotation.typePath, errorKey.get()); + result = false; + } + + result &= checker.checkTypeAnnotation(typeAnnotation); + } + + return result; + } + + private boolean checkClassTypeAnnotation(TypeAnnotationNode typeAnnotation, ClassNode clazz) { + if (!checkTypeRef(typeAnnotation.typeRef)) { + return false; + } + + TypeReference typeRef = new TypeReference(typeAnnotation.typeRef); + TypePathCheckerVisitor typePathChecker = new TypePathCheckerVisitor(typeAnnotation.typePath); + + switch (typeRef.getSort()) { + case TypeReference.CLASS_TYPE_PARAMETER -> { + return checkTypeParameterTypeAnnotation("class", typeAnnotation, clazz.signature, typeRef.getTypeParameterIndex()); + } + case TypeReference.CLASS_TYPE_PARAMETER_BOUND -> { + if (!checkTypeParameterBoundTypeAnnotation("class", typeAnnotation, clazz.signature, typeRef.getTypeParameterIndex(), typeRef.getTypeParameterBoundIndex(), typePathChecker)) { + return false; + } + } + case TypeReference.CLASS_EXTENDS -> { + int superTypeIndex = typeRef.getSuperTypeIndex(); + + if (superTypeIndex == -1) { + if (clazz.signature == null) { + typePathChecker.visitClassType(Objects.requireNonNullElse(clazz.superName, "java/lang/Object")); + typePathChecker.visitEnd(); + } else { + new SignatureReader(clazz.signature).accept(new SignatureVisitor(Constants.ASM_VERSION) { + @Override + public SignatureVisitor visitSuperclass() { + return typePathChecker; + } + }); + } + } else { + if (superTypeIndex >= clazz.interfaces.size()) { + error("Invalid type reference for class type annotation: {}, interface index {} out of bounds", typeAnnotation.typeRef, superTypeIndex); + return false; + } + + if (clazz.signature == null) { + typePathChecker.visitClassType(clazz.interfaces.get(superTypeIndex)); + typePathChecker.visitEnd(); + } else { + new SignatureReader(clazz.signature).accept(new SignatureVisitor(Constants.ASM_VERSION) { + int interfaceIndex = 0; + + @Override + public SignatureVisitor visitInterface() { + if (interfaceIndex++ == superTypeIndex) { + return typePathChecker; + } else { + return this; + } + } + }); + } + } + } + default -> { + error("Invalid type reference for class type annotation: {}", typeAnnotation.typeRef); + return false; + } + } + + String typePathError = typePathChecker.getError(); + + if (typePathError != null) { + error("Invalid type path for class type annotation, typeRef: {}, error: {}", typeAnnotation.typeRef, typePathError); + return false; + } + + return true; + } + + private boolean checkFieldTypeAnnotation(TypeAnnotationNode typeAnnotation, FieldNode field) { + if (!checkTypeRef(typeAnnotation.typeRef)) { + return false; + } + + if (new TypeReference(typeAnnotation.typeRef).getSort() != TypeReference.FIELD) { + error("Invalid type reference for field type annotation: {}", typeAnnotation.typeRef); + return false; + } + + String signature = Objects.requireNonNullElse(field.signature, field.desc); + TypePathCheckerVisitor typePathChecker = new TypePathCheckerVisitor(typeAnnotation.typePath); + new SignatureReader(signature).acceptType(typePathChecker); + String typePathError = typePathChecker.getError(); + + if (typePathError != null) { + error("Invalid type path for field type annotation, typeRef: {}, error: {}", typeAnnotation.typeRef, typePathError); + return false; + } + + return true; + } + + private boolean checkMethodTypeAnnotation(TypeAnnotationNode typeAnnotation, String className, MethodNode method) { + if (!checkTypeRef(typeAnnotation.typeRef)) { + return false; + } + + TypeReference typeRef = new TypeReference(typeAnnotation.typeRef); + TypePathCheckerVisitor typePathChecker = new TypePathCheckerVisitor(typeAnnotation.typePath); + + switch (typeRef.getSort()) { + case TypeReference.METHOD_TYPE_PARAMETER -> { + return checkTypeParameterTypeAnnotation("method", typeAnnotation, method.signature, typeRef.getTypeParameterIndex()); + } + case TypeReference.METHOD_TYPE_PARAMETER_BOUND -> { + if (!checkTypeParameterBoundTypeAnnotation("method", typeAnnotation, method.signature, typeRef.getTypeParameterIndex(), typeRef.getTypeParameterBoundIndex(), typePathChecker)) { + return false; + } + } + case TypeReference.METHOD_RETURN -> { + if (method.signature == null) { + new SignatureReader(Type.getReturnType(method.desc).getDescriptor()).acceptType(typePathChecker); + } else { + new SignatureReader(method.signature).accept(new SignatureVisitor(Constants.ASM_VERSION) { + @Override + public SignatureVisitor visitReturnType() { + return typePathChecker; + } + }); + } + } + case TypeReference.METHOD_RECEIVER -> { + if ((method.access & Opcodes.ACC_STATIC) != 0 || "".equals(method.name)) { + error("Invalid type reference for method type annotation: {}, method receiver used in a static context", typeAnnotation.typeRef); + return false; + } + + typePathChecker.visitClassType(className); + typePathChecker.visitEnd(); + } + case TypeReference.METHOD_FORMAL_PARAMETER -> { + int formalParamIndex = typeRef.getFormalParameterIndex(); + + if (method.signature == null) { + int nonSyntheticParams = 0; + boolean foundArgument = false; + + for (Type argumentType : Type.getArgumentTypes(method.desc)) { + if ((method.access & Opcodes.ACC_SYNTHETIC) == 0) { + if (nonSyntheticParams++ == formalParamIndex) { + foundArgument = true; + new SignatureReader(argumentType.getDescriptor()).acceptType(typePathChecker); + break; + } + } + } + + if (!foundArgument) { + error("Invalid type reference for method type annotation: {}, formal parameter index {} out of bounds", typeAnnotation.typeRef, formalParamIndex); + return false; + } + } else { + var visitor = new SignatureVisitor(Constants.ASM_VERSION) { + int paramIndex = 0; + boolean found = false; + + @Override + public SignatureVisitor visitParameterType() { + if (paramIndex++ == formalParamIndex) { + found = true; + return typePathChecker; + } else { + return this; + } + } + }; + new SignatureReader(method.signature).accept(visitor); + + if (!visitor.found) { + error("Invalid type reference for method type annotation: {}, formal parameter index {} out of bounds", typeAnnotation.typeRef, formalParamIndex); + return false; + } + } + } + case TypeReference.THROWS -> { + int throwsIndex = typeRef.getExceptionIndex(); + + if (method.signature == null) { + if (method.exceptions == null || throwsIndex >= method.exceptions.size()) { + error("Invalid type reference for method type annotation: {}, exception index {} out of bounds", typeAnnotation.typeRef, throwsIndex); + return false; + } + + typePathChecker.visitClassType(method.exceptions.get(throwsIndex)); + typePathChecker.visitEnd(); + } else { + var visitor = new SignatureVisitor(Constants.ASM_VERSION) { + int exceptionIndex = 0; + boolean found = false; + + @Override + public SignatureVisitor visitExceptionType() { + if (exceptionIndex++ == throwsIndex) { + found = true; + return typePathChecker; + } else { + return this; + } + } + }; + new SignatureReader(method.signature).accept(visitor); + + if (!visitor.found) { + error("Invalid type reference for method type annotation: {}, exception index {} out of bounds", typeAnnotation.typeRef, throwsIndex); + return false; + } + } + } + default -> { + error("Invalid type reference for method type annotation: {}", typeAnnotation.typeRef); + return false; + } + } + + String typePathError = typePathChecker.getError(); + + if (typePathError != null) { + error("Invalid type path for method type annotation, typeRef: {}, error: {}", typeAnnotation.typeRef, typePathError); + return false; + } + + return true; + } + + private boolean checkTypeParameterTypeAnnotation(String memberType, TypeAnnotationNode typeAnnotation, @Nullable String signature, int typeParamIndex) { + int formalParamCount; + + if (signature == null) { + formalParamCount = 0; + } else { + var formalParamCounter = new SignatureVisitor(Constants.ASM_VERSION) { + int count = 0; + + @Override + public void visitFormalTypeParameter(String name) { + count++; + } + }; + new SignatureReader(signature).accept(formalParamCounter); + formalParamCount = formalParamCounter.count; + } + + boolean result = true; + + if (typeParamIndex >= formalParamCount) { + error("Invalid type reference for {} type annotation: {}, formal parameter index {} out of bounds", memberType, typeAnnotation.typeRef, typeParamIndex); + result = false; + } + + if (typeAnnotation.typePath != null && typeAnnotation.typePath.getLength() != 0) { + error("Non-empty type path for annotation doesn't make sense for {}_TYPE_PARAMETER", memberType.toUpperCase(Locale.ROOT)); + result = false; + } + + return result; + } + + private boolean checkTypeParameterBoundTypeAnnotation(String memberType, TypeAnnotationNode typeAnnotation, @Nullable String signature, int typeParamIndex, int typeParamBoundIndex, TypePathCheckerVisitor typePathChecker) { + var visitor = new SignatureVisitor(Constants.ASM_VERSION) { + boolean found = false; + int formalParamIndex = -1; + int boundIndex = 0; + + @Override + public void visitFormalTypeParameter(String name) { + formalParamIndex++; + } + + @Override + public SignatureVisitor visitClassBound() { + if (formalParamIndex == typeParamIndex) { + if (boundIndex++ == typeParamBoundIndex) { + found = true; + return typePathChecker; + } + } + + return this; + } + + @Override + public SignatureVisitor visitInterfaceBound() { + return visitClassBound(); + } + }; + + if (signature != null) { + new SignatureReader(signature).accept(visitor); + } + + if (!visitor.found) { + error("Invalid type reference for {} type annotation: {}, formal parameter index {} bound index {} out of bounds", memberType, typeAnnotation.typeRef, typeParamIndex, typeParamBoundIndex); + return false; + } + + return true; + } + + // copied from CheckClassAdapter + private boolean checkTypeRef(int typeRef) { + int mask = switch (typeRef >>> 24) { + case TypeReference.CLASS_TYPE_PARAMETER, TypeReference.METHOD_TYPE_PARAMETER, + TypeReference.METHOD_FORMAL_PARAMETER -> 0xFFFF0000; + case TypeReference.FIELD, TypeReference.METHOD_RETURN, TypeReference.METHOD_RECEIVER, + TypeReference.LOCAL_VARIABLE, TypeReference.RESOURCE_VARIABLE, TypeReference.INSTANCEOF, + TypeReference.NEW, TypeReference.CONSTRUCTOR_REFERENCE, TypeReference.METHOD_REFERENCE -> 0xFF000000; + case TypeReference.CLASS_EXTENDS, TypeReference.CLASS_TYPE_PARAMETER_BOUND, + TypeReference.METHOD_TYPE_PARAMETER_BOUND, TypeReference.THROWS, TypeReference.EXCEPTION_PARAMETER -> 0xFFFFFF00; + case TypeReference.CAST, TypeReference.CONSTRUCTOR_INVOCATION_TYPE_ARGUMENT, + TypeReference.METHOD_INVOCATION_TYPE_ARGUMENT, TypeReference.CONSTRUCTOR_REFERENCE_TYPE_ARGUMENT, + TypeReference.METHOD_REFERENCE_TYPE_ARGUMENT -> 0xFF0000FF; + default -> 0; + }; + + if (mask == 0 || (typeRef & ~mask) != 0) { + error("Invalid type reference {}", typeRef); + return false; + } + + return true; + } + + private boolean checkAnnotation(Supplier errorKey, AnnotationNode annotation) { + if (!annotation.desc.startsWith("L") || !annotation.desc.endsWith(";")) { + error("Invalid annotation descriptor: {}", annotation.desc); + return false; + } + + String internalName = annotation.desc.substring(1, annotation.desc.length() - 1); + ClassNode annotationClass = getClass(internalName, true); + + if (annotationClass == null || (annotationClass.access & Opcodes.ACC_ANNOTATION) == 0) { + error("No such annotation class: {}", internalName); + return false; + } + + Set missingRequiredAttributes = new LinkedHashSet<>(); + Map attributeTypes = new HashMap<>(); + + for (MethodNode method : annotationClass.methods) { + if ((method.access & Opcodes.ACC_ABSTRACT) != 0) { + attributeTypes.put(method.name, Type.getReturnType(method.desc)); + + if (method.annotationDefault == null) { + missingRequiredAttributes.add(method.name); + } + } + } + + boolean result = true; + + if (annotation.values != null) { + for (int i = 0; i < annotation.values.size(); i += 2) { + String key = (String) annotation.values.get(i); + Object value = annotation.values.get(i + 1); + + Type expectedType = attributeTypes.get(key); + + if (expectedType == null) { + error("Unknown annotation attribute: {}.{}", internalName, key); + result = false; + continue; + } + + result &= checkAnnotationValue(errorKey, key, value, expectedType); + + missingRequiredAttributes.remove(key); + } + } + + if (!missingRequiredAttributes.isEmpty()) { + result = false; + error("Annotation applied to {} is missing required attributes: {}", errorKey.get(), missingRequiredAttributes); + } + + return result; + } + + private boolean checkAnnotationValue(Supplier errorKey, String name, Object value, Type expectedType) { + if (expectedType.getSort() == Type.ARRAY) { + if (!(value instanceof List values)) { + error("Annotation value is of type {}, expected array for attribute {}", getTypeName(value), name); + return false; + } + + boolean result = true; + + for (Object element : values) { + result &= checkAnnotationValue(errorKey, name, element, expectedType.getElementType()); + } + + return result; + } + + boolean result = true; + + boolean wrongType = switch (value) { + case Boolean ignored -> expectedType.getSort() != Type.BOOLEAN; + case Byte ignored -> expectedType.getSort() != Type.BYTE; + case Character ignored -> expectedType.getSort() != Type.CHAR; + case Short ignored -> expectedType.getSort() != Type.SHORT; + case Integer ignored -> expectedType.getSort() != Type.INT; + case Long ignored -> expectedType.getSort() != Type.LONG; + case Float ignored -> expectedType.getSort() != Type.FLOAT; + case Double ignored -> expectedType.getSort() != Type.DOUBLE; + case String ignored -> !expectedType.getDescriptor().equals("Ljava/lang/String;"); + case Type ignored -> !expectedType.getDescriptor().equals("Ljava/lang/Class;"); + case String[] enumValue -> { + if (!enumValue[0].startsWith("L") || !enumValue[0].endsWith(";")) { + error("Invalid enum descriptor: {}", enumValue[0]); + result = false; + yield false; + } + + boolean wrongEnumType = !expectedType.getDescriptor().equals(enumValue[0]); + + ClassNode enumClass = getClass(enumValue[0].substring(1, enumValue[0].length() - 1), true); + + if (enumClass == null) { + error("No such enum class: {}", enumValue[0]); + result = false; + yield wrongEnumType; + } + + if (!enumValueExists(enumClass, enumValue[1])) { + error("Enum value {} does not exist in class {}", enumValue[1], enumValue[0]); + result = false; + yield wrongEnumType; + } + + yield wrongEnumType; + } + case AnnotationNode annotation -> { + result &= checkAnnotation(errorKey, annotation); + yield !expectedType.getDescriptor().equals(annotation.desc); + } + case List ignored -> true; + default -> throw new AssertionError("Unexpected annotation value type: " + value.getClass().getName()); + }; + + if (wrongType) { + error("Annotation value is of type {}, expected {} for attribute {}", getTypeName(value), expectedType.getClassName(), name); + result = false; + } + + return result; + } + + @Nullable + private static FieldNode findField(ClassNode clazz, String nameAndDesc) { + for (FieldNode field : clazz.fields) { + if (nameAndDesc.equals(field.name + ":" + field.desc)) { + return field; + } + } + + return null; + } + + @Nullable + private static MethodNode findMethod(ClassNode clazz, String nameAndDesc) { + for (MethodNode method : clazz.methods) { + if (nameAndDesc.equals(method.name + method.desc)) { + return method; + } + } + + return null; + } + + private static boolean enumValueExists(ClassNode enumClass, String name) { + for (FieldNode field : enumClass.fields) { + if (field.name.equals(name) && (field.access & Opcodes.ACC_ENUM) != 0) { + return true; + } + } + + return false; + } + + private static String getTypeName(Object value) { + return switch (value) { + case Boolean ignored -> "boolean"; + case Byte ignored -> "byte"; + case Character ignored -> "char"; + case Short ignored -> "short"; + case Integer ignored -> "int"; + case Long ignored -> "long"; + case Float ignored -> "float"; + case Double ignored -> "double"; + case String ignored -> "java.lang.String"; + case Type ignored -> "java.lang.Class"; + case String[] enumValue -> getSafeClassNameFromDesc(enumValue[0]); + case AnnotationNode annotation -> getSafeClassNameFromDesc(annotation.desc); + case List ignored -> "array"; + default -> throw new AssertionError("Unexpected annotation value type: " + value.getClass().getName()); + }; + } + + private static String getSafeClassNameFromDesc(String desc) { + return desc.startsWith("L") && desc.endsWith(";") ? desc.substring(1, desc.length() - 1).replace('/', '.') : desc; + } + + private static String typePathToString(@Nullable TypePath typePath) { + return typePath == null ? "" : typePath.toString(); + } + + private static List concatLists(@Nullable List list1, @Nullable List list2) { + List result = new ArrayList<>(); + + if (list1 != null) { + result.addAll(list1); + } + + if (list2 != null) { + result.addAll(list2); + } + + return result; + } + + @FunctionalInterface + private interface TypeAnnotationChecker { + boolean checkTypeAnnotation(TypeAnnotationNode typeAnnotation); + } +} diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/validate/TypePathCheckerVisitor.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/validate/TypePathCheckerVisitor.java new file mode 100644 index 00000000..bc365058 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/validate/TypePathCheckerVisitor.java @@ -0,0 +1,248 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.extras.annotations.validate; + +import java.util.ArrayDeque; +import java.util.Deque; + +import org.jetbrains.annotations.Nullable; +import org.objectweb.asm.TypePath; +import org.objectweb.asm.signature.SignatureVisitor; + +import net.fabricmc.loom.util.Constants; + +public final class TypePathCheckerVisitor extends SignatureVisitor { + private final TypePath path; + private final int pathLen; + private int pathIndex = 0; + private boolean reached = false; + private String error = null; + + private final Deque argIndexStack = new ArrayDeque<>(); + private final SignatureVisitor sink = new SignatureVisitor(Constants.ASM_VERSION) { + }; + + public TypePathCheckerVisitor(final TypePath path) { + super(Constants.ASM_VERSION); + this.path = path; + this.pathLen = path == null ? 0 : path.getLength(); + + if (this.pathLen == 0) { + reached = true; + } + } + + private boolean hasMoreSteps() { + return pathIndex < pathLen; + } + + private int nextStepKind() { + return path.getStep(pathIndex); + } + + private int nextStepArgumentIndex() { + return path.getStepArgument(pathIndex); + } + + private String stepRepr(int i) { + return switch (path.getStep(i)) { + case TypePath.ARRAY_ELEMENT -> "["; + case TypePath.INNER_TYPE -> "."; + case TypePath.WILDCARD_BOUND -> "*"; + case TypePath.TYPE_ARGUMENT -> path.getStepArgument(i) + ";"; + default -> throw new AssertionError("Unexpected type path step kind: " + path.getStep(i)); + }; + } + + private String remainingSteps() { + if (path == null || !hasMoreSteps()) { + return ""; + } + + StringBuilder sb = new StringBuilder(); + + for (int i = pathIndex; i < pathLen; i++) { + sb.append(stepRepr(i)); + } + + return sb.toString(); + } + + private void consumeStep() { + pathIndex++; + + if (pathIndex == pathLen) { + reached = true; + } + } + + @Nullable + public String getError() { + if (error != null) { + return error; + } + + if (!reached) { + return "TypePath not fully consumed at index " + pathIndex + ", remaining steps: '" + remainingSteps() + "'"; + } + + return null; + } + + @Override + public void visitBaseType(final char descriptor) { + if (hasMoreSteps()) { + error = "TypePath has extra steps starting at index " + pathIndex + " ('" + remainingSteps() + + "') but reached base type descriptor '" + descriptor + "'."; + } else { + reached = true; + } + } + + @Override + public void visitTypeVariable(final String name) { + if (hasMoreSteps()) { + error = "TypePath has extra steps starting at index " + pathIndex + " ('" + remainingSteps() + + "') but reached type variable '" + name + "'."; + } else { + reached = true; + } + } + + @Override + public SignatureVisitor visitArrayType() { + if (hasMoreSteps()) { + if (nextStepKind() == TypePath.ARRAY_ELEMENT) { + consumeStep(); + } else { + error = "At step " + pathIndex + " expected array element '[' but found '" + stepRepr(pathIndex) + "'."; + } + } + + return this; + } + + @Override + public void visitClassType(final String name) { + argIndexStack.push(0); + + String[] innerParts = name.split("\\$"); + + for (int i = 1; i < innerParts.length; i++) { + visitInnerClassType(innerParts[i]); + } + } + + @Override + public void visitInnerClassType(final String name) { + if (hasMoreSteps()) { + if (nextStepKind() == TypePath.INNER_TYPE) { + consumeStep(); + } else { + error = "At step " + pathIndex + " expected inner type '.' but found '" + stepRepr(pathIndex) + "'."; + } + } + + argIndexStack.push(0); + } + + @Override + public void visitTypeArgument() { + // unbounded wildcard '*' — terminal + if (error != null) { + return; + } + + if (argIndexStack.isEmpty()) { + error = "Type signature has no enclosing class type for an unbounded wildcard at step " + pathIndex + "."; + return; + } + + int idx = argIndexStack.pop(); + boolean targeted = hasMoreSteps() && nextStepKind() == TypePath.TYPE_ARGUMENT && nextStepArgumentIndex() == idx; + + if (targeted) { + consumeStep(); + + if (hasMoreSteps()) { + error = "TypePath targets unbounded wildcard '*' at step " + (pathIndex - 1) + + " but contains further steps; '*' is terminal."; + } else { + reached = true; + } + } + + argIndexStack.push(idx + 1); + // no nested visits for unbounded wildcard + } + + @Override + public SignatureVisitor visitTypeArgument(final char wildcard) { + if (error != null) { + return sink; + } + + if (argIndexStack.isEmpty()) { + error = "Type signature has no enclosing class type for a type argument at step " + pathIndex + "."; + return sink; + } + + int idx = argIndexStack.pop(); + boolean targeted = hasMoreSteps() && nextStepKind() == TypePath.TYPE_ARGUMENT && nextStepArgumentIndex() == idx; + + if (targeted) { + consumeStep(); + + if (pathIndex == pathLen) { + // Path targets the whole type argument; success if no further navigation needed. + argIndexStack.push(idx + 1); + return sink; + } + } + + argIndexStack.push(idx + 1); + + if (hasMoreSteps() && nextStepKind() == TypePath.WILDCARD_BOUND) { + if (wildcard == SignatureVisitor.EXTENDS || wildcard == SignatureVisitor.SUPER) { + consumeStep(); + } else { + error = "At step " + pathIndex + " found wildcard bound '*' but the type argument is exact ('=')."; + } + } + + return this; + } + + @Override + public void visitEnd() { + if (argIndexStack.isEmpty()) { + if (error == null) { + error = "visitEnd encountered with no matching class/inner-class at step " + pathIndex + "."; + } + } else { + argIndexStack.pop(); + } + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/processor/AnnotationsDataValidatorTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/processor/AnnotationsDataValidatorTest.groovy new file mode 100644 index 00000000..90392727 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/processor/AnnotationsDataValidatorTest.groovy @@ -0,0 +1,951 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor + +import org.intellij.lang.annotations.Language +import org.objectweb.asm.ClassReader +import org.objectweb.asm.TypeReference +import org.objectweb.asm.tree.ClassNode +import spock.lang.Specification + +import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.AnnotationsData +import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.validate.AnnotationsDataValidator + +@SuppressWarnings("JsonStandardCompliance") +class AnnotationsDataValidatorTest extends Specification { + private static final String TEST_CLASSES_PACKAGE_INTERNAL = "net/fabricmc/loom/test/unit/processor/classes/" + + private static String internalName(String simpleName) { + return TEST_CLASSES_PACKAGE_INTERNAL + simpleName + } + + private static ClassNode loadClassNode(String internalName) { + ClassReader cr = new ClassReader(internalName) + ClassNode cn = new ClassNode() + cr.accept(cn, 0) + return cn + } + + class TestValidator extends AnnotationsDataValidator { + final List errors + + TestValidator(List errors) { + this.errors = errors + } + + @Override + protected ClassNode getClass(String name, boolean includeLibraries) { + try { + return loadClassNode(name) + } catch (Exception ignored) { + return null + } + } + + @Override + protected void error(String message, Object... args) { + errors << formatLog(message, args) + } + + private String formatLog(String message, Object... args) { + if (message == null) return "" + if (!args || args.length == 0) return message + StringBuilder sb = new StringBuilder() + int last = 0 + int argIdx = 0 + while (true) { + int idx = message.indexOf("{}", last) + if (idx == -1) { + sb.append(message.substring(last)) + break + } + sb.append(message.substring(last, idx)) + if (argIdx < args.length) { + sb.append(String.valueOf(args[argIdx++])) + } else { + sb.append("{}") + } + last = idx + 2 + } + return sb.toString() + } + } + + def "valid: removing an existing annotation should pass (class, field, method, param)"() { + given: + String classInternal = internalName("ClassWithAnnotations") + String annInternal = internalName("AnnPresent") + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "remove": [ + "${annInternal}" + ], + "fields": { + "bar:I": { + "remove": ["${annInternal}"] + } + }, + "methods": { + "method(Ljava/lang/String;)V": { + "remove": ["${annInternal}"], + "params": { + "0": { + "remove": ["${annInternal}"] + } + } + } + } + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + when: + boolean ok = validator.checkData(data) + + then: + ok + errors.isEmpty() + } + + def "valid: removing type parameter annotation on implements and return type"() { + given: + String classInternal = internalName("ClassWithImplements") + String classWithReturnTypeInternal = internalName("ClassWithReturnType") + String annInternal = internalName("AnnPresent") + def implTypeRef = TypeReference.newSuperTypeReference(0).value + def returnTypeRef = TypeReference.newTypeReference(TypeReference.METHOD_RETURN).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "type_remove": [ + { + "name": "${annInternal}", + "type_ref": ${implTypeRef}, + "type_path": "" + } + ] + }, + "${classWithReturnTypeInternal}": { + "methods": { + "annotatedGenericReturn()Ljava/util/List;": { + "type_remove": [ + { + "name": "${annInternal}", + "type_ref": ${returnTypeRef}, + "type_path": "0;" + } + ] + } + } + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + when: + boolean ok = validator.checkData(data) + + then: + ok + errors.isEmpty() + } + + def "valid: adding annotations and type annotations to class, field, method, and parameter"() { + given: + String classInternal = internalName("ClassWithoutAnnotations") + String genericInternal = internalName("AdvancedGenericTargetClass") + String addAnn = internalName("AnnAdd") + def classTypeParamRef = TypeReference.newTypeParameterReference(TypeReference.CLASS_TYPE_PARAMETER, 0).value + def fieldTypeRef = TypeReference.newTypeReference(TypeReference.FIELD).value + def methodReturnRef = TypeReference.newTypeReference(TypeReference.METHOD_RETURN).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "add": [ + { + "desc": "L${addAnn};" + } + ], + "fields": { + "otherField:I": { + "add": [ + { + "desc": "L${addAnn};" + } + ], + "type_add": [ + { + "desc": "L${addAnn};", + "type_ref": ${fieldTypeRef}, + "type_path": "" + } + ] + } + }, + "methods": { + "otherMethodWithParams(I)V": { + "add": [ + { + "desc": "L${addAnn};" + } + ], + "type_add": [ + { + "desc": "L${addAnn};", + "type_ref": ${methodReturnRef}, + "type_path": "" + } + ], + "params": { + "0": { + "add": [ + { + "desc": "L${addAnn};" + } + ] + } + } + } + } + }, + "${genericInternal}": { + "type_add": [ + { + "desc": "L${addAnn};", + "type_ref": ${classTypeParamRef}, + "type_path": "" + } + ] + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + when: + boolean ok = validator.checkData(data) + + then: + ok + errors.isEmpty() + } + + def "invalid: removing an annotation that isn't present should produce an error"() { + given: + String classInternal = internalName("ClassWithoutAnnotations") + String annNotPresentInternal = internalName("AnnNotPresent") + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "remove": [ + "${annNotPresentInternal}" + ] + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + String expected = "Trying to remove annotation ${annNotPresentInternal} from ${classInternal} but it's not present" + + when: + boolean ok = validator.checkData(data) + + then: + !ok + errors.contains(expected) + } + + def "invalid: adding an annotation already present should produce an error"() { + given: + String classInternal = internalName("ClassWithAnnotations") + String annPresentInternal = internalName("AnnPresent") + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "add": [ + { + "desc": "L${annPresentInternal};" + } + ] + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + String expected = "Trying to add annotation L${annPresentInternal}; to ${classInternal} but it's already present" + + when: + boolean ok = validator.checkData(data) + + then: + !ok + errors.contains(expected) + } + + def "invalid: adding/removing annotations to fields/methods/classes/params that don't exist should produce errors"() { + given: + String classInternal = internalName("ClassWithoutAnnotations") + String annAddInternal = internalName("AnnAdd") + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "fields": { + "noSuchField:I": { + "remove": ["${annAddInternal}"] + } + }, + "methods": { + "otherMethod()V": { + "add": [ + {"desc": "L${annAddInternal};"} + ], + "parameters": { + "5": { + "add": [{"desc": "L${annAddInternal};"}] + } + } + }, + "noSuchMethod()V": { + "add": [ + {"desc": "L${annAddInternal};"} + ] + } + } + }, + "net/fabricmc/loom/test/unit/processor/classes/NonExistentClass": { + "add": [ + {"desc": "L${annAddInternal};"} + ] + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + String expectedFieldErr = "No such target field: ${classInternal}.noSuchField:I" + String expectedMethodErr = "No such target method: ${classInternal}.noSuchMethod()V" + String expectedParamErr = "Invalid parameter index: 5 for method: ${classInternal}.otherMethod()V" + String expectedClassNotFound = "No such target class: net/fabricmc/loom/test/unit/processor/classes/NonExistentClass" + + when: + boolean ok = validator.checkData(data) + + then: + !ok + errors.contains(expectedFieldErr) + errors.contains(expectedMethodErr) + errors.contains(expectedParamErr) + errors.contains(expectedClassNotFound) + } + + def "invalid: adding annotation with attribute that doesn't exist; valid: adding with attribute that does exist"() { + given: + String classInternal = internalName("ClassWithoutAnnotations") + String annAttr = internalName("AnnWithAttr") + @Language("JSON") + String jsonBad = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "add": [ + { + "desc": "L${annAttr};", + "values": { + "nonexistent": { "type": "int", "value": 5 } + } + } + ] + } + }, + "namespace": "test" +} +""" + def readerBad = new BufferedReader(new StringReader(jsonBad)) + def dataBad = AnnotationsData.read(readerBad) + + List errors = [] + def validator = new TestValidator(errors) + + String expectedBad = "Unknown annotation attribute: ${annAttr}.nonexistent" + String expectedBadMissing = "Annotation applied to ${classInternal} is missing required attributes: [value]" + + when: + boolean okBad = validator.checkData(dataBad) + + then: + !okBad + errors.contains(expectedBad) + errors.contains(expectedBadMissing) + + when: "now add correct attribute" + errors.clear() + @Language("JSON") + String jsonGood = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "add": [ + { + "desc": "L${annAttr};", + "values": { + "value": { "type": "int", "value": 5 } + } + } + ] + } + }, + "namespace": "test" +} +""" + def readerGood = new BufferedReader(new StringReader(jsonGood)) + def dataGood = AnnotationsData.read(readerGood) + + boolean okGood = validator.checkData(dataGood) + + then: + okGood + errors.isEmpty() + } + + def "invalid: adding annotation with attribute value of wrong type"() { + given: + String classInternal = internalName("ClassWithoutAnnotations") + String annAttr = internalName("AnnWithAttr") + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "add": [ + { + "desc": "L${annAttr};", + "values": { + "value": { "type": "string", "value": "not-an-int" } + } + } + ] + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + String expected = "Annotation value is of type java.lang.String, expected int for attribute value" + + when: + boolean ok = validator.checkData(data) + + then: + !ok + errors.contains(expected) + } + + def "invalid: adding/removing type-parameters that don't exist"() { + given: + String classInternal = internalName("ClassWithGenericParams") + String annInternal = internalName("AnnAdd") + + def classTypeParamRef = TypeReference.newTypeParameterReference(TypeReference.CLASS_TYPE_PARAMETER, 5).value + def methodTypeParamRef = TypeReference.newTypeParameterReference(TypeReference.METHOD_TYPE_PARAMETER, 3).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "type_add": [ + { + "desc": "L${annInternal};", + "type_ref": ${classTypeParamRef}, + "type_path": "" + } + ], + "methods": { + "methodWithTypeParam()V": { + "type_add": [ + { + "desc": "L${annInternal};", + "type_ref": ${methodTypeParamRef}, + "type_path": "" + } + ] + } + } + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + String expectedClassTP = "Invalid type reference for class type annotation: ${classTypeParamRef}, formal parameter index 5 out of bounds" + String expectedMethodTP = "Invalid type reference for method type annotation: ${methodTypeParamRef}, formal parameter index 3 out of bounds" + + when: + boolean ok = validator.checkData(data) + + then: + !ok + errors.contains(expectedClassTP) + errors.contains(expectedMethodTP) + } + + def "valid: type annotation on class type parameter passes"() { + given: + String classInternal = internalName("ClassWithGenericParams") + String ann = internalName("AnnPresent") + def typeRef = TypeReference.newTypeParameterReference(TypeReference.CLASS_TYPE_PARAMETER, 1).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${typeRef}, + "type_path": "" + } + ] + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + when: + boolean ok = validator.checkData(data) + + then: + ok + errors.isEmpty() + } + + def "valid: type annotation on class type parameter bound passes"() { + given: + String classInternal = internalName("SelfGenericInterface") + String ann = internalName("AnnPresent") + def typeRef = TypeReference.newTypeParameterBoundReference(TypeReference.CLASS_TYPE_PARAMETER_BOUND, 0, 0).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${typeRef}, + "type_path": "" + } + ] + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + when: + boolean ok = validator.checkData(data) + + then: + ok + errors.isEmpty() + } + + def "valid: type annotation on extends (superclass) passes"() { + given: + String classInternal = internalName("ClassWithGenericParams") + String ann = internalName("AnnPresent") + def extendsRef = TypeReference.newSuperTypeReference(-1).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${extendsRef}, + "type_path": "" + } + ] + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + when: + boolean ok = validator.checkData(data) + + then: + ok + errors.isEmpty() + } + + def "valid: field type annotation with type path passes"() { + given: + String classInternal = internalName("ClassWithFieldTypes") + String ann = internalName("AnnPresent") + def fieldRef = TypeReference.newTypeReference(TypeReference.FIELD).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "fields": { + "listField:Ljava/util/List;": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${fieldRef}, + "type_path": "0;" + } + ] + }, + "arrayField:[Ljava/lang/String;": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${fieldRef}, + "type_path": "[" + } + ] + } + } + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + when: + boolean ok = validator.checkData(data) + + then: + ok + errors.isEmpty() + } + + def "valid: method return type annotation with type path passes"() { + given: + String classInternal = internalName("ClassWithReturnType") + String ann = internalName("AnnPresent") + def returnRef = TypeReference.newTypeReference(TypeReference.METHOD_RETURN).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "methods": { + "genericReturn()Ljava/util/List;": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${returnRef}, + "type_path": "0;" + } + ] + } + } + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + when: + boolean ok = validator.checkData(data) + + then: + ok + errors.isEmpty() + } + + def "receiver type annotation: valid on instance methods, invalid on static methods and constructors"() { + given: + String classInternal = internalName("ClassWithReceiverAndParams") + String ann = internalName("AnnPresent") + def receiverRef = TypeReference.newTypeReference(TypeReference.METHOD_RECEIVER).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "methods": { + "instanceMethod(ILjava/lang/String;)V": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${receiverRef}, + "type_path": "" + } + ] + }, + "staticMethod(I)V": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${receiverRef}, + "type_path": "" + } + ] + }, + "(I)V": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${receiverRef}, + "type_path": "" + } + ] + } + } + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + String expectedStaticReceiverErr = "Invalid type reference for method type annotation: ${receiverRef}, method receiver used in a static context" + + when: + boolean ok = validator.checkData(data) + + then: + !ok + errors.size() == 2 + errors.contains(expectedStaticReceiverErr) + } + + def "valid: type annotation on method formal parameter passes"() { + given: + String classInternal = internalName("ClassWithReceiverAndParams") + String ann = internalName("AnnPresent") + def formalParamRef = TypeReference.newFormalParameterReference(0).value + + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "methods": { + "instanceMethod(ILjava/lang/String;)V": { + "params": { + "0": { + "type_add": [ + { + "desc": "L${ann};", + "type_ref": ${formalParamRef}, + "type_path": "" + } + ] + } + } + } + } + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + when: + boolean ok = validator.checkData(data) + + then: + ok + errors.isEmpty() + } + + def "invalid: type annotation referring to out-of-bounds checked-exception index should produce an error"() { + given: + String classInternal = internalName("ClassWithoutAnnotations") + String annTypeInternal = internalName("AnnAdd") + int typeRef = TypeReference.newExceptionReference(0).value + @Language("JSON") + String json = """ +{ + "version": 1, + "classes": { + "${classInternal}": { + "type_add": [ + { + "desc": "L${annTypeInternal};", + "type_ref": ${typeRef}, + "type_path": "[" + } + ], + "methods": { + "otherMethod()V": { + "type_add": [ + { + "desc": "L${annTypeInternal};", + "type_ref": ${typeRef}, + "type_path": "" + } + ] + } + } + } + }, + "namespace": "test" +} +""" + def reader = new BufferedReader(new StringReader(json)) + def data = AnnotationsData.read(reader) + + List errors = [] + def validator = new TestValidator(errors) + + String expectedClassTypeRefErr = "Invalid type reference for class type annotation: ${typeRef}" + String expectedMethodTypeRefErr = "Invalid type reference for method type annotation: ${typeRef}, exception index 0 out of bounds" + + when: + boolean ok = validator.checkData(data) + + then: + !ok + errors.contains(expectedClassTypeRefErr) + errors.contains(expectedMethodTypeRefErr) + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/unit/processor/TypePathCheckerVisitorTest.groovy b/src/test/groovy/net/fabricmc/loom/test/unit/processor/TypePathCheckerVisitorTest.groovy new file mode 100644 index 00000000..1419d803 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/unit/processor/TypePathCheckerVisitorTest.groovy @@ -0,0 +1,94 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor + +import org.jetbrains.annotations.Nullable +import org.objectweb.asm.TypePath +import org.objectweb.asm.signature.SignatureReader +import spock.lang.Specification + +import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.validate.TypePathCheckerVisitor + +class TypePathCheckerVisitorTest extends Specification { + def "empty / null TypePath targets the whole type"() { + expect: + validate(null, "Ljava/lang/String;") == null + validate("", "Ljava/lang/String;") == null + } + + def "array element steps validate correctly"() { + expect: + validate("[", "[Ljava/lang/String;") == null + validate("[[", "[Ljava/lang/String;") == "TypePath not fully consumed at index 1, remaining steps: '['" + } + + def "type argument indexing and bounds"() { + expect: + // List + validate("0;", "Ljava/util/List;") == null + // second arg doesn't exist + validate("1;", "Ljava/util/List;") == "TypePath not fully consumed at index 0, remaining steps: '1;'" + + // List -> unbounded wildcard is a valid terminal target for type-argument 0 + validate("0;", "Ljava/util/List<*>;") == null + // but attempting to follow with a wildcard-bound step (0;*) is invalid for an unbounded wildcard + validate("0;*", "Ljava/util/List<*>;") == "TypePath targets unbounded wildcard '*' at step 0 but contains further steps; '*' is terminal." + + // List -> wildcard bound (*) after selecting arg 0 is valid + validate("0;*", "Ljava/util/List<+Ljava/lang/Number;>;") == null + } + + def "inner class stepping"() { + expect: + // Simple inner class descriptor + validate(".", "Lcom/example/Outer\$Inner;") == null + + // Wrong step (trying to use array step on a non-array) + validate("[", "Lcom/example/Outer\$Inner;") == "At step 0 expected inner type '.' but found '['." + } + + def "type variable cannot be followed by extra steps"() { + expect: + validate("0;", "TMyType;") == "TypePath has extra steps starting at index 0 ('0;') but reached type variable 'MyType'." + } + + def "complex: nested generics and bounds"() { + expect: + // Map> + String sig = "Ljava/util/Map;>;" + // target Map value type argument (index 1) and then the List's type argument (index 0) and its bound: + // path: 1;0;* -> TYPE_ARGUMENT(1) then TYPE_ARGUMENT(0) then WILDCARD_BOUND + String path = "1;0;*" + validate(path, sig) == null + } + + @Nullable + String validate(@Nullable String typePath, String signature) { + TypePath typePathObj = typePath == null ? null : TypePath.fromString(typePath) + TypePathCheckerVisitor visitor = new TypePathCheckerVisitor(typePathObj) + new SignatureReader(signature).acceptType(visitor) + return visitor.error + } +} \ No newline at end of file diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnAdd.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnAdd.java new file mode 100644 index 00000000..a4c73780 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnAdd.java @@ -0,0 +1,34 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.TYPE, ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD }) +public @interface AnnAdd { } diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnNotPresent.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnNotPresent.java new file mode 100644 index 00000000..883caf9e --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnNotPresent.java @@ -0,0 +1,34 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.TYPE, ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD }) +public @interface AnnNotPresent { } diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnPresent.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnPresent.java new file mode 100644 index 00000000..fa649175 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnPresent.java @@ -0,0 +1,34 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.TYPE, ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD, ElementType.TYPE_USE }) +public @interface AnnPresent { } diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnWithAttr.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnWithAttr.java new file mode 100644 index 00000000..be6c6f37 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/AnnWithAttr.java @@ -0,0 +1,37 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +// annotation with a single int attribute named `value` +@Retention(RetentionPolicy.RUNTIME) +@Target({ ElementType.TYPE, ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD, ElementType.TYPE_USE }) +public @interface AnnWithAttr { + int value(); +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithAnnotations.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithAnnotations.java new file mode 100644 index 00000000..150581d8 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithAnnotations.java @@ -0,0 +1,34 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +@AnnPresent +public class ClassWithAnnotations { + @AnnPresent + public int bar; + + @AnnPresent + public void method(@AnnPresent String param) { } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithFieldTypes.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithFieldTypes.java new file mode 100644 index 00000000..6a0d2045 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithFieldTypes.java @@ -0,0 +1,33 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +import java.util.List; + +public class ClassWithFieldTypes { + public List listField; + + public String[] arrayField; +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithGenericParams.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithGenericParams.java new file mode 100644 index 00000000..cc0396dd --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithGenericParams.java @@ -0,0 +1,30 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +public class ClassWithGenericParams<@AnnPresent T, U> { + public <@AnnPresent V> void methodWithTypeParam() { } +} + diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithImplements.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithImplements.java new file mode 100644 index 00000000..2d4a9ac9 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithImplements.java @@ -0,0 +1,28 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +public class ClassWithImplements implements @AnnPresent SimpleInterface { +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithReceiverAndParams.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithReceiverAndParams.java new file mode 100644 index 00000000..9baba5ef --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithReceiverAndParams.java @@ -0,0 +1,33 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +public class ClassWithReceiverAndParams { + public void instanceMethod(int a, String b) { } + + public static void staticMethod(int a) { } + + public ClassWithReceiverAndParams(int a) { } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithReturnType.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithReturnType.java new file mode 100644 index 00000000..6870538e --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithReturnType.java @@ -0,0 +1,37 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +import java.util.List; + +public class ClassWithReturnType { + public List genericReturn() { + return null; + } + + public List<@AnnPresent String> annotatedGenericReturn() { + return null; + } +} diff --git a/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithoutAnnotations.java b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithoutAnnotations.java new file mode 100644 index 00000000..f5416cc9 --- /dev/null +++ b/src/test/java/net/fabricmc/loom/test/unit/processor/classes/ClassWithoutAnnotations.java @@ -0,0 +1,35 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.processor.classes; + +public class ClassWithoutAnnotations { + public int otherField; + + public void otherMethod() { + } + + public void otherMethodWithParams(int param) { + } +} From 2dd467bdb753e21c969ad6d76417b9473a2f4fa1 Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 5 Oct 2025 09:53:49 +0100 Subject: [PATCH 05/25] Remove isRootProject from extension as it can hide `Project.getRootProject` when in the loom extension block. (#1381) --- src/main/java/net/fabricmc/loom/LoomGradleExtension.java | 2 -- .../java/net/fabricmc/loom/configuration/ide/RunConfig.java | 3 ++- .../fabricmc/loom/configuration/ide/RunConfigSettings.java | 3 ++- .../loom/configuration/ide/idea/IdeaConfiguration.java | 4 +--- .../net/fabricmc/loom/extension/LoomGradleExtensionImpl.java | 5 ----- src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java | 4 ++++ 6 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/LoomGradleExtension.java b/src/main/java/net/fabricmc/loom/LoomGradleExtension.java index 073fb794..68474957 100644 --- a/src/main/java/net/fabricmc/loom/LoomGradleExtension.java +++ b/src/main/java/net/fabricmc/loom/LoomGradleExtension.java @@ -99,8 +99,6 @@ public interface LoomGradleExtension extends LoomGradleExtensionAPI { FileCollection getMinecraftJarsCollection(MappingsNamespace mappingsNamespace); - boolean isRootProject(); - @Override MixinExtension getMixin(); diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java index 5c047009..0c42a784 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfig.java @@ -58,6 +58,7 @@ import net.fabricmc.loom.configuration.ide.idea.IdeaUtils; import net.fabricmc.loom.configuration.providers.BundleMetadata; import net.fabricmc.loom.configuration.providers.minecraft.library.LibraryContext; import net.fabricmc.loom.util.Constants; +import net.fabricmc.loom.util.gradle.GradleUtils; import net.fabricmc.loom.util.gradle.SourceSetReference; public class RunConfig { @@ -132,7 +133,7 @@ public class RunConfig { RunConfig runConfig = new RunConfig(); runConfig.configName = configName; - if (appendProjectPath && !extension.isRootProject()) { + if (appendProjectPath && !GradleUtils.isRootProject(project)) { runConfig.configName += " (" + project.getPath() + ")"; } diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java index 7a542829..248b8b2e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/RunConfigSettings.java @@ -46,6 +46,7 @@ import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftSourceSets; import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.Platform; +import net.fabricmc.loom.util.gradle.GradleUtils; import net.fabricmc.loom.util.gradle.SourceSetHelper; public abstract class RunConfigSettings implements Named { @@ -139,7 +140,7 @@ public abstract class RunConfigSettings implements Named { this.project = project; this.appendProjectPathToConfigName = project.getObjects().property(Boolean.class).convention(true); this.extension = LoomGradleExtension.get(project); - this.ideConfigGenerated = extension.isRootProject(); + this.ideConfigGenerated = GradleUtils.isRootProject(project); this.mainClass = project.getObjects().property(String.class).convention(project.provider(() -> { Objects.requireNonNull(environment, "Run config " + name + " must specify environment"); Objects.requireNonNull(defaultMainClass, "Run config " + name + " must specify default main class"); diff --git a/src/main/java/net/fabricmc/loom/configuration/ide/idea/IdeaConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/ide/idea/IdeaConfiguration.java index 95cf1438..0d7620bb 100644 --- a/src/main/java/net/fabricmc/loom/configuration/ide/idea/IdeaConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/ide/idea/IdeaConfiguration.java @@ -66,9 +66,7 @@ public abstract class IdeaConfiguration implements Runnable { } private void hookDownloadSources() { - LoomGradleExtension extension = LoomGradleExtension.get(getProject()); - - if (!extension.isRootProject()) { + if (!GradleUtils.isRootProject(getProject())) { return; } diff --git a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java index 40c59eb2..47ac3c3c 100644 --- a/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/LoomGradleExtensionImpl.java @@ -216,11 +216,6 @@ public abstract class LoomGradleExtensionImpl extends LoomGradleExtensionApiImpl return installerData; } - @Override - public boolean isRootProject() { - return project.getRootProject() == project; - } - @Override public MixinExtension getMixin() { return this.mixinApExtension; diff --git a/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java b/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java index 15685104..0303aa63 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java @@ -124,4 +124,8 @@ public final class GradleUtils { property.set(file); return property.getAsFile().get(); } + + public static boolean isRootProject(Project project) { + return project.getRootProject() == project; + } } From c97f9e4454f685685d74884d46607e816aabb91b Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 5 Oct 2025 10:18:30 +0100 Subject: [PATCH 06/25] Update to ASM 9.9 (#1382) --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 35d33302..c1093217 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,6 +1,6 @@ [versions] kotlin = "2.0.21" -asm = "9.8" +asm = "9.9" gson = "2.10.1" stitch = "0.6.2" From e2639f9e270bedda04eeef3ed9676b0113d60dcb Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 5 Oct 2025 16:26:37 +0100 Subject: [PATCH 07/25] Improve loom.mixin error message now that the AP is disabled by default. (#1383) --- .../net/fabricmc/loom/extension/MixinExtensionApiImpl.java | 4 +++- .../java/net/fabricmc/loom/extension/MixinExtensionImpl.java | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/extension/MixinExtensionApiImpl.java b/src/main/java/net/fabricmc/loom/extension/MixinExtensionApiImpl.java index 741ef986..6a4144ee 100644 --- a/src/main/java/net/fabricmc/loom/extension/MixinExtensionApiImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/MixinExtensionApiImpl.java @@ -40,6 +40,8 @@ import net.fabricmc.loom.api.MixinExtensionAPI; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; public abstract class MixinExtensionApiImpl implements MixinExtensionAPI { + static final String MIXIN_AP_DISABLED_ERROR = "The mixin annotation is no longer enabled by default, you should remove any loom.mixin configuration. If you wish to continue to use the mixin AP you can set useLegacyMixinAp = true."; + protected final Project project; protected final Property useMixinAp; private final Property refmapTargetNamespace; @@ -75,7 +77,7 @@ public abstract class MixinExtensionApiImpl implements MixinExtensionAPI { @Override public Property getRefmapTargetNamespace() { - if (!getUseLegacyMixinAp().get()) throw new IllegalStateException("You need to set useLegacyMixinAp = true to configure Mixin annotation processor."); + if (!getUseLegacyMixinAp().get()) throw new IllegalStateException(MIXIN_AP_DISABLED_ERROR); return refmapTargetNamespace; } diff --git a/src/main/java/net/fabricmc/loom/extension/MixinExtensionImpl.java b/src/main/java/net/fabricmc/loom/extension/MixinExtensionImpl.java index 4a05b7ae..4d74a9bf 100644 --- a/src/main/java/net/fabricmc/loom/extension/MixinExtensionImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/MixinExtensionImpl.java @@ -68,7 +68,7 @@ public class MixinExtensionImpl extends MixinExtensionApiImpl implements MixinEx @Override public Property getDefaultRefmapName() { - if (!super.getUseLegacyMixinAp().get()) throw new IllegalStateException("You need to set useLegacyMixinAp = true to configure Mixin annotation processor."); + if (!super.getUseLegacyMixinAp().get()) throw new IllegalStateException(MIXIN_AP_DISABLED_ERROR); return defaultRefmapName; } @@ -81,7 +81,7 @@ public class MixinExtensionImpl extends MixinExtensionApiImpl implements MixinEx @Override protected PatternSet add0(SourceSet sourceSet, Provider refmapName) { - if (!super.getUseLegacyMixinAp().get()) throw new IllegalStateException("You need to set useLegacyMixinAp = true to configure Mixin annotation processor."); + if (!super.getUseLegacyMixinAp().get()) throw new IllegalStateException(MIXIN_AP_DISABLED_ERROR); PatternSet pattern = new PatternSet().setIncludes(Collections.singletonList("**/*.json")); MixinExtension.setMixinInformationContainer(sourceSet, new MixinExtension.MixinInformationContainer(sourceSet, refmapName, pattern)); From d611d7f1b322d7ab8908158d0112b22173ff612e Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 5 Oct 2025 16:28:12 +0100 Subject: [PATCH 08/25] Replace TCA with forked fabric-log4j-util (#1375) * fabric-log4j-util * Update --- gradle/runtime.libs.versions.toml | 4 ++-- .../net/fabricmc/loom/configuration/LoomConfigurations.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gradle/runtime.libs.versions.toml b/gradle/runtime.libs.versions.toml index e930aad1..51a3dc2a 100644 --- a/gradle/runtime.libs.versions.toml +++ b/gradle/runtime.libs.versions.toml @@ -7,7 +7,7 @@ vineflower = "1.11.1" # Runtime depedencies mixin-compile-extensions = "0.6.0" dev-launch-injector = "0.2.1+build.8" -terminal-console-appender = "1.3.0" +fabric-log4j-util = "1.0.2" jetbrains-annotations = "26.0.2" native-support = "1.0.1" fabric-installer = "1.0.3" @@ -25,7 +25,7 @@ vineflower = { module = "org.vineflower:vineflower", version.ref = "vineflower" # Runtime depedencies mixin-compile-extensions = { module = "net.fabricmc:fabric-mixin-compile-extensions", version.ref = "mixin-compile-extensions" } dev-launch-injector = { module = "net.fabricmc:dev-launch-injector", version.ref = "dev-launch-injector" } -terminal-console-appender = { module = "net.minecrell:terminalconsoleappender", version.ref = "terminal-console-appender" } +fabric-log4j-util = { module = "net.fabricmc:fabric-log4j-util", version.ref = "fabric-log4j-util" } jetbrains-annotations = { module = "org.jetbrains:annotations", version.ref = "jetbrains-annotations" } native-support = { module = "net.fabricmc:fabric-loom-native-support", version.ref = "native-support" } fabric-installer = { module = "net.fabricmc:fabric-installer", version.ref = "fabric-installer" } diff --git a/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java b/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java index f774c9e7..72fa65a3 100644 --- a/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java +++ b/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java @@ -149,7 +149,7 @@ public abstract class LoomConfigurations implements Runnable { // Add the dev time dependencies getDependencies().add(Constants.Configurations.LOOM_DEVELOPMENT_DEPENDENCIES, LoomVersions.DEV_LAUNCH_INJECTOR.mavenNotation()); - getDependencies().add(Constants.Configurations.LOOM_DEVELOPMENT_DEPENDENCIES, LoomVersions.TERMINAL_CONSOLE_APPENDER.mavenNotation()); + getDependencies().add(Constants.Configurations.LOOM_DEVELOPMENT_DEPENDENCIES, LoomVersions.FABRIC_LOG4J_UTIL.mavenNotation()); getDependencies().add(JavaPlugin.COMPILE_ONLY_CONFIGURATION_NAME, LoomVersions.JETBRAINS_ANNOTATIONS.mavenNotation()); getDependencies().add(JavaPlugin.TEST_COMPILE_ONLY_CONFIGURATION_NAME, LoomVersions.JETBRAINS_ANNOTATIONS.mavenNotation()); From 4eac2e7845244725c1110a986e2d6bf42a8176f2 Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 5 Oct 2025 17:15:49 +0100 Subject: [PATCH 09/25] Fix CRF decompile failing on c0.30 (#1384) - ClassLineNumbers.readMappings throws a nicer error when the input data is empty. - GenerateSourcesTask include linemap filename when failed to read. - CRF decompiler wont write empty linemap data. - Add test --- .../decompilers/cfr/LoomCFRDecompiler.java | 4 +++ .../loom/decompilers/ClassLineNumbers.java | 19 +++++--------- .../loom/task/GenerateSourcesTask.java | 2 ++ .../test/integration/DecompileTest.groovy | 26 +++++++++++++++++++ 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/src/decompilers/cfr/net/fabricmc/loom/decompilers/cfr/LoomCFRDecompiler.java b/src/decompilers/cfr/net/fabricmc/loom/decompilers/cfr/LoomCFRDecompiler.java index 72f7b2e5..0f9510cc 100644 --- a/src/decompilers/cfr/net/fabricmc/loom/decompilers/cfr/LoomCFRDecompiler.java +++ b/src/decompilers/cfr/net/fabricmc/loom/decompilers/cfr/LoomCFRDecompiler.java @@ -98,6 +98,10 @@ public final class LoomCFRDecompiler implements LoomInternalDecompiler { } private void writeLineMap(Path output, Map> lineMap) { + if (lineMap.isEmpty()) { + return; + } + try (Writer writer = Files.newBufferedWriter(output, StandardCharsets.UTF_8)) { for (Map.Entry> classEntry : lineMap.entrySet()) { final String name = classEntry.getKey().replace(".", "/"); diff --git a/src/main/java/net/fabricmc/loom/decompilers/ClassLineNumbers.java b/src/main/java/net/fabricmc/loom/decompilers/ClassLineNumbers.java index 120972fa..64928cad 100644 --- a/src/main/java/net/fabricmc/loom/decompilers/ClassLineNumbers.java +++ b/src/main/java/net/fabricmc/loom/decompilers/ClassLineNumbers.java @@ -28,10 +28,7 @@ import static java.text.MessageFormat.format; import java.io.BufferedReader; import java.io.IOException; -import java.io.UncheckedIOException; import java.io.Writer; -import java.nio.file.Files; -import java.nio.file.Path; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -53,14 +50,6 @@ public record ClassLineNumbers(Map lineMap) { } } - public static ClassLineNumbers readMappings(Path lineMappingsPath) { - try (BufferedReader reader = Files.newBufferedReader(lineMappingsPath)) { - return readMappings(reader); - } catch (IOException e) { - throw new UncheckedIOException("Exception reading LineMappings file.", e); - } - } - public static ClassLineNumbers readMappings(BufferedReader reader) { var lineMap = new HashMap(); @@ -81,6 +70,7 @@ public record ClassLineNumbers(Map lineMap) { CurrentClass currentClass = null; Map currentMappings = new HashMap<>(); + boolean didRead = false; try { while ((line = reader.readLine()) != null) { @@ -88,6 +78,8 @@ public record ClassLineNumbers(Map lineMap) { continue; } + didRead = true; + final String[] segments = line.trim().split("\t"); if (line.charAt(0) != '\t') { @@ -108,7 +100,10 @@ public record ClassLineNumbers(Map lineMap) { throw new RuntimeException(format("Exception reading mapping line @{0}: {1}", lineNumber, line), e); } - assert currentClass != null; + if (!didRead) { + throw new IllegalArgumentException("Unable to read empty linemap data"); + } + currentClass.putEntry(lineMap, currentMappings); return new ClassLineNumbers(Collections.unmodifiableMap(lineMap)); diff --git a/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java b/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java index 9ff3ed2f..1fc99793 100644 --- a/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java +++ b/src/main/java/net/fabricmc/loom/task/GenerateSourcesTask.java @@ -631,6 +631,8 @@ public abstract class GenerateSourcesTask extends AbstractLoomTask { try (BufferedReader reader = Files.newBufferedReader(linemapFile, StandardCharsets.UTF_8)) { return ClassLineNumbers.readMappings(reader); + } catch (Exception e) { + throw new IOException("Failed to read line number map: " + linemapFile, e); } } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/DecompileTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/DecompileTest.groovy index 120d9e73..57b2d32f 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/DecompileTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/DecompileTest.groovy @@ -106,4 +106,30 @@ class DecompileTest extends Specification implements GradleProjectTestTrait { result2.task(":genSourcesWithVineflower").outcome == SUCCESS result3.task(":genSourcesWithVineflower").outcome == SUCCESS } + + // https://github.com/FabricMC/fabric-loom/issues/1362 + @Unroll + def "CFR legacy"() { + setup: + def gradle = gradleProject(project: "minimalBase", version: PRE_RELEASE_GRADLE) + gradle.buildGradle << ''' + loom { + noIntermediateMappings() + clientOnlyMinecraftJar() + } + + dependencies { + minecraft "com.mojang:minecraft:c0.30_01c" + mappings loom.layered() { + // No names + } + } + ''' + + when: + def result = gradle.run(task: "genSourcesWithCfr") + + then: + result.task(":genSourcesWithCfr").outcome == SUCCESS + } } From 53a99f860313644ba2a125da3a2504150f8eeaf9 Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 5 Oct 2025 17:57:56 +0100 Subject: [PATCH 10/25] Fix progress logger (#1386) --- src/main/java/net/fabricmc/loom/util/gradle/ProgressGroup.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/net/fabricmc/loom/util/gradle/ProgressGroup.java b/src/main/java/net/fabricmc/loom/util/gradle/ProgressGroup.java index 7eef1d2a..67ee00d3 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/ProgressGroup.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/ProgressGroup.java @@ -60,7 +60,7 @@ public class ProgressGroup implements Closeable { ProgressLogger progressLogger = this.progressLoggerFactory.newOperation(getClass(), progressGroup); progressLogger.setDescription(name); - progressLogger.start(name, null); + progressLogger.start(name, name); return progressLogger; } From ccb923d533cc0bcd56a61793e9a61554f1d72aa3 Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 5 Oct 2025 17:58:10 +0100 Subject: [PATCH 11/25] Fix remapping Kotlin classes containing $. (#1385) * Fix remapping Kotlin classes containing $. Closes #1363 * spotless --- .../loom/kotlin/remapping/KotlinClassRemapper.kt | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassRemapper.kt b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassRemapper.kt index 57aa3800..4a5924f1 100644 --- a/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassRemapper.kt +++ b/src/main/kotlin/net/fabricmc/loom/kotlin/remapping/KotlinClassRemapper.kt @@ -88,7 +88,15 @@ class KotlinClassRemapper( private fun remap(name: ClassName): ClassName { val local = name.isLocalClassName() - val remapped = remapper.map(name.toJvmInternalName()).replace('$', '.') + + // Ensure that none inner classes with names containing $ are persisted. + // See: https://github.com/FabricMC/fabric-loom/issues/1363 fix suggested by fan87 + val normalizedName = name.replace('$', '\n') + val remapped = + remapper + .map(normalizedName.toJvmInternalName()) + .replace('$', '.') + .replace('\n', '$') if (local) { return ".$remapped" From 3d2a0802425266e9a5d2e59fb75d05d54c6f4f9b Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Sun, 5 Oct 2025 18:07:27 +0100 Subject: [PATCH 12/25] Invalid remapped sources when using refresh dependencies. Closes #1142 --- .../loom/configuration/mods/ModConfigurationRemapper.java | 6 ++++-- src/main/java/net/fabricmc/loom/util/SourceRemapper.java | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java index 9e537c84..ab2c5ee9 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/ModConfigurationRemapper.java @@ -70,11 +70,11 @@ import net.fabricmc.loom.configuration.mods.dependency.ModDependency; import net.fabricmc.loom.configuration.mods.dependency.ModDependencyFactory; import net.fabricmc.loom.configuration.mods.dependency.ModDependencyOptions; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftSourceSets; +import net.fabricmc.loom.util.AsyncCache; import net.fabricmc.loom.util.Checksum; import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.ExceptionUtil; import net.fabricmc.loom.util.SourceRemapper; -import net.fabricmc.loom.util.AsyncCache; import net.fabricmc.loom.util.gradle.SourceSetHelper; import net.fabricmc.loom.util.service.ServiceFactory; @@ -361,7 +361,9 @@ public class ModConfigurationRemapper { return; } - if (dependency.isCacheInvalid(project, "sources")) { + LoomGradleExtension extension = LoomGradleExtension.get(project); + + if (dependency.isCacheInvalid(project, "sources") || extension.refreshDeps()) { final Path output = dependency.getWorkingFile(project, "sources"); sourceRemapper.scheduleRemapSources(sourcesInput.toFile(), output.toFile(), false, true, () -> { diff --git a/src/main/java/net/fabricmc/loom/util/SourceRemapper.java b/src/main/java/net/fabricmc/loom/util/SourceRemapper.java index f6f445c0..6c54bab0 100644 --- a/src/main/java/net/fabricmc/loom/util/SourceRemapper.java +++ b/src/main/java/net/fabricmc/loom/util/SourceRemapper.java @@ -67,6 +67,7 @@ public class SourceRemapper { remapTasks.add((logger) -> { try { logger.progress("remapping sources - " + source.getName()); + Files.deleteIfExists(destination.toPath()); remapSourcesInner(source, destination); ZipReprocessorUtil.reprocessZip(destination.toPath(), reproducibleFileOrder, preserveFileTimestamps); From fd2f807647c4d01bb6bd033717a09700b571614a Mon Sep 17 00:00:00 2001 From: modmuss Date: Sun, 5 Oct 2025 19:18:57 +0100 Subject: [PATCH 13/25] Only warn when attempting to configure the disabled mixin AP (#1387) --- .../loom/extension/MixinExtensionApiImpl.java | 18 ++++++++++++++++-- .../loom/extension/MixinExtensionImpl.java | 4 ++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/extension/MixinExtensionApiImpl.java b/src/main/java/net/fabricmc/loom/extension/MixinExtensionApiImpl.java index 6a4144ee..4f84dc29 100644 --- a/src/main/java/net/fabricmc/loom/extension/MixinExtensionApiImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/MixinExtensionApiImpl.java @@ -29,18 +29,22 @@ import java.util.Objects; import org.gradle.api.Action; import org.gradle.api.InvalidUserDataException; import org.gradle.api.Project; +import org.gradle.api.logging.configuration.WarningMode; import org.gradle.api.plugins.JavaPluginExtension; import org.gradle.api.provider.MapProperty; import org.gradle.api.provider.Property; import org.gradle.api.provider.Provider; import org.gradle.api.tasks.SourceSet; import org.gradle.api.tasks.util.PatternSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import net.fabricmc.loom.api.MixinExtensionAPI; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; public abstract class MixinExtensionApiImpl implements MixinExtensionAPI { - static final String MIXIN_AP_DISABLED_ERROR = "The mixin annotation is no longer enabled by default, you should remove any loom.mixin configuration. If you wish to continue to use the mixin AP you can set useLegacyMixinAp = true."; + private static final String MIXIN_AP_DISABLED_ERROR = "The mixin annotation is no longer enabled by default, you should remove any loom.mixin configuration. If you wish to continue to use the mixin AP you can set useLegacyMixinAp = true."; + private static final Logger LOGGER = LoggerFactory.getLogger(MixinExtensionApiImpl.class); protected final Project project; protected final Property useMixinAp; @@ -77,7 +81,7 @@ public abstract class MixinExtensionApiImpl implements MixinExtensionAPI { @Override public Property getRefmapTargetNamespace() { - if (!getUseLegacyMixinAp().get()) throw new IllegalStateException(MIXIN_AP_DISABLED_ERROR); + if (!getUseLegacyMixinAp().get()) logLegacyMixinAPConfiguration(); return refmapTargetNamespace; } @@ -175,4 +179,14 @@ public abstract class MixinExtensionApiImpl implements MixinExtensionAPI { throw new RuntimeException("Yeah... something is really wrong"); } } + + final void logLegacyMixinAPConfiguration() { + final WarningMode warningMode = project.getGradle().getStartParameter().getWarningMode(); + + if (warningMode == WarningMode.Fail) { + throw new IllegalStateException(MIXIN_AP_DISABLED_ERROR); + } else if (warningMode != WarningMode.None) { + LOGGER.warn(MIXIN_AP_DISABLED_ERROR); + } + } } diff --git a/src/main/java/net/fabricmc/loom/extension/MixinExtensionImpl.java b/src/main/java/net/fabricmc/loom/extension/MixinExtensionImpl.java index 4d74a9bf..29e3b0df 100644 --- a/src/main/java/net/fabricmc/loom/extension/MixinExtensionImpl.java +++ b/src/main/java/net/fabricmc/loom/extension/MixinExtensionImpl.java @@ -68,7 +68,7 @@ public class MixinExtensionImpl extends MixinExtensionApiImpl implements MixinEx @Override public Property getDefaultRefmapName() { - if (!super.getUseLegacyMixinAp().get()) throw new IllegalStateException(MIXIN_AP_DISABLED_ERROR); + if (!super.getUseLegacyMixinAp().get()) logLegacyMixinAPConfiguration(); return defaultRefmapName; } @@ -81,7 +81,7 @@ public class MixinExtensionImpl extends MixinExtensionApiImpl implements MixinEx @Override protected PatternSet add0(SourceSet sourceSet, Provider refmapName) { - if (!super.getUseLegacyMixinAp().get()) throw new IllegalStateException(MIXIN_AP_DISABLED_ERROR); + if (!super.getUseLegacyMixinAp().get()) logLegacyMixinAPConfiguration(); PatternSet pattern = new PatternSet().setIncludes(Collections.singletonList("**/*.json")); MixinExtension.setMixinInformationContainer(sourceSet, new MixinExtension.MixinInformationContainer(sourceSet, refmapName, pattern)); From 5250e9fb855f9b70ed3654fec12e1b9578d09269 Mon Sep 17 00:00:00 2001 From: Juuz <6596629+Juuxel@users.noreply.github.com> Date: Tue, 7 Oct 2025 00:00:03 +0300 Subject: [PATCH 14/25] Fix invisible log levels being enabled in the log4j config (#1388) The previous level made loggers report that e.g. the trace level is always enabled which is clearly wrong. --- src/main/resources/log4j2.fabric.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/log4j2.fabric.xml b/src/main/resources/log4j2.fabric.xml index 3f9d29bc..068c2038 100644 --- a/src/main/resources/log4j2.fabric.xml +++ b/src/main/resources/log4j2.fabric.xml @@ -59,7 +59,7 @@ - + From 501070a47a75ba0dbb28fff9cd6bb5c19a2b6140 Mon Sep 17 00:00:00 2001 From: Joseph Burton Date: Wed, 8 Oct 2025 12:01:23 +0100 Subject: [PATCH 15/25] Make annotations data a bit nicer to use (#1391) * Make annotations data a bit nicer to use * Add copy constructors --- .../extras/annotations/AnnotationsData.java | 40 +++++++++++++++++++ .../annotations/BaseAnnotationData.java | 38 ++++++++++++++++++ .../annotations/ClassAnnotationData.java | 17 +++++++- .../annotations/GenericAnnotationData.java | 15 ++++++- .../annotations/MethodAnnotationData.java | 16 +++++++- 5 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/BaseAnnotationData.java diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/AnnotationsData.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/AnnotationsData.java index b11cba57..51c79896 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/AnnotationsData.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/AnnotationsData.java @@ -27,11 +27,13 @@ package net.fabricmc.loom.configuration.providers.mappings.extras.annotations; import java.io.IOException; import java.io.Reader; import java.lang.reflect.Type; +import java.util.ArrayList; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.function.Function; +import java.util.function.UnaryOperator; import com.google.gson.FieldNamingPolicy; import com.google.gson.Gson; @@ -69,6 +71,14 @@ public record AnnotationsData(Map classes, String n } } + public AnnotationsData(String namespace) { + this(new LinkedHashMap<>(), namespace); + } + + public AnnotationsData(AnnotationsData other) { + this(copyMap(other.classes, ClassAnnotationData::new), other.namespace); + } + public static AnnotationsData read(Reader reader) { JsonObject json = GSON.fromJson(reader, JsonObject.class); checkVersion(json); @@ -118,6 +128,36 @@ public record AnnotationsData(Map classes, String n return result; } + static Map copyMap(Map map, UnaryOperator valueCopier) { + Map result = LinkedHashMap.newLinkedHashMap(map.size()); + map.forEach((key, value) -> result.put(key, valueCopier.apply(value))); + return result; + } + + static List copyAnnotations(List annotations) { + List result = new ArrayList<>(annotations.size()); + + for (AnnotationNode annotation : annotations) { + AnnotationNode newAnnotation = new AnnotationNode(annotation.desc); + annotation.accept(newAnnotation); + result.add(newAnnotation); + } + + return result; + } + + static List copyTypeAnnotations(List annotations) { + List result = new ArrayList<>(annotations.size()); + + for (TypeAnnotationNode annotation : annotations) { + TypeAnnotationNode newAnnotation = new TypeAnnotationNode(annotation.typeRef, annotation.typePath, annotation.desc); + annotation.accept(newAnnotation); + result.add(newAnnotation); + } + + return result; + } + public AnnotationsData merge(AnnotationsData other) { if (!namespace.equals(other.namespace)) { throw new IllegalArgumentException("Cannot merge annotations from namespace " + other.namespace + " into annotations from namespace " + this.namespace); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/BaseAnnotationData.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/BaseAnnotationData.java new file mode 100644 index 00000000..ef000b14 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/BaseAnnotationData.java @@ -0,0 +1,38 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.extras.annotations; + +import java.util.List; +import java.util.Set; + +import org.objectweb.asm.tree.AnnotationNode; +import org.objectweb.asm.tree.TypeAnnotationNode; + +public interface BaseAnnotationData { + Set annotationsToRemove(); + List annotationsToAdd(); + Set typeAnnotationsToRemove(); + List typeAnnotationsToAdd(); +} diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/ClassAnnotationData.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/ClassAnnotationData.java index 14ed43c5..b5ba93e1 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/ClassAnnotationData.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/ClassAnnotationData.java @@ -52,7 +52,7 @@ public record ClassAnnotationData( List typeAnnotationsToAdd, Map fields, Map methods -) { +) implements BaseAnnotationData { public ClassAnnotationData { if (annotationsToRemove == null) { annotationsToRemove = new LinkedHashSet<>(); @@ -79,6 +79,21 @@ public record ClassAnnotationData( } } + public ClassAnnotationData() { + this(new LinkedHashSet<>(), new ArrayList<>(), new LinkedHashSet<>(), new ArrayList<>(), new LinkedHashMap<>(), new LinkedHashMap<>()); + } + + public ClassAnnotationData(ClassAnnotationData other) { + this( + new LinkedHashSet<>(other.annotationsToRemove), + AnnotationsData.copyAnnotations(other.annotationsToAdd), + new LinkedHashSet<>(other.typeAnnotationsToRemove), + AnnotationsData.copyTypeAnnotations(other.typeAnnotationsToAdd), + AnnotationsData.copyMap(other.fields, GenericAnnotationData::new), + AnnotationsData.copyMap(other.methods, MethodAnnotationData::new) + ); + } + ClassAnnotationData merge(ClassAnnotationData other) { Set newAnnotationsToRemove = new LinkedHashSet<>(annotationsToRemove); newAnnotationsToRemove.addAll(other.annotationsToRemove); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/GenericAnnotationData.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/GenericAnnotationData.java index 94a2e96e..a0e96d2a 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/GenericAnnotationData.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/GenericAnnotationData.java @@ -46,7 +46,7 @@ public record GenericAnnotationData( Set typeAnnotationsToRemove, @SerializedName("type_add") List typeAnnotationsToAdd -) { +) implements BaseAnnotationData { public GenericAnnotationData { if (annotationsToRemove == null) { annotationsToRemove = new LinkedHashSet<>(); @@ -65,6 +65,19 @@ public record GenericAnnotationData( } } + public GenericAnnotationData() { + this(new LinkedHashSet<>(), new ArrayList<>(), new LinkedHashSet<>(), new ArrayList<>()); + } + + public GenericAnnotationData(GenericAnnotationData other) { + this( + new LinkedHashSet<>(other.annotationsToRemove), + AnnotationsData.copyAnnotations(other.annotationsToAdd), + new LinkedHashSet<>(other.typeAnnotationsToRemove), + AnnotationsData.copyTypeAnnotations(other.typeAnnotationsToAdd) + ); + } + GenericAnnotationData merge(GenericAnnotationData other) { Set newAnnotationToRemove = new LinkedHashSet<>(annotationsToRemove); newAnnotationToRemove.addAll(other.annotationsToRemove); diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/MethodAnnotationData.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/MethodAnnotationData.java index fde79765..deef4b45 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/MethodAnnotationData.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/MethodAnnotationData.java @@ -49,7 +49,7 @@ public record MethodAnnotationData( @SerializedName("type_add") List typeAnnotationsToAdd, Map parameters -) { +) implements BaseAnnotationData { public MethodAnnotationData { if (annotationsToRemove == null) { annotationsToRemove = new LinkedHashSet<>(); @@ -72,6 +72,20 @@ public record MethodAnnotationData( } } + public MethodAnnotationData() { + this(new LinkedHashSet<>(), new ArrayList<>(), new LinkedHashSet<>(), new ArrayList<>(), new LinkedHashMap<>()); + } + + public MethodAnnotationData(MethodAnnotationData other) { + this( + new LinkedHashSet<>(other.annotationsToRemove), + AnnotationsData.copyAnnotations(other.annotationsToAdd), + new LinkedHashSet<>(other.typeAnnotationsToRemove), + AnnotationsData.copyTypeAnnotations(other.typeAnnotationsToAdd), + AnnotationsData.copyMap(other.parameters, GenericAnnotationData::new) + ); + } + MethodAnnotationData merge(MethodAnnotationData other) { Set newAnnotationsToRemove = new LinkedHashSet<>(annotationsToRemove); newAnnotationsToRemove.addAll(other.annotationsToRemove); From 5656d4179202ab1f63300305369c66fb8c650a0c Mon Sep 17 00:00:00 2001 From: Joseph Burton Date: Wed, 15 Oct 2025 17:10:16 +0100 Subject: [PATCH 16/25] Apply record component annotations to getter methods and canonical constructor parameters (#1394) --- .../minecraft/AnnotationsApplyVisitor.java | 576 ++++++++++++------ 1 file changed, 392 insertions(+), 184 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/AnnotationsApplyVisitor.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/AnnotationsApplyVisitor.java index 786b6114..316b2faa 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/AnnotationsApplyVisitor.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/AnnotationsApplyVisitor.java @@ -24,6 +24,12 @@ package net.fabricmc.loom.configuration.providers.minecraft; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.function.BiConsumer; + +import org.jetbrains.annotations.Nullable; import org.objectweb.asm.AnnotationVisitor; import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.FieldVisitor; @@ -32,7 +38,9 @@ import org.objectweb.asm.Opcodes; import org.objectweb.asm.RecordComponentVisitor; import org.objectweb.asm.Type; import org.objectweb.asm.TypePath; +import org.objectweb.asm.TypeReference; import org.objectweb.asm.tree.AnnotationNode; +import org.objectweb.asm.tree.MethodNode; import org.objectweb.asm.tree.TypeAnnotationNode; import net.fabricmc.loom.configuration.providers.mappings.extras.annotations.AnnotationsData; @@ -58,6 +66,10 @@ public record AnnotationsApplyVisitor(AnnotationsData annotationsData) implement public static class AnnotationsApplyClassVisitor extends ClassVisitor { private final ClassAnnotationData classData; + private boolean isRecord; + private final List storedRecordMethods = new ArrayList<>(); + private final StringBuilder canonicalConstructorDesc = new StringBuilder("("); + private final List<@Nullable GenericAnnotationData> recordComponentAnnotationData = new ArrayList<>(); private boolean hasAddedAnnotations; public AnnotationsApplyClassVisitor(ClassVisitor cv, ClassAnnotationData classData) { @@ -69,6 +81,7 @@ public record AnnotationsApplyVisitor(AnnotationsData annotationsData) implement @Override public void visit(int version, int access, String name, String signature, String superName, String[] interfaces) { access = classData.modifyAccessFlags(access); + isRecord = (access & Opcodes.ACC_RECORD) != 0; super.visit(version, access, name, signature, superName, interfaces); } @@ -112,58 +125,21 @@ public record AnnotationsApplyVisitor(AnnotationsData annotationsData) implement public RecordComponentVisitor visitRecordComponent(String name, String descriptor, String signature) { addClassAnnotations(); + GenericAnnotationData fieldData = classData.getFieldData(name, descriptor); + canonicalConstructorDesc.append(descriptor); + recordComponentAnnotationData.add(fieldData); + RecordComponentVisitor rcv = super.visitRecordComponent(name, descriptor, signature); if (rcv == null) { return null; } - GenericAnnotationData fieldData = classData.getFieldData(name, descriptor); - if (fieldData == null) { return rcv; } - return new RecordComponentVisitor(Constants.ASM_VERSION, rcv) { - @Override - public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { - if (fieldData.annotationsToRemove().contains(Type.getType(descriptor).getInternalName())) { - return null; - } - - return super.visitAnnotation(descriptor, visible); - } - - @Override - public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { - if (fieldData.typeAnnotationsToRemove().contains(new TypeAnnotationKey(typeRef, typePath.toString(), Type.getType(descriptor).getInternalName()))) { - return null; - } - - return super.visitTypeAnnotation(typeRef, typePath, descriptor, visible); - } - - @Override - public void visitEnd() { - for (AnnotationNode annotation : fieldData.annotationsToAdd()) { - AnnotationVisitor av = delegate.visitAnnotation(annotation.desc, false); - - if (av != null) { - annotation.accept(av); - } - } - - for (TypeAnnotationNode typeAnnotation : fieldData.typeAnnotationsToAdd()) { - AnnotationVisitor av = delegate.visitTypeAnnotation(typeAnnotation.typeRef, typeAnnotation.typePath, typeAnnotation.desc, false); - - if (av != null) { - typeAnnotation.accept(av); - } - } - - super.visitEnd(); - } - }; + return new ApplyRecordComponentVisitor(rcv, fieldData); } @Override @@ -182,173 +158,71 @@ public record AnnotationsApplyVisitor(AnnotationsData annotationsData) implement return null; } - return new FieldVisitor(Constants.ASM_VERSION, fv) { - @Override - public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { - if (fieldData.annotationsToRemove().contains(Type.getType(descriptor).getInternalName())) { - return null; - } - - return super.visitAnnotation(descriptor, visible); - } - - @Override - public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { - if (fieldData.typeAnnotationsToRemove().contains(new TypeAnnotationKey(typeRef, typePath.toString(), Type.getType(descriptor).getInternalName()))) { - return null; - } - - return super.visitTypeAnnotation(typeRef, typePath, descriptor, visible); - } - - @Override - public void visitEnd() { - for (AnnotationNode annotation : fieldData.annotationsToAdd()) { - AnnotationVisitor av = fv.visitAnnotation(annotation.desc, false); - - if (av != null) { - annotation.accept(av); - } - } - - for (TypeAnnotationNode typeAnnotation : fieldData.typeAnnotationsToAdd()) { - AnnotationVisitor av = fv.visitTypeAnnotation(typeAnnotation.typeRef, typeAnnotation.typePath, typeAnnotation.desc, false); - - if (av != null) { - typeAnnotation.accept(av); - } - } - - super.visitEnd(); - } - }; + return new ApplyFieldVisitor(fv, fieldData); } @Override public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) { addClassAnnotations(); + if (isRecord) { + MethodNode node = new MethodNode(access, name, descriptor, signature, exceptions); + storedRecordMethods.add(node); + return node; + } + + return visitMethodInner(access, name, descriptor, signature, exceptions); + } + + private MethodVisitor visitMethodInner(int access, String name, String descriptor, String signature, String[] exceptions) { MethodAnnotationData methodData = classData.getMethodData(name, descriptor); - if (methodData == null) { - return super.visitMethod(access, name, descriptor, signature, exceptions); + if (isRecord && "".equals(name) && canonicalConstructorDesc.toString().equals(descriptor)) { + MethodVisitor mv = super.visitMethod(methodData != null ? methodData.modifyAccessFlags(access) : access, name, descriptor, signature, exceptions); + return new CanonicalConstructorApplyVisitor(mv, methodData, descriptor, recordComponentAnnotationData); } - MethodVisitor mv = super.visitMethod(methodData.modifyAccessFlags(access), name, descriptor, signature, exceptions); + if (methodData != null) { + MethodVisitor mv = super.visitMethod(methodData.modifyAccessFlags(access), name, descriptor, signature, exceptions); - if (mv == null) { - return null; + if (mv == null) { + return null; + } + + return new ApplyMethodVisitor(mv, methodData, descriptor); } - return new MethodVisitor(Constants.ASM_VERSION, mv) { - int syntheticParameterCount = 0; - boolean visitedAnnotableParameterCount = false; - boolean hasAddedAnnotations = false; + if (isRecord && descriptor.startsWith("()")) { + GenericAnnotationData fieldData = classData.getFieldData(name, descriptor.substring(2)); - @Override - public void visitParameter(String name, int access) { - if ((access & Opcodes.ACC_SYNTHETIC) != 0) { - syntheticParameterCount++; - } + if (fieldData != null) { + MethodVisitor mv = super.visitMethod(fieldData.modifyAccessFlags(access), name, descriptor, signature, exceptions); - super.visitParameter(name, access); - } - - @Override - public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { - if (methodData.annotationsToRemove().contains(Type.getType(descriptor).getInternalName())) { + if (mv == null) { return null; } - return super.visitAnnotation(descriptor, visible); + return new RecordComponentGetterApplyVisitor(mv, fieldData); } + } - @Override - public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { - if (methodData.typeAnnotationsToRemove().contains(new TypeAnnotationKey(typeRef, typePath.toString(), Type.getType(descriptor).getInternalName()))) { - return null; - } - - return super.visitTypeAnnotation(typeRef, typePath, descriptor, visible); - } - - @Override - public AnnotationVisitor visitParameterAnnotation(int parameter, String descriptor, boolean visible) { - GenericAnnotationData parameterData = methodData.parameters().get(parameter); - - if (parameterData != null && parameterData.annotationsToRemove().contains(Type.getType(descriptor).getInternalName())) { - return null; - } - - return super.visitParameterAnnotation(parameter, descriptor, visible); - } - - @Override - public void visitAnnotableParameterCount(int parameterCount, boolean visible) { - if (!visible && !methodData.parameters().isEmpty()) { - parameterCount = Math.max(parameterCount, Type.getArgumentCount(descriptor) - syntheticParameterCount); - visitedAnnotableParameterCount = true; - } - - super.visitAnnotableParameterCount(parameterCount, visible); - } - - @Override - public void visitCode() { - addMethodAnnotations(); - super.visitCode(); - } - - @Override - public void visitEnd() { - addMethodAnnotations(); - super.visitEnd(); - } - - void addMethodAnnotations() { - if (hasAddedAnnotations) { - return; - } - - hasAddedAnnotations = true; - - for (AnnotationNode annotation : methodData.annotationsToAdd()) { - AnnotationVisitor av = mv.visitAnnotation(annotation.desc, false); - - if (av != null) { - annotation.accept(av); - } - } - - for (TypeAnnotationNode typeAnnotation : methodData.typeAnnotationsToAdd()) { - AnnotationVisitor av = mv.visitTypeAnnotation(typeAnnotation.typeRef, typeAnnotation.typePath, typeAnnotation.desc, false); - - if (av != null) { - typeAnnotation.accept(av); - } - } - - if (!visitedAnnotableParameterCount && !methodData.parameters().isEmpty()) { - mv.visitAnnotableParameterCount(Type.getArgumentCount(descriptor) - syntheticParameterCount, false); - visitedAnnotableParameterCount = true; - } - - methodData.parameters().forEach((paramIndex, paramData) -> { - for (AnnotationNode annotation : paramData.annotationsToAdd()) { - AnnotationVisitor av = mv.visitParameterAnnotation(paramIndex, annotation.desc, false); - - if (av != null) { - annotation.accept(av); - } - } - }); - } - }; + return super.visitMethod(access, name, descriptor, signature, exceptions); } @Override public void visitEnd() { addClassAnnotations(); + + canonicalConstructorDesc.append(")V"); + + for (MethodNode method : storedRecordMethods) { + MethodVisitor mv = visitMethodInner(method.access, method.name, method.desc, method.signature, method.exceptions == null ? null : method.exceptions.toArray(new String[0])); + + if (mv != null) { + method.accept(mv); + } + } + super.visitEnd(); } @@ -376,4 +250,338 @@ public record AnnotationsApplyVisitor(AnnotationsData annotationsData) implement } } } + + private static class ApplyRecordComponentVisitor extends RecordComponentVisitor { + private final GenericAnnotationData fieldData; + + ApplyRecordComponentVisitor(RecordComponentVisitor rcv, GenericAnnotationData fieldData) { + super(Constants.ASM_VERSION, rcv); + this.fieldData = fieldData; + } + + @Override + public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { + if (fieldData.annotationsToRemove().contains(Type.getType(descriptor).getInternalName())) { + return null; + } + + return super.visitAnnotation(descriptor, visible); + } + + @Override + public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { + if (fieldData.typeAnnotationsToRemove().contains(new TypeAnnotationKey(typeRef, typePath.toString(), Type.getType(descriptor).getInternalName()))) { + return null; + } + + return super.visitTypeAnnotation(typeRef, typePath, descriptor, visible); + } + + @Override + public void visitEnd() { + for (AnnotationNode annotation : fieldData.annotationsToAdd()) { + AnnotationVisitor av = delegate.visitAnnotation(annotation.desc, false); + + if (av != null) { + annotation.accept(av); + } + } + + for (TypeAnnotationNode typeAnnotation : fieldData.typeAnnotationsToAdd()) { + AnnotationVisitor av = delegate.visitTypeAnnotation(typeAnnotation.typeRef, typeAnnotation.typePath, typeAnnotation.desc, false); + + if (av != null) { + typeAnnotation.accept(av); + } + } + + super.visitEnd(); + } + } + + private static class ApplyFieldVisitor extends FieldVisitor { + private final GenericAnnotationData fieldData; + + ApplyFieldVisitor(FieldVisitor fv, GenericAnnotationData fieldData) { + super(Constants.ASM_VERSION, fv); + this.fieldData = fieldData; + } + + @Override + public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { + if (fieldData.annotationsToRemove().contains(Type.getType(descriptor).getInternalName())) { + return null; + } + + return super.visitAnnotation(descriptor, visible); + } + + @Override + public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { + if (fieldData.typeAnnotationsToRemove().contains(new TypeAnnotationKey(typeRef, typePath.toString(), Type.getType(descriptor).getInternalName()))) { + return null; + } + + return super.visitTypeAnnotation(typeRef, typePath, descriptor, visible); + } + + @Override + public void visitEnd() { + for (AnnotationNode annotation : fieldData.annotationsToAdd()) { + AnnotationVisitor av = fv.visitAnnotation(annotation.desc, false); + + if (av != null) { + annotation.accept(av); + } + } + + for (TypeAnnotationNode typeAnnotation : fieldData.typeAnnotationsToAdd()) { + AnnotationVisitor av = fv.visitTypeAnnotation(typeAnnotation.typeRef, typeAnnotation.typePath, typeAnnotation.desc, false); + + if (av != null) { + typeAnnotation.accept(av); + } + } + + super.visitEnd(); + } + } + + private abstract static class AbstractApplyMethodVisitor extends MethodVisitor { + @Nullable + protected final MethodAnnotationData methodData; + private final String descriptor; + private int syntheticParameterCount = 0; + private boolean visitedAnnotableParameterCount = false; + private boolean hasAddedAnnotations = false; + + AbstractApplyMethodVisitor(MethodVisitor mv, @Nullable MethodAnnotationData methodData, String descriptor) { + super(Constants.ASM_VERSION, mv); + this.methodData = methodData; + this.descriptor = descriptor; + } + + @Nullable + protected abstract GenericAnnotationData getParameterData(int parameterIndex); + + protected abstract boolean hasAnnotableParameters(); + + protected abstract void forEachParameterData(BiConsumer action); + + @Override + public void visitParameter(String name, int access) { + if ((access & Opcodes.ACC_SYNTHETIC) != 0) { + syntheticParameterCount++; + } + + super.visitParameter(name, access); + } + + @Override + public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { + if (methodData != null && methodData.annotationsToRemove().contains(Type.getType(descriptor).getInternalName())) { + return null; + } + + return super.visitAnnotation(descriptor, visible); + } + + @Override + public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { + if (methodData != null && methodData.typeAnnotationsToRemove().contains(new TypeAnnotationKey(typeRef, typePath.toString(), Type.getType(descriptor).getInternalName()))) { + return null; + } + + return super.visitTypeAnnotation(typeRef, typePath, descriptor, visible); + } + + @Override + public AnnotationVisitor visitParameterAnnotation(int parameter, String descriptor, boolean visible) { + GenericAnnotationData parameterData = getParameterData(parameter); + + if (parameterData != null && parameterData.annotationsToRemove().contains(Type.getType(descriptor).getInternalName())) { + return null; + } + + return super.visitParameterAnnotation(parameter, descriptor, visible); + } + + @Override + public void visitAnnotableParameterCount(int parameterCount, boolean visible) { + if (!visible && hasAnnotableParameters()) { + parameterCount = Math.max(parameterCount, Type.getArgumentCount(descriptor) - syntheticParameterCount); + visitedAnnotableParameterCount = true; + } + + super.visitAnnotableParameterCount(parameterCount, visible); + } + + @Override + public void visitCode() { + addMethodAnnotations(); + super.visitCode(); + } + + @Override + public void visitEnd() { + addMethodAnnotations(); + super.visitEnd(); + } + + void addMethodAnnotations() { + if (hasAddedAnnotations) { + return; + } + + hasAddedAnnotations = true; + + if (methodData != null) { + for (AnnotationNode annotation : methodData.annotationsToAdd()) { + AnnotationVisitor av = mv.visitAnnotation(annotation.desc, false); + + if (av != null) { + annotation.accept(av); + } + } + + for (TypeAnnotationNode typeAnnotation : methodData.typeAnnotationsToAdd()) { + AnnotationVisitor av = mv.visitTypeAnnotation(typeAnnotation.typeRef, typeAnnotation.typePath, typeAnnotation.desc, false); + + if (av != null) { + typeAnnotation.accept(av); + } + } + } + + if (!visitedAnnotableParameterCount && hasAnnotableParameters()) { + mv.visitAnnotableParameterCount(Type.getArgumentCount(descriptor) - syntheticParameterCount, false); + visitedAnnotableParameterCount = true; + } + + forEachParameterData((paramIndex, paramData) -> { + for (AnnotationNode annotation : paramData.annotationsToAdd()) { + AnnotationVisitor av = mv.visitParameterAnnotation(paramIndex, annotation.desc, false); + + if (av != null) { + annotation.accept(av); + } + } + }); + } + } + + private static class ApplyMethodVisitor extends AbstractApplyMethodVisitor { + ApplyMethodVisitor(MethodVisitor mv, MethodAnnotationData methodData, String descriptor) { + super(mv, methodData, descriptor); + } + + @Override + @Nullable + protected GenericAnnotationData getParameterData(int parameterIndex) { + return methodData == null ? null : methodData.parameters().get(parameterIndex); + } + + @Override + protected boolean hasAnnotableParameters() { + return methodData != null && !methodData.parameters().isEmpty(); + } + + @Override + protected void forEachParameterData(BiConsumer action) { + if (methodData != null) { + methodData.parameters().forEach(action); + } + } + } + + private static class CanonicalConstructorApplyVisitor extends AbstractApplyMethodVisitor { + private final List<@Nullable GenericAnnotationData> parameterData; + + CanonicalConstructorApplyVisitor(MethodVisitor mv, @Nullable MethodAnnotationData methodData, String descriptor, List<@Nullable GenericAnnotationData> parameterData) { + super(mv, methodData, descriptor); + this.parameterData = parameterData; + } + + @Override + @Nullable + protected GenericAnnotationData getParameterData(int parameterIndex) { + return parameterIndex >= 0 && parameterIndex < parameterData.size() ? parameterData.get(parameterIndex) : null; + } + + @Override + protected boolean hasAnnotableParameters() { + return parameterData.stream().anyMatch(Objects::nonNull); + } + + @Override + protected void forEachParameterData(BiConsumer action) { + for (int i = 0; i < parameterData.size(); i++) { + GenericAnnotationData data = parameterData.get(i); + + if (data != null) { + action.accept(i, data); + } + } + } + } + + private static class RecordComponentGetterApplyVisitor extends MethodVisitor { + private final GenericAnnotationData fieldData; + + private RecordComponentGetterApplyVisitor(MethodVisitor mv, GenericAnnotationData fieldData) { + super(Constants.ASM_VERSION, mv); + this.fieldData = fieldData; + } + + @Override + public AnnotationVisitor visitAnnotation(String descriptor, boolean visible) { + if (fieldData.annotationsToRemove().contains(Type.getType(descriptor).getInternalName())) { + return null; + } + + return super.visitAnnotation(descriptor, visible); + } + + @Override + public AnnotationVisitor visitTypeAnnotation(int typeRef, TypePath typePath, String descriptor, boolean visible) { + int fieldTypeRef = typeRef; + + if (new TypeReference(fieldTypeRef).getSort() == TypeReference.METHOD_RETURN) { + fieldTypeRef = TypeReference.newTypeReference(TypeReference.FIELD).getValue(); + } + + if (fieldData.typeAnnotationsToRemove().contains(new TypeAnnotationKey(fieldTypeRef, typePath.toString(), Type.getType(descriptor).getInternalName()))) { + return null; + } + + return super.visitTypeAnnotation(typeRef, typePath, descriptor, visible); + } + + @Override + public void visitEnd() { + for (AnnotationNode annotation : fieldData.annotationsToAdd()) { + AnnotationVisitor av = mv.visitAnnotation(annotation.desc, false); + + if (av != null) { + annotation.accept(av); + } + } + + for (TypeAnnotationNode typeAnnotation : fieldData.typeAnnotationsToAdd()) { + int methodTypeRef = typeAnnotation.typeRef; + + if (new TypeReference(methodTypeRef).getSort() == TypeReference.METHOD_RETURN) { + methodTypeRef = TypeReference.newTypeReference(TypeReference.FIELD).getValue(); + } + + AnnotationVisitor av = mv.visitTypeAnnotation(methodTypeRef, typeAnnotation.typePath, typeAnnotation.desc, false); + + if (av != null) { + typeAnnotation.accept(av); + } + } + + super.visitEnd(); + } + } } From 3783f41ef34db0b92b6a31ab8fb496bedbd3daa1 Mon Sep 17 00:00:00 2001 From: Joseph Burton Date: Wed, 15 Oct 2025 21:52:49 +0100 Subject: [PATCH 17/25] Fix annotations not being applied in a project outside Enigma (#1395) --- .../extras/annotations/AnnotationsData.java | 26 +++++++++---------- .../minecraft/AnnotationsApplyVisitor.java | 3 ++- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/AnnotationsData.java b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/AnnotationsData.java index 51c79896..3143a754 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/AnnotationsData.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/mappings/extras/annotations/AnnotationsData.java @@ -24,11 +24,9 @@ package net.fabricmc.loom.configuration.providers.mappings.extras.annotations; -import java.io.IOException; import java.io.Reader; import java.lang.reflect.Type; import java.util.ArrayList; -import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -49,7 +47,7 @@ import org.objectweb.asm.tree.TypeAnnotationNode; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.configuration.providers.mappings.MappingConfiguration; -import net.fabricmc.loom.util.TinyRemapperHelper; +import net.fabricmc.loom.task.service.TinyRemapperService; import net.fabricmc.loom.util.service.ServiceFactory; import net.fabricmc.tinyremapper.TinyRemapper; @@ -204,34 +202,34 @@ public record AnnotationsData(Map classes, String n } @Nullable - public static AnnotationsData getRemappedAnnotations(MappingsNamespace targetNamespace, MappingConfiguration mappingConfiguration, Project project, ServiceFactory serviceFactory, String newNamespace) throws IOException { + public static AnnotationsData getRemappedAnnotations(MappingsNamespace targetNamespace, MappingConfiguration mappingConfiguration, Project project, ServiceFactory serviceFactory, String newNamespace) { List datas = mappingConfiguration.getAnnotationsData(); if (datas.isEmpty()) { return null; } - Map existingRemappers = new HashMap<>(); - AnnotationsData result = datas.getFirst().remap(targetNamespace, project, serviceFactory, newNamespace, existingRemappers); + AnnotationsData result = datas.getFirst().remap(targetNamespace, project, serviceFactory, newNamespace); for (int i = 1; i < datas.size(); i++) { - result = result.merge(datas.get(i).remap(targetNamespace, project, serviceFactory, newNamespace, existingRemappers)); + result = result.merge(datas.get(i).remap(targetNamespace, project, serviceFactory, newNamespace)); } return result; } - private AnnotationsData remap(MappingsNamespace targetNamespace, Project project, ServiceFactory serviceFactory, String newNamespace, Map existingRemappers) throws IOException { + private AnnotationsData remap(MappingsNamespace targetNamespace, Project project, ServiceFactory serviceFactory, String newNamespace) { if (namespace.equals(targetNamespace.toString())) { return this; } - TinyRemapper remapper = existingRemappers.get(namespace); - - if (remapper == null) { - remapper = TinyRemapperHelper.getTinyRemapper(project, serviceFactory, namespace, newNamespace); - existingRemappers.put(namespace, remapper); - } + TinyRemapperService remapperService = serviceFactory.get(TinyRemapperService.createSimple( + project, + project.provider(() -> namespace), + project.provider(() -> newNamespace), + TinyRemapperService.ClasspathLibraries.EXCLUDE + )); + TinyRemapper remapper = remapperService.getTinyRemapperForRemapping(); return remap(remapper, newNamespace); } diff --git a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/AnnotationsApplyVisitor.java b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/AnnotationsApplyVisitor.java index 316b2faa..b90c67c8 100644 --- a/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/AnnotationsApplyVisitor.java +++ b/src/main/java/net/fabricmc/loom/configuration/providers/minecraft/AnnotationsApplyVisitor.java @@ -55,7 +55,8 @@ import net.fabricmc.tinyremapper.api.TrClass; public record AnnotationsApplyVisitor(AnnotationsData annotationsData) implements TinyRemapper.ApplyVisitorProvider { @Override public ClassVisitor insertApplyVisitor(TrClass cls, ClassVisitor next) { - ClassAnnotationData classData = annotationsData.classes().get(cls.getName()); + String deobfName = cls.getEnvironment().getRemapper().map(cls.getName()); + ClassAnnotationData classData = annotationsData.classes().get(deobfName); if (classData == null) { return next; From b2c933d2c0192bd1b95ce1c663595692e59d5ca0 Mon Sep 17 00:00:00 2001 From: Joseph Burton Date: Fri, 17 Oct 2025 11:30:39 +0100 Subject: [PATCH 18/25] Add annotations apply integration test (#1396) --- .../integration/AnnotationsApplyTest.groovy | 75 +++++++++++++++++++ .../test/util/GradleProjectTestTrait.groovy | 31 +++++--- .../projects/annotationsApply/build.gradle | 9 +++ .../src/main/java/ExampleMod.java | 8 ++ 4 files changed, 111 insertions(+), 12 deletions(-) create mode 100644 src/test/groovy/net/fabricmc/loom/test/integration/AnnotationsApplyTest.groovy create mode 100644 src/test/resources/projects/annotationsApply/build.gradle create mode 100644 src/test/resources/projects/annotationsApply/src/main/java/ExampleMod.java diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/AnnotationsApplyTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/AnnotationsApplyTest.groovy new file mode 100644 index 00000000..e208b378 --- /dev/null +++ b/src/test/groovy/net/fabricmc/loom/test/integration/AnnotationsApplyTest.groovy @@ -0,0 +1,75 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.integration + +import java.util.function.Predicate + +import org.objectweb.asm.ClassReader +import org.objectweb.asm.tree.ClassNode +import org.objectweb.asm.tree.MethodNode +import spock.lang.Specification +import spock.lang.Unroll + +import net.fabricmc.loom.test.util.GradleProjectTestTrait +import net.fabricmc.loom.util.ZipUtils + +import static net.fabricmc.loom.test.LoomTestConstants.STANDARD_TEST_VERSIONS +import static org.gradle.testkit.runner.TaskOutcome.SUCCESS + +class AnnotationsApplyTest extends Specification implements GradleProjectTestTrait { + @Unroll + def "annotations apply (gradle #version)"() { + setup: + def gradle = gradleProject(project: "annotationsApply", version: version) + + when: + def result = gradle.run(task: "build") + + then: + result.task(":build").outcome == SUCCESS + checkClass(gradle, "net/minecraft/util/math/BlockPos") { + MethodNode method = it.methods.find { + it.name == "add" && it.desc == "(Lnet/minecraft/util/math/Vec3i;)Lnet/minecraft/util/math/BlockPos;" + } + + if (method == null || method.invisibleAnnotations == null) { + return false + } + + method.invisibleAnnotations.any { it.desc == "Lorg/jetbrains/annotations/Contract;" } + } + + where: + version << STANDARD_TEST_VERSIONS + } + + boolean checkClass(GradleProject gradle, String clazz, Predicate test) { + byte[] bytecode = ZipUtils.unpack(gradle.getGeneratedMinecraft("25w42a-net.fabricmc.yarn.25w42a.25w42a+build.9-v2").toPath(), "${clazz}.class") + ClassReader reader = new ClassReader(bytecode) + ClassNode node = new ClassNode() + reader.accept(node, 0) + return test.test(node) + } +} diff --git a/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy b/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy index d14673be..86df3a47 100644 --- a/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy @@ -24,7 +24,6 @@ package net.fabricmc.loom.test.util -import groovy.io.FileType import groovy.transform.Immutable import org.apache.commons.io.FileUtils import org.gradle.testkit.runner.BuildResult @@ -261,25 +260,33 @@ trait GradleProjectTestTrait { return ZipUtils.unpackNullable(file.toPath(), entryName) != null } - File getGeneratedSources(String mappings, String jarType = "merged") { - return new File(getGradleHomeDir(), "caches/fabric-loom/minecraftMaven/net/minecraft/minecraft-${jarType}/${mappings}/minecraft-${jarType}-${mappings}-sources.jar") + File getGeneratedMinecraft(String mappings, String jarType = "merged", String classifier = "") { + String classifierSuffix = classifier.isEmpty() ? "" : "-$classifier" + return new File(getGradleHomeDir(), "caches/fabric-loom/minecraftMaven/net/minecraft/minecraft-${jarType}/${mappings}/minecraft-${jarType}-${mappings}${classifierSuffix}.jar") } - File getGeneratedLocalSources(String mappings) { - File file - getProjectDir().traverse(type: FileType.FILES) { - if (it.name.startsWith("minecraft-merged-") - && it.name.contains(mappings) - && it.name.endsWith("-sources.jar")) { - file = it - } + File getGeneratedSources(String mappings, String jarType = "merged") { + return getGeneratedMinecraft(mappings, jarType, "sources") + } + + File getGeneratedLocalMinecraft(String mappings, String jarType = "merged", String classifier = "") { + String classifierSuffix = classifier.isEmpty() ? "" : "-$classifier" + + File file = new File(getProjectDir(), ".gradle/loom-cache/minecraftMaven/net/minecraft") + file = file.listFiles().find { + it.name.startsWith("minecraft-${jarType}-") } if (file == null) { throw new FileNotFoundException() } - return file + String jarFileName = "${file.name}-${mappings}${classifierSuffix}.jar" + return new File(file, "${mappings}/${jarFileName}") + } + + File getGeneratedLocalSources(String mappings, String jarType = "merged") { + return getGeneratedLocalMinecraft(mappings, jarType, "sources") } void buildSrc(String name, boolean apply = true) { diff --git a/src/test/resources/projects/annotationsApply/build.gradle b/src/test/resources/projects/annotationsApply/build.gradle new file mode 100644 index 00000000..7aec635b --- /dev/null +++ b/src/test/resources/projects/annotationsApply/build.gradle @@ -0,0 +1,9 @@ +plugins { + id 'fabric-loom' +} + +dependencies { + minecraft "com.mojang:minecraft:25w42a" + mappings "net.fabricmc:yarn:25w42a+build.9:v2" + modImplementation "net.fabricmc:fabric-loader:0.17.3" +} diff --git a/src/test/resources/projects/annotationsApply/src/main/java/ExampleMod.java b/src/test/resources/projects/annotationsApply/src/main/java/ExampleMod.java new file mode 100644 index 00000000..f6675524 --- /dev/null +++ b/src/test/resources/projects/annotationsApply/src/main/java/ExampleMod.java @@ -0,0 +1,8 @@ + +import net.fabricmc.api.ModInitializer; + +public class ExampleMod implements ModInitializer { + @Override + public void onInitialize() { + } +} From 09a4831f25079373ad86ecadde9550f9c439bf01 Mon Sep 17 00:00:00 2001 From: modmuss Date: Mon, 20 Oct 2025 22:46:36 +0100 Subject: [PATCH 19/25] Support classpath groups when using configure on demand. (#1392) * Support classpath groups when using configure on demand. * Cleanup * Work around Gradle 8.14 issue * Another fix * Rename plugin * Fix plugin versioning * Add some docs * More fixes * Ensure backwards compatible. --- build.gradle | 42 +++-- .../loom/LoomCompanionGradlePlugin.java | 44 +++++ .../net/fabricmc/loom/LoomGradlePlugin.java | 3 + .../net/fabricmc/loom/api/ModSettings.java | 53 +++++- .../configuration/CompileConfiguration.java | 28 +-- .../configuration/LoomConfigurations.java | 4 +- .../classpathgroups/ClasspathGroup.java | 53 ++++++ .../ExternalClasspathGroup.java | 30 ++++ .../ExternalClasspathGroupDTO.java | 87 ++++++++++ .../loom/task/launch/ExportClasspathTask.java | 93 ++++++++++ .../task/launch/GenerateDLIConfigTask.java | 37 ++-- .../task/service/ClasspathGroupService.java | 162 ++++++++++++++++++ .../net/fabricmc/loom/util/Constants.java | 5 + .../loom/util/gradle/GradleUtils.java | 3 +- .../loom/util/gradle/SourceSetHelper.java | 15 +- .../test/integration/MultiProjectTest.groovy | 15 ++ .../projects/multiproject/build.gradle | 7 +- .../multiproject/javalib/build.gradle | 4 + .../projects/multiproject/settings.gradle | 3 +- 19 files changed, 611 insertions(+), 77 deletions(-) create mode 100644 src/main/java/net/fabricmc/loom/LoomCompanionGradlePlugin.java create mode 100644 src/main/java/net/fabricmc/loom/configuration/classpathgroups/ClasspathGroup.java create mode 100644 src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroup.java create mode 100644 src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroupDTO.java create mode 100644 src/main/java/net/fabricmc/loom/task/launch/ExportClasspathTask.java create mode 100644 src/main/java/net/fabricmc/loom/task/service/ClasspathGroupService.java create mode 100644 src/test/resources/projects/multiproject/javalib/build.gradle diff --git a/build.gradle b/build.gradle index aaf025ef..7e68d615 100644 --- a/build.gradle +++ b/build.gradle @@ -237,6 +237,10 @@ gradlePlugin { id = 'fabric-loom' implementationClass = 'net.fabricmc.loom.LoomGradlePlugin' } + fabricLoomCompanion { + id = 'net.fabricmc.fabric-loom-companion' + implementationClass = 'net.fabricmc.loom.LoomCompanionGradlePlugin' + } } } @@ -292,25 +296,27 @@ publishing { from components.java } - // Manually crate the plugin marker for snapshot versions - snapshotPlugin(MavenPublication) { publication -> - groupId = 'fabric-loom' - artifactId = 'fabric-loom.gradle.plugin' - version = baseVersion + '-SNAPSHOT' + gradlePlugin.plugins.forEach { plugin -> + // Manually crate the plugin marker for snapshot versions + it.create(plugin.id + "SnapshotMarker", MavenPublication) { publication -> + groupId = plugin.id + artifactId = plugin.id + '.gradle.plugin' + version = baseVersion + '-SNAPSHOT' - pom.withXml({ - // Based off org.gradle.plugin.devel.plugins.MavenPluginPublishPlugin - Element root = asElement() - Document document = root.getOwnerDocument() - Node dependencies = root.appendChild(document.createElement('dependencies')) - Node dependency = dependencies.appendChild(document.createElement('dependency')) - Node groupId = dependency.appendChild(document.createElement('groupId')) - groupId.setTextContent('net.fabricmc') - Node artifactId = dependency.appendChild(document.createElement('artifactId')) - artifactId.setTextContent('fabric-loom') - Node version = dependency.appendChild(document.createElement('version')) - version.setTextContent(baseVersion + '-SNAPSHOT') - }) + pom.withXml({ + // Based off org.gradle.plugin.devel.plugins.MavenPluginPublishPlugin + Element root = asElement() + Document document = root.getOwnerDocument() + Node dependencies = root.appendChild(document.createElement('dependencies')) + Node dependency = dependencies.appendChild(document.createElement('dependency')) + Node groupId = dependency.appendChild(document.createElement('groupId')) + groupId.setTextContent('net.fabricmc') + Node artifactId = dependency.appendChild(document.createElement('artifactId')) + artifactId.setTextContent('fabric-loom') + Node version = dependency.appendChild(document.createElement('version')) + version.setTextContent(project.version) + }) + } } } } diff --git a/src/main/java/net/fabricmc/loom/LoomCompanionGradlePlugin.java b/src/main/java/net/fabricmc/loom/LoomCompanionGradlePlugin.java new file mode 100644 index 00000000..18a6ca99 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/LoomCompanionGradlePlugin.java @@ -0,0 +1,44 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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; + +import org.gradle.api.Plugin; +import org.gradle.api.Project; +import org.jetbrains.annotations.NotNull; + +import net.fabricmc.loom.configuration.LoomConfigurations; +import net.fabricmc.loom.task.launch.ExportClasspathTask; +import net.fabricmc.loom.util.Constants; + +public class LoomCompanionGradlePlugin implements Plugin { + public static final String NAME = "net.fabricmc.fabric-loom-companion"; + + @Override + public void apply(@NotNull Project project) { + var exportClassPathTask = project.getTasks().register(Constants.Task.EXPORT_CLASSPATH, ExportClasspathTask.class); + project.getConfigurations().register(Constants.Configurations.EXPORTED_CLASSPATH, LoomConfigurations.Role.CONSUMABLE::apply); + project.artifacts(artifactHandler -> artifactHandler.add(Constants.Configurations.EXPORTED_CLASSPATH, exportClassPathTask)); + } +} diff --git a/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java b/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java index c253d5ea..05a2cdeb 100644 --- a/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java +++ b/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java @@ -50,6 +50,7 @@ import net.fabricmc.loom.task.RemapTaskConfiguration; import net.fabricmc.loom.util.LibraryLocationLogger; public class LoomGradlePlugin implements Plugin { + public static final String NAME = "fabric-loom"; public static final Gson GSON = new GsonBuilder().setPrettyPrinting().create(); public static final String LOOM_VERSION = Objects.requireNonNullElse(LoomGradlePlugin.class.getPackage().getImplementationVersion(), "0.0.0+unknown"); @@ -92,5 +93,7 @@ public class LoomGradlePlugin implements Plugin { for (Class jobClass : SETUP_JOBS) { project.getObjects().newInstance(jobClass).run(); } + + project.apply(Map.of("plugin", LoomCompanionGradlePlugin.NAME)); } } diff --git a/src/main/java/net/fabricmc/loom/api/ModSettings.java b/src/main/java/net/fabricmc/loom/api/ModSettings.java index 7df3134c..d40c0e46 100644 --- a/src/main/java/net/fabricmc/loom/api/ModSettings.java +++ b/src/main/java/net/fabricmc/loom/api/ModSettings.java @@ -24,6 +24,10 @@ package net.fabricmc.loom.api; +import java.io.File; +import java.util.List; +import java.util.Map; + import javax.inject.Inject; import org.gradle.api.Named; @@ -35,6 +39,8 @@ import org.gradle.api.provider.ListProperty; import org.gradle.api.tasks.SourceSet; import org.jetbrains.annotations.ApiStatus; +import net.fabricmc.loom.LoomCompanionGradlePlugin; +import net.fabricmc.loom.configuration.classpathgroups.ExternalClasspathGroup; import net.fabricmc.loom.util.gradle.SourceSetHelper; import net.fabricmc.loom.util.gradle.SourceSetReference; @@ -49,7 +55,7 @@ public abstract class ModSettings implements Named { @Inject public ModSettings() { - getModSourceSets().finalizeValueOnRead(); + getExternalGroups().finalizeValueOnRead(); getModFiles().finalizeValueOnRead(); } @@ -82,18 +88,46 @@ public abstract class ModSettings implements Named { /** * Add {@link SourceSet}'s output directories from the supplied project to be grouped with the named mod. + * @deprecated Replaced with {@link #sourceSet(String, String)} to avoid passing a project reference. */ + @Deprecated public void sourceSet(SourceSet sourceSet, Project project) { - getModSourceSets().add(new SourceSetReference(sourceSet, project)); + ensureCompanion(project); + + sourceSet(sourceSet.getName(), project.getPath()); } /** * Add {@link SourceSet}'s output directories from the supplied project to be grouped with the named mod. * * @param name the name of the source set + * @deprecated Replaced with {@link #sourceSet(String, String)} to avoid passing a project reference. */ + @Deprecated public void sourceSet(String name, Project project) { - sourceSet(SourceSetHelper.getSourceSetByName(name, project), project); + ensureCompanion(project); + + sourceSet(name, project.getPath()); + } + + /** + * Add {@link SourceSet}'s output directories from the supplied project to be grouped with the named mod. + * + *

If the other project is not a Loom project you must apply the `net.fabricmc.fabric-loom-companion` plugin. + * + * @param sourceSetName the name of the source set + * @param projectPath the path of the project the source set belongs to + */ + public void sourceSet(String sourceSetName, String projectPath) { + if (projectPath.equals(getProject().getPath())) { + // Shortcut for source sets in our own project. + SourceSetReference ref = new SourceSetReference(SourceSetHelper.getSourceSetByName(sourceSetName, getProject()), getProject()); + List classpath = SourceSetHelper.getClasspath(ref); + getModFiles().from(classpath); + return; + } + + getExternalGroups().add(new ExternalClasspathGroup(projectPath, sourceSetName)); } /** @@ -114,11 +148,10 @@ public abstract class ModSettings implements Named { } /** - * List of classpath directories, used to populate the `fabric.classPathGroups` Fabric Loader system property. - * Use the {@link ModSettings#sourceSet} methods to add to this. + * List of {@link ExternalClasspathGroup} that will later be resolved to populate the classpath groups from another Gradle project. */ @ApiStatus.Internal - public abstract ListProperty getModSourceSets(); + public abstract ListProperty getExternalGroups(); @Inject public abstract Project getProject(); @@ -127,4 +160,12 @@ public abstract class ModSettings implements Named { public String toString() { return "ModSettings '" + getName() + "'"; } + + private void ensureCompanion(Project project) { + if (project == getProject()) { + return; + } + + project.apply(Map.of("plugin", LoomCompanionGradlePlugin.NAME)); + } } diff --git a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java index 6c050bd0..249a4f6c 100644 --- a/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java +++ b/src/main/java/net/fabricmc/loom/configuration/CompileConfiguration.java @@ -26,7 +26,6 @@ package net.fabricmc.loom.configuration; import static net.fabricmc.loom.util.Constants.Configurations; -import java.io.File; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; @@ -36,12 +35,13 @@ import java.time.Duration; import java.util.Objects; import java.util.Optional; import java.util.function.Consumer; -import java.util.stream.Collectors; import javax.inject.Inject; +import org.gradle.api.Action; import org.gradle.api.GradleException; import org.gradle.api.Project; +import org.gradle.api.Task; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; import org.gradle.api.plugins.JavaPlugin; @@ -71,6 +71,7 @@ import net.fabricmc.loom.configuration.providers.minecraft.mapped.AbstractMapped import net.fabricmc.loom.configuration.providers.minecraft.mapped.IntermediaryMinecraftProvider; import net.fabricmc.loom.configuration.providers.minecraft.mapped.NamedMinecraftProvider; import net.fabricmc.loom.extension.MixinExtension; +import net.fabricmc.loom.task.service.ClasspathGroupService; import net.fabricmc.loom.util.Checksum; import net.fabricmc.loom.util.ExceptionUtil; import net.fabricmc.loom.util.ProcessUtil; @@ -266,15 +267,22 @@ public abstract class CompileConfiguration implements Runnable { } getProject().getTasks().named(JavaPlugin.TEST_TASK_NAME, Test.class, test -> { - String classPathGroups = extension.getMods().stream() - .map(modSettings -> - SourceSetHelper.getClasspath(modSettings, getProject()).stream() - .map(File::getAbsolutePath) - .collect(Collectors.joining(File.pathSeparator)) - ) - .collect(Collectors.joining(File.pathSeparator+File.pathSeparator));; + test.getInputs().property("LoomClassPathGroups", ClasspathGroupService.create(getProject())); + test.doFirst(new Action() { + @Override + public void execute(Task task) { + try (ScopedServiceFactory serviceFactory = new ScopedServiceFactory()) { + var options = (ClasspathGroupService.Options) task.getInputs().getProperties().get("LoomClassPathGroups"); + ClasspathGroupService classpathGroupService = serviceFactory.get(options); - test.systemProperty("fabric.classPathGroups", classPathGroups); + if (classpathGroupService.hasGroups()) { + test.systemProperty("fabric.classPathGroups", classpathGroupService.getClasspathGroupsPropertyValue()); + } + } catch (IOException e) { + throw new UncheckedIOException("Failed to get classpath groups", e); + } + } + }); }); } diff --git a/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java b/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java index 72fa65a3..3586d4ce 100644 --- a/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java +++ b/src/main/java/net/fabricmc/loom/configuration/LoomConfigurations.java @@ -175,7 +175,7 @@ public abstract class LoomConfigurations implements Runnable { getConfigurations().getByName(a, configuration -> configuration.extendsFrom(getConfigurations().getByName(b))); } - enum Role { + public enum Role { NONE(false, false), CONSUMABLE(true, false), RESOLVABLE(false, true); @@ -188,7 +188,7 @@ public abstract class LoomConfigurations implements Runnable { this.canBeResolved = canBeResolved; } - void apply(Configuration configuration) { + public void apply(Configuration configuration) { configuration.setCanBeConsumed(canBeConsumed); configuration.setCanBeResolved(canBeResolved); } diff --git a/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ClasspathGroup.java b/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ClasspathGroup.java new file mode 100644 index 00000000..ae383520 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ClasspathGroup.java @@ -0,0 +1,53 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.classpathgroups; + +import java.io.File; +import java.io.Serializable; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import net.fabricmc.loom.api.ModSettings; + +public record ClasspathGroup(List paths, List externalGroups) implements Serializable { + public static List fromModSettings(Set modSettings) { + return modSettings.stream().map(s -> new ClasspathGroup(getPaths(s), s.getExternalGroups().get())).toList(); + } + + // TODO remove this constructor when updating to Gradle 9.0, works around an issue where config cache cannot serialize immutable lists + public ClasspathGroup(List paths, List externalGroups) { + this.paths = new ArrayList<>(paths); + this.externalGroups = new ArrayList<>(externalGroups); + } + + private static List getPaths(ModSettings modSettings) { + return modSettings.getModFiles() + .getFiles() + .stream() + .map(File::getAbsolutePath) + .toList(); + } +} diff --git a/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroup.java b/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroup.java new file mode 100644 index 00000000..780da174 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroup.java @@ -0,0 +1,30 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.classpathgroups; + +import java.io.Serializable; + +public record ExternalClasspathGroup(String projectPath, String sourceSetName) implements Serializable { +} diff --git a/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroupDTO.java b/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroupDTO.java new file mode 100644 index 00000000..38e02448 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroupDTO.java @@ -0,0 +1,87 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.classpathgroups; + +import java.io.File; +import java.io.IOException; +import java.io.Serializable; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +import org.gradle.api.Project; +import org.gradle.api.tasks.SourceSet; +import org.gradle.api.tasks.SourceSetContainer; + +import net.fabricmc.loom.LoomGradlePlugin; +import net.fabricmc.loom.util.gradle.SourceSetHelper; +import net.fabricmc.loom.util.gradle.SourceSetReference; + +/** + * This object is exported by projects as a json file to be consumed by others to correctly populate the classpath groups. + */ +public record ExternalClasspathGroupDTO(String projectPath, Map> classpaths) implements Serializable { + public static ExternalClasspathGroupDTO createFromProject(Project project) { + SourceSetContainer sourceSets = SourceSetHelper.getSourceSets(project); + + Map> classpaths = new HashMap<>(); + + for (SourceSet sourceSet : sourceSets) { + SourceSetReference ref = new SourceSetReference(sourceSet, project); + List classpath = SourceSetHelper.getClasspath(ref); + classpaths.put(sourceSet.getName(), classpath.stream().map(File::getAbsolutePath).toList()); + } + + return new ExternalClasspathGroupDTO(project.getPath(), Collections.unmodifiableMap(classpaths)); + } + + public static Map resolveExternal(Set files) { + Map map = new HashMap<>(); + + for (File file : files) { + String json; + + try { + json = Files.readString(file.toPath()); + } catch (IOException e) { + throw new UncheckedIOException("Failed to read external classpath group file: " + file, e); + } + + ExternalClasspathGroupDTO dto = LoomGradlePlugin.GSON.fromJson(json, ExternalClasspathGroupDTO.class); + map.put(dto.projectPath(), dto); + } + + return Collections.unmodifiableMap(map); + } + + public List getForSourceSet(String sourceSetName) { + return Objects.requireNonNull(classpaths.get(sourceSetName), "No classpath found for source set: " + sourceSetName); + } +} diff --git a/src/main/java/net/fabricmc/loom/task/launch/ExportClasspathTask.java b/src/main/java/net/fabricmc/loom/task/launch/ExportClasspathTask.java new file mode 100644 index 00000000..72eafccd --- /dev/null +++ b/src/main/java/net/fabricmc/loom/task/launch/ExportClasspathTask.java @@ -0,0 +1,93 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.task.launch; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; + +import javax.inject.Inject; + +import org.gradle.api.UncheckedIOException; +import org.gradle.api.file.RegularFileProperty; +import org.gradle.api.provider.Property; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.OutputFile; +import org.gradle.api.tasks.TaskAction; +import org.gradle.workers.WorkAction; +import org.gradle.workers.WorkParameters; +import org.gradle.workers.WorkQueue; +import org.gradle.workers.WorkerExecutor; + +import net.fabricmc.loom.LoomGradlePlugin; +import net.fabricmc.loom.configuration.classpathgroups.ExternalClasspathGroupDTO; +import net.fabricmc.loom.task.AbstractLoomTask; + +public abstract class ExportClasspathTask extends AbstractLoomTask { + @Input + public abstract Property getClasspathDtoJson(); + + @OutputFile + public abstract RegularFileProperty getOutput(); + + @Inject + protected abstract WorkerExecutor getWorkerExecutor(); + + @Inject + public ExportClasspathTask() { + getClasspathDtoJson().set(getProject() + .provider(() -> ExternalClasspathGroupDTO.createFromProject(getProject())) + .map(LoomGradlePlugin.GSON::toJson)); + getOutput().set(getProject().getLayout().getBuildDirectory().file("export_classpath.json")); + } + + @TaskAction + public void run() { + final WorkQueue workQueue = getWorkerExecutor().noIsolation(); + workQueue.submit(ExportClassPathWorkAction.class, p -> { + p.getClasspathDtoJson().set(getClasspathDtoJson()); + p.getOutput().set(getOutput()); + }); + } + + protected interface ExportClassPathWorkParameters extends WorkParameters { + Property getClasspathDtoJson(); + RegularFileProperty getOutput(); + } + + public abstract static class ExportClassPathWorkAction implements WorkAction { + @Override + public void execute() { + File outputFile = getParameters().getOutput().getAsFile().get(); + String json = getParameters().getClasspathDtoJson().get(); + + try { + Files.writeString(outputFile.toPath(), json); + } catch (IOException e) { + throw new UncheckedIOException("Failed to write classpath groups", e); + } + } + } +} diff --git a/src/main/java/net/fabricmc/loom/task/launch/GenerateDLIConfigTask.java b/src/main/java/net/fabricmc/loom/task/launch/GenerateDLIConfigTask.java index 004209dc..a6ab0b26 100644 --- a/src/main/java/net/fabricmc/loom/task/launch/GenerateDLIConfigTask.java +++ b/src/main/java/net/fabricmc/loom/task/launch/GenerateDLIConfigTask.java @@ -42,6 +42,7 @@ import org.gradle.api.logging.configuration.ConsoleOutput; import org.gradle.api.provider.Property; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.InputFile; +import org.gradle.api.tasks.Nested; import org.gradle.api.tasks.Optional; import org.gradle.api.tasks.OutputFile; import org.gradle.api.tasks.TaskAction; @@ -51,7 +52,8 @@ import net.fabricmc.loom.LoomGradlePlugin; import net.fabricmc.loom.configuration.providers.minecraft.MinecraftVersionMeta; import net.fabricmc.loom.configuration.providers.minecraft.mapped.MappedMinecraftProvider; import net.fabricmc.loom.task.AbstractLoomTask; -import net.fabricmc.loom.util.gradle.SourceSetHelper; +import net.fabricmc.loom.task.service.ClasspathGroupService; +import net.fabricmc.loom.util.service.ScopedServiceFactory; public abstract class GenerateDLIConfigTask extends AbstractLoomTask { @Input @@ -69,10 +71,6 @@ public abstract class GenerateDLIConfigTask extends AbstractLoomTask { @Input protected abstract Property getANSISupportedIDE(); - @Input - @Optional - protected abstract Property getClassPathGroups(); - @Input protected abstract Property getLog4jConfigPaths(); @@ -96,16 +94,16 @@ public abstract class GenerateDLIConfigTask extends AbstractLoomTask { @OutputFile protected abstract RegularFileProperty getDevLauncherConfig(); + @Nested + protected abstract Property getClasspathGroupOptions(); + public GenerateDLIConfigTask() { getVersionInfoJson().set(LoomGradlePlugin.GSON.toJson(getExtension().getMinecraftProvider().getVersionInfo())); getMinecraftVersion().set(getExtension().getMinecraftProvider().minecraftVersion()); getSplitSourceSets().set(getExtension().areEnvironmentSourceSetsSplit()); getANSISupportedIDE().set(ansiSupportedIde(getProject())); getPlainConsole().set(getProject().getGradle().getStartParameter().getConsoleOutput() == ConsoleOutput.Plain); - - if (!getExtension().getMods().isEmpty()) { - getClassPathGroups().set(buildClassPathGroups(getProject())); - } + getClasspathGroupOptions().set(ClasspathGroupService.create(getProject())); getLog4jConfigPaths().set(getAllLog4JConfigFiles(getProject())); @@ -152,8 +150,12 @@ public abstract class GenerateDLIConfigTask extends AbstractLoomTask { launchConfig.property("fabric.gameJarPath", getCommonGameJarPath().get()); } - if (getClassPathGroups().isPresent()) { - launchConfig.property("fabric.classPathGroups", getClassPathGroups().get()); + try (ScopedServiceFactory serviceFactory = new ScopedServiceFactory()) { + ClasspathGroupService classpathGroupService = serviceFactory.get(getClasspathGroupOptions()); + + if (classpathGroupService.hasGroups()) { + launchConfig.property("fabric.classPathGroups", classpathGroupService.getClasspathGroupsPropertyValue()); + } } //Enable ansi by default for idea and vscode when gradle is not ran with plain console. @@ -180,19 +182,6 @@ public abstract class GenerateDLIConfigTask extends AbstractLoomTask { }; } - /** - * See: https://github.com/FabricMC/fabric-loader/pull/585. - */ - private static String buildClassPathGroups(Project project) { - return LoomGradleExtension.get(project).getMods().stream() - .map(modSettings -> - SourceSetHelper.getClasspath(modSettings, project).stream() - .map(File::getAbsolutePath) - .collect(Collectors.joining(File.pathSeparator)) - ) - .collect(Collectors.joining(File.pathSeparator+File.pathSeparator)); - } - private static boolean ansiSupportedIde(Project project) { File rootDir = project.getRootDir(); return new File(rootDir, ".vscode").exists() diff --git a/src/main/java/net/fabricmc/loom/task/service/ClasspathGroupService.java b/src/main/java/net/fabricmc/loom/task/service/ClasspathGroupService.java new file mode 100644 index 00000000..c3af919d --- /dev/null +++ b/src/main/java/net/fabricmc/loom/task/service/ClasspathGroupService.java @@ -0,0 +1,162 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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.task.service; + +import java.io.File; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +import org.gradle.api.NamedDomainObjectContainer; +import org.gradle.api.Project; +import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.Dependency; +import org.gradle.api.file.ConfigurableFileCollection; +import org.gradle.api.provider.ListProperty; +import org.gradle.api.provider.Provider; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.InputFiles; +import org.gradle.api.tasks.Optional; + +import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.api.ModSettings; +import net.fabricmc.loom.configuration.classpathgroups.ClasspathGroup; +import net.fabricmc.loom.configuration.classpathgroups.ExternalClasspathGroup; +import net.fabricmc.loom.configuration.classpathgroups.ExternalClasspathGroupDTO; +import net.fabricmc.loom.util.Constants; +import net.fabricmc.loom.util.Lazy; +import net.fabricmc.loom.util.service.Service; +import net.fabricmc.loom.util.service.ServiceFactory; +import net.fabricmc.loom.util.service.ServiceType; + +public class ClasspathGroupService extends Service { + public static ServiceType TYPE = new ServiceType(Options.class, ClasspathGroupService.class); + + public interface Options extends Service.Options { + @Input + @Optional + ListProperty getClasspathGroups(); + + @InputFiles + @Optional + ConfigurableFileCollection getExternalClasspathGroups(); + } + + public static Provider create(Project project) { + return TYPE.create(project, options -> { + LoomGradleExtension extension = LoomGradleExtension.get(project); + NamedDomainObjectContainer modSettings = extension.getMods(); + + if (modSettings.isEmpty()) { + return; + } + + options.getClasspathGroups().set(ClasspathGroup.fromModSettings(modSettings)); + + if (!hasExternalClasspathGroups(modSettings)) { + return; + } + + List externalDependencies = getExternalDependencies(project, modSettings); + Configuration externalClasspathGroups = project.getConfigurations().detachedConfiguration(externalDependencies.toArray(new Dependency[0])); + options.getExternalClasspathGroups().from(externalClasspathGroups); + }); + } + + private static boolean hasExternalClasspathGroups(Set modSettings) { + return modSettings.stream() + .anyMatch(s -> + s.getExternalGroups().isPresent() + && !s.getExternalGroups().get().isEmpty() + ); + } + + private static List getExternalDependencies(Project project, Set modSettings) { + List requiredProjects = modSettings.stream() + .flatMap(s -> s.getExternalGroups().get().stream()) + .map(ExternalClasspathGroup::projectPath) + .distinct() + .toList(); + + List dependencies = new ArrayList<>(); + + for (String projectPath : requiredProjects) { + Dependency externalDependency = project.getDependencies() + .project(Map.of( + "path", projectPath, + "configuration", Constants.Configurations.EXPORTED_CLASSPATH + )); + dependencies.add(externalDependency); + } + + return Collections.unmodifiableList(dependencies); + } + + private final Supplier> externalClasspathGroups = Lazy.of(() -> ExternalClasspathGroupDTO.resolveExternal(getOptions().getExternalClasspathGroups().getFiles())); + + public ClasspathGroupService(Options options, ServiceFactory serviceFactory) { + super(options, serviceFactory); + } + + public List getClasspath(ClasspathGroup classpathGroup) { + final List paths = new ArrayList<>(); + + for (ExternalClasspathGroup externalGroup : classpathGroup.externalGroups()) { + ExternalClasspathGroupDTO dto = externalClasspathGroups.get().get(externalGroup.projectPath()); + + if (dto == null) { + throw new IllegalStateException("Could not find resolved external classpath group for project: " + externalGroup.projectPath()); + } + + paths.addAll(dto.getForSourceSet(externalGroup.sourceSetName())); + } + + paths.addAll(classpathGroup.paths()); + + return paths.stream().map(File::new).toList(); + } + + /** + * See: https://github.com/FabricMC/fabric-loader/pull/585. + */ + public String getClasspathGroupsPropertyValue() { + return getOptions().getClasspathGroups().get() + .stream() + .map(group -> + getClasspath(group).stream() + .map(File::getAbsolutePath) + .collect(Collectors.joining(File.pathSeparator)) + ) + .collect(Collectors.joining(File.pathSeparator+File.pathSeparator)); + } + + public boolean hasGroups() { + return getOptions().getClasspathGroups().isPresent() && !getOptions().getClasspathGroups().get().isEmpty(); + } +} diff --git a/src/main/java/net/fabricmc/loom/util/Constants.java b/src/main/java/net/fabricmc/loom/util/Constants.java index 24294c0d..c051f3c1 100644 --- a/src/main/java/net/fabricmc/loom/util/Constants.java +++ b/src/main/java/net/fabricmc/loom/util/Constants.java @@ -90,6 +90,10 @@ public class Constants { * Mods to be used by {@link net.fabricmc.loom.task.prod.AbstractProductionRunTask} tasks by default. */ public static final String PRODUCTION_RUNTIME_MODS = "productionRuntimeMods"; + /** + * Used to query classpath data across project boundaries. + */ + public static final String EXPORTED_CLASSPATH = "loomExportedClasspath"; private Configurations() { } @@ -125,6 +129,7 @@ public class Constants { public static final class Task { public static final String PROCESS_INCLUDE_JARS = "processIncludeJars"; + public static final String EXPORT_CLASSPATH = "exportClasspath"; private Task() { } diff --git a/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java b/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java index 0303aa63..1cfcc2b7 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java @@ -33,6 +33,7 @@ import org.gradle.api.invocation.Gradle; import org.gradle.api.provider.Provider; import net.fabricmc.loom.LoomGradleExtension; +import net.fabricmc.loom.LoomGradlePlugin; public final class GradleUtils { private GradleUtils() { @@ -59,7 +60,7 @@ public final class GradleUtils { } public static boolean isLoomProject(Project project) { - return project.getPluginManager().hasPlugin("fabric-loom"); + return project.getPluginManager().hasPlugin(LoomGradlePlugin.NAME); } public static Provider getBooleanPropertyProvider(Project project, String key) { diff --git a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java index e605648c..4ab589e0 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java @@ -52,7 +52,6 @@ import org.jetbrains.annotations.VisibleForTesting; import org.xml.sax.InputSource; import net.fabricmc.loom.LoomGradleExtension; -import net.fabricmc.loom.api.ModSettings; import net.fabricmc.loom.configuration.ide.idea.IdeaUtils; import net.fabricmc.loom.util.Constants; @@ -113,18 +112,8 @@ public final class SourceSetHelper { return it.hasNext() ? it.next().getProject() : null; } - public static List getClasspath(ModSettings modSettings, Project project) { - final List files = new ArrayList<>(); - - files.addAll(modSettings.getModSourceSets().get().stream() - .flatMap(sourceSet -> getClasspath(sourceSet, project).stream()) - .toList()); - files.addAll(modSettings.getModFiles().getFiles()); - - return Collections.unmodifiableList(files); - } - - public static List getClasspath(SourceSetReference reference, Project project) { + public static List getClasspath(SourceSetReference reference) { + final Project project = reference.project(); final List classpath = getGradleClasspath(reference, project); classpath.addAll(getIdeaClasspath(reference, project)); diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/MultiProjectTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/MultiProjectTest.groovy index f163703d..78083ded 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/MultiProjectTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/MultiProjectTest.groovy @@ -57,4 +57,19 @@ class MultiProjectTest extends Specification implements GradleProjectTestTrait { where: version << STANDARD_TEST_VERSIONS } + + @Unroll + def "classpath groups (gradle #version)"() { + setup: + def gradle = gradleProject(project: "multiproject", version: version) + + when: + def result = gradle.run(tasks: [":generateDLIConfig",]) + + then: + result.task(":generateDLIConfig").outcome == SUCCESS + + where: + version << STANDARD_TEST_VERSIONS + } } diff --git a/src/test/resources/projects/multiproject/build.gradle b/src/test/resources/projects/multiproject/build.gradle index ae3e637a..1e414ec3 100644 --- a/src/test/resources/projects/multiproject/build.gradle +++ b/src/test/resources/projects/multiproject/build.gradle @@ -5,6 +5,8 @@ plugins { } allprojects { + if (it.path == ":javalib") return; + apply plugin: "fabric-loom" version = "1.0.0" @@ -42,13 +44,14 @@ allprojects { loom { mods { core { - sourceSet project(':core').sourceSets.main + sourceSet("main", ":core") } example { - sourceSet project(':example').sourceSets.main + sourceSet("main", ":example") } root { sourceSet sourceSets.main + sourceSet("main", ":javalib") } } } diff --git a/src/test/resources/projects/multiproject/javalib/build.gradle b/src/test/resources/projects/multiproject/javalib/build.gradle new file mode 100644 index 00000000..1fb413eb --- /dev/null +++ b/src/test/resources/projects/multiproject/javalib/build.gradle @@ -0,0 +1,4 @@ +plugins { + id 'java-library' + id 'net.fabricmc.fabric-loom-companion' +} \ No newline at end of file diff --git a/src/test/resources/projects/multiproject/settings.gradle b/src/test/resources/projects/multiproject/settings.gradle index df4ff932..e0e1c14b 100644 --- a/src/test/resources/projects/multiproject/settings.gradle +++ b/src/test/resources/projects/multiproject/settings.gradle @@ -1,4 +1,5 @@ rootProject.name = "fabric-example-mod" include 'core' -include 'example' \ No newline at end of file +include 'example' +include 'javalib' \ No newline at end of file From 2a47c835d155c40bf1da95ed5aa13fa1e5b746b6 Mon Sep 17 00:00:00 2001 From: modmuss Date: Wed, 22 Oct 2025 21:28:57 +0100 Subject: [PATCH 20/25] Fix exporting the dev jar path in a projects classpath. (#1397) --- src/main/java/net/fabricmc/loom/api/ModSettings.java | 2 +- .../classpathgroups/ExternalClasspathGroupDTO.java | 2 +- .../fabricmc/loom/util/gradle/SourceSetHelper.java | 12 +++++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/api/ModSettings.java b/src/main/java/net/fabricmc/loom/api/ModSettings.java index d40c0e46..2c9970fc 100644 --- a/src/main/java/net/fabricmc/loom/api/ModSettings.java +++ b/src/main/java/net/fabricmc/loom/api/ModSettings.java @@ -122,7 +122,7 @@ public abstract class ModSettings implements Named { if (projectPath.equals(getProject().getPath())) { // Shortcut for source sets in our own project. SourceSetReference ref = new SourceSetReference(SourceSetHelper.getSourceSetByName(sourceSetName, getProject()), getProject()); - List classpath = SourceSetHelper.getClasspath(ref); + List classpath = SourceSetHelper.getClasspath(ref, false); getModFiles().from(classpath); return; } diff --git a/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroupDTO.java b/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroupDTO.java index 38e02448..c1c88702 100644 --- a/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroupDTO.java +++ b/src/main/java/net/fabricmc/loom/configuration/classpathgroups/ExternalClasspathGroupDTO.java @@ -55,7 +55,7 @@ public record ExternalClasspathGroupDTO(String projectPath, Map classpath = SourceSetHelper.getClasspath(ref); + List classpath = SourceSetHelper.getClasspath(ref, true); classpaths.put(sourceSet.getName(), classpath.stream().map(File::getAbsolutePath).toList()); } diff --git a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java index 4ab589e0..a85d0610 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java @@ -112,9 +112,12 @@ public final class SourceSetHelper { return it.hasNext() ? it.next().getProject() : null; } - public static List getClasspath(SourceSetReference reference) { + /** + * @param forExport set to true when this classpath is going to be exported for another project to consume. + */ + public static List getClasspath(SourceSetReference reference, boolean forExport) { final Project project = reference.project(); - final List classpath = getGradleClasspath(reference, project); + final List classpath = getGradleClasspath(reference, forExport); classpath.addAll(getIdeaClasspath(reference, project)); classpath.addAll(getIdeaModuleCompileOutput(reference)); @@ -124,7 +127,7 @@ public final class SourceSetHelper { return classpath; } - private static List getGradleClasspath(SourceSetReference reference, Project project) { + private static List getGradleClasspath(SourceSetReference reference, boolean forExport) { final SourceSetOutput output = reference.sourceSet().getOutput(); final File resources = output.getResourcesDir(); @@ -137,8 +140,7 @@ public final class SourceSetHelper { } // Add dev jars from dependency projects if the source set is "main". - if (SourceSet.MAIN_SOURCE_SET_NAME.equals(reference.sourceSet().getName()) && !reference.project().getPath().equals(project.getPath()) - && GradleUtils.isLoomProject(reference.project())) { + if (forExport && SourceSet.MAIN_SOURCE_SET_NAME.equals(reference.sourceSet().getName()) && GradleUtils.isLoomProject(reference.project())) { final Configuration namedElements = reference.project().getConfigurations().getByName(Constants.Configurations.NAMED_ELEMENTS); // Note: We're not looking at the artifacts from configuration variants. It's probably not needed From e67de3f9af849dc29c018dc4c95aea5792325b9d Mon Sep 17 00:00:00 2001 From: modmuss50 Date: Wed, 22 Oct 2025 23:20:14 +0100 Subject: [PATCH 21/25] Export runtime jars from companion projects. --- .../java/net/fabricmc/loom/util/gradle/GradleUtils.java | 5 +++++ .../net/fabricmc/loom/util/gradle/SourceSetHelper.java | 7 +++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java b/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java index 1cfcc2b7..7f97bfc3 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/GradleUtils.java @@ -32,6 +32,7 @@ import org.gradle.api.file.RegularFileProperty; import org.gradle.api.invocation.Gradle; import org.gradle.api.provider.Provider; +import net.fabricmc.loom.LoomCompanionGradlePlugin; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.LoomGradlePlugin; @@ -63,6 +64,10 @@ public final class GradleUtils { return project.getPluginManager().hasPlugin(LoomGradlePlugin.NAME); } + public static boolean isLoomCompanionProject(Project project) { + return project.getPluginManager().hasPlugin(LoomCompanionGradlePlugin.NAME); + } + public static Provider getBooleanPropertyProvider(Project project, String key) { LoomGradleExtension extension = LoomGradleExtension.get(project); diff --git a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java index a85d0610..32399b25 100644 --- a/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java +++ b/src/main/java/net/fabricmc/loom/util/gradle/SourceSetHelper.java @@ -42,6 +42,7 @@ import javax.xml.xpath.XPathFactory; import org.gradle.api.Project; import org.gradle.api.Task; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.plugins.JavaPlugin; import org.gradle.api.plugins.JavaPluginExtension; import org.gradle.api.tasks.SourceSet; import org.gradle.api.tasks.SourceSetContainer; @@ -140,8 +141,10 @@ public final class SourceSetHelper { } // Add dev jars from dependency projects if the source set is "main". - if (forExport && SourceSet.MAIN_SOURCE_SET_NAME.equals(reference.sourceSet().getName()) && GradleUtils.isLoomProject(reference.project())) { - final Configuration namedElements = reference.project().getConfigurations().getByName(Constants.Configurations.NAMED_ELEMENTS); + if (forExport && SourceSet.MAIN_SOURCE_SET_NAME.equals(reference.sourceSet().getName()) && GradleUtils.isLoomCompanionProject(reference.project())) { + String configurationName = GradleUtils.isLoomProject(reference.project()) + ? Constants.Configurations.NAMED_ELEMENTS : JavaPlugin.RUNTIME_ELEMENTS_CONFIGURATION_NAME; + final Configuration namedElements = reference.project().getConfigurations().getByName(configurationName); // Note: We're not looking at the artifacts from configuration variants. It's probably not needed // (certainly not with Loom's setup), but technically someone could add child variants that add additional From 692ab936939a5e7c0efd1a441f15f69d49f56f24 Mon Sep 17 00:00:00 2001 From: modmuss Date: Thu, 23 Oct 2025 08:54:46 +0100 Subject: [PATCH 22/25] Support depending on type safe projects. (#1399) --- .../java/net/fabricmc/loom/api/ModSettings.java | 13 +++++++++++++ .../resources/projects/multiproject/build.gradle | 6 +++--- .../resources/projects/multiproject/settings.gradle | 2 ++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/main/java/net/fabricmc/loom/api/ModSettings.java b/src/main/java/net/fabricmc/loom/api/ModSettings.java index 2c9970fc..b2e0b300 100644 --- a/src/main/java/net/fabricmc/loom/api/ModSettings.java +++ b/src/main/java/net/fabricmc/loom/api/ModSettings.java @@ -34,6 +34,7 @@ import org.gradle.api.Named; import org.gradle.api.Project; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.Dependency; +import org.gradle.api.artifacts.ProjectDependency; import org.gradle.api.file.ConfigurableFileCollection; import org.gradle.api.provider.ListProperty; import org.gradle.api.tasks.SourceSet; @@ -130,6 +131,18 @@ public abstract class ModSettings implements Named { getExternalGroups().add(new ExternalClasspathGroup(projectPath, sourceSetName)); } + /** + * Add {@link SourceSet}'s output directories from the supplied project to be grouped with the named mod. + * + *

If the other project is not a Loom project you must apply the `net.fabricmc.fabric-loom-companion` plugin. + * + * @param sourceSetName the name of the source set + * @param projectDependency the {@link ProjectDependency} the source set belongs to + */ + public void sourceSet(String sourceSetName, ProjectDependency projectDependency) { + sourceSet(sourceSetName, projectDependency.getPath()); + } + /** * Add a number of {@link Dependency} to the mod's classpath group. Should be used to include all dependencies that are shaded into your mod. * diff --git a/src/test/resources/projects/multiproject/build.gradle b/src/test/resources/projects/multiproject/build.gradle index 1e414ec3..05237cc9 100644 --- a/src/test/resources/projects/multiproject/build.gradle +++ b/src/test/resources/projects/multiproject/build.gradle @@ -44,14 +44,14 @@ allprojects { loom { mods { core { - sourceSet("main", ":core") + sourceSet("main", projects.core) } example { - sourceSet("main", ":example") + sourceSet("main", projects.example) } root { sourceSet sourceSets.main - sourceSet("main", ":javalib") + sourceSet("main", projects.javalib) } } } diff --git a/src/test/resources/projects/multiproject/settings.gradle b/src/test/resources/projects/multiproject/settings.gradle index e0e1c14b..dd3f0f37 100644 --- a/src/test/resources/projects/multiproject/settings.gradle +++ b/src/test/resources/projects/multiproject/settings.gradle @@ -1,5 +1,7 @@ rootProject.name = "fabric-example-mod" +enableFeaturePreview 'TYPESAFE_PROJECT_ACCESSORS' + include 'core' include 'example' include 'javalib' \ No newline at end of file From 36cf73997f6a1362f97e3b0d12921bf58bf92e3d Mon Sep 17 00:00:00 2001 From: modmuss Date: Thu, 23 Oct 2025 19:25:17 +0100 Subject: [PATCH 23/25] Make the intergration tests easier to debug (#1401) * Make the intergration tests easier to debug * Spotless --- .../net/fabricmc/loom/test/LoomTestConstants.groovy | 5 +++++ .../loom/test/util/GradleProjectTestTrait.groovy | 12 +++--------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy index b7ec5d12..9759794c 100644 --- a/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/LoomTestConstants.groovy @@ -24,6 +24,8 @@ package net.fabricmc.loom.test +import java.lang.management.ManagementFactory + import org.gradle.util.GradleVersion class LoomTestConstants { @@ -46,6 +48,9 @@ class LoomTestConstants { public static final File TEST_DIR = new File("./.gradle/test-files") + // Try to detect if the debugging agent is enabled. + public static final boolean IS_DEBUGGING_ENABLED = ManagementFactory.runtimeMXBean.inputArguments.any { it.startsWith('-agentlib:jdwp') } + /** * Nightly gradle versions get removed after a certain amount of time, lets check to see if its still online before running the tests. */ diff --git a/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy b/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy index 86df3a47..06bf5ab6 100644 --- a/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/util/GradleProjectTestTrait.groovy @@ -28,7 +28,6 @@ import groovy.transform.Immutable import org.apache.commons.io.FileUtils import org.gradle.testkit.runner.BuildResult import org.gradle.testkit.runner.GradleRunner -import org.gradle.util.GradleVersion import spock.lang.Shared import net.fabricmc.loom.test.LoomTestConstants @@ -149,7 +148,6 @@ trait GradleProjectTestTrait { private String gradleHomeDir private String warningMode private boolean useBuildSrc - private boolean enableDebugging = true BuildResult run(Map options) { // Setup the system props to tell loom that its running in a test env @@ -165,7 +163,8 @@ trait GradleProjectTestTrait { args << options.task } - boolean configurationCache = true + // Configuration cache is not compatible with the debugging agent. + boolean configurationCache = !LoomTestConstants.IS_DEBUGGING_ENABLED if (options.containsKey("configurationCache")) { configurationCache = options.configurationCache @@ -197,10 +196,6 @@ trait GradleProjectTestTrait { writeBuildSrcDeps(runner) } - if (options.disableDebugging) { - enableDebugging = false - } - return options.expectFailure ? runner.buildAndFail() : runner.build() } @@ -210,8 +205,7 @@ trait GradleProjectTestTrait { .withPluginClasspath() .withGradleVersion(gradleVersion) .forwardOutput() - // Only enable debugging when the current gradle version matches the version we are testing - .withDebug(enableDebugging && gradleVersion == GradleVersion.current().getVersion()) + .withDebug(LoomTestConstants.IS_DEBUGGING_ENABLED) } File getProjectDir() { From c08bfbe5bee605f576ab09e04cf3c68f6b7ce36b Mon Sep 17 00:00:00 2001 From: modmuss Date: Thu, 23 Oct 2025 22:19:33 +0100 Subject: [PATCH 24/25] Warn when the project is stored in OneDrive (#1402) * Warn when the project is stored in OneDrive This is a large source of random errors we see in the support channels. * Dont event check if it exists --- .../net/fabricmc/loom/LoomGradlePlugin.java | 2 + .../java/net/fabricmc/loom/util/OneDrive.java | 91 +++++++++++++++++++ 2 files changed, 93 insertions(+) create mode 100644 src/main/java/net/fabricmc/loom/util/OneDrive.java diff --git a/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java b/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java index 05a2cdeb..1c416b30 100644 --- a/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java +++ b/src/main/java/net/fabricmc/loom/LoomGradlePlugin.java @@ -48,6 +48,7 @@ import net.fabricmc.loom.extension.LoomGradleExtensionImpl; import net.fabricmc.loom.task.LoomTasks; import net.fabricmc.loom.task.RemapTaskConfiguration; import net.fabricmc.loom.util.LibraryLocationLogger; +import net.fabricmc.loom.util.OneDrive; public class LoomGradlePlugin implements Plugin { public static final String NAME = "fabric-loom"; @@ -81,6 +82,7 @@ public class LoomGradlePlugin implements Plugin { project.getLogger().lifecycle("Fabric Loom: " + LOOM_VERSION); LibraryLocationLogger.logLibraryVersions(); + OneDrive.verify(project); // Apply default plugins project.apply(Map.of("plugin", "java-library")); diff --git a/src/main/java/net/fabricmc/loom/util/OneDrive.java b/src/main/java/net/fabricmc/loom/util/OneDrive.java new file mode 100644 index 00000000..72ee9ab7 --- /dev/null +++ b/src/main/java/net/fabricmc/loom/util/OneDrive.java @@ -0,0 +1,91 @@ +/* + * This file is part of fabric-loom, licensed under the MIT License (MIT). + * + * Copyright (c) 2025 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 java.nio.file.InvalidPathException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.function.Supplier; + +import org.gradle.api.Project; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +// OneDrive causes all kinds of issues, log a warning if the users project is stored there. +public final class OneDrive { + private static final List ENV_VARS = List.of("OneDrive", "OneDriveConsumer", "OneDriveCommercial"); + private static final Supplier> ONE_DRIVE_PATHS = Lazy.of(OneDrive::getOneDrivePaths); + private static final Logger LOGGER = LoggerFactory.getLogger(OneDrive.class); + + private OneDrive() { + } + + public static void verify(Project project) { + if (!Platform.CURRENT.getOperatingSystem().isWindows()) { + return; + } + + Path projectDir = project.getProjectDir().toPath(); + + if (isInOneDrive(projectDir)) { + LOGGER.warn("Project directory '{}' is located in OneDrive. This is known to cause issues and slower performance. Consider moving the project to a different location.", projectDir); + } + } + + private static boolean isInOneDrive(Path path) { + Path normalized = path.toAbsolutePath().normalize(); + + for (Path oneDrivePath : ONE_DRIVE_PATHS.get()) { + if (normalized.startsWith(oneDrivePath)) { + return true; + } + } + + return false; + } + + private static List getOneDrivePaths() { + var paths = new ArrayList(); + + for (String envVar : ENV_VARS) { + String value = System.getenv(envVar); + + if (value == null || value.isEmpty()) { + continue; + } + + try { + Path path = Path.of(value); + paths.add(path.toAbsolutePath().normalize()); + } catch (InvalidPathException ignored) { + // Don't care + } + } + + return Collections.unmodifiableList(paths); + } +} From e076ac7bcf4e3361cd850bd3514df6e1f240d62b Mon Sep 17 00:00:00 2001 From: modmuss Date: Fri, 24 Oct 2025 18:06:26 +0100 Subject: [PATCH 25/25] Direct port to class tweaker (#1398) * Direct port to class tweaker * Debugging help * Checkstyle * Update CT --- build.gradle | 2 +- gradle/libs.versions.toml | 4 +- .../accesswidener/AccessWidenerEntry.java | 4 +- .../AccessWidenerJarProcessor.java | 4 +- .../AccessWidenerTransformer.java | 29 ++-- .../LocalAccessWidenerEntry.java | 10 +- .../accesswidener/ModAccessWidenerEntry.java | 20 +-- ...nsitiveAccessWidenerMappingsProcessor.java | 129 ++++++++++-------- .../AccessWidenerAnalyzeVisitorProvider.java | 15 +- .../mods/AccessWidenerUtils.java | 22 +-- .../net/fabricmc/loom/task/RemapJarTask.java | 18 +-- .../loom/task/ValidateAccessWidenerTask.java | 47 ++----- .../test/integration/AccessWidenerTest.groovy | 11 +- 13 files changed, 159 insertions(+), 156 deletions(-) diff --git a/build.gradle b/build.gradle index 7e68d615..edeb45e2 100644 --- a/build.gradle +++ b/build.gradle @@ -103,7 +103,7 @@ dependencies { // tinyfile management implementation libs.fabric.tiny.remapper - implementation libs.fabric.access.widener + implementation libs.fabric.clazz.tweaker implementation libs.fabric.mapping.io implementation (libs.fabric.lorenz.tiny) { transitive = false diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c1093217..bc51d2a7 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -5,7 +5,7 @@ gson = "2.10.1" stitch = "0.6.2" tiny-remapper = "0.12.0" -access-widener = "2.1.0" +clazz-tweaker = "0.1.1" mapping-io = "0.7.1" lorenz-tiny = "4.0.2" mercury = "0.4.2" @@ -30,7 +30,7 @@ gson = { module = "com.google.code.gson:gson", version.ref = "gson" } fabric-stitch = { module = "net.fabricmc:stitch", version.ref = "stitch" } fabric-tiny-remapper = { module = "net.fabricmc:tiny-remapper", version.ref = "tiny-remapper" } -fabric-access-widener = { module = "net.fabricmc:access-widener", version.ref = "access-widener" } +fabric-clazz-tweaker = { module = "net.fabricmc:class-tweaker", version.ref = "clazz-tweaker" } fabric-mapping-io = { module = "net.fabricmc:mapping-io", version.ref = "mapping-io" } fabric-lorenz-tiny = { module = "net.fabricmc:lorenz-tiny", version.ref = "lorenz-tiny" } fabric-mercury = { module = "net.fabricmc:mercury", version.ref = "mercury" } diff --git a/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerEntry.java b/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerEntry.java index 802b7c85..a2bf7d94 100644 --- a/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerEntry.java +++ b/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerEntry.java @@ -28,7 +28,7 @@ import java.io.IOException; import org.jetbrains.annotations.Nullable; -import net.fabricmc.accesswidener.AccessWidenerVisitor; +import net.fabricmc.classtweaker.api.visitor.ClassTweakerVisitor; import net.fabricmc.loom.util.LazyCloseable; import net.fabricmc.loom.util.fmj.ModEnvironment; import net.fabricmc.tinyremapper.TinyRemapper; @@ -44,5 +44,5 @@ public interface AccessWidenerEntry { String getSortKey(); - void read(AccessWidenerVisitor visitor, LazyCloseable remapper) throws IOException; + void read(ClassTweakerVisitor visitor, LazyCloseable remapper) throws IOException; } diff --git a/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerJarProcessor.java b/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerJarProcessor.java index 212ef932..ef0f6ce6 100644 --- a/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerJarProcessor.java +++ b/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerJarProcessor.java @@ -38,7 +38,7 @@ import javax.inject.Inject; import org.gradle.api.file.RegularFileProperty; import org.jetbrains.annotations.Nullable; -import net.fabricmc.accesswidener.AccessWidener; +import net.fabricmc.classtweaker.api.ClassTweaker; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.api.processor.MinecraftJarProcessor; import net.fabricmc.loom.api.processor.ProcessorContext; @@ -131,7 +131,7 @@ public class AccessWidenerJarProcessor implements MinecraftJarProcessor accessWideners = spec.accessWidenersForContext(context); - final var accessWidener = new AccessWidener(); + final var accessWidener = ClassTweaker.newInstance(); try (LazyCloseable remapper = context.createRemapper(MappingsNamespace.INTERMEDIARY, MappingsNamespace.NAMED)) { for (AccessWidenerEntry widener : accessWideners) { diff --git a/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerTransformer.java b/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerTransformer.java index 0eb195af..7cd33b5e 100644 --- a/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerTransformer.java +++ b/src/main/java/net/fabricmc/loom/configuration/accesswidener/AccessWidenerTransformer.java @@ -37,8 +37,7 @@ import org.objectweb.asm.ClassWriter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import net.fabricmc.accesswidener.AccessWidener; -import net.fabricmc.accesswidener.AccessWidenerClassVisitor; +import net.fabricmc.classtweaker.api.ClassTweaker; import net.fabricmc.loom.util.Constants; import net.fabricmc.loom.util.Pair; import net.fabricmc.loom.util.ZipUtils; @@ -46,9 +45,9 @@ import net.fabricmc.loom.util.ZipUtils; final class AccessWidenerTransformer { private static final Logger LOGGER = LoggerFactory.getLogger(AccessWidenerTransformer.class); - private final AccessWidener accessWidener; + private final ClassTweaker accessWidener; - AccessWidenerTransformer(AccessWidener accessWidener) { + AccessWidenerTransformer(ClassTweaker accessWidener) { this.accessWidener = accessWidener; } @@ -57,7 +56,14 @@ final class AccessWidenerTransformer { */ void apply(Path jarFile) { try { - ZipUtils.transform(jarFile, getTransformers(accessWidener.getTargets())); + Set targets = accessWidener.getTargets(); + int transformed = ZipUtils.transform(jarFile, getTransformers(targets)); + + LOGGER.debug("Applied access wideners to {} classes in {}", transformed, jarFile); + + if (targets.size() != transformed) { + LOGGER.debug("Access widener target count ({}) does not match transformed class count ({}).", targets.size(), transformed); + } } catch (IOException e) { throw new UncheckedIOException("Failed to apply access wideners to %s".formatted(jarFile), e); } @@ -65,17 +71,22 @@ final class AccessWidenerTransformer { private List>> getTransformers(Set classes) { return classes.stream() - .map(string -> new Pair<>(string.replaceAll("\\.", "/") + ".class", getTransformer(string))) + .map(string -> new Pair<>(string + ".class", getTransformer(string))) .collect(Collectors.toList()); } private ZipUtils.UnsafeUnaryOperator getTransformer(String className) { return input -> { ClassReader reader = new ClassReader(input); - ClassWriter writer = new ClassWriter(0); - ClassVisitor classVisitor = AccessWidenerClassVisitor.createClassVisitor(Constants.ASM_VERSION, writer, accessWidener); - LOGGER.debug("Applying access widener to " + className); + if (!reader.getClassName().equals(className)) { + throw new IllegalStateException("Class name mismatch: expected %s but transforming %s".formatted(className, reader.getClassName())); + } + + ClassWriter writer = new ClassWriter(0); + ClassVisitor classVisitor = accessWidener.createClassVisitor(Constants.ASM_VERSION, writer, null); + + LOGGER.debug("Applying access widener to {}", className); reader.accept(classVisitor, 0); return writer.toByteArray(); diff --git a/src/main/java/net/fabricmc/loom/configuration/accesswidener/LocalAccessWidenerEntry.java b/src/main/java/net/fabricmc/loom/configuration/accesswidener/LocalAccessWidenerEntry.java index c2aed2d6..24db25ea 100644 --- a/src/main/java/net/fabricmc/loom/configuration/accesswidener/LocalAccessWidenerEntry.java +++ b/src/main/java/net/fabricmc/loom/configuration/accesswidener/LocalAccessWidenerEntry.java @@ -30,8 +30,8 @@ import java.nio.file.Path; import org.jetbrains.annotations.Nullable; -import net.fabricmc.accesswidener.AccessWidenerReader; -import net.fabricmc.accesswidener.AccessWidenerVisitor; +import net.fabricmc.classtweaker.api.ClassTweakerReader; +import net.fabricmc.classtweaker.api.visitor.ClassTweakerVisitor; import net.fabricmc.loom.util.Checksum; import net.fabricmc.loom.util.LazyCloseable; import net.fabricmc.loom.util.fmj.ModEnvironment; @@ -43,9 +43,9 @@ public record LocalAccessWidenerEntry(Path path, String hash) implements AccessW } @Override - public void read(AccessWidenerVisitor visitor, LazyCloseable remapper) throws IOException { - var reader = new AccessWidenerReader(visitor); - reader.read(Files.readAllBytes(path)); + public void read(ClassTweakerVisitor visitor, LazyCloseable remapper) throws IOException { + var reader = ClassTweakerReader.create(visitor); + reader.read(Files.readAllBytes(path), null); } @Override diff --git a/src/main/java/net/fabricmc/loom/configuration/accesswidener/ModAccessWidenerEntry.java b/src/main/java/net/fabricmc/loom/configuration/accesswidener/ModAccessWidenerEntry.java index 4731422c..32904c78 100644 --- a/src/main/java/net/fabricmc/loom/configuration/accesswidener/ModAccessWidenerEntry.java +++ b/src/main/java/net/fabricmc/loom/configuration/accesswidener/ModAccessWidenerEntry.java @@ -32,10 +32,10 @@ import java.util.Map; import org.jetbrains.annotations.Nullable; -import net.fabricmc.accesswidener.AccessWidenerReader; -import net.fabricmc.accesswidener.AccessWidenerRemapper; -import net.fabricmc.accesswidener.AccessWidenerVisitor; -import net.fabricmc.accesswidener.TransitiveOnlyFilter; +import net.fabricmc.classtweaker.api.ClassTweakerReader; +import net.fabricmc.classtweaker.api.visitor.ClassTweakerVisitor; +import net.fabricmc.classtweaker.visitors.ClassTweakerRemapperVisitor; +import net.fabricmc.classtweaker.visitors.TransitiveOnlyFilter; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.util.LazyCloseable; import net.fabricmc.loom.util.fmj.FabricModJson; @@ -67,26 +67,26 @@ public record ModAccessWidenerEntry(FabricModJson mod, String path, ModEnvironme } @Override - public void read(AccessWidenerVisitor visitor, LazyCloseable remapper) throws IOException { + public void read(ClassTweakerVisitor visitor, LazyCloseable remapper) throws IOException { if (transitiveOnly) { // Filter for only transitive rules visitor = new TransitiveOnlyFilter(visitor); } final byte[] data = readRaw(); - final AccessWidenerReader.Header header = AccessWidenerReader.readHeader(data); + final ClassTweakerReader.Header header = ClassTweakerReader.readHeader(data); if (!header.getNamespace().equals(MappingsNamespace.NAMED.toString())) { // Remap the AW if needed visitor = getRemapper(visitor, remapper.get()); } - var reader = new AccessWidenerReader(visitor); - reader.read(data); + var reader = ClassTweakerReader.create(visitor); + reader.read(data, mod.getId()); } - private static AccessWidenerRemapper getRemapper(AccessWidenerVisitor visitor, TinyRemapper tinyRemapper) { - return new AccessWidenerRemapper( + private static ClassTweakerRemapperVisitor getRemapper(ClassTweakerVisitor visitor, TinyRemapper tinyRemapper) { + return new ClassTweakerRemapperVisitor( visitor, tinyRemapper.getEnvironment().getRemapper(), MappingsNamespace.INTERMEDIARY.toString(), diff --git a/src/main/java/net/fabricmc/loom/configuration/accesswidener/TransitiveAccessWidenerMappingsProcessor.java b/src/main/java/net/fabricmc/loom/configuration/accesswidener/TransitiveAccessWidenerMappingsProcessor.java index e7197b7b..1df5cbc4 100644 --- a/src/main/java/net/fabricmc/loom/configuration/accesswidener/TransitiveAccessWidenerMappingsProcessor.java +++ b/src/main/java/net/fabricmc/loom/configuration/accesswidener/TransitiveAccessWidenerMappingsProcessor.java @@ -31,8 +31,8 @@ import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import net.fabricmc.accesswidener.AccessWidenerReader; -import net.fabricmc.accesswidener.AccessWidenerVisitor; +import net.fabricmc.classtweaker.api.visitor.AccessWidenerVisitor; +import net.fabricmc.classtweaker.api.visitor.ClassTweakerVisitor; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.api.processor.MappingProcessorContext; import net.fabricmc.loom.api.processor.MinecraftJarProcessor; @@ -44,6 +44,8 @@ import net.fabricmc.tinyremapper.TinyRemapper; public final class TransitiveAccessWidenerMappingsProcessor implements MinecraftJarProcessor.MappingsProcessor { public static final TransitiveAccessWidenerMappingsProcessor INSTANCE = new TransitiveAccessWidenerMappingsProcessor(); + private static final Logger LOGGER = LoggerFactory.getLogger(TransitiveAccessWidenerMappingsProcessor.class); + private TransitiveAccessWidenerMappingsProcessor() { } @@ -63,7 +65,7 @@ public final class TransitiveAccessWidenerMappingsProcessor implements Minecraft try (LazyCloseable remapper = context.createRemapper(MappingsNamespace.INTERMEDIARY, MappingsNamespace.NAMED)) { for (AccessWidenerEntry accessWidener : accessWideners) { - var visitor = new MappingCommentVisitor(accessWidener.mappingId(), mappings); + var visitor = new MappingCommentClassTweakerVisitor(accessWidener.mappingId(), mappings); accessWidener.read(visitor, remapper); } } catch (IOException e) { @@ -73,80 +75,91 @@ public final class TransitiveAccessWidenerMappingsProcessor implements Minecraft return true; } - private record MappingCommentVisitor(String modId, MemoryMappingTree mappingTree) implements AccessWidenerVisitor { - private static final Logger LOGGER = LoggerFactory.getLogger(MappingCommentVisitor.class); - + private record MappingCommentClassTweakerVisitor(String modId, MemoryMappingTree mappingTree) implements ClassTweakerVisitor { @Override - public void visitClass(String name, AccessWidenerReader.AccessType access, boolean transitive) { - MappingTree.ClassMapping classMapping = mappingTree.getClass(name); - - if (classMapping == null) { - LOGGER.info("Failed to find class ({}) to mark access widened by mod ({})", name, modId()); - return; - } - - classMapping.setComment(appendComment(classMapping.getComment(), access)); + public AccessWidenerVisitor visitAccessWidener(String owner) { + return new MappingCommentAccessWidenerVisitor(owner); } - @Override - public void visitMethod(String owner, String name, String descriptor, AccessWidenerReader.AccessType access, boolean transitive) { - // Access is also applied to the class, so also add the comment to the class - visitClass(owner, access, transitive); + private class MappingCommentAccessWidenerVisitor implements AccessWidenerVisitor { + private final String className; - MappingTree.ClassMapping classMapping = mappingTree.getClass(owner); - - if (classMapping == null) { - LOGGER.info("Failed to find class ({}) to mark access widened by mod ({})", owner, modId()); - return; + private MappingCommentAccessWidenerVisitor(String className) { + this.className = className; } - MappingTree.MethodMapping methodMapping = classMapping.getMethod(name, descriptor); + @Override + public void visitClass(AccessType access, boolean transitive) { + MappingTree.ClassMapping classMapping = mappingTree.getClass(className); - if (methodMapping == null) { - LOGGER.info("Failed to find method ({}) in ({}) to mark access widened by mod ({})", name, owner, modId()); - return; + if (classMapping == null) { + LOGGER.info("Failed to find class ({}) to mark access widened by mod ({})", className, modId()); + return; + } + + classMapping.setComment(appendComment(classMapping.getComment(), access)); } - methodMapping.setComment(appendComment(methodMapping.getComment(), access)); - } + @Override + public void visitMethod(String name, String descriptor, AccessType access, boolean transitive) { + // Access is also applied to the class, so also add the comment to the class + visitClass(access, transitive); - @Override - public void visitField(String owner, String name, String descriptor, AccessWidenerReader.AccessType access, boolean transitive) { - // Access is also applied to the class, so also add the comment to the class - visitClass(owner, access, transitive); + MappingTree.ClassMapping classMapping = mappingTree.getClass(className); - MappingTree.ClassMapping classMapping = mappingTree.getClass(owner); + if (classMapping == null) { + LOGGER.info("Failed to find class ({}) to mark access widened by mod ({})", className, modId()); + return; + } - if (classMapping == null) { - LOGGER.info("Failed to find class ({}) to mark access widened by mod ({})", name, modId()); - return; + MappingTree.MethodMapping methodMapping = classMapping.getMethod(name, descriptor); + + if (methodMapping == null) { + LOGGER.info("Failed to find method ({}) in ({}) to mark access widened by mod ({})", name, className, modId()); + return; + } + + methodMapping.setComment(appendComment(methodMapping.getComment(), access)); } - MappingTree.FieldMapping fieldMapping = classMapping.getField(name, descriptor); + @Override + public void visitField(String name, String descriptor, AccessType access, boolean transitive) { + // Access is also applied to the class, so also add the comment to the class + visitClass(access, transitive); - if (fieldMapping == null) { - LOGGER.info("Failed to find field ({}) in ({}) to mark access widened by mod ({})", name, owner, modId()); - return; + MappingTree.ClassMapping classMapping = mappingTree.getClass(className); + + if (classMapping == null) { + LOGGER.info("Failed to find class ({}) to mark access widened by mod ({})", name, modId()); + return; + } + + MappingTree.FieldMapping fieldMapping = classMapping.getField(name, descriptor); + + if (fieldMapping == null) { + LOGGER.info("Failed to find field ({}) in ({}) to mark access widened by mod ({})", name, className, modId()); + return; + } + + fieldMapping.setComment(appendComment(fieldMapping.getComment(), access)); } - fieldMapping.setComment(appendComment(fieldMapping.getComment(), access)); - } + private String appendComment(String comment, AccessType access) { + if (comment == null) { + comment = ""; + } else { + comment += "\n"; + } - private String appendComment(String comment, AccessWidenerReader.AccessType access) { - if (comment == null) { - comment = ""; - } else { - comment += "\n"; + String awComment = "Access widened by %s to %s".formatted(modId(), access); + + if (!comment.contains(awComment)) { + // Ensure we don't comment the same thing twice. A bit of a cheap way to do this, but should work ok. + comment += awComment; + } + + return comment; } - - String awComment = "Access widened by %s to %s".formatted(modId(), access); - - if (!comment.contains(awComment)) { - // Ensure we don't comment the same thing twice. A bit of a cheap way to do this, but should work ok. - comment += awComment; - } - - return comment; } } } diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/AccessWidenerAnalyzeVisitorProvider.java b/src/main/java/net/fabricmc/loom/configuration/mods/AccessWidenerAnalyzeVisitorProvider.java index b59373d6..9b218417 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/AccessWidenerAnalyzeVisitorProvider.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/AccessWidenerAnalyzeVisitorProvider.java @@ -29,16 +29,15 @@ import java.util.List; import org.objectweb.asm.ClassVisitor; -import net.fabricmc.accesswidener.AccessWidener; -import net.fabricmc.accesswidener.AccessWidenerClassVisitor; -import net.fabricmc.accesswidener.AccessWidenerReader; +import net.fabricmc.classtweaker.api.ClassTweaker; +import net.fabricmc.classtweaker.api.ClassTweakerReader; import net.fabricmc.loom.configuration.mods.dependency.ModDependency; import net.fabricmc.loom.util.Constants; import net.fabricmc.tinyremapper.TinyRemapper; -public record AccessWidenerAnalyzeVisitorProvider(AccessWidener accessWidener) implements TinyRemapper.AnalyzeVisitorProvider { +public record AccessWidenerAnalyzeVisitorProvider(ClassTweaker accessWidener) implements TinyRemapper.AnalyzeVisitorProvider { static AccessWidenerAnalyzeVisitorProvider createFromMods(String namespace, List mods) throws IOException { - AccessWidener accessWidener = new AccessWidener(); + ClassTweaker accessWidener = ClassTweaker.newInstance(); accessWidener.visitHeader(namespace); for (ModDependency mod : mods) { @@ -48,8 +47,8 @@ public record AccessWidenerAnalyzeVisitorProvider(AccessWidener accessWidener) i continue; } - final var reader = new AccessWidenerReader(accessWidener); - reader.read(accessWidenerData.content()); + final var reader = ClassTweakerReader.create(accessWidener); + reader.read(accessWidenerData.content(), null); // TODO pass mod id } return new AccessWidenerAnalyzeVisitorProvider(accessWidener); @@ -57,6 +56,6 @@ public record AccessWidenerAnalyzeVisitorProvider(AccessWidener accessWidener) i @Override public ClassVisitor insertAnalyzeVisitor(int mrjVersion, String className, ClassVisitor next) { - return AccessWidenerClassVisitor.createClassVisitor(Constants.ASM_VERSION, next, accessWidener); + return accessWidener.createClassVisitor(Constants.ASM_VERSION, next, null); } } diff --git a/src/main/java/net/fabricmc/loom/configuration/mods/AccessWidenerUtils.java b/src/main/java/net/fabricmc/loom/configuration/mods/AccessWidenerUtils.java index 725e4fb1..8b2cb86d 100644 --- a/src/main/java/net/fabricmc/loom/configuration/mods/AccessWidenerUtils.java +++ b/src/main/java/net/fabricmc/loom/configuration/mods/AccessWidenerUtils.java @@ -30,9 +30,9 @@ import java.util.List; import org.objectweb.asm.commons.Remapper; -import net.fabricmc.accesswidener.AccessWidenerReader; -import net.fabricmc.accesswidener.AccessWidenerRemapper; -import net.fabricmc.accesswidener.AccessWidenerWriter; +import net.fabricmc.classtweaker.api.ClassTweakerReader; +import net.fabricmc.classtweaker.api.ClassTweakerWriter; +import net.fabricmc.classtweaker.visitors.ClassTweakerRemapperVisitor; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.util.fmj.FabricModJson; import net.fabricmc.loom.util.fmj.FabricModJsonFactory; @@ -42,18 +42,18 @@ public class AccessWidenerUtils { * Remap a mods access widener from intermediary to named, so that loader can apply it in our dev-env. */ public static byte[] remapAccessWidener(byte[] input, Remapper remapper) { - int version = AccessWidenerReader.readVersion(input); + int version = ClassTweakerReader.readVersion(input); - AccessWidenerWriter writer = new AccessWidenerWriter(version); - AccessWidenerRemapper awRemapper = new AccessWidenerRemapper( + ClassTweakerWriter writer = ClassTweakerWriter.create(version); + ClassTweakerRemapperVisitor awRemapper = new ClassTweakerRemapperVisitor( writer, remapper, MappingsNamespace.INTERMEDIARY.toString(), MappingsNamespace.NAMED.toString() ); - AccessWidenerReader reader = new AccessWidenerReader(awRemapper); - reader.read(input); - return writer.write(); + ClassTweakerReader reader = ClassTweakerReader.create(awRemapper); + reader.read(input, null); // TODO pass modid + return writer.getOutput(); } public static AccessWidenerData readAccessWidenerData(Path inputJar) throws IOException { @@ -74,11 +74,11 @@ public class AccessWidenerUtils { final String accessWidenerPath = classTweakers.get(0); final byte[] accessWidener = fabricModJson.getSource().read(accessWidenerPath); - final AccessWidenerReader.Header header = AccessWidenerReader.readHeader(accessWidener); + final ClassTweakerReader.Header header = ClassTweakerReader.readHeader(accessWidener); return new AccessWidenerData(accessWidenerPath, header, accessWidener); } - public record AccessWidenerData(String path, AccessWidenerReader.Header header, byte[] content) { + public record AccessWidenerData(String path, ClassTweakerReader.Header header, byte[] content) { } } diff --git a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java index 3a51a33e..745da380 100644 --- a/src/main/java/net/fabricmc/loom/task/RemapJarTask.java +++ b/src/main/java/net/fabricmc/loom/task/RemapJarTask.java @@ -52,9 +52,9 @@ import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import net.fabricmc.accesswidener.AccessWidenerReader; -import net.fabricmc.accesswidener.AccessWidenerRemapper; -import net.fabricmc.accesswidener.AccessWidenerWriter; +import net.fabricmc.classtweaker.api.ClassTweakerReader; +import net.fabricmc.classtweaker.api.ClassTweakerWriter; +import net.fabricmc.classtweaker.visitors.ClassTweakerRemapperVisitor; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.build.nesting.JarNester; import net.fabricmc.loom.build.nesting.NestableJarGenerationTask; @@ -268,19 +268,19 @@ public abstract class RemapJarTask extends AbstractRemapJarTask { private byte[] remapAccessWidener(byte[] input) { Objects.requireNonNull(tinyRemapper, "tinyRemapper"); - int version = AccessWidenerReader.readVersion(input); + int version = ClassTweakerReader.readVersion(input); - AccessWidenerWriter writer = new AccessWidenerWriter(version); - AccessWidenerRemapper remapper = new AccessWidenerRemapper( + ClassTweakerWriter writer = ClassTweakerWriter.create(version); + ClassTweakerRemapperVisitor remapper = new ClassTweakerRemapperVisitor( writer, tinyRemapper.getEnvironment().getRemapper(), getParameters().getSourceNamespace().get(), getParameters().getTargetNamespace().get() ); - AccessWidenerReader reader = new AccessWidenerReader(remapper); - reader.read(input); + ClassTweakerReader reader = ClassTweakerReader.create(remapper); + reader.read(input, null); // TODO pass mod id - return writer.write(); + return writer.getOutput(); } private void addNestedJars() { diff --git a/src/main/java/net/fabricmc/loom/task/ValidateAccessWidenerTask.java b/src/main/java/net/fabricmc/loom/task/ValidateAccessWidenerTask.java index c47f8b0b..33383175 100644 --- a/src/main/java/net/fabricmc/loom/task/ValidateAccessWidenerTask.java +++ b/src/main/java/net/fabricmc/loom/task/ValidateAccessWidenerTask.java @@ -30,6 +30,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.util.concurrent.atomic.AtomicBoolean; import javax.inject.Inject; @@ -41,14 +42,12 @@ import org.gradle.api.tasks.InputFiles; import org.gradle.api.tasks.SkipWhenEmpty; import org.gradle.api.tasks.TaskAction; -import net.fabricmc.accesswidener.AccessWidenerFormatException; -import net.fabricmc.accesswidener.AccessWidenerReader; -import net.fabricmc.accesswidener.AccessWidenerVisitor; +import net.fabricmc.classtweaker.api.ClassTweakerReader; +import net.fabricmc.classtweaker.validator.ClassTweakerValidatingVisitor; import net.fabricmc.loom.LoomGradleExtension; import net.fabricmc.loom.api.mappings.layered.MappingsNamespace; import net.fabricmc.loom.util.TinyRemapperLoggerAdapter; import net.fabricmc.tinyremapper.TinyRemapper; -import net.fabricmc.tinyremapper.api.TrEnvironment; public abstract class ValidateAccessWidenerTask extends DefaultTask { @SkipWhenEmpty @@ -77,44 +76,26 @@ public abstract class ValidateAccessWidenerTask extends DefaultTask { tinyRemapper.readClassPath(file.toPath()); } - final AccessWidenerValidator validator = new AccessWidenerValidator(tinyRemapper.getEnvironment()); - final AccessWidenerReader accessWidenerReader = new AccessWidenerReader(validator); + AtomicBoolean hasProblem = new AtomicBoolean(false); + + final ClassTweakerValidatingVisitor validator = new ClassTweakerValidatingVisitor(tinyRemapper.getEnvironment(), (lineNumber, message) -> { + hasProblem.set(true); + getLogger().error("{} on line {}", message, lineNumber); + }); + + final ClassTweakerReader accessWidenerReader = ClassTweakerReader.create(validator); try (BufferedReader reader = Files.newBufferedReader(getAccessWidener().get().getAsFile().toPath(), StandardCharsets.UTF_8)) { accessWidenerReader.read(reader, "named"); - } catch (AccessWidenerFormatException e) { - getLogger().error("Failed to validate access-widener file {} on line {}: {}", getAccessWidener().get().getAsFile().getName(), e.getLineNumber(), e.getMessage()); - throw e; } catch (IOException e) { throw new UncheckedIOException("Failed to read access widener", e); } finally { tinyRemapper.finish(); } - } - /** - * Validates that all entries in an access-widner file relate to a class/method/field in the mc jar. - */ - private record AccessWidenerValidator(TrEnvironment environment) implements AccessWidenerVisitor { - @Override - public void visitClass(String name, AccessWidenerReader.AccessType access, boolean transitive) { - if (environment().getClass(name) == null) { - throw new RuntimeException("Could not find class (%s)".formatted(name)); - } - } - - @Override - public void visitMethod(String owner, String name, String descriptor, AccessWidenerReader.AccessType access, boolean transitive) { - if (environment().getMethod(owner, name, descriptor) == null) { - throw new RuntimeException("Could not find method (%s%s) in class (%s)".formatted(name, descriptor, owner)); - } - } - - @Override - public void visitField(String owner, String name, String descriptor, AccessWidenerReader.AccessType access, boolean transitive) { - if (environment().getField(owner, name, descriptor) == null) { - throw new RuntimeException("Could not find field (%s%s) in class (%s)".formatted(name, descriptor, owner)); - } + if (hasProblem.get()) { + getLogger().error("access-widener validation failed for {}", getAccessWidener().get().getAsFile().getName()); + throw new RuntimeException("access-widener validation failed for %s".formatted(getAccessWidener().get().getAsFile().getName())); } } } diff --git a/src/test/groovy/net/fabricmc/loom/test/integration/AccessWidenerTest.groovy b/src/test/groovy/net/fabricmc/loom/test/integration/AccessWidenerTest.groovy index a405f602..a0754717 100644 --- a/src/test/groovy/net/fabricmc/loom/test/integration/AccessWidenerTest.groovy +++ b/src/test/groovy/net/fabricmc/loom/test/integration/AccessWidenerTest.groovy @@ -72,18 +72,17 @@ class AccessWidenerTest extends Specification implements GradleProjectTestTrait setup: def gradle = gradleProject(project: "accesswidener", version: version) new File(gradle.projectDir, "src/main/resources/modid.accesswidener").append(awLine) - def errorPrefix = "Failed to validate access-widener file modid.accesswidener on line 10: java.lang.RuntimeException: " when: def result = gradle.run(task: "check", expectFailure: true) then: - result.output.contains(errorPrefix + error) + result.output.contains(error) where: - awLine | error | version - 'accessible\tclass\tnet/minecraft/DoesntExists' | "Could not find class (net/minecraft/DoesntExists)" | DEFAULT_GRADLE - 'accessible\tfield\tnet/minecraft/screen/slot/Slot\tabc\tI' | "Could not find field (abcI) in class (net/minecraft/screen/slot/Slot)" | DEFAULT_GRADLE - 'accessible\tmethod\tnet/minecraft/client/main/Main\tmain\t([Ljava/lang/NotAString;)V' | "Could not find method (main([Ljava/lang/NotAString;)V) in class (net/minecraft/client/main/Main)" | DEFAULT_GRADLE + awLine | error | version + 'accessible\tclass\tnet/minecraft/DoesntExists' | "Could not find class (net/minecraft/DoesntExists) on line 10" | DEFAULT_GRADLE + 'accessible\tfield\tnet/minecraft/screen/slot/Slot\tabc\tI' | "Could not find field (abc:I) in class (net/minecraft/screen/slot/Slot) on line 10" | DEFAULT_GRADLE + 'accessible\tmethod\tnet/minecraft/client/main/Main\tmain\t([Ljava/lang/NotAString;)V' | "Could not find method (main([Ljava/lang/NotAString;)V) in class (net/minecraft/client/main/Main) on line 10" | DEFAULT_GRADLE } }