Skip to content

Commit d84205d

Browse files
shartteTechnici4n
shartte
and
Technici4n
authored
Support reporting of problems from NFRT via the Gradle Problems API if supported (8.13+) (#248)
Co-authored-by: Technici4n <[email protected]>
1 parent 13e9391 commit d84205d

File tree

10 files changed

+254
-18
lines changed

10 files changed

+254
-18
lines changed

‎build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ dependencies {
119119
shaded("net.neoforged:JarJarMetadata:0.4.2") {
120120
exclude group: 'org.slf4j'
121121
}
122+
shaded 'net.neoforged.installertools:problems-api:3.0.4'
122123

123124
java8CompileOnly gradleApi()
124125

‎src/main/java/net/neoforged/moddevgradle/internal/ModDevArtifactsWorkflow.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,17 @@ public static ModDevArtifactsWorkflow create(Project project,
124124
.map(javaLauncher -> javaLauncher.getExecutablePath().getAsFile().getAbsolutePath()));
125125

126126
task.getAccessTransformers().from(accessTransformers);
127+
// If AT validation is enabled, add the user-supplied AT paths as files to be validated,
128+
// they're also part of the normal AT collection, so if AT validation is disabled, just return an empty list.
129+
task.getValidatedAccessTransformers().from(
130+
extension.getValidateAccessTransformers().map(validate -> {
131+
if (validate) {
132+
return extension.getAccessTransformers().getFiles();
133+
} else {
134+
return project.files();
135+
}
136+
}));
127137
task.getInterfaceInjectionData().from(interfaceInjectionData);
128-
task.getValidateAccessTransformers().set(extension.getValidateAccessTransformers());
129138
task.getParchmentData().from(parchmentData);
130139
task.getParchmentEnabled().set(parchment.getEnabled());
131140
task.getParchmentConflictResolutionPrefix().set(parchment.getConflictResolutionPrefix());

‎src/main/java/net/neoforged/moddevgradle/internal/utils/ProblemReportingUtil.java‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
package net.neoforged.moddevgradle.internal.utils;
2+
3+
import java.lang.reflect.InvocationTargetException;
4+
import java.util.Map;
5+
import java.util.Objects;
6+
import java.util.concurrent.ConcurrentHashMap;
7+
import net.neoforged.problems.Problem;
8+
import org.gradle.api.Action;
9+
import org.gradle.api.problems.ProblemGroup;
10+
import org.gradle.api.problems.ProblemId;
11+
import org.gradle.api.problems.ProblemSpec;
12+
import org.gradle.api.problems.Problems;
13+
import org.gradle.api.problems.Severity;
14+
import org.jetbrains.annotations.Nullable;
15+
import org.slf4j.Logger;
16+
import org.slf4j.LoggerFactory;
17+
18+
/**
19+
* Our lowest supported Gradle version is 8.8. The Problems Api has significantly changed
20+
* since then. This class aims to hide the details of supporting this transparently.
21+
* <p>
22+
* The {@link Problems} interface has been added in Gradle 8.6 and is thus safe to use in
23+
* the public interface of this class.
24+
* <p>
25+
* {@link ProblemId} and {@link ProblemGroup} are also fine to use, since they're available since Gradle 8.8.
26+
*/
27+
public final class ProblemReportingUtil {
28+
private static final Logger LOG = LoggerFactory.getLogger(ProblemReportingUtil.class);
29+
30+
private static final Map<net.neoforged.problems.ProblemGroup, ProblemGroup> PROBLEM_GROUPS = new ConcurrentHashMap<>();
31+
private static final Map<net.neoforged.problems.ProblemId, ProblemId> PROBLEM_IDS = new ConcurrentHashMap<>();
32+
33+
private ProblemReportingUtil() {}
34+
35+
public static void report(Problems problems, Problem problem) {
36+
try {
37+
var id = getProblemId(problem.problemId());
38+
Gradle813Adapter.report(problems, id, spec -> applyProblemToProblemSpec(problem, spec));
39+
return;
40+
} catch (ProblemsApiUnsupported ignored) {}
41+
42+
// As a fallback, report on the console
43+
switch (problem.severity()) {
44+
case ADVICE -> LOG.info("{}", problem);
45+
case WARNING -> LOG.warn("{}", problem);
46+
case ERROR -> LOG.error("{}", problem);
47+
}
48+
}
49+
50+
private static void applyProblemToProblemSpec(Problem problem, ProblemSpec spec) {
51+
switch (problem.severity()) {
52+
case ADVICE -> spec.severity(Severity.ADVICE);
53+
case WARNING -> spec.severity(Severity.WARNING);
54+
case ERROR -> spec.severity(Severity.ERROR);
55+
}
56+
57+
if (problem.contextualLabel() != null) {
58+
spec.contextualLabel(problem.contextualLabel());
59+
}
60+
if (problem.details() != null) {
61+
spec.details(problem.details());
62+
}
63+
if (problem.solution() != null) {
64+
spec.solution(problem.solution());
65+
}
66+
if (problem.documentedAt() != null) {
67+
spec.documentedAt(problem.documentedAt());
68+
}
69+
70+
var location = problem.location();
71+
if (location != null) {
72+
var filePath = location.file().toAbsolutePath().toString();
73+
if (location.offset() != null) {
74+
int length = Objects.requireNonNullElse(location.length(), 0);
75+
spec.offsetInFileLocation(filePath, location.offset(), length);
76+
} else if (location.line() != null) {
77+
if (location.column() != null) {
78+
if (location.length() != null) {
79+
spec.lineInFileLocation(filePath, location.line(), location.column(), location.length());
80+
} else {
81+
spec.lineInFileLocation(filePath, location.line(), location.column());
82+
}
83+
} else {
84+
spec.lineInFileLocation(filePath, location.line());
85+
}
86+
} else {
87+
spec.fileLocation(filePath);
88+
}
89+
}
90+
}
91+
92+
private static ProblemId getProblemId(net.neoforged.problems.ProblemId problemId) {
93+
return PROBLEM_IDS.computeIfAbsent(problemId, ProblemReportingUtil::convertProblemId);
94+
}
95+
96+
private static ProblemId convertProblemId(net.neoforged.problems.ProblemId problemId) {
97+
return Gradle813Adapter.createProblemId(
98+
problemId.id(),
99+
problemId.displayName(),
100+
getProblemGroup(problemId.group()));
101+
}
102+
103+
private static ProblemGroup getProblemGroup(net.neoforged.problems.ProblemGroup problemGroup) {
104+
return PROBLEM_GROUPS.computeIfAbsent(problemGroup, ProblemReportingUtil::convertProblemGroup);
105+
}
106+
107+
private static ProblemGroup convertProblemGroup(net.neoforged.problems.ProblemGroup problemGroup) {
108+
return Gradle813Adapter.createProblemGroup(
109+
problemGroup.id(),
110+
problemGroup.displayName(),
111+
problemGroup.parent() == null ? null : getProblemGroup(problemGroup.parent()));
112+
}
113+
114+
/**
115+
* We're compiling against Gradle 8.8, but these methods have become available only in 8.13.
116+
*/
117+
private static class Gradle813Adapter {
118+
public static ProblemId createProblemId(String id, String displayName, ProblemGroup problemGroup) {
119+
try {
120+
var factory = ProblemId.class.getMethod("create", String.class, String.class, ProblemGroup.class);
121+
return (ProblemId) factory.invoke(null, id, displayName, problemGroup);
122+
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
123+
LOG.debug("Problems API is unsupported.", e);
124+
throw new ProblemsApiUnsupported();
125+
}
126+
}
127+
128+
public static ProblemGroup createProblemGroup(String id, String displayName, @Nullable ProblemGroup parent) {
129+
try {
130+
if (parent == null) {
131+
var factory = ProblemGroup.class.getMethod("create", String.class, String.class);
132+
return (ProblemGroup) factory.invoke(null, id, displayName);
133+
} else {
134+
var factory = ProblemGroup.class.getMethod("create", String.class, String.class, ProblemGroup.class);
135+
return (ProblemGroup) factory.invoke(null, id, displayName, parent);
136+
}
137+
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
138+
LOG.debug("Problems API is unsupported.", e);
139+
throw new ProblemsApiUnsupported();
140+
}
141+
}
142+
143+
public static void report(Problems problems, ProblemId id, Action<ProblemSpec> consumer) {
144+
try {
145+
var reporter = problems.getClass().getMethod("getReporter").invoke(problems);
146+
147+
var problemFactory = reporter.getClass().getMethod("create", ProblemId.class, Action.class);
148+
var problem = problemFactory.invoke(reporter, id, consumer);
149+
150+
var problemClass = Class.forName("org.gradle.api.problems.Problem", true, reporter.getClass().getClassLoader());
151+
152+
var problemConsumer = reporter.getClass().getMethod("report", problemClass);
153+
problemConsumer.invoke(reporter, problem);
154+
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException | ClassNotFoundException e) {
155+
LOG.debug("Problems API is unsupported.", e);
156+
throw new ProblemsApiUnsupported();
157+
}
158+
}
159+
}
160+
161+
private static class ProblemsApiUnsupported extends RuntimeException {}
162+
}

‎src/main/java/net/neoforged/nfrtgradle/CreateMinecraftArtifacts.java‎

Lines changed: 68 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
package net.neoforged.nfrtgradle;
22

33
import java.io.File;
4+
import java.io.IOException;
45
import java.util.ArrayList;
56
import java.util.Collections;
7+
import java.util.HashSet;
68
import java.util.List;
79
import javax.inject.Inject;
10+
import net.neoforged.moddevgradle.internal.utils.ProblemReportingUtil;
11+
import net.neoforged.problems.FileProblemReporter;
12+
import net.neoforged.problems.Problem;
813
import org.gradle.api.GradleException;
914
import org.gradle.api.file.ConfigurableFileCollection;
1015
import org.gradle.api.file.RegularFileProperty;
16+
import org.gradle.api.problems.Problems;
1117
import org.gradle.api.provider.MapProperty;
1218
import org.gradle.api.provider.Property;
1319
import org.gradle.api.tasks.Input;
@@ -54,6 +60,15 @@ public CreateMinecraftArtifacts() {
5460
@InputFiles
5561
public abstract ConfigurableFileCollection getAccessTransformers();
5662

63+
/**
64+
* Files added to this collection will be passed to NFRT via the {@code --validated-access-transformer}
65+
* command line option and will fail the build if they contain errors.
66+
* <p>
67+
* This is a more precise version of setting {@link #getValidateAccessTransformers()} to {@code true}.
68+
*/
69+
@InputFiles
70+
public abstract ConfigurableFileCollection getValidatedAccessTransformers();
71+
5772
/**
5873
* Files added to this collection will be passed to NFRT via the {@code --interface-injection-data}
5974
* command line option.
@@ -62,10 +77,15 @@ public CreateMinecraftArtifacts() {
6277
public abstract ConfigurableFileCollection getInterfaceInjectionData();
6378

6479
/**
65-
* If set to true, passes {@code --validate-access-transformers} to NFRT.
80+
* If set to true, all files from {@link #getAccessTransformers()} are added as validated ATs and will fail the build
81+
* if they contain errors, or they target non-existent code elements.
82+
* <p>
6683
* Defaults to false.
84+
*
85+
* @deprecated Prefer the more precise {@link #getValidatedAccessTransformers()} setting.
6786
*/
6887
@Input
88+
@Deprecated
6989
public abstract Property<Boolean> getValidateAccessTransformers();
7090

7191
/**
@@ -172,6 +192,9 @@ public CreateMinecraftArtifacts() {
172192
@Optional
173193
public abstract RegularFileProperty getResourcesArtifact();
174194

195+
@Inject
196+
protected abstract Problems getProblems();
197+
175198
@TaskAction
176199
public void createArtifacts() {
177200
var args = new ArrayList<String>();
@@ -182,20 +205,31 @@ public void createArtifacts() {
182205
args.add(getToolsJavaExecutable().get());
183206
}
184207

208+
// If an AT path is added twice, the validated variant takes precedence
209+
var accessTransformersAdded = new HashSet<File>();
210+
for (var accessTransformer : getValidatedAccessTransformers().getFiles()) {
211+
if (accessTransformersAdded.add(accessTransformer)) {
212+
args.add("--validated-access-transformer");
213+
args.add(accessTransformer.getAbsolutePath());
214+
}
215+
}
216+
185217
for (var accessTransformer : getAccessTransformers().getFiles()) {
186-
args.add("--access-transformer");
187-
args.add(accessTransformer.getAbsolutePath());
218+
if (accessTransformersAdded.add(accessTransformer)) {
219+
if (getValidateAccessTransformers().get()) {
220+
args.add("--validated-access-transformer");
221+
} else {
222+
args.add("--access-transformer");
223+
}
224+
args.add(accessTransformer.getAbsolutePath());
225+
}
188226
}
189227

190228
for (var interfaceInjectionFile : getInterfaceInjectionData().getFiles()) {
191229
args.add("--interface-injection-data");
192230
args.add(interfaceInjectionFile.getAbsolutePath());
193231
}
194232

195-
if (getValidateAccessTransformers().get()) {
196-
args.add("--validate-access-transformers");
197-
}
198-
199233
if (getParchmentEnabled().get()) {
200234
var parchmentData = getParchmentData().getFiles();
201235
if (parchmentData.size() == 1) {
@@ -275,7 +309,33 @@ public void createArtifacts() {
275309
args.add(requestedResult.id() + ":" + requestedResult.destination().getAbsolutePath());
276310
}
277311

278-
run(args);
312+
var problemsReport = new File(getTemporaryDir(), "nfrt-problem-report.json");
313+
args.add("--problems-report");
314+
args.add(problemsReport.getAbsolutePath());
315+
316+
try {
317+
run(args);
318+
} finally {
319+
reportProblems(problemsReport);
320+
}
321+
}
322+
323+
private void reportProblems(File problemsReport) {
324+
if (!problemsReport.exists()) {
325+
return; // Not created -> nothing to report
326+
}
327+
328+
List<Problem> problems;
329+
try {
330+
problems = FileProblemReporter.loadRecords(problemsReport.toPath());
331+
} catch (IOException e) {
332+
getLogger().warn("Failed to load NFRT problems report from {}", problemsReport, e);
333+
return;
334+
}
335+
336+
for (Problem problem : problems) {
337+
ProblemReportingUtil.report(getProblems(), problem);
338+
}
279339
}
280340

281341
record RequestedResult(String id, File destination) {}

‎src/main/java/net/neoforged/nfrtgradle/NeoFormRuntimeExtension.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
public abstract class NeoFormRuntimeExtension {
1313
public static final String NAME = "neoFormRuntime";
1414

15-
private static final String DEFAULT_NFRT_VERSION = "1.0.22";
15+
private static final String DEFAULT_NFRT_VERSION = "1.0.23";
1616

1717
@Inject
1818
public NeoFormRuntimeExtension(Project project) {

‎testproject/build.gradle‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ plugins {
55

66
evaluationDependsOn(":subproject") // Because of the sourceset reference
77

8+
tasks.named('wrapper') {
9+
distributionType = Wrapper.DistributionType.ALL
10+
}
11+
812
sourceSets {
913
api
1014
}
@@ -29,6 +33,7 @@ neoForge {
2933
version = project.neoforge_version
3034
addModdingDependenciesTo(sourceSets.api)
3135

36+
validateAccessTransformers = true
3237
interfaceInjectionData = files("interfaces.json")
3338

3439
runs {

‎testproject/gradle/wrapper/gradle-wrapper.jar‎

260 Bytes
Binary file not shown.

‎testproject/gradle/wrapper/gradle-wrapper.properties‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
distributionBase=GRADLE_USER_HOME
22
distributionPath=wrapper/dists
3-
distributionUrl=https\://services.gradle.org/distributions/gradle-8.9-all.zip
3+
distributionUrl=https\://services.gradle.org/distributions/gradle-8.14-all.zip
44
networkTimeout=10000
55
validateDistributionUrl=true
66
zipStoreBase=GRADLE_USER_HOME

‎testproject/gradlew‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ done
8686
# shellcheck disable=SC2034
8787
APP_BASE_NAME=${0##*/}
8888
# Discard cd standard output in case $CDPATH is set (https://github.com/gradle/gradle/issues/25036)
89-
APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s
90-
' "$PWD" ) || exit
89+
APP_HOME=$( cd -P "${APP_HOME:-./}" > /dev/null && printf '%s\n' "$PWD" ) || exit
9190

9291
# Use the maximum available, or set MAX_FD != -1 to use that value.
9392
MAX_FD=maximum
@@ -115,7 +114,7 @@ case "$( uname )" in #(
115114
NONSTOP* ) nonstop=true ;;
116115
esac
117116

118-
CLASSPATH=$APP_HOME/gradle/wrapper/gradle-wrapper.jar
117+
CLASSPATH="\\\"\\\""
119118

120119

121120
# Determine the Java command to use to start the JVM.
@@ -206,15 +205,15 @@ fi
206205
DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"'
207206

208207
# Collect all arguments for the java command:
209-
# * DEFAULT_JVM_OPTS, JAVA_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,
208+
# * DEFAULT_JVM_OPTS, JAVA_OPTS, and optsEnvironmentVar are not allowed to contain shell fragments,
210209
# and any embedded shellness will be escaped.
211210
# * For example: A user cannot expect ${Hostname} to be expanded, as it is an environment variable and will be
212211
# treated as '${Hostname}' itself on the command line.
213212

214213
set -- \
215214
"-Dorg.gradle.appname=$APP_BASE_NAME" \
216215
-classpath "$CLASSPATH" \
217-
org.gradle.wrapper.GradleWrapperMain \
216+
-jar "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" \
218217
"$@"
219218

220219
# Stop when "xargs" is not available.

0 commit comments

Comments
 (0)