Leverage existing requirements.txt parsing logic to close #225

[vreuter]

Once aboutcode-org/pip-requirements-parser#17 is merged, we could more directly use pip-requirements-parser (namely, the RequirementsParser.from_string method). Until then, this works around a NameError that comes up when using the less common from_string method in the RequirementsFile class in that project.

[jherland]

Hmm. I took a closer look at pip-requirements-parser's own code, and AFAICS it boils down to a single Python script of almost 3000 lines (😱), the vast majority of which is copied directly from pip itself. After reading through the project's own README, my initial WTF!? reaction is somewhat softened, and the project argues well for why things are done this way.

Still, what I don't like about this integration here is having to write out a temporary file to parse some requirements text, when this text (with the sole exception of parsing setup.cfg) already comes directly from a file!

However, fixing this requires firrst some moderate refactoring in our extract_declared_dependencies.py to adjust our various parsers to work on Paths directly rather than str + Location arguments.

Coincidentally, this also frees up our parser to decide for themselves exactly how to read the given file (i.e. they may choose something more appropriate than Path.read_text(), depending on their context. I think this refactoring might be a net win overall, and worth doing in any case...

After this refactoring parse_requirements_contents() would be reduced to a single call to RequirementsFile.from_file(filename=str(path), include_nested=False).requirements.

Then there is the special case of parse_setup_cfg_contents() which also leans on parse_requirements_contents() to parse its requirements values, but given that requirements.txt probably outnumber setup.cfg files 1000:1, I'd rather put the clumsy workaround (writing a temp file) inside parse_setup_cfg_contents() rather than inside parse_requirements_contents().

I started looking at this refactoring, and I'd rather you rebuild this PR on top of that, instead of having to write temporary files for every single requirements.txt file we will ever parse... Will post a branch as soon as I got something that passes tests...

[jherland]

I started looking at this refactoring, and I'd rather you rebuild this PR on top of that, instead of having to write temporary files for every single requirements.txt file we will ever parse... Will post a branch as soon as I got something that passes tests...

#229

[jherland]

Looks great! Two things missing:

• An added test (demonstrating Line continuations aren't fully supported for per-requirement options in requirements.txt #225) that is fixed by this PR.
• A comment/consideration on how to possibly make the setup.cfg situation better.

For the second one, I've been looking at pip-requirements-parser to see if there is anything in there we could benefit from¹.

AFAICS, we have to dig all the way in to RequirementsFileParser._parse_file() before we've actually opened and read the file. so a cleaner implementation of RequirementsFile.from_string() would have to copy all but the first line of ._parse_file()...

Footnotes

1. Even if we were to call into its internal functions, this could still be considered acceptable as we can pin our use of pip-requirements-parser (and control when we upgrade it) as opposed to pip itself where we are only given whatever is available on the user's machine. ↩

[vreuter]

Looks great! Two things missing:

* An added test (demonstrating #225) that is fixed by this PR.

* A comment/consideration on how to possibly make the `setup.cfg` situation better.

Thanks for reviewing @jherland !
The test I add here: 0dac8e8
And the comment I add here: dab88d4