Skip to content

Commit e9a4b93

Browse files
committed
Expose compiler warnings in CompilationException
This commit improves TestCompiler to expose both errors and warnings instead of an opaque message. When compilation fails, both errors and warnings are displayed. This is particularly useful when combined with the `-Werror` option that turns the presence of a warning into an error. Closes gh-36037
1 parent 86e89d5 commit e9a4b93

File tree

4 files changed

+176
-45
lines changed

4 files changed

+176
-45
lines changed

spring-core-test/src/main/java/org/springframework/core/test/tools/CompilationException.java

Lines changed: 85 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,38 +16,105 @@
1616

1717
package org.springframework.core.test.tools;
1818

19+
import java.io.PrintWriter;
20+
import java.io.StringWriter;
21+
import java.util.Arrays;
22+
import java.util.List;
23+
import java.util.function.Function;
24+
import java.util.stream.Collectors;
25+
26+
import javax.tools.Diagnostic;
27+
1928
/**
20-
* Exception thrown when code cannot compile.
29+
* Exception thrown when code cannot compile. Expose the {@linkplain Problem
30+
* problems} for further inspection.
2131
*
2232
* @author Phillip Webb
33+
* @author Stephane Nicoll
2334
* @since 6.0
2435
*/
2536
@SuppressWarnings("serial")
2637
public class CompilationException extends RuntimeException {
2738

39+
private final List<Problem> problems;
2840

29-
CompilationException(String errors, SourceFiles sourceFiles, ResourceFiles resourceFiles) {
30-
super(buildMessage(errors, sourceFiles, resourceFiles));
41+
CompilationException(List<Problem> problems, SourceFiles sourceFiles, ResourceFiles resourceFiles) {
42+
super(buildMessage(problems, sourceFiles, resourceFiles));
43+
this.problems = problems;
3144
}
3245

33-
34-
private static String buildMessage(String errors, SourceFiles sourceFiles,
46+
private static String buildMessage(List<Problem> problems, SourceFiles sourceFiles,
3547
ResourceFiles resourceFiles) {
36-
StringBuilder message = new StringBuilder();
37-
message.append("Unable to compile source\n\n");
38-
message.append(errors);
39-
message.append("\n\n");
40-
for (SourceFile sourceFile : sourceFiles) {
41-
message.append("---- source: ").append(sourceFile.getPath()).append("\n\n");
42-
message.append(sourceFile.getContent());
43-
message.append("\n\n");
48+
StringWriter out = new StringWriter();
49+
PrintWriter writer = new PrintWriter(out);
50+
writer.println("Unable to compile source");
51+
Function<List<Problem>, String> createBulletList = elements -> elements.stream()
52+
.map(warning -> "- %s".formatted(warning.message()))
53+
.collect(Collectors.joining("\n"));
54+
55+
List<Problem> errors = problems.stream()
56+
.filter(problem -> problem.kind == Diagnostic.Kind.ERROR).toList();
57+
if (!errors.isEmpty()) {
58+
writer.println();
59+
writer.println("Errors:");
60+
writer.println(createBulletList.apply(errors));
4461
}
45-
for (ResourceFile resourceFile : resourceFiles) {
46-
message.append("---- resource: ").append(resourceFile.getPath()).append("\n\n");
47-
message.append(resourceFile.getContent());
48-
message.append("\n\n");
62+
List<Problem> warnings = problems.stream()
63+
.filter(problem -> problem.kind == Diagnostic.Kind.WARNING ||
64+
problem.kind == Diagnostic.Kind.MANDATORY_WARNING).toList();
65+
if (!warnings.isEmpty()) {
66+
writer.println();
67+
writer.println("Warnings:");
68+
writer.println(createBulletList.apply(warnings));
4969
}
50-
return message.toString();
70+
if (!sourceFiles.isEmpty()) {
71+
for (SourceFile sourceFile : sourceFiles) {
72+
writer.println();
73+
writer.printf("---- source: %s%n".formatted(sourceFile.getPath()));
74+
writer.println(sourceFile.getContent());
75+
}
76+
}
77+
if (!resourceFiles.isEmpty()) {
78+
for (ResourceFile resourceFile : resourceFiles) {
79+
writer.println();
80+
writer.printf("---- resource: %s%n".formatted(resourceFile.getPath()));
81+
writer.println(resourceFile.getContent());
82+
}
83+
}
84+
return out.toString();
85+
}
86+
87+
/**
88+
* Return the {@linkplain Problem problems} that lead to this exception.
89+
* @return the problems
90+
* @since 7.0.3
91+
*/
92+
public List<Problem> getProblems() {
93+
return this.problems;
94+
}
95+
96+
/**
97+
* Return the {@linkplain Problem problems} of the given {@code kinds}.
98+
* @param kinds the {@linkplain Diagnostic.Kind kinds} to filter on
99+
* @return the problems with the given kinds, or an empty list
100+
* @since 7.0.3
101+
*/
102+
public List<Problem> getProblems(Diagnostic.Kind... kinds) {
103+
List<Diagnostic.Kind> toMatch = Arrays.asList(kinds);
104+
return this.problems.stream().filter(problem -> toMatch.contains(problem.kind())).toList();
105+
}
106+
107+
/**
108+
* Description of a problem that lead to a compilation failure.
109+
* <p>{@linkplain Diagnostic.Kind#ERROR errors} are the most important, but
110+
* they might not be enough in case an error is triggered by the presence
111+
* of a warning, see {@link Diagnostic.Kind#MANDATORY_WARNING}.
112+
* @since 7.0.3
113+
* @param kind the kind of problem
114+
* @param message the description of the problem
115+
*/
116+
public record Problem(Diagnostic.Kind kind, String message) {
117+
51118
}
52119

53120
}

spring-core-test/src/main/java/org/springframework/core/test/tools/TestCompiler.java

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -307,13 +307,13 @@ private DynamicClassLoader compile() {
307307
DynamicJavaFileManager fileManager = new DynamicJavaFileManager(
308308
standardFileManager, classLoaderToUse, this.classFiles, this.resourceFiles);
309309
if (!this.sourceFiles.isEmpty()) {
310-
Errors errors = new Errors();
311-
CompilationTask task = this.compiler.getTask(null, fileManager, errors,
310+
Problems problems = new Problems();
311+
CompilationTask task = this.compiler.getTask(null, fileManager, problems,
312312
this.compilerOptions, null, compilationUnits);
313313
task.setProcessors(this.processors);
314314
boolean result = task.call();
315-
if (!result || errors.hasReportedErrors()) {
316-
throw new CompilationException(errors.toString(), this.sourceFiles, this.resourceFiles);
315+
if (!result || problems.hasReportedErrors()) {
316+
throw new CompilationException(problems.elements, this.sourceFiles, this.resourceFiles);
317317
}
318318
}
319319
return new DynamicClassLoader(classLoaderToUse, this.classFiles, this.resourceFiles,
@@ -342,34 +342,38 @@ public TestCompiler printFiles(PrintStream printStream) {
342342

343343

344344
/**
345-
* {@link DiagnosticListener} used to collect errors.
345+
* {@link DiagnosticListener} used to collect errors and warnings.
346346
*/
347-
static class Errors implements DiagnosticListener<JavaFileObject> {
347+
static class Problems implements DiagnosticListener<JavaFileObject> {
348348

349-
private final StringBuilder message = new StringBuilder();
349+
private static final List<Diagnostic.Kind> HANDLED_DIAGNOSTICS = List.of(
350+
Diagnostic.Kind.ERROR, Diagnostic.Kind.MANDATORY_WARNING, Diagnostic.Kind.WARNING);
351+
352+
private final List<CompilationException.Problem> elements = new ArrayList<>();
350353

351354
@Override
352355
public void report(Diagnostic<? extends JavaFileObject> diagnostic) {
353-
if (diagnostic.getKind() == Diagnostic.Kind.ERROR) {
354-
this.message.append('\n');
355-
this.message.append(diagnostic.getMessage(Locale.getDefault()));
356-
if (diagnostic.getSource() != null) {
357-
this.message.append(' ');
358-
this.message.append(diagnostic.getSource().getName());
359-
this.message.append(' ');
360-
this.message.append(diagnostic.getLineNumber()).append(':')
361-
.append(diagnostic.getColumnNumber());
362-
}
356+
Diagnostic.Kind kind = diagnostic.getKind();
357+
if (HANDLED_DIAGNOSTICS.contains(kind)) {
358+
this.elements.add(new CompilationException.Problem(kind, toMessage(diagnostic)));
363359
}
364360
}
365361

366-
boolean hasReportedErrors() {
367-
return !this.message.isEmpty();
362+
private String toMessage(Diagnostic<? extends JavaFileObject> diagnostic) {
363+
StringBuilder message = new StringBuilder();
364+
message.append(diagnostic.getMessage(Locale.getDefault()));
365+
if (diagnostic.getSource() != null) {
366+
message.append(' ');
367+
message.append(diagnostic.getSource().getName());
368+
message.append(' ');
369+
message.append(diagnostic.getLineNumber()).append(':')
370+
.append(diagnostic.getColumnNumber());
371+
}
372+
return message.toString();
368373
}
369374

370-
@Override
371-
public String toString() {
372-
return this.message.toString();
375+
boolean hasReportedErrors() {
376+
return this.elements.stream().anyMatch(problem -> problem.kind() == Diagnostic.Kind.ERROR);
373377
}
374378

375379
}

spring-core-test/src/test/java/org/springframework/core/test/tools/CompilationExceptionTests.java

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,72 @@
1616

1717
package org.springframework.core.test.tools;
1818

19+
import java.util.List;
20+
21+
import javax.tools.Diagnostic;
22+
1923
import org.junit.jupiter.api.Test;
2024

25+
import org.springframework.core.test.tools.CompilationException.Problem;
26+
2127
import static org.assertj.core.api.Assertions.assertThat;
2228

2329
/**
2430
* Tests for {@link CompilationException}.
2531
*
2632
* @author Phillip Webb
33+
* @author Stephane Nicoll
2734
*/
2835
class CompilationExceptionTests {
2936

3037
@Test
31-
void getMessageReturnsMessage() {
32-
CompilationException exception = new CompilationException("message", SourceFiles.none(), ResourceFiles.none());
33-
assertThat(exception).hasMessageContaining("message");
38+
void exceptionMessageReportsSingleError() {
39+
CompilationException exception = new CompilationException(
40+
List.of(new Problem(Diagnostic.Kind.ERROR, "error message")),
41+
SourceFiles.none(), ResourceFiles.none());
42+
assertThat(exception.getMessage().lines()).containsExactly(
43+
"Unable to compile source", "", "Errors:", "- error message");
44+
}
45+
46+
@Test
47+
void exceptionMessageReportsSingleWarning() {
48+
CompilationException exception = new CompilationException(
49+
List.of(new Problem(Diagnostic.Kind.MANDATORY_WARNING, "warning message")),
50+
SourceFiles.none(), ResourceFiles.none());
51+
assertThat(exception.getMessage().lines()).containsExactly(
52+
"Unable to compile source", "", "Warnings:", "- warning message");
53+
}
54+
55+
@Test
56+
void exceptionMessageReportsProblems() {
57+
CompilationException exception = new CompilationException(List.of(
58+
new Problem(Diagnostic.Kind.MANDATORY_WARNING, "warning message"),
59+
new Problem(Diagnostic.Kind.ERROR, "error message"),
60+
new Problem(Diagnostic.Kind.WARNING, "warning message2"),
61+
new Problem(Diagnostic.Kind.ERROR, "error message2")), SourceFiles.none(), ResourceFiles.none());
62+
assertThat(exception.getMessage().lines()).containsExactly(
63+
"Unable to compile source", "", "Errors:", "- error message", "- error message2", "" ,
64+
"Warnings:", "- warning message","- warning message2");
65+
}
66+
67+
@Test
68+
void exceptionMessageReportsSourceCode() {
69+
CompilationException exception = new CompilationException(
70+
List.of(new Problem(Diagnostic.Kind.ERROR, "error message")),
71+
SourceFiles.of(SourceFile.of("public class Hello {}")), ResourceFiles.none());
72+
assertThat(exception.getMessage().lines()).containsExactly(
73+
"Unable to compile source", "", "Errors:", "- error message", "",
74+
"---- source: Hello.java", "public class Hello {}");
75+
}
76+
77+
@Test
78+
void exceptionMessageReportsResource() {
79+
CompilationException exception = new CompilationException(
80+
List.of(new Problem(Diagnostic.Kind.ERROR, "error message")),
81+
SourceFiles.none(), ResourceFiles.of(ResourceFile.of("application.properties", "test=value")));
82+
assertThat(exception.getMessage().lines()).containsExactly(
83+
"Unable to compile source", "", "Errors:", "- error message", "",
84+
"---- resource: application.properties", "test=value");
3485
}
3586

3687
}

spring-core-test/src/test/java/org/springframework/core/test/tools/TestCompilerTests.java

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import javax.annotation.processing.RoundEnvironment;
3131
import javax.annotation.processing.SupportedAnnotationTypes;
3232
import javax.lang.model.element.TypeElement;
33+
import javax.tools.Diagnostic;
3334
import javax.tools.FileObject;
3435
import javax.tools.StandardLocation;
3536

@@ -136,7 +137,8 @@ void compileWhenSourceHasCompileErrors() {
136137
assertThatExceptionOfType(CompilationException.class).isThrownBy(
137138
() -> TestCompiler.forSystem().withSources(
138139
SourceFile.of(HELLO_BAD)).compile(compiled -> {
139-
}));
140+
})).satisfies(ex -> assertThat(ex.getProblems()).singleElement()
141+
.satisfies(problem -> assertThat(problem.message()).contains("Supplier")));
140142
}
141143

142144
@Test
@@ -177,7 +179,14 @@ public static void main(String[] args) {
177179
assertThatExceptionOfType(CompilationException.class).isThrownBy(
178180
() -> TestCompiler.forSystem().failOnWarning().withSources(
179181
SourceFile.of(HELLO_DEPRECATED), main).compile(compiled -> {
180-
}));
182+
})).satisfies(compilationException -> {
183+
assertThat(compilationException.getProblems(Diagnostic.Kind.ERROR)).singleElement()
184+
.satisfies(error -> assertThat(error.message())
185+
.contains("-Werror"));
186+
assertThat(compilationException.getProblems(Diagnostic.Kind.MANDATORY_WARNING)).singleElement()
187+
.satisfies(warning -> assertThat(warning.message())
188+
.contains("get()", "com.example.Hello"));
189+
});
181190
}
182191

183192
@Test

0 commit comments

Comments
 (0)