Skip to content

Commit

Permalink
constraints: fix intersection of MultiConstraint and ConstraintUnion …
Browse files Browse the repository at this point in the history
…and increase test coverage for intersection of generic constraints
  • Loading branch information
radoering committed Nov 2, 2022
1 parent 89e96f5 commit 67f9134
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 32 deletions.
20 changes: 11 additions & 9 deletions src/poetry/core/constraints/generic/multi_constraint.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

from poetry.core.constraints.generic import EmptyConstraint
from poetry.core.constraints.generic.base_constraint import BaseConstraint
from poetry.core.constraints.generic.constraint import Constraint

Expand Down Expand Up @@ -62,18 +63,19 @@ def allows_any(self, other: BaseConstraint) -> bool:

def intersect(self, other: BaseConstraint) -> BaseConstraint:
if not isinstance(other, Constraint):
raise ValueError("Unimplemented constraint intersection")
return other.intersect(self)

constraints = self._constraints
if other not in constraints:
constraints += (other,)
else:
constraints = (other,)
if other in self._constraints:
return self

if len(constraints) == 1:
return constraints[0]
if other.value in (c.value for c in self._constraints):
# same value but different operator, e.g. '== "linux"' and '!= "linux"'
return EmptyConstraint()

return MultiConstraint(*constraints)
if other.operator == "==":
return other

return MultiConstraint(*self._constraints, other)

def __eq__(self, other: object) -> bool:
if not isinstance(other, MultiConstraint):
Expand Down
17 changes: 5 additions & 12 deletions src/poetry/core/constraints/generic/union_constraint.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,21 +71,13 @@ def intersect(self, other: BaseConstraint) -> BaseConstraint:
return other

if isinstance(other, Constraint):
if self.allows(other):
return other
# (A or B) and C => (A and C) or (B and C)
# just a special case of UnionConstraint
other = UnionConstraint(other)

return EmptyConstraint()

# Two remaining cases: an intersection with another union, or an intersection
# with a multi.
#
# In the first case:
# (A or B) and (C or D) => (A and C) or (A and D) or (B and C) or (B and D)
#
# In the second case:
# (A or B) and (C and D) => (A and C and D) or (B and C and D)
new_constraints = []
if isinstance(other, UnionConstraint):
# (A or B) and (C or D) => (A and C) or (A and D) or (B and C) or (B and D)
for our_constraint in self._constraints:
for their_constraint in other.constraints:
intersection = our_constraint.intersect(their_constraint)
Expand All @@ -98,6 +90,7 @@ def intersect(self, other: BaseConstraint) -> BaseConstraint:

else:
assert isinstance(other, MultiConstraint)
# (A or B) and (C and D) => (A and C and D) or (B and C and D)

for our_constraint in self._constraints:
intersection = our_constraint
Expand Down
111 changes: 107 additions & 4 deletions tests/constraints/generic/test_constraint.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,51 @@ def test_allows_all() -> None:
@pytest.mark.parametrize(
("constraint1", "constraint2", "expected"),
[
(
EmptyConstraint(),
Constraint("win32"),
EmptyConstraint(),
),
(
EmptyConstraint(),
MultiConstraint(Constraint("darwin", "!="), Constraint("linux", "!=")),
EmptyConstraint(),
),
(
EmptyConstraint(),
UnionConstraint(Constraint("win32"), Constraint("linux")),
EmptyConstraint(),
),
(
AnyConstraint(),
Constraint("win32"),
Constraint("win32"),
),
(
AnyConstraint(),
MultiConstraint(Constraint("darwin", "!="), Constraint("linux", "!=")),
MultiConstraint(Constraint("darwin", "!="), Constraint("linux", "!=")),
),
(
AnyConstraint(),
UnionConstraint(Constraint("win32"), Constraint("linux")),
UnionConstraint(Constraint("win32"), Constraint("linux")),
),
(
EmptyConstraint(),
AnyConstraint(),
EmptyConstraint(),
),
(
EmptyConstraint(),
EmptyConstraint(),
EmptyConstraint(),
),
(
AnyConstraint(),
AnyConstraint(),
AnyConstraint(),
),
(
Constraint("win32"),
Constraint("win32"),
Expand All @@ -65,6 +110,16 @@ def test_allows_all() -> None:
Constraint("linux"),
EmptyConstraint(),
),
(
Constraint("win32"),
MultiConstraint(Constraint("darwin", "!="), Constraint("linux", "!=")),
Constraint("win32"),
),
(
Constraint("win32"),
MultiConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
EmptyConstraint(),
),
(
Constraint("win32"),
UnionConstraint(Constraint("win32"), Constraint("linux")),
Expand All @@ -90,25 +145,73 @@ def test_allows_all() -> None:
Constraint("linux", "!="),
MultiConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
),
(
Constraint("win32", "!="),
MultiConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
MultiConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
),
(
Constraint("darwin", "!="),
MultiConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
MultiConstraint(
Constraint("win32", "!="),
Constraint("linux", "!="),
Constraint("darwin", "!="),
),
),
(
Constraint("win32", "!="),
UnionConstraint(Constraint("win32"), Constraint("linux")),
Constraint("linux"),
),
(
Constraint("win32", "!="),
UnionConstraint(
Constraint("win32"), Constraint("linux"), Constraint("darwin")
),
UnionConstraint(Constraint("linux"), Constraint("darwin")),
),
(
Constraint("win32", "!="),
UnionConstraint(Constraint("linux"), Constraint("linux2")),
UnionConstraint(Constraint("linux"), Constraint("linux2")),
),
(
UnionConstraint(Constraint("win32"), Constraint("linux")),
UnionConstraint(Constraint("win32"), Constraint("darwin")),
Constraint("win32"),
),
(
UnionConstraint(
Constraint("win32"), Constraint("linux"), Constraint("darwin")
),
UnionConstraint(
Constraint("win32"), Constraint("cygwin"), Constraint("darwin")
),
UnionConstraint(
Constraint("win32"),
Constraint("darwin"),
),
),
(
UnionConstraint(Constraint("win32"), Constraint("linux")),
MultiConstraint(Constraint("win32", "!="), Constraint("darwin", "!=")),
Constraint("linux"),
),
(
UnionConstraint(Constraint("win32"), Constraint("linux")),
MultiConstraint(Constraint("win32", "!="), Constraint("linux", "!=")),
EmptyConstraint(),
),
],
)
def test_intersect(
constraint1: BaseConstraint,
constraint2: BaseConstraint,
expected: BaseConstraint,
) -> None:
intersection = constraint1.intersect(constraint2)
assert intersection == expected
assert constraint1.intersect(constraint2) == expected
assert constraint2.intersect(constraint1) == expected


@pytest.mark.parametrize(
Expand Down Expand Up @@ -158,8 +261,8 @@ def test_union(
constraint2: BaseConstraint,
expected: BaseConstraint,
) -> None:
union = constraint1.union(constraint2)
assert union == expected
assert constraint1.union(constraint2) == expected
assert constraint2.union(constraint1) == expected


def test_difference() -> None:
Expand Down
7 changes: 0 additions & 7 deletions tests/constraints/generic/test_multi_constraint.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,3 @@ def test_allows_all() -> None:
assert not c.allows_all(
MultiConstraint(Constraint("win32", "!="), Constraint("darwin", "!="))
)


def test_intersect() -> None:
c = MultiConstraint(Constraint("win32", "!="), Constraint("linux", "!="))

intersection = c.intersect(Constraint("win32", "!="))
assert intersection == Constraint("win32", "!=")

0 comments on commit 67f9134

Please sign in to comment.