Skip to content

Commit

Permalink
simplify markers fully in _compact_markers
Browse files Browse the repository at this point in the history
Exploit the same cnf / dnf trick that is used in marker union
  • Loading branch information
dimbleby committed Dec 3, 2022
1 parent 24ef146 commit 913c651
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 31 deletions.
48 changes: 27 additions & 21 deletions src/poetry/core/version/markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -702,21 +702,28 @@ def parse_marker(marker: str) -> BaseMarker:
return markers


def _compact_markers(tree_elements: Tree, tree_prefix: str = "") -> BaseMarker:
def _compact_markers(
tree_elements: Tree, tree_prefix: str = "", top_level: bool = True
) -> BaseMarker:
from lark import Token

groups: list[BaseMarker] = [MultiMarker()]
# groups is a disjunction of conjunctions
# eg [[A, B], [C, D]] represents "(A and B) or (C and D)"
groups: list[list[BaseMarker]] = [[]]

for token in tree_elements:
if isinstance(token, Token):
if token.type == f"{tree_prefix}BOOL_OP" and token.value == "or":
groups.append(MultiMarker())
groups.append([])

continue

if token.data == "marker":
groups[-1] = MultiMarker.of(
groups[-1], _compact_markers(token.children, tree_prefix=tree_prefix)
sub_marker = _compact_markers(
token.children, tree_prefix=tree_prefix, top_level=False
)
groups[-1].append(sub_marker)

elif token.data == f"{tree_prefix}item":
name, op, value = token.children
if value.type == f"{tree_prefix}MARKER_NAME":
Expand All @@ -726,27 +733,26 @@ def _compact_markers(tree_elements: Tree, tree_prefix: str = "") -> BaseMarker:
)

value = value[1:-1]
groups[-1] = MultiMarker.of(
groups[-1], SingleMarker(str(name), f"{op}{value}")
)
elif token.data == f"{tree_prefix}BOOL_OP" and token.children[0] == "or":
groups.append(MultiMarker())
sub_marker = SingleMarker(str(name), f"{op}{value}")
groups[-1].append(sub_marker)

for i, group in enumerate(reversed(groups)):
if group.is_empty():
del groups[len(groups) - 1 - i]
continue
elif token.data == f"{tree_prefix}BOOL_OP" and token.children[0] == "or":
groups.append([])

if isinstance(group, MultiMarker) and len(group.markers) == 1:
groups[len(groups) - 1 - i] = group.markers[0]
# Combine the groups.
sub_markers = [MultiMarker(*group) for group in groups]
union = MarkerUnion(*sub_markers)

if not groups:
return EmptyMarker()
# This function calls itself recursively. In the inner calls we don't perform any
# simplification, instead doing it all only when we have the complete marker.
if not top_level:
return union

if len(groups) == 1:
return groups[0]
conjunction = cnf(union)
if not isinstance(conjunction, MultiMarker):
return conjunction

return MarkerUnion.of(*groups)
return dnf(conjunction)


def cnf(marker: BaseMarker) -> BaseMarker:
Expand Down
10 changes: 5 additions & 5 deletions tests/packages/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ def test_dependency_from_pep_508_complex() -> None:
assert dep.python_versions == ">=2.7 !=3.2.*"
assert (
str(dep.marker)
== 'python_version >= "2.7" and python_version != "3.2" '
'and (sys_platform == "win32" or sys_platform == "darwin") '
'and extra == "foo"'
== 'python_version >= "2.7" and python_version != "3.2" and sys_platform =='
' "win32" and extra == "foo" or python_version >= "2.7" and python_version'
' != "3.2" and sys_platform == "darwin" and extra == "foo"'
)


Expand Down Expand Up @@ -278,11 +278,11 @@ def test_dependency_from_pep_508_with_python_full_version() -> None:
assert dep.name == "requests"
assert str(dep.constraint) == "2.18.0"
assert dep.extras == frozenset()
assert dep.python_versions == ">=2.7 <2.8 || >=3.4 <3.5.4"
assert dep.python_versions == ">=2.7 <2.8 || >=3.4.0 <3.5.4"
assert (
str(dep.marker)
== 'python_version >= "2.7" and python_version < "2.8" '
'or python_full_version >= "3.4" and python_full_version < "3.5.4"'
'or python_full_version >= "3.4.0" and python_full_version < "3.5.4"'
)


Expand Down
7 changes: 2 additions & 5 deletions tests/packages/utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
{
"python_version": [
[("<", "3.6")],
[("<", "3.6"), (">=", "3.3")],
[("<", "3.3")],
[("<", "3.6"), (">=", "3.3")],
],
"sys_platform": [
[("==", "win32")],
Expand All @@ -41,10 +41,7 @@
' "win32" and python_version < "3.6" and python_version >= "3.3" or'
' sys_platform == "win32" and python_version < "3.3"'
),
{
"python_version": [[("<", "3.6")], [("<", "3.3")]],
"sys_platform": [[("==", "win32")]],
},
{"python_version": [[("<", "3.6")]], "sys_platform": [[("==", "win32")]]},
),
(
'python_version == "2.7" or python_version == "2.6"',
Expand Down
10 changes: 10 additions & 0 deletions tests/version/test_markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,16 @@ def test_empty_marker_is_found_in_complex_intersection(
assert m2.intersect(m1).is_empty()


def test_empty_marker_is_found_in_complex_parse() -> None:
marker = parse_marker(
'(python_implementation != "pypy" or python_version != "3.6") and '
'((python_implementation != "pypy" and python_version != "3.6") or'
' (python_implementation == "pypy" and python_version == "3.6")) and '
'(python_implementation == "pypy" or python_version == "3.6")'
)
assert marker.is_empty()


@pytest.mark.parametrize(
"python_version, python_full_version, "
"expected_intersection_version, expected_union_version",
Expand Down

0 comments on commit 913c651

Please sign in to comment.