gnodet commented on code in PR #12698:
URL: https://github.com/apache/maven/pull/12698#discussion_r3743197726
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/CommonsCliOptions.java:
##########
@@ -456,6 +465,13 @@ protected void
prepareOptions(org.apache.commons.cli.Options options) {
+ " interactive TTYs use 'rich' (status bar)."
+ " 'machine' outputs one JSON line per lifecycle
event.")
.get());
+ options.addOption(Option.builder()
+ .longOpt(WARNING_MODE)
+ .hasArg()
Review Comment:
No validation of `--warning-mode` values. An invalid value like
`--warning-mode=quiet` is silently accepted and defaults to `summary` behavior.
`BaseParser.validate()` already validates `--color` and `--fail-on-severity`
against allowed values — `--warning-mode` should follow the same pattern for
consistency.
##########
impl/maven-core/src/main/java/org/apache/maven/internal/build/DefaultDiagnosticCollector.java:
##########
@@ -0,0 +1,221 @@
+/*
+ * 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.internal.build;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.LongAdder;
+
+import org.apache.maven.api.services.BuilderProblem;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * Thread-safe collector for {@link BuilderProblem}s with deduplication
support.
+ * <p>
+ * Problems with a non-null {@link BuilderProblem#getKey()} are deduplicated:
+ * the first occurrence is stored, subsequent duplicates only increment the
+ * counter. Problems without a key are always stored (up to the cap).
+ * <p>
+ * This implementation is safe for use from parallel module builds
+ * ({@code -T}) and from any thread within a plugin execution.
+ *
+ * @since 4.1.0
+ */
+@Named
+@Singleton
+public final class DefaultDiagnosticCollector {
+
+ /**
+ * Maximum number of unique problems to store.
+ * Protects against runaway plugins that produce unbounded problems.
+ */
+ static final int MAX_DIAGNOSTICS = 1000;
+
+ /**
+ * Preserves insertion order: key → first problem.
+ * Using ConcurrentHashMap for thread safety; insertion order is tracked
+ * separately in {@link #orderedKeys}.
+ */
+ private final Map<String, BuilderProblem> uniqueProblems = new
ConcurrentHashMap<>();
+
+ /** Counts per key (including the first occurrence). */
+ private final Map<String, LongAdder> counts = new ConcurrentHashMap<>();
+
+ /**
+ * Insertion-order tracking. Synchronized on itself for ordered access.
+ * The key list mirrors {@link #uniqueProblems} keys in insertion order.
+ */
+ private final List<String> orderedKeys = Collections.synchronizedList(new
ArrayList<>());
+
+ /** Counter for problems without a key, used to generate synthetic keys. */
+ private final LongAdder noKeyCounter = new LongAdder();
+
+ /**
+ * Keys to suppress. Problems with a key in this set are silently dropped.
+ * Configured via {@link #setSuppressedKeys(Set)}, typically from the
+ * {@code maven.diagnostic.suppress} user property.
+ */
+ private volatile Set<String> suppressedKeys = Set.of();
+
+ /**
+ * Sets the keys to suppress. Problems with a matching key will be
+ * silently dropped from {@link #report(BuilderProblem)}.
+ * <p>
+ * Supports both exact keys ({@code "deprecated-source-target"}) and
+ * prefix matching with wildcard ({@code "auto:*"} to suppress all
+ * auto-collected warnings from Maven 3 plugins).
+ *
+ * @param keys the set of keys to suppress; must not be null
+ */
+ public void setSuppressedKeys(Set<String> keys) {
+ this.suppressedKeys = Set.copyOf(requireNonNull(keys, "keys"));
+ }
+
+ /**
+ * Reports a problem.
+ * <p>
+ * If the problem has a non-null {@link BuilderProblem#getKey()} and a
+ * problem with the same key has already been reported, the duplicate is
+ * counted but not stored again.
+ *
+ * @param problem the problem to report
+ */
+ public void report(BuilderProblem problem) {
+ requireNonNull(problem, "problem");
+ String key = problem.getKey();
+
+ // Problems without a key get a synthetic key for storage
+ if (key == null) {
Review Comment:
Race condition: `noKeyCounter.increment()` and `noKeyCounter.longValue()`
are not atomic together. Two concurrent threads reporting null-key problems can
both get the same counter value, producing the same synthetic key
`"__no_key__N"`, and the second problem is silently dropped by `putIfAbsent`.
Fix: use `AtomicLong` instead of `LongAdder` with `incrementAndGet()` for an
atomic unique counter:
```suggestion
key = "__no_key__" + noKeyCounter.incrementAndGet();
```
(also change `noKeyCounter` field type from `LongAdder` to `AtomicLong`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
