Requirement('installer') errors after 2bd5da391c302f2f5a18fd3e2bd9fb3b75c02e34

[KOLANICH]

from packaging.requirements import Requirement
Requirement('installer')

The regression has been introduced in 2bd5da391c302f2f5a18fd3e2bd9fb3b75c02e34.

[pradyunsg]

OK, I spent a bit of time looking at trying to fuzz our tests for the tokeniser with hypothesis, but I've hit multiple issues with trying to use hypothesis/hypofuzz at this point.

It's really cool tho, especially with the pytrace integration. :)

[pradyunsg]

FWIW, #619 has been significantly increased in size and scope.

[pradyunsg]

Well, here's another regression:

❯ python -c "import packaging; print(packaging.__version__); from packaging.requirements import Requirement; print(vars(Requirement(\"  name == 3.0.*  \")))"
21.4.dev0
IDENTIFIER   name
OP  ==
VERSION  3.0.*
Traceback (most recent call last):
  File "/Users/pradyunsg/Developer/github/packaging/packaging/requirements.py", line 35, in __init__
    req = parse_named_requirement(requirement_string)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pradyunsg/Developer/github/packaging/packaging/_parser.py", line 79, in parse_named_requirement
    specifier = parse_specifier(tokens)
                ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pradyunsg/Developer/github/packaging/packaging/_parser.py", line 119, in parse_specifier
    parsed_specifiers = parse_version_many(tokens)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pradyunsg/Developer/github/packaging/packaging/_parser.py", line 136, in parse_version_many
    if not tokens.match("COMMA"):
           ^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pradyunsg/Developer/github/packaging/packaging/_tokenizer.py", line 102, in match
    token = self.peek()
            ^^^^^^^^^^^
  File "/Users/pradyunsg/Developer/github/packaging/packaging/_tokenizer.py", line 95, in peek
    self.next_token = next(self.generator)
                      ^^^^^^^^^^^^^^^^^^^^
  File "/Users/pradyunsg/Developer/github/packaging/packaging/_tokenizer.py", line 163, in _tokenize
    raise self.raise_syntax_error(message="Unrecognized token")
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/pradyunsg/Developer/github/packaging/packaging/_tokenizer.py", line 138, in raise_syntax_error
    raise ParseExceptionError(
packaging._tokenizer.ParseExceptionError: Unrecognized token
at position 15:
      name == 3.0.*  
                   ^

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/pradyunsg/Developer/github/packaging/packaging/requirements.py", line 37, in __init__
    raise InvalidRequirement(str(e))
packaging.requirements.InvalidRequirement: Unrecognized token
at position 15:
      name == 3.0.*  
                   ^
❯ python -c "import packaging; print(packaging.__version__); from packaging.requirements import Requirement; print(vars(Requirement(\"  name == 3.0.*  \")))"
21.3
{'name': 'name', 'url': None, 'extras': set(), 'specifier': <SpecifierSet('==3.0.*')>, 'marker': None}

[pradyunsg]

I've started a parser rewrite, to couple the parsing and tokenising such that tokenising is context-sensitive.

[hrnciar]

I apologise for the issues that caused my contribution. I'd be happy to address them had I known about them. Since nobody mentioned me sooner, I learned about the problems during the weekend after @pradyunsg already spend time rewriting it and opened #624.

[pradyunsg]

@hrnciar There's no need to apologise. My work in that PR is very-much based off of your work, and I certainly wouldn't have gotten as far as I have without the existing base that you built! :)

I was already looking improving the error messages around requirement parsing back (partly what motivated this parser rewrite for me), so it was fairly organic to dive into this for me.

[pradyunsg]

On a different note... what do we want to do with strings like:

package(===arbitrarystring)

The behaviours we have:

(21.3)

❯ echo "package(===arbitrarystring)" | python -c "import packaging; from packaging.requirements import Requirement; print(packaging.__version__); print(vars(Requirement(input())))"
21.3
Traceback (most recent call last):
  [snip]
packaging.requirements.InvalidRequirement: Parse error at "'(===arbi'": Expected string_end

(main)

❯ echo "package(===arbitrarystring)" | python -c "import packaging; from packaging.requirements import Requirement; print(packaging.__version__); print(vars(Requirement(input())))"
21.4.dev0
Traceback (most recent call last):
  [snip]
packaging.requirements.InvalidRequirement: Closing right parenthesis is missing
at position 27:
    package(===arbitrarystring)
                               ^

And... I have two options:

(a) Error out:

❯ echo "package(===arbitrarystring)" | python -c "import packaging; from packaging.requirements import Requirement; print(packaging.__version__); print(vars(Requirement(input())))"
21.4.dev0
Traceback (most recent call last):
  [snip]
packaging.requirements.InvalidRequirement: Expected closing RIGHT_PARENTHESIS
    package(===arbitrarystring)
           ~~~~~~~~~~~~~~~~~~~~^

(b) Modify how Specifier parses arbitrary versions strings to parse this (and package ( === arbitrarystring )) as package===arbitrarystring

❯ echo "package(===arbitrarystring)" | python -c "import packaging; from packaging.requirements import Requirement; print(packaging.__version__); print(vars(Requirement(input())))"
21.4.dev0
{'name': 'package', 'url': None, 'extras': set(), 'specifier': <SpecifierSet('===arbitrarystring')>, 'marker': None}

[pradyunsg]

I'm inclined to go with option (b).

[pradyunsg]

The same situation as ) happens with ; for markers.

[brettcannon]

If someone messes up their specifier so it isn't valid, how will they know that with option (b)? Will it be based on the fact that the specifier will never be true?

[pradyunsg]

This is only an issue with ===, along with ) or ;. Currently, they both error out if you don't have whitespace after the arbitrary string; even though version strings are unlikely to contain ; or ).

I can't imagine someone messing up their version specifier in a manner that makes it difficult to understand what's happening TBH.

[pradyunsg]

FWIW, 39ae524 (#624) is the relevant change for fixing how this parses things.