Structured BuilderProblem pipeline for DiagnosticCollector - #12702
Structured BuilderProblem pipeline for DiagnosticCollector#12702gnodet wants to merge 2 commits into
Conversation
6124231 to
dc3bccd
Compare
0e0c4e7 to
4463770
Compare
dc3bccd to
a7c83db
Compare
4463770 to
8ef52fb
Compare
a7c83db to
7243fd7
Compare
8ef52fb to
c5cdbba
Compare
7243fd7 to
af945c2
Compare
c5cdbba to
b0d3de4
Compare
af945c2 to
850a04c
Compare
dbf0ac6 to
84d77ae
Compare
- mvnlog / mvnlog --json CLI viewer for build-report JSON files - BuildReportRenderer with ANSI colors, timing, failure details - SimpleJsonReader dependency-free streaming JSON parser - mvn --log routes to MavenLogCling, mvnlog shell scripts - MavenITgh12571BuildReportTest: 8 ITs covering build report, console modes, warning mode, version info on failure, and mvnlog viewer
…nager migration - Pipe structured BuilderProblems into DiagnosticCollector for pathways 2-4: plugin parameter validation, dependency validation, Contextualizable check - Migrate PluginValidationManager interface to native BuilderProblem API - Deprecated String-based methods with backward-compat adapters - Updated all 9 call sites across 8 files
84d77ae to
b6a409b
Compare
850a04c to
5913a2b
Compare
gnodet
left a comment
gnodet
left a comment
There was a problem hiding this comment.
Well-structured PR that cleanly pipes structured BuilderProblems into the DiagnosticCollector. Two observations after verification (a finding about source-incompatible API changes was a false positive — old String-based methods are preserved as deprecated defaults).
Also noted:
- The mvnlog tool addition is complete with comprehensive unit and integration tests.
- The EXCLUDED_LOGGERS set correctly prevents double-counting for classes that now pipe BuilderProblems directly.
- The shell script changes correctly handle
--logrouting.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
| */ | ||
| private static BuilderProblem toBuilderProblem(ModelProblem problem) { | ||
| BuilderProblem.Severity severity = | ||
| switch (problem.getSeverity()) { |
There was a problem hiding this comment.
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.
| @@ -100,4 +112,26 @@ public List<MavenProject> selectProjects(List<File> files, MavenExecutionRequest | |||
|
|
|||
There was a problem hiding this comment.
This toBuilderProblem(ModelProblem) method is an exact duplicate of the one in DefaultMaven.java. If the conversion logic changes, both copies must be updated in lockstep. Consider extracting to a shared utility method.
1 participant
Summary
Follow-up to the build report / warning mode chain. This PR implements the structured
BuilderProblempipeline for the 4 pathways identified in the foundation work, plus a full migration of thePluginValidationManagerinterface.Commit 1: Pipe structured BuilderProblems into DiagnosticCollector (Pathways 1–4)
Pathway 2 — Plugin parameter validation (
AbstractMavenPluginParametersValidator): 3 validators now createBuilderProblemwith structured key, severity, and suggestion, and pipe throughPluginValidationManager→DiagnosticCollector.Pathway 3 — Plugin dependency validation (
AbstractMavenPluginDependenciesValidator): 4 validators now createBuilderProblemwith structured key and pipe throughPluginValidationManager→DiagnosticCollector.Pathway 4 — Plugin manager Contextualizable check (
DefaultMavenPluginManager): createsBuilderProblemwith keyplugin-validation:contextualizableand pipes throughPluginValidationManager→DiagnosticCollector.Commit 2: Migrate PluginValidationManager to native BuilderProblem API
PluginValidationManagerinterface: 3 abstract methods now acceptBuilderProbleminstead ofString@Deprecated(since = "4.1.0", forRemoval = true)String-based default methods as backward-compat adaptersBuilderProblemnativelyPR chain
mvnlogviewerTest plan
mvn test -pl impl/maven-core— tests pass🤖 Generated with Claude Code