fix: mvnup: widen exact Maven version pins to allow Maven 4#12505
fix: mvnup: widen exact Maven version pins to allow Maven 4#12505gnodet wants to merge 1 commit into
Conversation
Projects like Apache Flink pin their Maven version to an exact version using ranges like [3.8.6,3.8.6] (both bounds equal, both inclusive). The existing MAVEN4_EXCLUSIVE_UPPER_BOUND pattern only matched ranges with an exclusive upper bound at 4.x (e.g. [3.8.8,4)), so exact pins and ranges with an upper bound below 4 were silently skipped. Add a new UPPER_BOUND_BELOW_MAVEN4 pattern that catches any version range whose upper bound has a major version less than 4. This covers: - Exact version pins: [3.8.6,3.8.6] -> [3.8.6,5) - Sub-4 upper bounds: [3.8.0,3.9) -> [3.8.0,5) - Unbounded lower ranges: (,3.9] -> (,5) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
gnodet
left a comment
There was a problem hiding this comment.
Review: mvnup: widen exact Maven version pins to allow Maven 4
Clean fix that closes an important gap in mvnup's EnforcerVersionRangeStrategy — exact version pins like [3.8.6,3.8.6] and sub-4 upper bounds like [3.8.0,3.9) now correctly get widened to allow Maven 4.
What this PR does well
- Correct regex design —
UPPER_BOUND_BELOW_MAVEN4captures the right cases, and the pattern ordering (checkingMAVEN4_EXCLUSIVE_UPPER_BOUNDfirst) is essential and well-designed - Safe fallback —
getMajorVersion()returnsInteger.MAX_VALUEfor unparseable versions, which acts as a "do not widen" guard - Comprehensive tests — 5 new unit tests + 1 integration test covering exact pins, short versions, and Maven 4 non-widening
- Good Javadoc — the new pattern documents matched/unmatched examples clearly
Suggestions (low priority)
-
[Low] Missing test for unbounded lower bound
(,3.9]— the Javadoc lists this as a matched example, but there's no test exercising it. The regex handles it correctly (group 2 captures empty string), but a test would document the behavior. -
[Low] No test documenting
[3.8.8,4]behavior — a range with an inclusive upper bound at exactly 4 is not widened by either pattern (first requires), second sees major=4 which is not < 4). In Maven semantics[3.8.8,4]includes4.0.0but blocks4.0.1+. The non-widening behavior is reasonable, but a test documenting this decision would be useful.
Checklist
| Check | Status |
|---|---|
| Tests | ✓ Comprehensive coverage |
| Docs | ✓ Javadoc updated |
| Commit convention | ✓ fix: prefix |
| Public API / backward compat | ✓ Package-private method |
| Security | N/A |
| CI | ✓ All 24 checks pass |
LGTM.
Reviewed with Claude Code on behalf of Guillaume Nodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.
Summary
[3.8.6,3.8.6](both bounds equal, both inclusive). The existingMAVEN4_EXCLUSIVE_UPPER_BOUNDpattern only matched ranges with an exclusive upper bound at4.x(e.g.[3.8.8,4)), so exact pins and ranges with an upper bound below 4 were silently skipped.UPPER_BOUND_BELOW_MAVEN4pattern that catches any version range whose upper bound has a major version less than 4, covering exact version pins ([3.8.6,3.8.6]->[3.8.6,5)), sub-4 upper bounds ([3.8.0,3.9)->[3.8.0,5)), and unbounded lower ranges ((,3.9]->(,5)).Test plan
EnforcerVersionRangeStrategyTesttests pass (was 22 before)[3.8.6,3.8.6]is now widened to[3.8.6,5)[3.0,3.0],[3.9.0,3.9.0],[4.0.0,4.0.0](not widened)[3.8.8,3.9)is now widened to[3.8.8,5)[3.8.8,4),[3,4), etc. still pass unchanged🤖 Generated with Claude Code