Fix RemoveDependency.unlessUsing for multi-module Maven projects#183
Open
steve-aom-elliott wants to merge 2 commits into
Open
Fix RemoveDependency.unlessUsing for multi-module Maven projects#183steve-aom-elliott wants to merge 2 commits into
steve-aom-elliott wants to merge 2 commits into
Conversation
`RemoveDependency` scans every source file and records whether `unlessUsing` matched against the file's `JavaProject` marker. In a multi-module Maven project the parent pom carries its own `JavaProject` distinct from the children, so the parent module's accumulator entry never reflects type usages found in child modules. The visitor would then strip the `<dependency>` from the parent pom even though child modules in the same reactor still relied on inheriting it. Now the scanner additionally records the `MavenResolutionResult.id -> JavaProject` mapping for every pom it visits. When the visitor is about to remove a `<dependency>` from a pom, it walks that pom's descendant modules via `MavenResolutionResult.getModules()` and preserves the declaration if any descendant's `JavaProject` shows the type in use.
…f-declare The descendant-aware preservation introduced in the previous commit was intentionally conservative: any descendant module whose Java sources used the `unlessUsing` type blocked removal of the dep from an ancestor pom. That over-preserves when a descendant module also declares the same dependency directly in its own `<dependencies>` — in that case the descendant is self-sufficient and the ancestor's declaration is dead weight. Refine the check: a descendant only blocks removal if it uses the type AND does not declare the dependency itself. The self-declaration check looks at the raw `requested` pom (as-written), not the resolved/inherited dependency list, so a child that inherits the dep from the ancestor we might modify still correctly blocks removal.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RemoveDependency'sunlessUsingguard is per-JavaProjectmarker, which corresponds to a single Maven module. In a multi-module reactor the parent pom carries a differentJavaProjectthan its children, so type usages found in child modules never gate removal from the parent pom — the<dependency>gets stripped from the parent while child modules still rely on inheriting it, producing surprise compile errors in downstream builds.This came up while looking into a Spring Boot 4 migration failure where
io.moderne.java.spring.boot4.MigrateSpringRetry(which callsRemoveDependency: unlessUsing: org.springframework.retry..*at the end of its recipe list) removedspring-retryfrom a parent pom even though child modules contained extensiveRetryTemplate/RetryPolicy/BackOffPolicyusage.Two commits:
MavenResolutionResult.id -> JavaProject. When the visitor is about to remove from a pom, it walksMavenResolutionResult.getModules()recursively and preserves the dep if any descendant'sJavaProjectshows the type in use.Pom.getRequested().getDependencies()rather than the resolved/inherited list, so a child that inherits the dep from the very ancestor we're modifying still correctly blocks removal.Test plan
Five new tests in
RemoveDependencyTest(all passing):doNotRemoveSpringRetryWhenRetryTemplateInUse— single module, body usage → preserveddoNotRemoveSpringRetryWhenOnlyImportsPresent— single module, imports only → preserveddoNotRemoveSpringRetryWhenDepAndUsageInSameChildModule— multi-module, dep + Java in same child → preserveddoNotRemoveSpringRetryInMultiModuleWhenChildUsesIt— previously failing, parent has dep, child uses → preservedremoveParentDependencyWhenChildSelfDeclaresAndUses— parent has dep, child redeclares and uses → parent dep removed (commit 2 behavior)Existing tests continue to pass.