Better errors when an invalid requirement is encountered

[woodruffw]

See #478: this isn't a full fix, but will hopefully improve the overall quality and debuggability of the errors produced.

[woodruffw]

Example with a subdependency:

$ pip-audit -r <(echo 'celery==4.4.7')
ERROR:pip_audit._cli:celery has invalid requirement 'pytz (>dev)': Expected closing RIGHT_PARENTHESIS
    pytz (>dev)
         ~^

The direct dependency error still isn't great:

$ pip-audit -r <(echo 'pytz (>dev)')
ERROR:pip_audit._cli:requirement file /dev/fd/63 contains invalid lines: [InvalidRequirementLine(requirement_line=RequirementLine(line_number=1, line='pytz (>dev)', filename=PosixPath('/dev/fd/63')), error_message='Invalid requirement: : Expected closing RIGHT_PARENTHESIS\n    pytz (>dev)\n         ~^')]

...so I'll fix that one up as well.

[woodruffw]

Well, it's still not fantastic, but this makes it a bit better:

$ pip-audit -r <(echo 'pytz (>dev)')
ERROR:pip_audit._cli:requirement file /dev/fd/63 contains invalid specifier at line 1: Invalid requirement: : Expected closing RIGHT_PARENTHESIS
    pytz (>dev)
         ~^

[woodruffw]

cc @norg: if you get a chance, please give these changes a spin and let us know what you think!

[norg]

This is already a nice improvement. But it still prevents the further audit of other packages :)
So if you use a requrements.txt with the celery but also a compliant one, you would still not get any report on that.

[woodruffw]

Yeah, I need to do some more thinking about how we should handle that...it's arguably incorrect of us to skip things just because we can't parse them, since there's probably a positive relationship between "old enough to have an invalid specifier" and "more likely to have known vulnerabilities."

[di]

...we also could continue supporting legacy versions: see https://github.com/di/packaging_legacy and pypa/packaging#669.

[woodruffw]

...we also could continue supporting legacy versions: see https://github.com/di/packaging_legacy and pypa/packaging#669.

That works for me, although I wonder if we should establish some kind of (weak) policy around these kinds of compatibility concerns. Three ideas:

1. "Forcing function": as it is, pip-audit serves as a forcing function for packaging ecosystem changes: we adopt changes to packaging and resolvelib before pip and others do, meaning that we make potentially breaking changes ahead of the rest of the ecosystem. This is good in terms of encouraging users to align with the standards, but makes adoption for entrenched users harder.
2. "Same as": we generally aim for CLI flag compatibility with pip, and maybe we should follow a similar policy around standards behavior: we should only adopt major incompatible changes when pip rolls them out. In that case, we should do what you suggested and use packaging_legacy, etc. to handle LegacyVersions here.
3. "Conservative": we could lag behind pip and other tooling, since pip-audit should help users with older codebases/dependency trees (as that's where we're especially likely to be useful). This however entails being more conservative than we currently are with e.g. pip support, which might be burdensome for a small tail of users (who we'd like to encourage to upgrade anyways).

[tetsuo-cpp]

LGTM!

[tetsuo-cpp]

As discussed, we can probably do better here. Either way, this is still a good improvement so let's get it in.

[woodruffw]

Yep! I'll keep #478 open so we can come up with a more permanent solution.

[norg]

Thanks again for working on that :)