Copilot commented on code in PR #2217: URL: https://github.com/apache/maven-resolver/pull/2217#discussion_r4329741826
########## maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/PrefixTree.java: ########## @@ -42,13 +43,27 @@ * <li><a href="https://repo.eclipse.org/content/repositories/tycho/.meta/prefixes.txt";>Eclipse Tycho</a></li> * </ul> */ -public class PrefixTree extends Node<PrefixTree> { +public class PrefixTree { private static List<String> elementsOfPath(final String path) { return Arrays.stream(path.split("/")).filter(e -> !e.isEmpty()).collect(toList()); } + private final String name; + private final HashMap<String, PrefixTree> children; + private final boolean leaf; Review Comment: `PrefixTree` now uses `HashMap` for `children`, whereas the prior shared `Node` implementation used `ConcurrentHashMap`. If `PrefixTree` is ever accessed concurrently during/after construction (e.g., load + reads, or publication without strict happens-before), this can introduce data races or visibility issues. Consider using `ConcurrentHashMap` here as well, or make the structure immutable after `loadNodes()` (e.g., build with mutable maps, then freeze) and ensure safe publication. ########## maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/filter/ruletree/PrefixTree.java: ########## @@ -76,12 +117,24 @@ public boolean acceptedPath(String path) { final List<String> pathElements = elementsOfPath(path); PrefixTree currentNode = this; for (String pathElement : pathElements) { - currentNode = currentNode.siblings.get(pathElement); + currentNode = currentNode.children.get(pathElement); if (currentNode == null || currentNode.isLeaf()) { break; } } // path is accepted if its elements lead to an existing node that is a leaf node return currentNode != null && currentNode.isLeaf(); } + + @Override + public String toString() { + return name + (leaf ? "*" : ""); + } + + public void dump(String prefix) { + System.out.println(prefix + this); + for (PrefixTree node : children.values()) { + node.dump(prefix + " "); + } + } Review Comment: This introduces (and exposes publicly) a `dump()` method that writes directly to stdout from production code. Direct `System.out.println` can cause noisy output in embedded/library usage and is hard to control operationally. Prefer either removing `dump()` entirely, making it package-private for debugging only, or routing output through the project’s logging framework at an appropriate level. -- 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]
