Structured BuilderProblem pipeline for DiagnosticCollector

[gnodet]

Summary

Follow-up to the build report / warning mode chain. This PR implements the structured BuilderProblem pipeline for the 4 pathways identified in the foundation work, plus a full migration of the PluginValidationManager interface.

Commit 1: Pipe structured BuilderProblems into DiagnosticCollector (Pathways 1–4)

Pathway 2 — Plugin parameter validation (AbstractMavenPluginParametersValidator): 3 validators now create BuilderProblem with structured key, severity, and suggestion, and pipe through PluginValidationManager → DiagnosticCollector.

Pathway 3 — Plugin dependency validation (AbstractMavenPluginDependenciesValidator): 4 validators now create BuilderProblem with structured key and pipe through PluginValidationManager → DiagnosticCollector.

Pathway 4 — Plugin manager Contextualizable check (DefaultMavenPluginManager): creates BuilderProblem with key plugin-validation:contextualizable and pipes through PluginValidationManager → DiagnosticCollector.

Commit 2: Migrate PluginValidationManager to native BuilderProblem API

• Changed PluginValidationManager interface: 3 abstract methods now accept BuilderProblem instead of String
• Added @Deprecated(since = "4.1.0", forRemoval = true) String-based default methods as backward-compat adapters
• Updated all 9 call sites across 8 files to create BuilderProblem natively

PR chain

#	PR	Feature
1	#12694	Logging foundation
2	#12695	Build report
3	#12697	Console modes
4	#12698	Warning mode + diagnostics
5	#12699	mvnlog viewer
6	This PR	Structured problems pipeline
7	#12714	TRACE level migration

Test plan

• mvn test -pl impl/maven-core — tests pass
• CI

🤖 Generated with Claude Code

[gnodet]

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 --log routing.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

[gnodet]

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.

[gnodet]

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.

[gnodet]

Well-structured PR that correctly implements the structured BuilderProblem pipeline. The API migration preserves backward compatibility via deprecated default methods. A few items to address:

Confirmed findings (verified independently):

1. [Medium — Windows parity gap] mvn.cmd:278 — Windows batch script sets MAVEN_MAIN_CLASS for --log but does not strip the routing flag from %* before passing it to the Java process. The Unix mvn script (lines 316-327) explicitly strips --debug, --yjp, --enc, --shell, --up, and --log to prevent Commons CLI prefix-matching collision (e.g. --log matching --log-file). On Windows, running mvn --log validate will pass --log through to the Java process, causing the documented collision.

2. [Low — Code duplication] DefaultMaven.java:673 — The static method toBuilderProblem(ModelProblem) is duplicated character-for-character in DefaultMaven.java and DefaultProjectsSelector.java. Consider extracting to a shared utility class.

3. [Low — Fragile dedup key] DefaultMaven.java:691 — Using message.hashCode() as part of the dedup key ("model:" + problem.getMessage().hashCode()) is fragile since String.hashCode() is a 32-bit hash that can produce collisions. Two different model problems with the same hashCode would be silently deduplicated. Using the full message or a stronger hash would be more robust.

4. [Low — Dead code] BuildReportRenderer.java:45 — MAX_PADDED_BUILD_TIME_DURATION_LENGTH is declared but never referenced. Leftover copy from ExecutionEventLogger.

This review was generated by an AI agent (Claude Code) and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

[Medium — Windows parity gap] The Unix mvn script strips routing flags (--debug, --yjp, --enc, --shell, --up, --log) from $@ before exec (lines 316-327), but the Windows .cmd passes %* unmodified. Running mvn --log validate on Windows will pass --log through to Commons CLI, where it can collide with --log-file via prefix matching.

Windows batch's %* cannot be modified by shift, so equivalent stripping logic would need a for loop to rebuild the argument list.

[gnodet]

[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.