Changes to integrate new parser into packaging + test adjustments

packaging/_tokenizer.py

@@ -16,7 +16,7 @@ def matches(self, name: str = "") -> bool:
         return True
 
 
-class ParseException(Exception):
+class ParseExceptionError(Exception):
     """
     Parsing failed.
     """
@@ -80,9 +80,7 @@ class Tokenizer:
 
     next_token: Optional[Token]
 
-    def __init__(
-        self, source: str, rules: Dict[Optional[str], object] = DEFAULT_RULES
-    ) -> None:
+    def __init__(self, source: str, rules: Dict[str, object] = DEFAULT_RULES) -> None:
         self.source = source
         self.rules = {name: re.compile(pattern) for name, pattern in rules.items()}
         self.next_token = None
@@ -137,7 +135,7 @@ def raise_syntax_error(self, *, message: str) -> NoReturn:
         """
         at = f"at position {self.position}:"
         marker = " " * self.position + "^"
-        raise ParseException(
+        raise ParseExceptionError(
             f"{message}\n{at}\n    {self.source}\n    {marker}",
             self.position,
         )

packaging/markers.py

@@ -8,6 +8,8 @@
 import sys
 from typing import Any, Callable, Dict, List, Optional, Tuple, Union
 
+from ._parser import MarkerAtom, MarkerList, Op, Variable, parse_marker_expr
+from ._tokenizer import ParseExceptionError, Tokenizer
 from .specifiers import InvalidSpecifier, Specifier
 from .utils import canonicalize_name
 
@@ -41,37 +43,8 @@ class UndefinedEnvironmentName(ValueError):
     """
 
 
-class Node:
-    def __init__(self, value: Any) -> None:
-        self.value = value
-
-    def __str__(self) -> str:
-        return str(self.value)
-
-    def __repr__(self) -> str:
-        return f"<{self.__class__.__name__}('{self}')>"
-
-    def serialize(self) -> str:
-        raise NotImplementedError
-
-
-class Variable(Node):
-    def serialize(self) -> str:
-        return str(self)
-
-
-class Value(Node):
-    def serialize(self) -> str:
-        return f'"{self}"'
-
-
-class Op(Node):
-    def serialize(self) -> str:
-        return str(self)
-
-
 def _format_marker(
-    marker: Union[List[str], Tuple[Node, ...], str], first: Optional[bool] = True
+    marker: Union[List[str], MarkerAtom, str], first: Optional[bool] = True
 ) -> str:
 
     assert isinstance(marker, (list, tuple, str))
@@ -138,7 +111,7 @@ def _normalize(*values: str, key: str) -> Tuple[str, ...]:
     return values
 
 
-def _evaluate_markers(markers: List[Any], environment: Dict[str, str]) -> bool:
+def _evaluate_markers(markers: MarkerList, environment: Dict[str, str]) -> bool:
     groups: List[List[bool]] = [[]]
 
     for marker in markers:
@@ -197,7 +170,7 @@ def default_environment() -> Dict[str, str]:
 class Marker:
     def __init__(self, marker: str) -> None:
         try:
-            self._markers = _coerce_parse_result(MARKER.parseString(marker))
+            self._markers = parse_marker_expr(Tokenizer(marker))
             # The attribute `_markers` can be described in terms of a recursive type:
             # MarkerList = List[Union[Tuple[Node, ...], str, MarkerList]]
             #
@@ -214,10 +187,10 @@ def __init__(self, marker: str) -> None:
             #         (<Variable('os_name')>, <Op('==')>, <Value('unix')>)
             #     ]
             # ]
-        except ParseException as e:
+        except ParseExceptionError as e:
             raise InvalidMarker(
                 f"Invalid marker: {marker!r}, parse error at "
-                f"{marker[e.loc : e.loc + 8]!r}"
+                f"{marker[e.position : e.position + 8]!r}"
             )
 
     def __str__(self) -> str:

packaging/requirements.py

@@ -2,13 +2,18 @@
 # 2.0, and the BSD License. See the LICENSE file in the root of this repository
 # for complete details.
 
-import re
-import string
 import urllib.parse
+from collections import namedtuple
 from typing import Any, List, Optional, Set
 
-from .markers import Marker
-from .specifiers import LegacySpecifier, Specifier, SpecifierSet
+from ._parser import parse_named_requirement
+from ._tokenizer import ParseExceptionError
+from .markers import InvalidMarker, Marker
+from .specifiers import SpecifierSet
+
+_RequirementTuple = namedtuple(
+    "_RequirementTuple", ["name", "url", "extras", "specifier", "marker"]
+)
 
 
 class InvalidRequirement(ValueError):
@@ -32,11 +37,9 @@ class Requirement:
 
     def __init__(self, requirement_string: str) -> None:
         try:
-            req = REQUIREMENT.parseString(requirement_string)
-        except ParseException as e:
-            raise InvalidRequirement(
-                f'Parse error at "{ requirement_string[e.loc : e.loc + 8]!r}": {e.msg}'
-            )
+            req = _RequirementTuple(*parse_named_requirement(requirement_string))
+        except ParseExceptionError as e:
+            raise InvalidRequirement(str(e))
 
         self.name: str = req.name
         if req.url:
@@ -51,9 +54,12 @@ def __init__(self, requirement_string: str) -> None:
             self.url: Optional[str] = req.url
         else:
             self.url = None
-        self.extras: Set[str] = set(req.extras.asList() if req.extras else [])
+        self.extras: Set[str] = set(req.extras if req.extras else [])
         self.specifier: SpecifierSet = SpecifierSet(req.specifier)
-        self.marker: Optional[Marker] = req.marker if req.marker else None
+        try:
+            self.marker: Optional[Marker] = Marker(req.marker) if req.marker else None
+        except InvalidMarker as e:
+            raise InvalidRequirement(str(e))
 
     def __str__(self) -> str:
         parts: List[str] = [self.name]

tests/test_markers.py

@@ -10,10 +10,10 @@
 
 import pytest
 
+from packaging._parser import Node
 from packaging.markers import (
     InvalidMarker,
     Marker,
-    Node,
     UndefinedComparison,
     default_environment,
     format_full_version,
@@ -61,11 +61,11 @@ class TestNode:
     def test_accepts_value(self, value):
         assert Node(value).value == value
 
-    @pytest.mark.parametrize("value", ["one", "two", None, 3, 5, []])
+    @pytest.mark.parametrize("value", ["one", "two"])
     def test_str(self, value):
         assert str(Node(value)) == str(value)
 
-    @pytest.mark.parametrize("value", ["one", "two", None, 3, 5, []])
+    @pytest.mark.parametrize("value", ["one", "two"])
     def test_repr(self, value):
         assert repr(Node(value)) == f"<Node({str(value)!r})>"
 
@@ -165,6 +165,7 @@ def test_parses_valid(self, marker_string):
             "python_version",
             "(python_version)",
             "python_version >= 1.0 and (python_version)",
+            '(python_version == "2.7" and os_name == "linux"',
         ],
     )
     def test_parses_invalid(self, marker_string):

tests/test_requirements.py

@@ -5,7 +5,7 @@
 import pytest
 
 from packaging.markers import Marker
-from packaging.requirements import URL, URL_AND_MARKER, InvalidRequirement, Requirement
+from packaging.requirements import InvalidRequirement, Requirement
 from packaging.specifiers import SpecifierSet
 
 
@@ -60,17 +60,34 @@ def test_name_with_version(self):
         self._assert_requirement(req, "name", specifier=">=3")
 
     def test_with_legacy_version(self):
-        with pytest.raises(InvalidRequirement):
+        with pytest.raises(InvalidRequirement) as e:
             Requirement("name==1.0.org1")
+        assert "Expected semicolon (followed by markers) or end of string" in str(e)
 
     def test_with_legacy_version_and_marker(self):
-        with pytest.raises(InvalidRequirement):
+        with pytest.raises(InvalidRequirement) as e:
             Requirement("name>=1.x.y;python_version=='2.6'")
+        assert "Expected semicolon (followed by markers) or end of string" in str(e)
+
+    def test_missing_name(self):
+        with pytest.raises(InvalidRequirement) as e:
+            Requirement("@ http://example.com")
+        assert "Expression must begin with package name" in str(e)
+
+    def test_name_with_missing_version(self):
+        with pytest.raises(InvalidRequirement) as e:
+            Requirement("name>=")
+        assert "Missing version" in str(e)
 
     def test_version_with_parens_and_whitespace(self):
         req = Requirement("name (==4)")
         self._assert_requirement(req, "name", specifier="==4")
 
+    def test_version_with_missing_closing_paren(self):
+        with pytest.raises(InvalidRequirement) as e:
+            Requirement("name(==4")
+        assert "Closing right parenthesis is missing" in str(e)
+
     def test_name_with_multiple_versions(self):
         req = Requirement("name>=3,<2")
         self._assert_requirement(req, "name", specifier="<2,>=3")
@@ -79,6 +96,22 @@ def test_name_with_multiple_versions_and_whitespace(self):
         req = Requirement("name >=2, <3")
         self._assert_requirement(req, "name", specifier="<3,>=2")
 
+    def test_name_with_multiple_versions_in_parenthesis(self):
+        req = Requirement("name (>=2,<3)")
+        self._assert_requirement(req, "name", specifier="<3,>=2")
+
+    def test_name_with_no_extras_no_versions_in_parenthesis(self):
+        req = Requirement("name []()")
+        self._assert_requirement(req, "name", specifier="", extras=[])
+
+    def test_name_with_extra_and_multiple_versions_in_parenthesis(self):
+        req = Requirement("name [foo, bar](>=2,<3)")
+        self._assert_requirement(req, "name", specifier="<3,>=2", extras=["foo", "bar"])
+
+    def test_name_with_no_versions_in_parenthesis(self):
+        req = Requirement("name ()")
+        self._assert_requirement(req, "name", specifier="")
+
     def test_extras(self):
         req = Requirement("foobar [quux,bar]")
         self._assert_requirement(req, "foobar", extras=["bar", "quux"])
@@ -87,16 +120,27 @@ def test_empty_extras(self):
         req = Requirement("foo[]")
         self._assert_requirement(req, "foo")
 
+    def test_unclosed_extras(self):
+        with pytest.raises(InvalidRequirement) as e:
+            Requirement("foo[")
+        assert "Closing square bracket is missing" in str(e)
+
+    def test_extras_without_comma(self):
+        with pytest.raises(InvalidRequirement) as e:
+            Requirement("foobar[quux bar]")
+        assert "Missing comma after extra" in str(e)
+
     def test_url(self):
-        url_section = "@ http://example.com"
-        parsed = URL.parseString(url_section)
-        assert parsed.url == "http://example.com"
+        url_section = "test @ http://example.com"
+        req = Requirement(url_section)
+        self._assert_requirement(req, "test", "http://example.com", extras=[])
 
     def test_url_and_marker(self):
-        instring = "@ http://example.com ; os_name=='a'"
-        parsed = URL_AND_MARKER.parseString(instring)
-        assert parsed.url == "http://example.com"
-        assert str(parsed.marker) == 'os_name == "a"'
+        instring = "test @ http://example.com ; os_name=='a'"
+        req = Requirement(instring)
+        self._assert_requirement(
+            req, "test", "http://example.com", extras=[], marker='os_name == "a"'
+        )
 
     def test_invalid_url(self):
         with pytest.raises(InvalidRequirement) as e:
@@ -147,6 +191,11 @@ def test_invalid_marker(self):
         with pytest.raises(InvalidRequirement):
             Requirement("name; foobar=='x'")
 
+    def test_marker_with_missing_semicolon(self):
+        with pytest.raises(InvalidRequirement) as e:
+            Requirement('name[bar]>=3 python_version == "2.7"')
+        assert "Expected semicolon (followed by markers) or end of string" in str(e)
+
     def test_types(self):
         req = Requirement("foobar[quux]<2,>=3; os_name=='a'")
         assert isinstance(req.name, str)
@@ -190,9 +239,7 @@ def test_sys_platform_linux_in(self):
     def test_parseexception_error_msg(self):
         with pytest.raises(InvalidRequirement) as e:
             Requirement("toto 42")
-        assert "Expected stringEnd" in str(e.value) or (
-            "Expected string_end" in str(e.value)  # pyparsing>=3.0.0
-        )
+        assert "Expected semicolon (followed by markers) or end of string" in str(e)
 
     EQUAL_DEPENDENCIES = [
         ("packaging>20.1", "packaging>20.1"),