[MENFORCER-467] banDynamicVersions excludedScopes on project level (#262)

slawekjaranowski · kwin · web-flow · commit 5895ff288ec2 · 2023-03-27T22:36:41.000+02:00
Co-authored-by: Konrad Windszus &lt;kwin@apache.org&gt;
diff --git a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BanDynamicVersions.java b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BanDynamicVersions.java
@@ -16,7 +16,7 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-package org.apache.maven.enforcer.rules;
+package org.apache.maven.enforcer.rules.dependency;
 
 import javax.inject.Inject;
 import javax.inject.Named;
@@ -32,25 +32,16 @@
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
-import org.apache.maven.RepositoryUtils;
 import org.apache.maven.enforcer.rule.api.EnforcerRuleException;
+import org.apache.maven.enforcer.rules.AbstractStandardEnforcerRule;
 import org.apache.maven.enforcer.rules.utils.ArtifactMatcher;
 import org.apache.maven.enforcer.rules.utils.ArtifactUtils;
 import org.apache.maven.execution.MavenSession;
 import org.apache.maven.project.MavenProject;
-import org.eclipse.aether.DefaultRepositorySystemSession;
 import org.eclipse.aether.RepositorySystem;
-import org.eclipse.aether.RepositorySystemSession;
-import org.eclipse.aether.collection.CollectRequest;
-import org.eclipse.aether.collection.CollectResult;
 import org.eclipse.aether.collection.DependencyCollectionException;
-import org.eclipse.aether.collection.DependencySelector;
-import org.eclipse.aether.graph.Dependency;
 import org.eclipse.aether.graph.DependencyNode;
 import org.eclipse.aether.graph.DependencyVisitor;
-import org.eclipse.aether.util.graph.selector.AndDependencySelector;
-import org.eclipse.aether.util.graph.selector.OptionalDependencySelector;
-import org.eclipse.aether.util.graph.selector.ScopeDependencySelector;
 import org.eclipse.aether.util.graph.visitor.TreeDependencyVisitor;
 import org.eclipse.aether.version.VersionConstraint;
 
@@ -108,7 +99,7 @@ public final class BanDynamicVersions extends AbstractStandardEnforcerRule {
     /**
      * the scopes of dependencies which should be excluded from this rule
      */
-    private String[] excludedScopes;
+    private List<String> excludedScopes;
 
     /**
      * Specify the ignored dependencies. This can be a list of artifacts in the format
@@ -119,17 +110,12 @@ public final class BanDynamicVersions extends AbstractStandardEnforcerRule {
      */
     private List<String> ignores = null;
 
-    private final MavenProject project;
-
-    private final RepositorySystem repoSystem;
-
-    private final MavenSession mavenSession;
+    private final ResolverUtil resolverUtil;
 
     @Inject
-    public BanDynamicVersions(MavenProject project, RepositorySystem repoSystem, MavenSession mavenSession) {
-        this.project = Objects.requireNonNull(project);
-        this.repoSystem = Objects.requireNonNull(repoSystem);
-        this.mavenSession = Objects.requireNonNull(mavenSession);
+    public BanDynamicVersions(
+            MavenProject project, RepositorySystem repoSystem, MavenSession mavenSession, ResolverUtil resolverUtil) {
+        this.resolverUtil = Objects.requireNonNull(resolverUtil);
     }
 
     private final class BannedDynamicVersionCollector implements DependencyVisitor {
@@ -138,19 +124,19 @@ private final class BannedDynamicVersionCollector implements DependencyVisitor {
 
         private boolean isRoot = true;
 
-        private int numViolations;
+        private List<String> violations;
 
         private final Predicate<DependencyNode> predicate;
 
-        public int getNumViolations() {
-            return numViolations;
+        public List<String> getViolations() {
+            return violations;
         }
 
         BannedDynamicVersionCollector(Predicate<DependencyNode> predicate) {
-            nodeStack = new ArrayDeque<>();
+            this.nodeStack = new ArrayDeque<>();
             this.predicate = predicate;
             this.isRoot = true;
-            numViolations = 0;
+            this.violations = new ArrayList<>();
         }
 
         private boolean isBannedDynamicVersion(VersionConstraint versionConstraint) {
@@ -183,13 +169,11 @@ public boolean visitEnter(DependencyNode node) {
             } else {
                 getLog().debug("Found node " + node + " with version constraint " + node.getVersionConstraint());
                 if (predicate.test(node) && isBannedDynamicVersion(node.getVersionConstraint())) {
-                    getLog().warnOrError(() -> new StringBuilder()
-                            .append("Dependency ")
-                            .append(node.getDependency())
-                            .append(dumpIntermediatePath(nodeStack))
-                            .append(" is referenced with a banned dynamic version ")
-                            .append(node.getVersionConstraint()));
-                    numViolations++;
+                    violations.add("Dependency "
+                            + node.getDependency()
+                            + dumpIntermediatePath(nodeStack)
+                            + " is referenced with a banned dynamic version "
+                            + node.getVersionConstraint());
                     return false;
                 }
                 nodeStack.addLast(node);
@@ -209,26 +193,17 @@ public boolean visitLeave(DependencyNode node) {
     @Override
     public void execute() throws EnforcerRuleException {
 
-        // get a new session to be able to tweak the dependency selector
-        DefaultRepositorySystemSession newRepoSession =
-                new DefaultRepositorySystemSession(mavenSession.getRepositorySession());
-
-        Collection<DependencySelector> depSelectors = new ArrayList<>();
-        depSelectors.add(new ScopeDependencySelector(excludedScopes));
-        if (excludeOptionals) {
-            depSelectors.add(new OptionalDependencySelector());
-        }
-        newRepoSession.setDependencySelector(new AndDependencySelector(depSelectors));
-
-        Dependency rootDependency = RepositoryUtils.toDependency(project.getArtifact(), null);
         try {
-            // use root dependency with unresolved direct dependencies
-            int numViolations = emitDependenciesWithBannedDynamicVersions(rootDependency, newRepoSession);
-            if (numViolations > 0) {
+            DependencyNode rootDependency =
+                    resolverUtil.resolveTransitiveDependencies(excludeOptionals, excludedScopes);
+
+            List<String> violations = collectDependenciesWithBannedDynamicVersions(rootDependency);
+            if (!violations.isEmpty()) {
                 ChoiceFormat dependenciesFormat = new ChoiceFormat("1#dependency|1<dependencies");
-                throw new EnforcerRuleException("Found " + numViolations + " "
-                        + dependenciesFormat.format(numViolations)
-                        + " with dynamic versions. Look at the warnings emitted above for the details.");
+                throw new EnforcerRuleException("Found " + violations.size() + " "
+                        + dependenciesFormat.format(violations.size())
+                        + " with dynamic versions." + System.lineSeparator()
+                        + String.join(System.lineSeparator(), violations));
             }
         } catch (DependencyCollectionException e) {
             throw new EnforcerRuleException("Could not retrieve dependency metadata for project", e);
@@ -256,10 +231,8 @@ public boolean test(DependencyNode depNode) {
         }
     }
 
-    private int emitDependenciesWithBannedDynamicVersions(
-            Dependency rootDependency, RepositorySystemSession repoSession) throws DependencyCollectionException {
-        CollectRequest collectRequest = new CollectRequest(rootDependency, project.getRemoteProjectRepositories());
-        CollectResult collectResult = repoSystem.collectDependencies(repoSession, collectRequest);
+    private List<String> collectDependenciesWithBannedDynamicVersions(DependencyNode rootDependency)
+            throws DependencyCollectionException {
         Predicate<DependencyNode> predicate;
         if (ignores != null && !ignores.isEmpty()) {
             predicate = new ExcludeArtifactPatternsPredicate(ignores);
@@ -268,8 +241,8 @@ private int emitDependenciesWithBannedDynamicVersions(
         }
         BannedDynamicVersionCollector bannedDynamicVersionCollector = new BannedDynamicVersionCollector(predicate);
         DependencyVisitor depVisitor = new TreeDependencyVisitor(bannedDynamicVersionCollector);
-        collectResult.getRoot().accept(depVisitor);
-        return bannedDynamicVersionCollector.getNumViolations();
+        rootDependency.accept(depVisitor);
+        return bannedDynamicVersionCollector.getViolations();
     }
 
     @Override
diff --git a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/ResolverUtil.java b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/ResolverUtil.java
@@ -77,7 +77,7 @@ class ResolverUtil {
      * Please consult {@link ConflictResolver} and {@link DependencyManagerUtils}>
      * </p>
      *
-     * @param excludedScopes a project dependency scope to excluded
+     * @param excludedScopes the scopes of direct dependencies to ignore
      * @return a Dependency Node which is the root of the project's dependency tree
      * @throws EnforcerRuleException thrown if the lookup fails
      */
@@ -96,6 +96,20 @@ DependencyNode resolveTransitiveDependencies() throws EnforcerRuleException {
         return resolveTransitiveDependencies(false, true, Arrays.asList(SCOPE_TEST, SCOPE_PROVIDED));
     }
 
+    /**
+     * Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency
+     * for the current {@link MavenProject}.
+     *
+     * @param excludeOptional ignore optional project artifacts
+     * @param excludedScopes the scopes of direct dependencies to ignore
+     * @return a Dependency Node which is the root of the project's dependency tree
+     * @throws EnforcerRuleException thrown if the lookup fails
+     */
+    DependencyNode resolveTransitiveDependencies(boolean excludeOptional, List<String> excludedScopes)
+            throws EnforcerRuleException {
+        return resolveTransitiveDependencies(false, excludeOptional, excludedScopes);
+    }
+
     private DependencyNode resolveTransitiveDependencies(
             boolean verbose, boolean excludeOptional, List<String> excludedScopes) throws EnforcerRuleException {
 
@@ -134,6 +148,7 @@ private DependencyNode resolveTransitiveDependencies(
             return repositorySystem
                     .collectDependencies(repositorySystemSession, collectRequest)
                     .getRoot();
+
         } catch (DependencyCollectionException e) {
             throw new EnforcerRuleException("Could not build dependency tree " + e.getLocalizedMessage(), e);
         }
diff --git a/maven-enforcer-plugin/src/it/projects/ban-dynamic-versions/verify.groovy b/maven-enforcer-plugin/src/it/projects/ban-dynamic-versions/verify.groovy
@@ -21,5 +21,5 @@ assert buildLog.text.contains( '[ERROR] Dependency org.apache.maven.plugins.enfo
 assert buildLog.text.contains( '[ERROR] Dependency org.apache.maven.plugins.enforcer.its:menforcer138_io:jar:LATEST (compile) is referenced with a banned dynamic version LATEST' )
 assert buildLog.text.contains( '[ERROR] Dependency org.apache.maven.plugins.enforcer.its:menforcer134_model:jar:1.0-SNAPSHOT (compile) is referenced with a banned dynamic version 1.0-SNAPSHOT' )
 assert buildLog.text.contains( '[ERROR] Dependency org.apache.maven.plugins.enforcer.its:menforcer427-a:jar:1.0 (compile) via org.apache.maven.plugins.enforcer.its:menforcer427:jar:1.0 is referenced with a banned dynamic version [1.0,2)' )
-assert buildLog.text.contains( '[ERROR] Rule 0: org.apache.maven.enforcer.rules.BanDynamicVersions failed with message' )
-assert buildLog.text.contains( 'Found 4 dependencies with dynamic versions. Look at the warnings emitted above for the details.' )
+assert buildLog.text.contains( '[ERROR] Rule 0: org.apache.maven.enforcer.rules.dependency.BanDynamicVersions failed with message' )
+assert buildLog.text.contains( 'ERROR] Found 4 dependencies with dynamic versions.' )
