From 913c651fa1642af7f3e53f3d36a6e0fcdfd251c7 Mon Sep 17 00:00:00 2001 From: David Hotham Date: Tue, 29 Nov 2022 20:33:07 +0000 Subject: [PATCH] simplify markers fully in _compact_markers Exploit the same cnf / dnf trick that is used in marker union --- src/poetry/core/version/markers.py | 48 +++++++++++++++++------------- tests/packages/test_main.py | 10 +++---- tests/packages/utils/test_utils.py | 7 ++--- tests/version/test_markers.py | 10 +++++++ 4 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/poetry/core/version/markers.py b/src/poetry/core/version/markers.py index dc6555339..ddef93703 100644 --- a/src/poetry/core/version/markers.py +++ b/src/poetry/core/version/markers.py @@ -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": @@ -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: diff --git a/tests/packages/test_main.py b/tests/packages/test_main.py index 2dd465b7a..5a880e12a 100644 --- a/tests/packages/test_main.py +++ b/tests/packages/test_main.py @@ -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"' ) @@ -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"' ) diff --git a/tests/packages/utils/test_utils.py b/tests/packages/utils/test_utils.py index 9a96f2100..1006031de 100644 --- a/tests/packages/utils/test_utils.py +++ b/tests/packages/utils/test_utils.py @@ -25,8 +25,8 @@ { "python_version": [ [("<", "3.6")], - [("<", "3.6"), (">=", "3.3")], [("<", "3.3")], + [("<", "3.6"), (">=", "3.3")], ], "sys_platform": [ [("==", "win32")], @@ -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"', diff --git a/tests/version/test_markers.py b/tests/version/test_markers.py index 2f53e2abf..3f1ce54bf 100644 --- a/tests/version/test_markers.py +++ b/tests/version/test_markers.py @@ -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",