[MENFORCER-494] Allow banning dynamic versions in whole tree (#294)

* [MENFORCER-494] Allow banning dynamic versions in whole tree

This commit introduces the possibility of banning dynamic versions
in the entire dependency tree before Maven computes the final
dependency tree.

* Fix format

---------

Co-authored-by: Slawomir Jaranowski <[email protected]>

Author: JimmyAx

enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BanDynamicVersions.java

@@ -22,11 +22,9 @@
 import javax.inject.Named;
 
 import java.text.ChoiceFormat;
-import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Deque;
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Predicate;
@@ -40,9 +38,12 @@
 import org.apache.maven.project.MavenProject;
 import org.eclipse.aether.RepositorySystem;
 import org.eclipse.aether.collection.DependencyCollectionException;
+import org.eclipse.aether.graph.DependencyFilter;
 import org.eclipse.aether.graph.DependencyNode;
-import org.eclipse.aether.graph.DependencyVisitor;
-import org.eclipse.aether.util.graph.visitor.TreeDependencyVisitor;
+import org.eclipse.aether.util.graph.manager.DependencyManagerUtils;
+import org.eclipse.aether.util.graph.visitor.PathRecordingDependencyVisitor;
+import org.eclipse.aether.util.version.GenericVersionScheme;
+import org.eclipse.aether.version.InvalidVersionSpecificationException;
 import org.eclipse.aether.version.VersionConstraint;
 
 /**
@@ -110,6 +111,14 @@ public final class BanDynamicVersions extends AbstractStandardEnforcerRule {
      */
     private List<String> ignores = null;
 
+    /**
+     * {@code true} if dependencies should be checked before Maven computes the final
+     * dependency tree. Setting this property will make the rule check dependencies
+     * before any conflicts are resolved. This is similar to the {@code verbose}
+     * parameter for the {@code tree} goal for {@code maven-dependency-plugin}.
+     */
+    private boolean verbose;
+
     private final ResolverUtil resolverUtil;
 
     @Inject
@@ -118,25 +127,24 @@ public BanDynamicVersions(
         this.resolverUtil = Objects.requireNonNull(resolverUtil);
     }
 
-    private final class BannedDynamicVersionCollector implements DependencyVisitor {
-
-        private final Deque<DependencyNode> nodeStack; // all intermediate nodes (without the root node)
+    private final class BannedDynamicVersionCollector implements DependencyFilter {
 
         private boolean isRoot = true;
 
         private List<String> violations;
 
         private final Predicate<DependencyNode> predicate;
 
+        private GenericVersionScheme versionScheme;
+
         public List<String> getViolations() {
             return violations;
         }
 
         BannedDynamicVersionCollector(Predicate<DependencyNode> predicate) {
-            this.nodeStack = new ArrayDeque<>();
             this.predicate = predicate;
-            this.isRoot = true;
             this.violations = new ArrayList<>();
+            this.versionScheme = new GenericVersionScheme();
         }
 
         private boolean isBannedDynamicVersion(VersionConstraint versionConstraint) {
@@ -163,30 +171,51 @@ private boolean isBannedDynamicVersion(VersionConstraint versionConstraint) {
         }
 
         @Override
-        public boolean visitEnter(DependencyNode node) {
+        public boolean accept(DependencyNode node, List<DependencyNode> parents) {
             if (isRoot) {
                 isRoot = false;
-            } else {
-                getLog().debug("Found node " + node + " with version constraint " + node.getVersionConstraint());
-                if (predicate.test(node) && isBannedDynamicVersion(node.getVersionConstraint())) {
-                    violations.add("Dependency "
-                            + node.getDependency()
-                            + dumpIntermediatePath(nodeStack)
-                            + " is referenced with a banned dynamic version "
-                            + node.getVersionConstraint());
-                    return false;
+                return false;
+            }
+            getLog().debug("Found node " + node + " with version constraint " + node.getVersionConstraint());
+            if (!predicate.test(node)) {
+                return false;
+            }
+            VersionConstraint versionConstraint = node.getVersionConstraint();
+            if (isBannedDynamicVersion(versionConstraint)) {
+                addViolation(versionConstraint, node, parents);
+                return true;
+            }
+            try {
+                if (verbose) {
+                    String premanagedVersion = DependencyManagerUtils.getPremanagedVersion(node);
+                    if (premanagedVersion != null) {
+                        VersionConstraint premanagedContraint = versionScheme.parseVersionConstraint(premanagedVersion);
+                        if (isBannedDynamicVersion(premanagedContraint)) {
+                            addViolation(premanagedContraint, node, parents);
+                            return true;
+                        }
+                    }
                 }
-                nodeStack.addLast(node);
+            } catch (InvalidVersionSpecificationException ex) {
+                // This should never happen.
+                throw new RuntimeException("Failed to parse version for " + node, ex);
             }
-            return true;
+            return false;
         }
 
-        @Override
-        public boolean visitLeave(DependencyNode node) {
-            if (!nodeStack.isEmpty()) {
-                nodeStack.removeLast();
+        private void addViolation(
+                VersionConstraint versionContraint, DependencyNode node, List<DependencyNode> parents) {
+            List<DependencyNode> intermediatePath = new ArrayList<>(parents);
+            if (!intermediatePath.isEmpty()) {
+                // This project is also included in the path, but we do
+                // not want that in the report.
+                intermediatePath.remove(intermediatePath.size() - 1);
             }
-            return true;
+            violations.add("Dependency "
+                    + node.getDependency()
+                    + dumpIntermediatePath(intermediatePath)
+                    + " is referenced with a banned dynamic version "
+                    + versionContraint);
         }
     }
 
@@ -195,7 +224,7 @@ public void execute() throws EnforcerRuleException {
 
         try {
             DependencyNode rootDependency =
-                    resolverUtil.resolveTransitiveDependencies(excludeOptionals, excludedScopes);
+                    resolverUtil.resolveTransitiveDependencies(verbose, excludeOptionals, excludedScopes);
 
             List<String> violations = collectDependenciesWithBannedDynamicVersions(rootDependency);
             if (!violations.isEmpty()) {
@@ -239,23 +268,27 @@ private List<String> collectDependenciesWithBannedDynamicVersions(DependencyNode
         } else {
             predicate = d -> true;
         }
-        BannedDynamicVersionCollector bannedDynamicVersionCollector = new BannedDynamicVersionCollector(predicate);
-        DependencyVisitor depVisitor = new TreeDependencyVisitor(bannedDynamicVersionCollector);
-        rootDependency.accept(depVisitor);
-        return bannedDynamicVersionCollector.getViolations();
+        BannedDynamicVersionCollector collector = new BannedDynamicVersionCollector(predicate);
+        rootDependency.accept(new PathRecordingDependencyVisitor(collector));
+        return collector.getViolations();
+    }
+
+    public void setVerbose(boolean verbose) {
+        this.verbose = verbose;
     }
 
     @Override
     public String toString() {
         return String.format(
-                "BanDynamicVersions[allowSnapshots=%b, allowLatest=%b, allowRelease=%b, allowRanges=%b, allowRangesWithIdenticalBounds=%b, excludeOptionals=%b, excludedScopes=%s, ignores=%s]",
+                "BanDynamicVersions[allowSnapshots=%b, allowLatest=%b, allowRelease=%b, allowRanges=%b, allowRangesWithIdenticalBounds=%b, excludeOptionals=%b, excludedScopes=%s, ignores=%s, verbose=%b]",
                 allowSnapshots,
                 allowLatest,
                 allowRelease,
                 allowRanges,
                 allowRangesWithIdenticalBounds,
                 excludeOptionals,
                 excludedScopes,
-                ignores);
+                ignores,
+                verbose);
     }
 }

enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/ResolverUtil.java

@@ -110,8 +110,8 @@ DependencyNode resolveTransitiveDependencies(boolean excludeOptional, List<Strin
         return resolveTransitiveDependencies(false, excludeOptional, excludedScopes);
     }
 
-    private DependencyNode resolveTransitiveDependencies(
-            boolean verbose, boolean excludeOptional, List<String> excludedScopes) throws EnforcerRuleException {
+    DependencyNode resolveTransitiveDependencies(boolean verbose, boolean excludeOptional, List<String> excludedScopes)
+            throws EnforcerRuleException {
 
         try {
             RepositorySystemSession repositorySystemSession = session.getRepositorySession();

enforcer-rules/src/site/apt/banDynamicVersions.apt.vm

@@ -69,6 +69,8 @@ Ban Dynamic Versions
 
         []
 
+  * <<verbose>> - if <<<true>>> the dependency tree is checked before Maven computes the final dependency tree. Setting this property will make the rule check dependencies before any conflicts are resolved. This is similar to the <<<verbose>>> parameter for the <<<tree>>> goal for <<<maven-dependency-plugin>>>. Default is <<<false>>>.
+  
   []
 
   Sample Plugin Configuration:

maven-enforcer-plugin/src/it/mrm/repository/menforcer494_dependency-1.0-SNAPSHOT.pom

@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  * 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. 
+  *
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>org.apache.maven.plugins.enforcer.its</groupId>
+  <artifactId>menforcer494_dependency</artifactId>
+  <version>1.0-SNAPSHOT</version>
+</project>

maven-enforcer-plugin/src/it/mrm/repository/menforcer494_dependency-2.0.pom

@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  * 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. 
+  *
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>org.apache.maven.plugins.enforcer.its</groupId>
+  <artifactId>menforcer494_dependency</artifactId>
+  <version>2.0</version>
+</project>

maven-enforcer-plugin/src/it/mrm/repository/menforcer494_project-1.0-SNAPSHOT.pom

@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  * 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. 
+  *
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <groupId>org.apache.maven.plugins.enforcer.its</groupId>
+  <artifactId>menforcer494_project</artifactId>
+  <version>1.0-SNAPSHOT</version>
+  
+  <dependencies>
+    <dependency>
+      <groupId>org.apache.maven.plugins.enforcer.its</groupId>
+      <artifactId>menforcer494_dependency</artifactId>
+      <version>1.0-SNAPSHOT</version>
+    </dependency>  
+  </dependencies>
+</project>

maven-enforcer-plugin/src/it/projects/ban-dynamic-versions-verbose/invoker.properties

@@ -0,0 +1,18 @@
+# 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.
+
+invoker.buildResult = failure

maven-enforcer-plugin/src/it/projects/ban-dynamic-versions-verbose/pom.xml

@@ -0,0 +1,72 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+  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.
+-->
+
+<project>
+  <modelVersion>4.0.0</modelVersion>
+
+  <groupId>org.apache.maven.its.enforcer</groupId>
+  <artifactId>ban-dynamic-versions-test</artifactId>
+  <version>1.0</version>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-enforcer-plugin</artifactId>
+        <version>@project.version@</version>
+        <executions>
+          <execution>
+            <id>test</id>
+            <goals>
+              <goal>enforce</goal>
+            </goals>
+            <configuration>
+              <rules>
+                <banDynamicVersions>
+                  <ignores>
+                    <ignore>*:menforcer494_project</ignore>
+                  </ignores>
+                  <verbose>true</verbose>
+                </banDynamicVersions>
+              </rules>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
+
+  <dependencies>
+    <dependency>
+      <!-- banned SNAPSHOT, but ignored (though it's transitive dependency on menforcerxxx_dependency is not) -->
+      <groupId>org.apache.maven.plugins.enforcer.its</groupId>
+      <artifactId>menforcer494_project</artifactId>
+      <version>1.0-SNAPSHOT</version>
+    </dependency>
+    <dependency>
+      <!-- this overrides the version in menforcerxxx_project, but we still want the failure from menforcerxxx_project to show -->
+      <groupId>org.apache.maven.plugins.enforcer.its</groupId>
+      <artifactId>menforcer494_dependency</artifactId>
+      <version>2.0</version>
+    </dependency>
+  </dependencies>
+
+</project>