Visitar URL original
fix: support groupId-only includes in RequireUpperBoundDeps rule (#998) by waterWang · Pull Request #1002 · apache/maven-enforcer · GitHub
Skip to content

fix: support groupId-only includes in RequireUpperBoundDeps rule (#998) - #1002

Open
waterWang wants to merge 1 commit into
apache:masterfrom
waterWang:fix/requireUpperBoundDeps-groupId-includes
Open

waterWang wants to merge 1 commit into
apache:masterfrom
waterWang:fix/requireUpperBoundDeps-groupId-includes

Conversation

@waterWang

Copy link
Copy Markdown

Summary

Fixes #998.

The RequireUpperBoundDeps rule documentation states that the includes parameter accepts groupId[:artifactId] format (with artifactId being optional), but the implementation only supported exact string matching via List.contains(). This meant specifying just a groupId (e.g., com.example) would not match dependencies like com.example:my-lib.

Changes

  • Changed RequireUpperBoundDepsVisitor.includes from List<String> to Predicate<Artifact> includesMatcher
  • Used ArtifactUtils.prepareDependencyArtifactMatcher() to create the predicate — the same approach used by BannedDependencies, RequireReleaseDeps, and other rules that already support groupId[:artifactId] format
  • Updated visitEnter() to use the predicate instead of direct string comparison

Tests

  • Added testIncludesWithGroupIdOnly() — verifies that includes = ["default-group"] correctly detects conflicts for dependencies with that groupId
  • Added testIncludesWithGroupIdOnlyExcludesOtherGroup() — verifies that includes = ["other-group"] correctly ignores dependencies with a different groupId
  • All 3 tests pass (1 existing + 2 new)

@waterWang

Copy link
Copy Markdown
Author

Hi, friendly reminder! All CI checks passed on this PR. Could you take a look when you have a moment? Let me know if anything needs to be changed. Thanks!


if (includes != null && !includes.isEmpty() && !includes.contains(key)) {
return true;
if (includesMatcher != null) {

@elharo elharo Aug 19, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if instead of a null check we used a matcher that matches everything as the default?

@elharo elharo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

run spotless

22
Caused by: org.apache.maven.plugin.MojoExecutionException: The following files had format violations:
src/test/java/org/apache/maven/enforcer/rules/dependency/RequireUpperBoundDepsTest.java
@@ -18,6 +18,8 @@

@slawekjaranowski

Copy link
Copy Markdown
Member

please preserve line:

 I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

form PR template in PR description

@slawekjaranowski slawekjaranowski added bug Something isn't working waiting-for-feedback Waiting for 90 days until issues or pull request will be closed labels Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working waiting-for-feedback Waiting for 90 days until issues or pull request will be closed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RequireUpperBoundDeps does not support includes with only groupId

3 participants