diff --git a/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java b/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java index c00d35a65..69c5f9bb3 100644 --- a/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java +++ b/src/main/java/org/apache/maven/plugin/compiler/AbstractCompilerMojo.java @@ -885,6 +885,16 @@ final void amendincrementalCompilation(EnumSet aspects, @Parameter(property = "maven.compiler.maxmem") protected String maxmem; + /** + * Should maven log a summary of compiler messages (if any) by type. + * When there are a lot of messages of the same type (unchecked, deprecation etc), + * it can make the output more readable. + * + * @since 4.0.0-beta-5 + */ + @Parameter(property = "maven.compiler.messageLogType", defaultValue = "COMPILER") + protected MessageLogType messageLogType; + // ---------------------------------------------------------------------- // Read-only parameters // ---------------------------------------------------------------------- @@ -1227,7 +1237,7 @@ public void execute() throws MojoException { * Therefore, the executor can safely be executed in a background thread, * provided that the {@link #logger} is thread-safe. * - * @param listener where to send compilation warnings, or {@code null} for the Maven logger + * @param listener where to send compilation messages, or {@code null} for the Maven logger * @return the task to execute for compiling the project using the configuration in this MOJO * @throws MojoException if this method identifies an invalid parameter in this MOJO * @throws IOException if an error occurred while creating the output directory or scanning the source directories @@ -1248,6 +1258,18 @@ public ToolExecutor createExecutor(DiagnosticListener li return executor; } + /** + * Simple utility to enforce a listener. + * @param listener caller provided listener. + * @return user listener if not null else maven internal one. + */ + private DiagnosticListener useOrCreateListener( + final DiagnosticListener listener) { + return listener == null + ? new DiagnosticLogger(logger, messageBuilderFactory, null, project.getRootDirectory(), messageLogType) + : listener; + } + /** * {@return the compiler to use for compiling the code} * If {@link #fork} is {@code true}, the returned compiler will be a wrapper for a command line. @@ -1370,7 +1392,7 @@ public Options parseParameters(final OptionChecker compiler) { */ @SuppressWarnings("UseSpecificCatch") private void compile(final JavaCompiler compiler, final Options configuration) throws IOException { - final ToolExecutor executor = createExecutor(null); + final ToolExecutor executor = createExecutor(useOrCreateListener(null)); if (!executor.applyIncrementalBuild(this, configuration)) { return; } diff --git a/src/main/java/org/apache/maven/plugin/compiler/DiagnosticLogger.java b/src/main/java/org/apache/maven/plugin/compiler/DiagnosticLogger.java index 9bce7a546..7c51cd30e 100644 --- a/src/main/java/org/apache/maven/plugin/compiler/DiagnosticLogger.java +++ b/src/main/java/org/apache/maven/plugin/compiler/DiagnosticLogger.java @@ -21,18 +21,22 @@ import javax.tools.Diagnostic; import javax.tools.DiagnosticListener; import javax.tools.JavaFileObject; +import javax.tools.ToolProvider; import java.nio.file.Path; -import java.util.Arrays; import java.util.LinkedHashMap; import java.util.Locale; import java.util.Map; import java.util.Optional; +import java.util.ResourceBundle; +import java.util.function.Consumer; import org.apache.maven.api.plugin.Log; import org.apache.maven.api.services.MessageBuilder; import org.apache.maven.api.services.MessageBuilderFactory; +import static java.util.Optional.ofNullable; + /** * A Java compiler diagnostic listener which send the messages to the Maven logger. * @@ -59,6 +63,11 @@ final class DiagnosticLogger implements DiagnosticListener { */ private final Path directory; + /** + * Type of output of the logger. + */ + private final MessageLogType logType; + /** * Number of errors or warnings. */ @@ -77,23 +86,30 @@ final class DiagnosticLogger implements DiagnosticListener { /** * Creates a listener which will send the diagnostics to the given logger. * - * @param logger the logger where to send diagnostics + * @param logger the logger where to send diagnostics * @param messageBuilderFactory the factory for creating message builders - * @param locale the locale for compiler message - * @param directory the base directory with which to relativize the paths to source files + * @param locale the locale for compiler message + * @param directory the base directory with which to relativize the paths to source files + * @param logType output flavor */ - DiagnosticLogger(Log logger, MessageBuilderFactory messageBuilderFactory, Locale locale, Path directory) { + DiagnosticLogger( + Log logger, + MessageBuilderFactory messageBuilderFactory, + Locale locale, + Path directory, + MessageLogType logType) { this.logger = logger; this.messageBuilderFactory = messageBuilderFactory; this.locale = locale; this.directory = directory; + this.logType = logType; codeCount = new LinkedHashMap<>(); } /** * Makes the given file relative to the base directory. * - * @param file the path to make relative to the base directory + * @param file the path to make relative to the base directory * @return the given path, potentially relative to the base directory */ private String relativize(String file) { @@ -118,63 +134,67 @@ public void report(Diagnostic diagnostic) { if (message == null || message.isBlank()) { return; } - MessageBuilder record = messageBuilderFactory.builder(); - record.a(message); - JavaFileObject source = diagnostic.getSource(); - Diagnostic.Kind kind = diagnostic.getKind(); - String style; - switch (kind) { - case ERROR: - style = ".error:-bold,f:red"; - break; - case MANDATORY_WARNING: - case WARNING: - style = ".warning:-bold,f:yellow"; - break; - default: - style = ".info:-bold,f:blue"; - if (diagnostic.getLineNumber() == Diagnostic.NOPOS) { - source = null; // Some messages are generic, e.g. "Recompile with -Xlint:deprecation". - } - break; - } - if (source != null) { - record.newline().a(" at ").a(relativize(source.getName())); - long line = diagnostic.getLineNumber(); - long column = diagnostic.getColumnNumber(); - if (line != Diagnostic.NOPOS || column != Diagnostic.NOPOS) { - record.style(style).a('['); - if (line != Diagnostic.NOPOS) { - record.a(line); - } - if (column != Diagnostic.NOPOS) { - record.a(',').a(column); - } - record.a(']').resetStyle(); + if (logType == MessageLogType.COMPILER || logType == MessageLogType.ALL) { + MessageBuilder record = messageBuilderFactory.builder(); + record.a(message); + JavaFileObject source = diagnostic.getSource(); + Diagnostic.Kind kind = diagnostic.getKind(); + String style; + switch (kind) { + case ERROR: + style = ".error:-bold,f:red"; + break; + case MANDATORY_WARNING: + case WARNING: + style = ".warning:-bold,f:yellow"; + break; + default: + style = ".info:-bold,f:blue"; + if (diagnostic.getLineNumber() == Diagnostic.NOPOS) { + source = null; // Some messages are generic, e.g. "Recompile with -Xlint:deprecation". + } + break; } - } - String log = record.toString(); - switch (kind) { - case ERROR: - if (firstError == null) { - firstError = message; + if (source != null) { + record.newline().a(" at ").a(relativize(source.getName())); + long line = diagnostic.getLineNumber(); + long column = diagnostic.getColumnNumber(); + if (line != Diagnostic.NOPOS || column != Diagnostic.NOPOS) { + record.style(style).a('['); + if (line != Diagnostic.NOPOS) { + record.a(line); + } + if (column != Diagnostic.NOPOS) { + record.a(',').a(column); + } + record.a(']').resetStyle(); } - logger.error(log); - numErrors++; - break; - case MANDATORY_WARNING: - case WARNING: - logger.warn(log); - numWarnings++; - break; - default: - logger.info(log); - break; + } + String log = record.toString(); + switch (kind) { + case ERROR: + if (firstError == null) { + firstError = message; + } + logger.error(log); + numErrors++; + break; + case MANDATORY_WARNING: + case WARNING: + logger.warn(log); + numWarnings++; + break; + default: + logger.info(log); + break; + } } - // Statistics - String code = diagnostic.getCode(); - if (code != null) { - codeCount.merge(code, 1, (old, initial) -> old + 1); + if (logType == MessageLogType.SUMMARY || logType == MessageLogType.ALL) { + // Statistics + String code = diagnostic.getCode(); + if (code != null) { + codeCount.merge(code, 1, (old, initial) -> old + 1); + } } } @@ -191,50 +211,93 @@ Optional firstError(Throwable cause) { * Reports summary after the compilation finished. */ void logSummary() { - MessageBuilder message = messageBuilderFactory.builder(); - final String patternForCount; if (!codeCount.isEmpty()) { - @SuppressWarnings("unchecked") - Map.Entry[] entries = codeCount.entrySet().toArray(Map.Entry[]::new); - Arrays.sort(entries, (a, b) -> Integer.compare(b.getValue(), a.getValue())); - patternForCount = patternForCount(Math.max(entries[0].getValue(), Math.max(numWarnings, numErrors))); - message.strong("Summary of compiler messages:").newline(); - for (Map.Entry entry : entries) { + final var bundle = tryGetCompilerBundle(); + for (final var entry : codeCount.entrySet().stream() + // sort occurrence then key to have an absolute ordering + .sorted(Map.Entry.comparingByValue() + .reversed() + .thenComparing(Map.Entry.comparingByKey())) + .toList()) { int count = entry.getValue(); - message.format(patternForCount, count, entry.getKey()).newline(); + String key = entry.getKey(); + if (bundle != null) { + try { + // not great but the code is worse when you read it + key = key + " (" + bundle.getString(key) + ")"; + } catch (final RuntimeException re) { + // ignore, use the plain key + } + } + Consumer log; + if (entry.getKey().startsWith("compiler.")) { + final var sub = entry.getKey().substring("compiler.".length()); + if (sub.startsWith("err.")) { + log = logger::error; + } else if (sub.startsWith("warn.")) { + log = logger::warn; + } else { + log = logger::info; + } + } else { + log = logger::info; + } + log.accept(messageBuilderFactory + .builder() + .strong(key + ": ") + .append(String.valueOf(count)) + .build()); } - } else { - patternForCount = patternForCount(Math.max(numWarnings, numErrors)); } + if ((numWarnings | numErrors) != 0) { + MessageBuilder message = messageBuilderFactory.builder(); message.strong("Total:"); + if (numWarnings != 0) { + message.append(' ').append(String.valueOf(numWarnings)).append(" warning"); + if (numWarnings > 1) { + message.append('s'); + } + } + if (numErrors != 0) { + message.append(' ').append(String.valueOf(numErrors)).append(" error"); + if (numErrors > 1) { + message.append('s'); + } + } + logger.info(message.build()); } - if (numWarnings != 0) { - writeCount(message, patternForCount, numWarnings, "warning"); - } - if (numErrors != 0) { - writeCount(message, patternForCount, numErrors, "error"); - } - logger.info(message.toString()); - } - - /** - * {@return the pattern for formatting the specified number followed by a label} - * The given number should be the widest number to format. - * A margin of 4 spaces is added at the beginning of the line. - */ - private static String patternForCount(int n) { - return " %" + Integer.toString(n).length() + "d %s"; } - /** - * Appends the count of warnings or errors, making them plural if needed. - */ - private static void writeCount(MessageBuilder message, String patternForCount, int count, String name) { - message.newline(); - message.format(patternForCount, count, name); - if (count > 1) { - message.append('s'); + // we mainly know the one for javac as of today, this impl is best effort + private ResourceBundle tryGetCompilerBundle() { + // ignore the locale since we do log everything in english, + // use only default bundle to avoid mixed outputs + final var bundleName = "com.sun.tools.javac.resources.compiler"; + try { + final var clazz = ToolProvider.getSystemJavaCompiler().getClass(); + final var resources = bundleName.replace('.', '/') + ".class"; + final var is = clazz.getModule() == null + ? ofNullable(clazz.getClassLoader()) + .orElseGet(ClassLoader::getSystemClassLoader) + .getResourceAsStream(resources) + : clazz.getModule().getResourceAsStream(resources); + if (is == null) { + return null; + } + try (is) { + final var bytes = is.readAllBytes(); + final var rbClass = new ClassLoader() { + { + super.defineClass(bundleName, bytes, 0, bytes.length); + } + }.loadClass(bundleName); + final var cons = rbClass.getConstructor(); + cons.setAccessible(true); + return (ResourceBundle) cons.newInstance(); + } + } catch (final Exception e) { + return null; } } } diff --git a/src/main/java/org/apache/maven/plugin/compiler/MessageLogType.java b/src/main/java/org/apache/maven/plugin/compiler/MessageLogType.java new file mode 100644 index 000000000..a01e14192 --- /dev/null +++ b/src/main/java/org/apache/maven/plugin/compiler/MessageLogType.java @@ -0,0 +1,34 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.plugin.compiler; + +public enum MessageLogType { + /** + * SUMMARY+COMPILER + */ + ALL, + /** + * 1-1 between output and compiler messages. + */ + COMPILER, + /** + * Only the summary. + */ + SUMMARY, +} diff --git a/src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java b/src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java index 907d32390..62bd1bc05 100644 --- a/src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java +++ b/src/main/java/org/apache/maven/plugin/compiler/ToolExecutor.java @@ -43,7 +43,6 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; -import java.util.Locale; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -55,6 +54,8 @@ import org.apache.maven.api.services.DependencyResolverResult; import org.apache.maven.api.services.MavenException; +import static java.util.Objects.requireNonNull; + /** * A task which configures and executes a Java tool such as the Java compiler. * This class takes a snapshot of the information provided in the MOJO. @@ -72,11 +73,6 @@ * @author Martin Desruisseaux */ public class ToolExecutor { - /** - * The locale for diagnostics, or {@code null} for the platform default. - */ - private static final Locale LOCALE = null; - /** * The character encoding of source files, or {@code null} for the platform default encoding. * @@ -222,12 +218,8 @@ public class ToolExecutor { protected ToolExecutor(final AbstractCompilerMojo mojo, DiagnosticListener listener) throws IOException { + this.listener = requireNonNull(listener, "DiagnosticListener can't be null in ToolExecutor"); logger = mojo.logger; - if (listener == null) { - Path root = mojo.project.getRootDirectory(); - listener = new DiagnosticLogger(logger, mojo.messageBuilderFactory, LOCALE, root); - } - this.listener = listener; encoding = mojo.charset(); incrementalBuildConfig = mojo.incrementalCompilationConfiguration(); outputDirectory = Files.createDirectories(mojo.getOutputDirectory()); @@ -937,7 +929,7 @@ public boolean compile(JavaCompiler compiler, final Options configuration, final compiler = new WorkaroundForPatchModule(compiler); } boolean success = true; - try (StandardJavaFileManager fileManager = compiler.getStandardFileManager(listener, LOCALE, encoding)) { + try (StandardJavaFileManager fileManager = compiler.getStandardFileManager(listener, null, encoding)) { setDependencyPaths(fileManager); if (!generatedSourceDirectories.isEmpty()) { fileManager.setLocationFromPaths(StandardLocation.SOURCE_OUTPUT, generatedSourceDirectories); diff --git a/src/test/java/org/apache/maven/plugin/compiler/DiagnosticLoggerTest.java b/src/test/java/org/apache/maven/plugin/compiler/DiagnosticLoggerTest.java new file mode 100644 index 000000000..2d3b7e6cf --- /dev/null +++ b/src/test/java/org/apache/maven/plugin/compiler/DiagnosticLoggerTest.java @@ -0,0 +1,131 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.plugin.compiler; + +import javax.tools.Diagnostic; +import javax.tools.JavaFileObject; + +import java.nio.file.Path; +import java.util.Locale; + +import org.apache.maven.impl.DefaultMessageBuilderFactory; +import org.apache.maven.internal.impl.DefaultLog; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.slf4j.helpers.NOPLogger; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class DiagnosticLoggerTest { + @Test + void summary(@TempDir final Path tmp) { + final var builder = new StringBuilder(); + final DiagnosticLogger logger = new DiagnosticLogger( + new DefaultLog(NOPLogger.NOP_LOGGER) { + @Override + public boolean isInfoEnabled() { + return true; + } + + @Override + public boolean isWarnEnabled() { + return true; + } + + @Override + public void info(final CharSequence content) { + builder.append("[INFO] ").append(content).append('\n'); + } + + @Override + public void warn(final CharSequence content) { + builder.append("[WARNING] ").append(content).append('\n'); + } + }, + new DefaultMessageBuilderFactory(), + Locale.ROOT, + tmp, + MessageLogType.SUMMARY); + logger.report(new SimpleDiagnostic(Diagnostic.Kind.NOTE, "some note", "compiler.note.deprecated.filename")); + logger.report(new SimpleDiagnostic(Diagnostic.Kind.WARNING, "some warning", "compiler.warn.invalid.path")); + logger.logSummary(); + + assertEquals(""" + [INFO] compiler.note.deprecated.filename ({0} uses or overrides a deprecated API.): 1 + [WARNING] compiler.warn.invalid.path (Invalid filename: {0}): 1 + """, builder.toString()); + } + + private static class SimpleDiagnostic implements Diagnostic { + private final Kind kind; + private final String message; + private final String code; + + private SimpleDiagnostic(final Kind kind, final String message, final String code) { + this.kind = kind; + this.message = message; + this.code = code; + } + + @Override + public Kind getKind() { + return kind; + } + + @Override + public JavaFileObject getSource() { + return null; + } + + @Override + public long getPosition() { + return 0; + } + + @Override + public long getStartPosition() { + return 0; + } + + @Override + public long getEndPosition() { + return 0; + } + + @Override + public long getLineNumber() { + return 0; + } + + @Override + public long getColumnNumber() { + return 0; + } + + @Override + public String getCode() { + return code; + } + + @Override + public String getMessage(final Locale locale) { + return message; + } + } +}