[MNG-8561] SourceRoot should be more lenient wrt duplicates

Add includes/excludes to the SourceKey

Author: gnodet

impl/maven-core/src/main/java/org/apache/maven/project/MavenProject.java

@@ -33,6 +33,7 @@
 import java.util.Objects;
 import java.util.Properties;
 import java.util.Set;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
 
@@ -143,40 +144,10 @@ public class MavenProject implements Cloneable {
 
     private List<MavenProject> collectedProjects;
 
-    /**
-     * A tuple of {@link SourceRoot} properties for which we decide that no duplicated value should exist in a project.
-     * The set of properties that we choose to put in this record may be modified in any future Maven version.
-     * The intent is to detect some configuration errors.
-     */
-    private record SourceKey(ProjectScope scope, Language language, Path directory) {
-        /**
-         * Converts this key into a source root.
-         * Used for adding a new source when no other information is available.
-         *
-         * @return the source root for the properties of this key.
-         */
-        SourceRoot createSource() {
-            return new DefaultSourceRoot(scope, language, directory);
-        }
-
-        /**
-         * {@return an error message to report when a conflict is detected}.
-         *
-         * @param baseDir value of {@link #getBaseDirectory()}, in order to make the message shorter
-         */
-        String conflictMessage(Path baseDir) {
-            return "Directory " + baseDir.relativize(directory)
-                    + " is specified twice for the scope \"" + scope.id()
-                    + "\" and language \"" + language.id() + "\".";
-        }
-    }
-
     /**
      * All sources of this project. The map includes main and test codes for all languages.
-     * However, we put some restrictions on what information can be repeated.
-     * Those restrictions are expressed in {@link SourceKey}.
      */
-    private HashMap<SourceKey, SourceRoot> sources = new LinkedHashMap<>(); // Need access to the `clone()` method.
+    private List<SourceRoot> sources = new CopyOnWriteArrayList<>();
 
     @Deprecated
     private ArtifactRepository releaseArtifactRepository;
@@ -358,11 +329,12 @@ public DependencyManagement getDependencyManagement() {
      * @since 4.0.0
      */
     public void addSourceRoot(SourceRoot source) {
-        var key = new SourceKey(source.scope(), source.language(), source.directory());
-        SourceRoot current = sources.putIfAbsent(key, source);
-        if (current != null && !current.equals(source)) {
-            throw new IllegalArgumentException(key.conflictMessage(getBaseDirectory()));
+        if (sources.contains(source)) {
+            throw new IllegalArgumentException("Directory " + getBaseDirectory().relativize(source.directory())
+                    + " is specified twice for the scope \"" + source.scope().id()
+                    + "\" and language \"" + source.language().id() + "\".");
         }
+        sources.add(source);
     }
 
     /**
@@ -381,8 +353,7 @@ public void addSourceRoot(SourceRoot source) {
      */
     public void addSourceRoot(ProjectScope scope, Language language, Path directory) {
         directory = getBaseDirectory().resolve(directory).normalize();
-        var key = new SourceKey(scope, language, directory);
-        sources.computeIfAbsent(key, SourceKey::createSource);
+        addSourceRoot(new DefaultSourceRoot(scope, language, directory));
     }
 
     /**
@@ -402,8 +373,7 @@ public void addSourceRoot(ProjectScope scope, Language language, String director
             directory = directory.trim();
             if (!directory.isBlank()) {
                 Path path = getBaseDirectory().resolve(directory).normalize();
-                var key = new SourceKey(scope, language, path);
-                sources.computeIfAbsent(key, SourceKey::createSource);
+                addSourceRoot(scope, language, path);
             }
         }
     }
@@ -432,7 +402,7 @@ public void addTestCompileSourceRoot(String path) {
      * @see #addSourceRoot(SourceRoot)
      */
     public Collection<SourceRoot> getSourceRoots() {
-        return Collections.unmodifiableCollection(sources.values());
+        return Collections.unmodifiableCollection(sources);
     }
 
     /**
@@ -449,7 +419,7 @@ public Collection<SourceRoot> getSourceRoots() {
      * @since 4.0.0
      */
     public Stream<SourceRoot> getEnabledSourceRoots(ProjectScope scope, Language language) {
-        Stream<SourceRoot> s = sources.values().stream().filter(SourceRoot::enabled);
+        Stream<SourceRoot> s = sources.stream().filter(SourceRoot::enabled);
         if (scope != null) {
             s = s.filter((source) -> scope.equals(source.scope()));
         }
@@ -1249,7 +1219,7 @@ protected void setAttachedArtifacts(List<Artifact> attachedArtifacts) {
      */
     @Deprecated
     private void setSourceRootDirs(ProjectScope scope, Language language, List<String> roots) {
-        sources.values().removeIf((source) -> scope.equals(source.scope()) && language.equals(source.language()));
+        sources.removeIf((source) -> scope.equals(source.scope()) && language.equals(source.language()));
         Path directory = getBaseDirectory();
         for (String root : roots) {
             addSourceRoot(new DefaultSourceRoot(scope, language, directory.resolve(root)));
@@ -1335,7 +1305,7 @@ private void deepCopy(MavenProject project) {
         // This property is not handled like others as we don't use public API.
         // The whole implementation of this `deepCopy` method may need revision,
         // but it would be the topic for a separated commit.
-        sources = (HashMap<SourceKey, SourceRoot>) project.sources.clone();
+        sources = new CopyOnWriteArrayList<>(project.sources);
 
         if (project.getModel() != null) {
             setModel(project.getModel().clone());

impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultSourceRoot.java

@@ -117,15 +117,15 @@ public DefaultSourceRoot(final Path baseDir, ProjectScope scope, Resource resour
     }
 
     /**
-     * Creates a new instance for the given directory and scope. The language is assumed Java.
+     * Creates a new instance for the given directory and scope.
      *
      * @param scope scope of source code (main or test)
      * @param language language of the source code
      * @param directory directory of the source code
      */
     public DefaultSourceRoot(final ProjectScope scope, final Language language, final Path directory) {
         this.scope = Objects.requireNonNull(scope);
-        this.language = language;
+        this.language = Objects.requireNonNull(language);
         this.directory = Objects.requireNonNull(directory);
         includes = List.of();
         excludes = List.of();
@@ -136,6 +136,31 @@ public DefaultSourceRoot(final ProjectScope scope, final Language language, fina
         enabled = true;
     }
 
+    /**
+     * Creates a new instance for the given directory and scope.
+     *
+     * @param scope scope of source code (main or test)
+     * @param language language of the source code
+     * @param directory directory of the source code
+     */
+    public DefaultSourceRoot(
+            final ProjectScope scope,
+            final Language language,
+            final Path directory,
+            List<PathMatcher> includes,
+            List<PathMatcher> excludes) {
+        this.scope = Objects.requireNonNull(scope);
+        this.language = language;
+        this.directory = Objects.requireNonNull(directory);
+        this.includes = includes != null ? List.copyOf(includes) : List.of();
+        this.excludes = excludes != null ? List.copyOf(excludes) : List.of();
+        moduleName = null;
+        targetVersion = null;
+        targetPath = null;
+        stringFiltering = false;
+        enabled = true;
+    }
+
     /**
      * {@return the given value as a trimmed non-blank string, or null otherwise}.
      */
@@ -162,11 +187,21 @@ private static String nonBlank(String value) {
     private static List<PathMatcher> matchers(FileSystem fs, List<String> patterns) {
         final var matchers = new PathMatcher[patterns.size()];
         for (int i = 0; i < matchers.length; i++) {
-            String pattern = patterns.get(i);
-            if (pattern.indexOf(':') < 0) {
-                pattern = "glob:" + pattern;
-            }
-            matchers[i] = fs.getPathMatcher(pattern);
+            String rawPattern = patterns.get(i);
+            String pattern = rawPattern.contains(":") ? rawPattern : "glob:" + rawPattern;
+            matchers[i] = new PathMatcher() {
+                final PathMatcher delegate = fs.getPathMatcher(pattern);
+
+                @Override
+                public boolean matches(Path path) {
+                    return delegate.matches(path);
+                }
+
+                @Override
+                public String toString() {
+                    return pattern;
+                }
+            };
         }
         return List.of(matchers);
     }