Fix union(range, public_version) to fill local-variant punctures

[dimbleby]

The constraint algebra treated '==X' as the literal point [X, X] when computing 'range.union(public_X)', returning the range unchanged whenever the range already allowed X. But per PEP 440 release-equality '==X' also matches every local-tagged variant 'X+local', so a range whose bound is such a variant (e.g. '>=X,<X+local' or '>X+local,<Y') must be broadened to include those variants in the union.

Apply the broadening symmetrically in VersionRange.union(Version), handling both bounds in a single pass, and delegate Version.union(non Version) to the other operand so the rules apply regardless of operand order.

Resolves: python-poetry#

• Added tests for changed code.
• Updated documentation for changed code.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In the parametrized test_union_with_public_version_is_symmetric, the parameter names min and max shadow builtins and make the test harder to read; consider renaming them (e.g., min_version, max_version) for clarity.
• The broadening of the upper bound using other.stable.next_patch() in VersionRange.union assumes other behaves like a final release; it may be worth explicitly considering or guarding how this behaves for non-final versions (pre/dev/post releases) to ensure it matches the intended PEP 440 semantics.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In the parametrized `test_union_with_public_version_is_symmetric`, the parameter names `min` and `max` shadow builtins and make the test harder to read; consider renaming them (e.g., `min_version`, `max_version`) for clarity.
- The broadening of the upper bound using `other.stable.next_patch()` in `VersionRange.union` assumes `other` behaves like a final release; it may be worth explicitly considering or guarding how this behaves for non-final versions (pre/dev/post releases) to ensure it matches the intended PEP 440 semantics.

## Individual Comments

### Comment 1
<location path="tests/constraints/version/test_version_range.py" line_range="738-756" />
<code_context>
     assert isinstance(result, EmptyConstraint)


+def test_union_upper_bound_local_with_public_extends_to_next_patch() -> None:
+    """``>=X,<X+local ∪ ==X`` broadens the upper bound to
+    ``X.next_patch()`` (exclusive). ``==X`` matches every ``X+local``
+    variant by PEP 440 release-equality, so the union must cover them.
+    """
+    range_below = VersionRange(
+        Version.parse("0.21.0"),
+        Version.parse("0.21.0+cpu"),
+        include_min=True,
+        include_max=False,
+    )
+    public = Version.parse("0.21.0")
+    expected = VersionRange(
+        Version.parse("0.21.0"),
+        Version.parse("0.21.1"),
+        include_min=True,
+        include_max=False,
+    )
+    assert range_below.union(public) == expected
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a case where both bounds are local variants of the same public version to ensure the range collapses as intended

Current tests only cover cases where one bound is local and the other is public. Please add a case where both bounds are local variants of the same public version (e.g. `>=0.21.0+cpu,<=0.21.0+gpu ∪ ==0.21.0`) and assert it collapses to `[0.21.0, 0.21.1)`, checking both `range ∪ ==X` and `==X ∪ range` for symmetry.

```suggestion
def test_union_upper_bound_local_with_public_extends_to_next_patch() -> None:
    """``>=X,<X+local ∪ ==X`` broadens the upper bound to
    ``X.next_patch()`` (exclusive). ``==X`` matches every ``X+local``
    variant by PEP 440 release-equality, so the union must cover them.
    """
    range_below = VersionRange(
        Version.parse("0.21.0"),
        Version.parse("0.21.0+cpu"),
        include_min=True,
        include_max=False,
    )
    public = Version.parse("0.21.0")
    expected = VersionRange(
        Version.parse("0.21.0"),
        Version.parse("0.21.1"),
        include_min=True,
        include_max=False,
    )
    assert range_below.union(public) == expected


def test_union_local_bounds_with_public_collapses_to_next_patch() -> None:
    """``>=X+local1,<=X+local2 ∪ ==X`` collapses to ``[X, X.next_patch())``.

    Both bounds are local variants of the same public version ``X``.
    Since ``==X`` matches all of them by PEP 440 release-equality, the
    union must cover the entire public range up to ``X.next_patch()``.
    """
    range_locals = VersionRange(
        Version.parse("0.21.0+cpu"),
        Version.parse("0.21.0+gpu"),
        include_min=True,
        include_max=True,
    )
    public = Version.parse("0.21.0")
    expected = VersionRange(
        Version.parse("0.21.0"),
        Version.parse("0.21.1"),
        include_min=True,
        include_max=False,
    )

    # range ∪ ==X
    assert range_locals.union(public) == expected
    # ==X ∪ range (symmetry)
    assert public.union(range_locals) == expected
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}