Skip to content

Structured BuilderProblem pipeline for DiagnosticCollector #12702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gnodet wants to merge 1 commit into
base: feature/mvnlog
Choose a base branch
from
Open

Structured BuilderProblem pipeline for DiagnosticCollector #12702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
import org.apache.maven.eventspy.internal.EventSpyDispatcher;
import org.apache.maven.execution.MavenExecutionRequest;
import org.apache.maven.impl.SettingsUtilsV4;
import org.apache.maven.internal.build.DefaultDiagnosticCollector;
import org.apache.maven.jline.FastTerminal;
import org.apache.maven.jline.MessageUtils;
import org.apache.maven.logging.BuildEventListener;
Expand Down Expand Up @@ -750,6 +751,13 @@ protected Runnable settings(C context, boolean emitSettingsWarnings, SettingsBui
}
}
context.logger.info("");

// Pipe structured problems directly to DiagnosticCollector so that
// key, suggestion, documentationUrl, and source location are preserved
// in the build report (instead of being lost to plain-text logging).
// This runs before SessionStarted, so the SLF4J auto-collection hook
// is not active yet — no double-counting risk.
pipeSettingsProblems(context, settingsResult);
}
return () -> {
context.installationSettingsPath = null;
Expand All @@ -761,6 +769,20 @@ protected Runnable settings(C context, boolean emitSettingsWarnings, SettingsBui
};
}

/**
* Pipes structured settings validation problems to the DiagnosticCollector.
* This preserves key, suggestion, documentationUrl, and source location
* that would otherwise be lost when problems are logged as plain text.
*/
private void pipeSettingsProblems(C context, SettingsBuilderResult settingsResult) {
context.lookup.lookupOptional(DefaultDiagnosticCollector.class).ifPresent(collector -> {
for (BuilderProblem problem :
settingsResult.getProblems().problems().toList()) {
collector.report(problem);
}
});
}

protected void customizeSettingsRequest(C context, SettingsBuilderRequest settingsBuilderRequest)
throws Exception {}

Expand Down
12 changes: 12 additions & 0 deletions impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import org.apache.maven.execution.MavenExecutionResult;
import org.apache.maven.execution.ProfileActivation;
import org.apache.maven.execution.ProjectActivation;
import org.apache.maven.internal.build.DefaultDiagnosticCollector;
import org.apache.maven.jline.MessageUtils;
import org.apache.maven.lifecycle.LifecycleExecutionException;
import org.apache.maven.logging.BuildEventListener;
Expand Down Expand Up @@ -220,6 +221,17 @@ protected void toolchains(MavenContext context, MavenExecutionRequest request) t
}

context.logger.info("");

// Pipe structured problems directly to DiagnosticCollector so that
// key, suggestion, documentationUrl, and source location are preserved
// in the build report. This runs before SessionStarted, so the SLF4J
// auto-collection hook is not active yet — no double-counting risk.
context.lookup.lookupOptional(DefaultDiagnosticCollector.class).ifPresent(collector -> {
for (BuilderProblem problem :
toolchainsResult.getProblems().problems().toList()) {
collector.report(problem);
}
});
}
}

Expand Down
92 changes: 3 additions & 89 deletions impl/maven-core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,6 @@ under the License.
<name>Maven 4 Core</name>
<description>Maven Core classes.</description>

<properties>
<!-- Default lifecycle plugin versions.
Maintained here so that dependency-update bots (Dependabot, Renovate)
can propose version bumps automatically. Values are filtered into
plugin-versions.properties at build time and loaded at runtime. -->
<version.maven-clean-plugin>3.4.0</version.maven-clean-plugin>
<version.maven-compiler-plugin>3.13.0</version.maven-compiler-plugin>
<version.maven-deploy-plugin>3.1.3</version.maven-deploy-plugin>
<version.maven-ear-plugin>3.3.0</version.maven-ear-plugin>
<version.maven-ejb-plugin>3.2.1</version.maven-ejb-plugin>
<version.maven-install-plugin>3.1.3</version.maven-install-plugin>
<version.maven-jar-plugin>3.4.2</version.maven-jar-plugin>
<version.maven-plugin-plugin>3.15.1</version.maven-plugin-plugin>
<version.maven-rar-plugin>3.0.0</version.maven-rar-plugin>
<version.maven-resources-plugin>3.3.1</version.maven-resources-plugin>
<version.maven-site-plugin>3.21.0</version.maven-site-plugin>
<version.maven-surefire-plugin>3.5.2</version.maven-surefire-plugin>
<version.maven-war-plugin>3.4.0</version.maven-war-plugin>
</properties>

<dependencies>
<!-- Maven4 API -->
<dependency>
Expand Down Expand Up @@ -280,74 +260,6 @@ under the License.
</resources>
<pluginManagement>
<plugins>
<!-- Default lifecycle plugins declared here so that dependency-update bots
(Dependabot, Renovate) see the version.maven-*-plugin properties as
actual plugin version references and can propose automated bumps. -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-clean-plugin</artifactId>
<version>${version.maven-clean-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>${version.maven-compiler-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-deploy-plugin</artifactId>
<version>${version.maven-deploy-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-ear-plugin</artifactId>
<version>${version.maven-ear-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-ejb-plugin</artifactId>
<version>${version.maven-ejb-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-install-plugin</artifactId>
<version>${version.maven-install-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
<version>${version.maven-jar-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-plugin-plugin</artifactId>
<version>${version.maven-plugin-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-rar-plugin</artifactId>
<version>${version.maven-rar-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-resources-plugin</artifactId>
<version>${version.maven-resources-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-site-plugin</artifactId>
<version>${version.maven-site-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>${version.maven-surefire-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-war-plugin</artifactId>
<version>${version.maven-war-plugin}</version>
</plugin>
<plugin>
<groupId>org.apache.rat</groupId>
<artifactId>apache-rat-plugin</artifactId>
Expand Down Expand Up @@ -484,10 +396,12 @@ under the License.
<exclude>org.apache.maven.toolchain.ToolchainManagerPrivate</exclude>
<exclude>org.apache.maven.toolchain.ToolchainPrivate</exclude>
<exclude>org.apache.maven.toolchain.ToolchainsBuilder</exclude>
<!-- PluginValidationManager: String issue → BuilderProblem problem (4.1.0) -->
<exclude>org.apache.maven.plugin.PluginValidationManager</exclude>
</excludes>
</parameter>
</configuration>
</plugin>
</plugins>
</build>
</project>
</project>
60 changes: 45 additions & 15 deletions impl/maven-core/src/main/java/org/apache/maven/DefaultMaven.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.apache.maven.api.model.Model;
import org.apache.maven.api.model.Prerequisites;
import org.apache.maven.api.model.Profile;
import org.apache.maven.api.services.BuilderProblem;
import org.apache.maven.api.services.Lookup;
import org.apache.maven.api.services.LookupException;
import org.apache.maven.artifact.ArtifactUtils;
Expand All @@ -59,6 +60,7 @@
import org.apache.maven.execution.ProjectDependencyGraph;
import org.apache.maven.graph.GraphBuilder;
import org.apache.maven.graph.ProjectSelector;
import org.apache.maven.internal.build.DefaultDiagnosticCollector;
import org.apache.maven.internal.impl.DefaultSessionFactory;
import org.apache.maven.internal.impl.InternalMavenSession;
import org.apache.maven.lifecycle.LifecycleExecutionException;
Expand Down Expand Up @@ -113,6 +115,8 @@ public class DefaultMaven implements Maven {

private final ProjectSelector projectSelector;

private final DefaultDiagnosticCollector diagnosticCollector;

@Inject
@SuppressWarnings("checkstyle:ParameterNumber")
public DefaultMaven(
Expand All @@ -126,7 +130,8 @@ public DefaultMaven(
BuildResumptionDataRepository buildResumptionDataRepository,
SuperPomProvider superPomProvider,
DefaultSessionFactory defaultSessionFactory,
@Nullable @Named("ide") WorkspaceReader ideWorkspaceReader) {
@Nullable @Named("ide") WorkspaceReader ideWorkspaceReader,
DefaultDiagnosticCollector diagnosticCollector) {
this.lookup = lookup;
this.eventCatapult = eventCatapult;
this.legacySupport = legacySupport;
Expand All @@ -138,6 +143,7 @@ public DefaultMaven(
this.superPomProvider = superPomProvider;
this.ideWorkspaceReader = ideWorkspaceReader;
this.defaultSessionFactory = defaultSessionFactory;
this.diagnosticCollector = diagnosticCollector;
this.projectSelector = new ProjectSelector(); // if necessary switch to DI
}

Expand Down Expand Up @@ -210,22 +216,20 @@ private MavenExecutionResult doExecute(MavenExecutionRequest request) {
// so that @SessionScoped components can be @Injected into AbstractLifecycleParticipants.
//
sessionScope.enter();
try {
MavenChainedWorkspaceReader chainedWorkspaceReader =
new MavenChainedWorkspaceReader(request.getWorkspaceReader(), ideWorkspaceReader);
try (CloseableSession closeableSession = newCloseableSession(request, chainedWorkspaceReader)) {
MavenSession session = new MavenSession(closeableSession, request, result);
session.setSession(defaultSessionFactory.newSession(session));
MavenChainedWorkspaceReader chainedWorkspaceReader =
new MavenChainedWorkspaceReader(request.getWorkspaceReader(), ideWorkspaceReader);
try (CloseableSession closeableSession = newCloseableSession(request, chainedWorkspaceReader)) {
MavenSession session = new MavenSession(closeableSession, request, result);
session.setSession(defaultSessionFactory.newSession(session));

sessionScope.seed(MavenSession.class, session);
sessionScope.seed(RepositorySystemSession.class, closeableSession); // fixed in Maven 3.10.x
sessionScope.seed(Session.class, session.getSession());
sessionScope.seed(InternalMavenSession.class, InternalMavenSession.from(session.getSession()));
sessionScope.seed(MavenSession.class, session);
sessionScope.seed(RepositorySystemSession.class, closeableSession); // fixed in Maven 3.10.x
sessionScope.seed(Session.class, session.getSession());
sessionScope.seed(InternalMavenSession.class, InternalMavenSession.from(session.getSession()));

legacySupport.setSession(session);
legacySupport.setSession(session);

return doExecute(request, session, result, chainedWorkspaceReader);
}
return doExecute(request, session, result, chainedWorkspaceReader);
} finally {
sessionScope.exit();
}
Expand Down Expand Up @@ -650,6 +654,10 @@ private Result<? extends ProjectDependencyGraph> buildGraph(MavenSession session
} else {
logger.error(problem.getMessage());
}
// Pipe structured problem directly to DiagnosticCollector so that
// source location and severity are preserved in the build report.
// The SLF4J hook excludes this logger to avoid double-counting.
diagnosticCollector.report(toBuilderProblem(problem));
}

if (!graphResult.hasErrors()) {
Expand All @@ -662,9 +670,31 @@ private Result<? extends ProjectDependencyGraph> buildGraph(MavenSession session
return graphResult;
}

/**
* Converts a compat {@link ModelProblem} to the Maven 4 {@link BuilderProblem} API,
* preserving source, line, column, severity, and message.
*/
private static BuilderProblem toBuilderProblem(ModelProblem problem) {
BuilderProblem.Severity severity =
switch (problem.getSeverity()) {

@gnodet gnodet Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diagnostic key is generated as "model:" + problem.getMessage().hashCode(). Using String.hashCode() for deduplication keys is fragile — hash collisions would cause unrelated problems to be silently deduplicated. The same pattern appears in DefaultProjectsSelector and the deprecated adapters in PluginValidationManager.

Elsewhere in this PR, human-readable keys are used (e.g. "plugin-validation:contextualizable", "plugin-validation:maven2-plugin"). Consider using a more collision-resistant approach here too — e.g., incorporating the problem source and a truncated/normalized message.

case FATAL -> BuilderProblem.Severity.FATAL;
case ERROR -> BuilderProblem.Severity.ERROR;
default -> BuilderProblem.Severity.WARNING;
};
return BuilderProblem.builder()
.source(problem.getSource())
.lineNumber(problem.getLineNumber())
.columnNumber(problem.getColumnNumber())
.exception(problem.getException())
.message(problem.getMessage())
.severity(severity)
.key("model:" + problem.getMessage().hashCode())

@gnodet gnodet Aug 16, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Low — Fragile dedup key] String.hashCode() is a 32-bit hash that can produce collisions (e.g., "Aa" and "BB" both hash to 2112). If two different model problems collide, one gets silently deduplicated. Consider using the full message string or a stronger hash for the key.

.build();
}

@Deprecated
// 5 January 2014
protected Logger getLogger() {
return logger;
}
}
}
19 changes: 17 additions & 2 deletions impl/maven-core/src/main/java/org/apache/maven/internal/build/BuildReportCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,19 @@ public final class BuildReportCollector extends AbstractEventSpy {

private static final int MAX_STACKTRACE_LINES = 30;

/**
* Logger names excluded from SLF4J auto-collection because these classes
* already pipe structured {@link org.apache.maven.api.services.BuilderProblem}
* objects directly to the {@link DefaultDiagnosticCollector}. Without this
* exclusion, each problem would be counted twice: once from the direct pipe
* and once from the SLF4J WARN interception.
*/
private static final Set<String> EXCLUDED_LOGGERS = Set.of(
BuildReportCollector.class.getName(),
"org.apache.maven.DefaultMaven",
"org.apache.maven.project.collector.DefaultProjectsSelector",
"org.apache.maven.plugin.internal.DefaultPluginValidationManager");

private final DefaultDiagnosticCollector diagnosticCollector;

@Inject
Expand Down Expand Up @@ -353,10 +366,12 @@ private void captureLogEvent(LogEvent event) {

// Auto-collect WARN-level log events as build problems, giving Maven 3 plugins
// automatic deduplication and summary at end of build without code changes.
// Skip our own logger to avoid feedback loops from problem summary printing.
// Skip loggers that already pipe structured BuilderProblems directly to the
// DiagnosticCollector (avoiding double-counting), and our own logger to avoid
// feedback loops from problem summary printing.
if (event.level() == LogLevel.WARN
&& event.message() != null
&& !event.loggerName().equals(BuildReportCollector.class.getName())) {
&& !EXCLUDED_LOGGERS.contains(event.loggerName())) {
String syntheticKey = syntheticDiagnosticKey(event.loggerName(), event.message());
diagnosticCollector.report(BuilderProblem.builder()
.source(event.loggerName())
Expand Down
Loading