From e9a4b9347752cc16234f86b09f60463638681580 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Wed, 17 Dec 2025 11:45:20 +0100 Subject: [PATCH] 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 --- .../core/test/tools/CompilationException.java | 103 +++++++++++++++--- .../core/test/tools/TestCompiler.java | 48 ++++---- .../test/tools/CompilationExceptionTests.java | 57 +++++++++- .../core/test/tools/TestCompilerTests.java | 13 ++- 4 files changed, 176 insertions(+), 45 deletions(-) diff --git a/spring-core-test/src/main/java/org/springframework/core/test/tools/CompilationException.java b/spring-core-test/src/main/java/org/springframework/core/test/tools/CompilationException.java index d6c7dff7c67..41465a59ab6 100644 --- a/spring-core-test/src/main/java/org/springframework/core/test/tools/CompilationException.java +++ b/spring-core-test/src/main/java/org/springframework/core/test/tools/CompilationException.java @@ -16,38 +16,105 @@ package org.springframework.core.test.tools; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.Arrays; +import java.util.List; +import java.util.function.Function; +import java.util.stream.Collectors; + +import javax.tools.Diagnostic; + /** - * Exception thrown when code cannot compile. + * Exception thrown when code cannot compile. Expose the {@linkplain Problem + * problems} for further inspection. * * @author Phillip Webb + * @author Stephane Nicoll * @since 6.0 */ @SuppressWarnings("serial") public class CompilationException extends RuntimeException { + private final List problems; - CompilationException(String errors, SourceFiles sourceFiles, ResourceFiles resourceFiles) { - super(buildMessage(errors, sourceFiles, resourceFiles)); + CompilationException(List problems, SourceFiles sourceFiles, ResourceFiles resourceFiles) { + super(buildMessage(problems, sourceFiles, resourceFiles)); + this.problems = problems; } - - private static String buildMessage(String errors, SourceFiles sourceFiles, + private static String buildMessage(List problems, SourceFiles sourceFiles, ResourceFiles resourceFiles) { - StringBuilder message = new StringBuilder(); - message.append("Unable to compile source\n\n"); - message.append(errors); - message.append("\n\n"); - for (SourceFile sourceFile : sourceFiles) { - message.append("---- source: ").append(sourceFile.getPath()).append("\n\n"); - message.append(sourceFile.getContent()); - message.append("\n\n"); + StringWriter out = new StringWriter(); + PrintWriter writer = new PrintWriter(out); + writer.println("Unable to compile source"); + Function, String> createBulletList = elements -> elements.stream() + .map(warning -> "- %s".formatted(warning.message())) + .collect(Collectors.joining("\n")); + + List errors = problems.stream() + .filter(problem -> problem.kind == Diagnostic.Kind.ERROR).toList(); + if (!errors.isEmpty()) { + writer.println(); + writer.println("Errors:"); + writer.println(createBulletList.apply(errors)); } - for (ResourceFile resourceFile : resourceFiles) { - message.append("---- resource: ").append(resourceFile.getPath()).append("\n\n"); - message.append(resourceFile.getContent()); - message.append("\n\n"); + List warnings = problems.stream() + .filter(problem -> problem.kind == Diagnostic.Kind.WARNING || + problem.kind == Diagnostic.Kind.MANDATORY_WARNING).toList(); + if (!warnings.isEmpty()) { + writer.println(); + writer.println("Warnings:"); + writer.println(createBulletList.apply(warnings)); } - return message.toString(); + if (!sourceFiles.isEmpty()) { + for (SourceFile sourceFile : sourceFiles) { + writer.println(); + writer.printf("---- source: %s%n".formatted(sourceFile.getPath())); + writer.println(sourceFile.getContent()); + } + } + if (!resourceFiles.isEmpty()) { + for (ResourceFile resourceFile : resourceFiles) { + writer.println(); + writer.printf("---- resource: %s%n".formatted(resourceFile.getPath())); + writer.println(resourceFile.getContent()); + } + } + return out.toString(); + } + + /** + * Return the {@linkplain Problem problems} that lead to this exception. + * @return the problems + * @since 7.0.3 + */ + public List getProblems() { + return this.problems; + } + + /** + * Return the {@linkplain Problem problems} of the given {@code kinds}. + * @param kinds the {@linkplain Diagnostic.Kind kinds} to filter on + * @return the problems with the given kinds, or an empty list + * @since 7.0.3 + */ + public List getProblems(Diagnostic.Kind... kinds) { + List toMatch = Arrays.asList(kinds); + return this.problems.stream().filter(problem -> toMatch.contains(problem.kind())).toList(); + } + + /** + * Description of a problem that lead to a compilation failure. + *

{@linkplain Diagnostic.Kind#ERROR errors} are the most important, but + * they might not be enough in case an error is triggered by the presence + * of a warning, see {@link Diagnostic.Kind#MANDATORY_WARNING}. + * @since 7.0.3 + * @param kind the kind of problem + * @param message the description of the problem + */ + public record Problem(Diagnostic.Kind kind, String message) { + } } diff --git a/spring-core-test/src/main/java/org/springframework/core/test/tools/TestCompiler.java b/spring-core-test/src/main/java/org/springframework/core/test/tools/TestCompiler.java index 9d646048769..c373dd56230 100644 --- a/spring-core-test/src/main/java/org/springframework/core/test/tools/TestCompiler.java +++ b/spring-core-test/src/main/java/org/springframework/core/test/tools/TestCompiler.java @@ -307,13 +307,13 @@ public final class TestCompiler { DynamicJavaFileManager fileManager = new DynamicJavaFileManager( standardFileManager, classLoaderToUse, this.classFiles, this.resourceFiles); if (!this.sourceFiles.isEmpty()) { - Errors errors = new Errors(); - CompilationTask task = this.compiler.getTask(null, fileManager, errors, + Problems problems = new Problems(); + CompilationTask task = this.compiler.getTask(null, fileManager, problems, this.compilerOptions, null, compilationUnits); task.setProcessors(this.processors); boolean result = task.call(); - if (!result || errors.hasReportedErrors()) { - throw new CompilationException(errors.toString(), this.sourceFiles, this.resourceFiles); + if (!result || problems.hasReportedErrors()) { + throw new CompilationException(problems.elements, this.sourceFiles, this.resourceFiles); } } return new DynamicClassLoader(classLoaderToUse, this.classFiles, this.resourceFiles, @@ -342,34 +342,38 @@ public final class TestCompiler { /** - * {@link DiagnosticListener} used to collect errors. + * {@link DiagnosticListener} used to collect errors and warnings. */ - static class Errors implements DiagnosticListener { + static class Problems implements DiagnosticListener { - private final StringBuilder message = new StringBuilder(); + private static final List HANDLED_DIAGNOSTICS = List.of( + Diagnostic.Kind.ERROR, Diagnostic.Kind.MANDATORY_WARNING, Diagnostic.Kind.WARNING); + + private final List elements = new ArrayList<>(); @Override public void report(Diagnostic diagnostic) { - if (diagnostic.getKind() == Diagnostic.Kind.ERROR) { - this.message.append('\n'); - this.message.append(diagnostic.getMessage(Locale.getDefault())); - if (diagnostic.getSource() != null) { - this.message.append(' '); - this.message.append(diagnostic.getSource().getName()); - this.message.append(' '); - this.message.append(diagnostic.getLineNumber()).append(':') - .append(diagnostic.getColumnNumber()); - } + Diagnostic.Kind kind = diagnostic.getKind(); + if (HANDLED_DIAGNOSTICS.contains(kind)) { + this.elements.add(new CompilationException.Problem(kind, toMessage(diagnostic))); } } - boolean hasReportedErrors() { - return !this.message.isEmpty(); + private String toMessage(Diagnostic diagnostic) { + StringBuilder message = new StringBuilder(); + message.append(diagnostic.getMessage(Locale.getDefault())); + if (diagnostic.getSource() != null) { + message.append(' '); + message.append(diagnostic.getSource().getName()); + message.append(' '); + message.append(diagnostic.getLineNumber()).append(':') + .append(diagnostic.getColumnNumber()); + } + return message.toString(); } - @Override - public String toString() { - return this.message.toString(); + boolean hasReportedErrors() { + return this.elements.stream().anyMatch(problem -> problem.kind() == Diagnostic.Kind.ERROR); } } diff --git a/spring-core-test/src/test/java/org/springframework/core/test/tools/CompilationExceptionTests.java b/spring-core-test/src/test/java/org/springframework/core/test/tools/CompilationExceptionTests.java index 8212892d256..847987860fe 100644 --- a/spring-core-test/src/test/java/org/springframework/core/test/tools/CompilationExceptionTests.java +++ b/spring-core-test/src/test/java/org/springframework/core/test/tools/CompilationExceptionTests.java @@ -16,21 +16,72 @@ package org.springframework.core.test.tools; +import java.util.List; + +import javax.tools.Diagnostic; + import org.junit.jupiter.api.Test; +import org.springframework.core.test.tools.CompilationException.Problem; + import static org.assertj.core.api.Assertions.assertThat; /** * Tests for {@link CompilationException}. * * @author Phillip Webb + * @author Stephane Nicoll */ class CompilationExceptionTests { @Test - void getMessageReturnsMessage() { - CompilationException exception = new CompilationException("message", SourceFiles.none(), ResourceFiles.none()); - assertThat(exception).hasMessageContaining("message"); + void exceptionMessageReportsSingleError() { + CompilationException exception = new CompilationException( + List.of(new Problem(Diagnostic.Kind.ERROR, "error message")), + SourceFiles.none(), ResourceFiles.none()); + assertThat(exception.getMessage().lines()).containsExactly( + "Unable to compile source", "", "Errors:", "- error message"); + } + + @Test + void exceptionMessageReportsSingleWarning() { + CompilationException exception = new CompilationException( + List.of(new Problem(Diagnostic.Kind.MANDATORY_WARNING, "warning message")), + SourceFiles.none(), ResourceFiles.none()); + assertThat(exception.getMessage().lines()).containsExactly( + "Unable to compile source", "", "Warnings:", "- warning message"); + } + + @Test + void exceptionMessageReportsProblems() { + CompilationException exception = new CompilationException(List.of( + new Problem(Diagnostic.Kind.MANDATORY_WARNING, "warning message"), + new Problem(Diagnostic.Kind.ERROR, "error message"), + new Problem(Diagnostic.Kind.WARNING, "warning message2"), + new Problem(Diagnostic.Kind.ERROR, "error message2")), SourceFiles.none(), ResourceFiles.none()); + assertThat(exception.getMessage().lines()).containsExactly( + "Unable to compile source", "", "Errors:", "- error message", "- error message2", "" , + "Warnings:", "- warning message","- warning message2"); + } + + @Test + void exceptionMessageReportsSourceCode() { + CompilationException exception = new CompilationException( + List.of(new Problem(Diagnostic.Kind.ERROR, "error message")), + SourceFiles.of(SourceFile.of("public class Hello {}")), ResourceFiles.none()); + assertThat(exception.getMessage().lines()).containsExactly( + "Unable to compile source", "", "Errors:", "- error message", "", + "---- source: Hello.java", "public class Hello {}"); + } + + @Test + void exceptionMessageReportsResource() { + CompilationException exception = new CompilationException( + List.of(new Problem(Diagnostic.Kind.ERROR, "error message")), + SourceFiles.none(), ResourceFiles.of(ResourceFile.of("application.properties", "test=value"))); + assertThat(exception.getMessage().lines()).containsExactly( + "Unable to compile source", "", "Errors:", "- error message", "", + "---- resource: application.properties", "test=value"); } } diff --git a/spring-core-test/src/test/java/org/springframework/core/test/tools/TestCompilerTests.java b/spring-core-test/src/test/java/org/springframework/core/test/tools/TestCompilerTests.java index 59d80e70fba..97eb09623b2 100644 --- a/spring-core-test/src/test/java/org/springframework/core/test/tools/TestCompilerTests.java +++ b/spring-core-test/src/test/java/org/springframework/core/test/tools/TestCompilerTests.java @@ -30,6 +30,7 @@ import javax.annotation.processing.Processor; import javax.annotation.processing.RoundEnvironment; import javax.annotation.processing.SupportedAnnotationTypes; import javax.lang.model.element.TypeElement; +import javax.tools.Diagnostic; import javax.tools.FileObject; import javax.tools.StandardLocation; @@ -136,7 +137,8 @@ class TestCompilerTests { assertThatExceptionOfType(CompilationException.class).isThrownBy( () -> TestCompiler.forSystem().withSources( SourceFile.of(HELLO_BAD)).compile(compiled -> { - })); + })).satisfies(ex -> assertThat(ex.getProblems()).singleElement() + .satisfies(problem -> assertThat(problem.message()).contains("Supplier"))); } @Test @@ -177,7 +179,14 @@ class TestCompilerTests { assertThatExceptionOfType(CompilationException.class).isThrownBy( () -> TestCompiler.forSystem().failOnWarning().withSources( SourceFile.of(HELLO_DEPRECATED), main).compile(compiled -> { - })); + })).satisfies(compilationException -> { + assertThat(compilationException.getProblems(Diagnostic.Kind.ERROR)).singleElement() + .satisfies(error -> assertThat(error.message()) + .contains("-Werror")); + assertThat(compilationException.getProblems(Diagnostic.Kind.MANDATORY_WARNING)).singleElement() + .satisfies(warning -> assertThat(warning.message()) + .contains("get()", "com.example.Hello")); + }); } @Test