From 932e1286ce4355ea3767af6564ea793db7ecc17a Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Sat, 25 Mar 2023 16:43:25 +0100 Subject: [PATCH 1/5] [MENFORCER-467] banDynamicVersions excludedScopes on project level We should not change default Maven scope filtering for transitive dependencies Put message of violation into exception to be consistent with other rules --- .../{ => dependency}/BanDynamicVersions.java | 85 +++++++------------ .../rules/dependency/ResolverUtil.java | 14 +++ .../ban-dynamic-versions/verify.groovy | 4 +- 3 files changed, 45 insertions(+), 58 deletions(-) rename enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/{ => dependency}/BanDynamicVersions.java (71%) diff --git a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/BanDynamicVersions.java b/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BanDynamicVersions.java similarity index 71% rename from enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/BanDynamicVersions.java rename to enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BanDynamicVersions.java index d82aade3..cc36c255 100644 --- a/enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/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 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 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 violations; private final Predicate predicate; - public int getNumViolations() { - return numViolations; + public List getNumViolations() { + return violations; } BannedDynamicVersionCollector(Predicate 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 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 violations = emitDependenciesWithBannedDynamicVersions(rootDependency); + if (!violations.isEmpty()) { ChoiceFormat dependenciesFormat = new ChoiceFormat("1#dependency|1 emitDependenciesWithBannedDynamicVersions(DependencyNode rootDependency) + throws DependencyCollectionException { Predicate predicate; if (ignores != null && !ignores.isEmpty()) { predicate = new ExcludeArtifactPatternsPredicate(ignores); @@ -268,7 +241,7 @@ private int emitDependenciesWithBannedDynamicVersions( } BannedDynamicVersionCollector bannedDynamicVersionCollector = new BannedDynamicVersionCollector(predicate); DependencyVisitor depVisitor = new TreeDependencyVisitor(bannedDynamicVersionCollector); - collectResult.getRoot().accept(depVisitor); + rootDependency.accept(depVisitor); return bannedDynamicVersionCollector.getNumViolations(); } 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 index 3bbe8674..3ae59b54 100644 --- 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 @@ -96,6 +96,19 @@ 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 excludedScopes a project dependency scope to excluded + * @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 excludedScopes) + throws EnforcerRuleException { + return resolveTransitiveDependencies(false, excludeOptional, excludedScopes); + } + private DependencyNode resolveTransitiveDependencies( boolean verbose, boolean excludeOptional, List excludedScopes) throws EnforcerRuleException { @@ -134,6 +147,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 index cdf90a98..e2a4ae94 100644 --- 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.' ) From 24674cf46795699fe649b4f11b5e0f6f4862be29 Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Sun, 26 Mar 2023 13:45:37 +0200 Subject: [PATCH 2/5] Update enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BanDynamicVersions.java Co-authored-by: Konrad Windszus --- .../maven/enforcer/rules/dependency/BanDynamicVersions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index cc36c255..f4474214 100644 --- 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 @@ -128,7 +128,7 @@ private final class BannedDynamicVersionCollector implements DependencyVisitor { private final Predicate predicate; - public List getNumViolations() { + public List getViolations() { return violations; } From b482b7be64c86ae3a476774ce52e673464c3fdad Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Sun, 26 Mar 2023 13:45:50 +0200 Subject: [PATCH 3/5] Update enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/BanDynamicVersions.java Co-authored-by: Konrad Windszus --- .../maven/enforcer/rules/dependency/BanDynamicVersions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index f4474214..0af128b3 100644 --- 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 @@ -231,7 +231,7 @@ public boolean test(DependencyNode depNode) { } } - private List emitDependenciesWithBannedDynamicVersions(DependencyNode rootDependency) + private List collectDependenciesWithBannedDynamicVersions(DependencyNode rootDependency) throws DependencyCollectionException { Predicate predicate; if (ignores != null && !ignores.isEmpty()) { From 115889e8ad081b2a744b18e69232e0266218773e Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Sun, 26 Mar 2023 13:46:10 +0200 Subject: [PATCH 4/5] Update enforcer-rules/src/main/java/org/apache/maven/enforcer/rules/dependency/ResolverUtil.java Co-authored-by: Konrad Windszus --- .../apache/maven/enforcer/rules/dependency/ResolverUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 3ae59b54..1c58e667 100644 --- 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 @@ -100,7 +100,7 @@ DependencyNode resolveTransitiveDependencies() throws EnforcerRuleException { * Retrieves the {@link DependencyNode} instance containing the result of the transitive dependency * for the current {@link MavenProject}. * - * @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 */ From dafb1f8213776a1a8cb1f55a7426e40d274c32fb Mon Sep 17 00:00:00 2001 From: Slawomir Jaranowski Date: Sun, 26 Mar 2023 13:57:03 +0200 Subject: [PATCH 5/5] Apply suggestions from review --- .../maven/enforcer/rules/dependency/BanDynamicVersions.java | 4 ++-- .../apache/maven/enforcer/rules/dependency/ResolverUtil.java | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) 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 index 0af128b3..7224b848 100644 --- 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 @@ -197,7 +197,7 @@ public void execute() throws EnforcerRuleException { DependencyNode rootDependency = resolverUtil.resolveTransitiveDependencies(excludeOptionals, excludedScopes); - List violations = emitDependenciesWithBannedDynamicVersions(rootDependency); + List violations = collectDependenciesWithBannedDynamicVersions(rootDependency); if (!violations.isEmpty()) { ChoiceFormat dependenciesFormat = new ChoiceFormat("1#dependency|1 collectDependenciesWithBannedDynamicVersions(DependencyNode BannedDynamicVersionCollector bannedDynamicVersionCollector = new BannedDynamicVersionCollector(predicate); DependencyVisitor depVisitor = new TreeDependencyVisitor(bannedDynamicVersionCollector); rootDependency.accept(depVisitor); - return bannedDynamicVersionCollector.getNumViolations(); + 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 index 1c58e667..774812af 100644 --- 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}> *

* - * @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 */ @@ -100,6 +100,7 @@ DependencyNode resolveTransitiveDependencies() throws EnforcerRuleException { * 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