Skip to content

DependencyManagementDependencyRequiresVersion: remove only entries that manage nothing - #8445

Draft
martinfrancois wants to merge 5 commits into
openrewrite:mainfrom
martinfrancois:fix/dependency-management-versionless-entry-semantics
Draft

DependencyManagementDependencyRequiresVersion: remove only entries that manage nothing#8445
martinfrancois wants to merge 5 commits into
openrewrite:mainfrom
martinfrancois:fix/dependency-management-versionless-entry-semantics

Conversation

@martinfrancois

@martinfrancois martinfrancois commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Suggested review order: 13 of 52 (Score: 7)
Review first: #8509

What's changed?

On main DependencyManagementDependencyRequiresVersion removes every <dependencyManagement> entry with no <version> child. Its description claims such an entry "can't possibly affect dependency resolution anywhere". It can, in two ways:

  1. Without a version an entry still manages scope, exclusions, optional and systemPath for a dependency versioned somewhere else.
  2. Maven merges dependency management one entry at a time on the key groupId:artifactId:type:classifier, not field by field, so an entry declaring only coordinates overrides, rather than inherits from, an entry for the same key in a parent or an imported BOM. Removing it lets that hidden entry take effect.

For the child POM described under motivation, main deletes the versionless guava entry and the emptied container elements. mvn help:effective-pom with Maven 3.9.16 then shows guava moving from runtime to compile scope and the jsr305 exclusion disappearing. This also occurs in published code: org.openrewrite.maven:rewrite-maven-plugin:5.40.0 has three versionless entries with <scope>provided</scope> that main's recipe deletes. Reproduced on v8.87.0, v8.88.3, and main at 80d1f1b7.

With this change an entry goes only when it declares nothing but groupId and artifactId and nothing the run can see manages those coordinates elsewhere. Management the run cannot see counts as unknown, and unknown is reason to keep. The entry stays when:

  • Either coordinate uses a property that does not resolve here.
  • The POM is packaged as pom, or another POM in the same run declares it as its parent, so what a consumer manages cannot be established from here.
  • The POM declares a parent and that parent (or an ancestor's parent) was not parsed in the same run, or the resolved parent manages those coordinates, or an ancestor declares dependency management inside a <profile>, or the ancestry is cyclic.
  • The POM, or one of its profiles, imports a BOM that may manage those coordinates.
  • A sibling entry declares the same groupId and artifactId, or has coordinates that do not resolve here.

The two key comparisons are deliberately different. Against a parent the lookup uses the entry's exact key - groupId:artifactId:jar, no classifier, passing a null type that ResolvedPom reads as jar. Against siblings it is coarser, on groupId and artifactId only, which can keep an otherwise removable entry but never removes an entry Maven would have kept.

The recipe now removes a strict subset of what it removed before, so its display name, description and recipes.csv row are updated to match.

What's your motivation?

Recipe: org.openrewrite.maven.cleanup.DependencyManagementDependencyRequiresVersion.

Before

The parent POM manages guava at 33.4.8-jre. The child POM overrides its scope and exclusion without repeating the version:

<dependency>
  <groupId>com.google.guava</groupId>
  <artifactId>guava</artifactId>
  <scope>runtime</scope>
  <exclusions>
    <exclusion>
      <groupId>com.google.code.findbugs</groupId>
      <artifactId>jsr305</artifactId>
    </exclusion>
  </exclusions>
</dependency>

Actual after the recipe

<!-- dependency entry deleted -->

Deleting the entry changes guava from runtime to compile scope and removes the jsr305 exclusion.

Expected after the recipe

(unchanged)

Affected code in real projects

  • operaton/operaton pom.xml: the parent POM of the Operaton Run distribution modules manages 14 versionless dependencies with <scope>provided</scope>, so that Spring Boot and engine jars already shipped in the Run distribution stay out of the dependency sets the modules copy with maven-dependency-plugin (includeScope=runtime). The implementation on main deletes all 14 entries because they carry no <version>, returning those dependencies to compile scope and pulling the duplicated jars back into the distribution modules.
  • cibseven/cibseven pom.xml: the CIB seven Run modules parent POM uses the same pattern, 14 versionless <scope>provided</scope> entries whose own comment states they mark dependencies already present in the Spring Boot ZIP. The implementation on main deletes every one of them, so the scope management is lost and the excluded jars are copied into the distribution again.

Anything in particular you'd like reviewers to focus on?

No existing test expectation changed; the test for #1084 still passes as written.

Two cases keep the most entries: a POM whose declared parent this run did not parse (spring-boot-starter-parent, say) keeps every entry, and a POM importing any BOM keeps every coordinates-only entry, including ones that BOM does not manage. A third limit is inherited - the recipe still does not visit entries under <profiles>.

The test file still has no @DocumentExample. examples.yml is generated and checked in, and a hand-written entry would probably not match what the generator produces; 14 recipes in this module carry no example today.

Have you considered any alternatives or workarounds?

A more precise rule can look the coordinates up in ResolvedPom.getDependencyManagement(), which already holds entries inherited from parents and imported from BOMs. That list also contains the resolved form of the entry under review and records only the declared ManagedDependency each resolved entry came from, not the POM that declared it, so a naive lookup always matches the entry itself and removes nothing; it has to compare against the entry's own declared ManagedDependency to tell the self match apart. The work stays inside managedElsewhere. Say the word and I will make it.

Any additional context

Upstream names the test class ManagedDependencyRequiresVersionTest even though the recipe class is DependencyManagementDependencyRequiresVersion; left as is. The class goes from 1 to 21 tests, 17 of which fail without the code change. They cover an entry declaring more than coordinates; a coordinates-only entry hiding a parent or BOM entry; unresolvable coordinates; management the run cannot establish (unparsed parent, profile, cyclic ancestry, a consumer that inherits or imports); and a sibling repeating the coordinates. Four pass either way, covering removals the recipe must still perform.

This change was prepared with AI assistance (Claude Code). I reviewed the code, the tests and this description.

Checklist

The recipe removed every managed dependency without a `<version>`, on
the premise that such an entry cannot affect resolution anywhere.
Maven does not honor that premise. An entry without a version still
manages `scope`, `exclusions`, `optional` and `systemPath` for a
dependency versioned elsewhere, and because Maven merges dependency
management one entry at a time on the management key rather than
field by field, an entry declaring only coordinates hides, rather
than inherits from, a same-key entry coming from a parent or an
imported BOM. Removing it silently widened effective scope and
dropped exclusions.

An entry is now removed only when it declares nothing but `groupId`
and `artifactId`, both resolve, and no entry for that key can be
hidden: the POM is not one that others inherit from or import, its
parent's effective management does not cover the key, no ancestor
declares dependency management in a profile or is unresolved here,
neither the POM nor its profiles import a BOM, and no sibling entry
shares the key. The display description and the generated recipes.csv
row are updated to match.

The recipe is now strictly more conservative, so it also keeps
entries that happen to be inert but whose surroundings cannot be
established from the POM under review. The one hidden entry it still
cannot see is a BOM import of a POM packaged as something other than
`pom`, since only the parent relation is recorded on either side. The
existing test for issue 1084 is untouched and still passes: a
coordinates-only entry in a standalone POM is still removed.
…im commentary

The display name promised a version on every entry, which the recipe no longer enforces. Also simplifies the sibling lookup and hoists the recipe into defaults(RecipeSpec).
@timtebeek timtebeek changed the title DependencyManagementDependencyRequiresVersion: remove only inert entries DependencyManagementDependencyRequiresVersion: remove only entries that manage nothing Aug 11, 2026
…ment-versionless-entry-semantics

# Conflicts:
#	rewrite-maven/src/main/resources/META-INF/rewrite/recipes.csv
…ess-entry-semantics' into fix/dependency-management-versionless-entry-semantics

# Conflicts:
#	rewrite-maven/src/main/resources/META-INF/rewrite/recipes.csv
@martinfrancois
martinfrancois force-pushed the fix/dependency-management-versionless-entry-semantics branch from a125cad to 7060026 Compare August 16, 2026 20:17
@martinfrancois
martinfrancois marked this pull request as draft August 17, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

2 participants