Skip to content

Preserve imported BOM precedence in SortDependencies - #8508

Open
martinfrancois wants to merge 6 commits into
openrewrite:mainfrom
martinfrancois:agent/repro-sort-dependencies-bom-precedence
Open

Preserve imported BOM precedence in SortDependencies#8508
martinfrancois wants to merge 6 commits into
openrewrite:mainfrom
martinfrancois:agent/repro-sort-dependencies-bom-precedence

Conversation

@martinfrancois

@martinfrancois martinfrancois commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Suggested review order: 1 of 52 (Score: 10)
Review first: None - this is the first item in the queue.

What's changed?

Preserves the positions of imported BOM dependencies when SortDependencies sorts Maven dependencies. Other dependencies still sort around those fixed positions, and comments remain attached to their original dependency.

What's your motivation?

Recipe: org.openrewrite.maven.SortDependencies.

I found this by running org.openrewrite.maven.SortDependencies from org.openrewrite:rewrite-maven:8.87.0 on pom.xml in Symphony-Trello at a8013f27. I reproduced the same result on that exact checkout with the latest released recipe artifact, org.openrewrite:rewrite-maven:8.90.0, using org.openrewrite.maven:rewrite-maven-plugin:6.46.1.

Before

<!-- Keep the stable OpenTelemetry modules aligned while the Quarkus BOM trails the security fix. -->
<dependency>
  <groupId>io.opentelemetry</groupId>
  <artifactId>opentelemetry-bom</artifactId>
</dependency>
<dependency>
  <groupId>${quarkus.platform.group-id}</groupId>
  <artifactId>${quarkus.platform.artifact-id}</artifactId>
</dependency>

Actual after the recipe

<dependency>
  <groupId>${quarkus.platform.group-id}</groupId>
  <artifactId>${quarkus.platform.artifact-id}</artifactId>
</dependency>
<!-- Keep the stable OpenTelemetry modules aligned while the Quarkus BOM trails the security fix. -->
<dependency>
  <groupId>io.opentelemetry</groupId>
  <artifactId>opentelemetry-bom</artifactId>
</dependency>

Expected after the recipe

(unchanged)

Imported BOM order controls dependency-management precedence. Sorting these two entries changes which managed version wins, so the recipe changes project behavior rather than presentation alone.

Confirmed real-world execution

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

Please review the fixed-position treatment of imported BOMs and the conservative handling of property placeholders. A placeholder in a BOM marker remains fixed because profile-local properties are not always present in the effective model used during the visit.

Have you considered any alternatives or workarounds?

Reapplying the intended BOM order after every recipe run is fragile and makes the recipe non-idempotent for this project.

Any additional context

Pre-existing tests changed: None.

This change was prepared with AI assistance. I reviewed the target execution evidence, implementation, tests, generated metadata, and contribution text.

Checklist

@martinfrancois
martinfrancois force-pushed the agent/repro-sort-dependencies-bom-precedence branch from 364ebe3 to b574ac0 Compare August 15, 2026 22:51
@martinfrancois
martinfrancois marked this pull request as ready for review August 17, 2026 08:08
Replace the hand-rolled profile activation and property precedence
resolver with ResolvedPom value resolution, keeping the conservative
treatment of placeholders the effective model cannot resolve.

Rework the tests that only existed to exercise that resolver so they
assert the simplified contract instead.
…mparison

The CDATA existed only to exercise a trim() that the XML parser makes
unnecessary; surrounding whitespace never reaches CharData text. Compare
type and scope the way MavenVisitor.isManagedDependencyImportTag does.
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