pylint icon indicating copy to clipboard operation
pylint copied to clipboard

Pylint no longer support separating options from positional arguments with --

Open phst opened this issue 2 years ago • 6 comments

Bug description

With Pylint 2.12, pylint -- file.py works as expected, with Pylint 2.14 it raises an error like "pylint: error: Unrecognized option found:". I guess that's related to the argparse migration.

Configuration

No response

Command used

pylint -- file.py

Pylint output

pylint: error: Unrecognized option found:

Expected behavior

The -- should separate options from positional arguments. In this case the invocation should have identical results to pylint file.py.

Pylint version

pylint 2.14.3
astroid 2.11.6
Python 3.9.12 (main, May  6 2022, 20:15:26) 
[GCC 11.2.0]

OS / Environment

Debian GNU/Linux

Additional dependencies

No response

phst avatar Jun 22 '22 15:06 phst

Thank you for opening the issue.

I added the discussion label because I don't know if we want to deviate from what argparse is doing and if it's worth it. In your example it's possible to use pylint file.py directly, right ? What is the standard we're talking about here, is it just something that optparse was doing or is it something well established for CLI ?

Pierre-Sassoulas avatar Jun 22 '22 18:06 Pierre-Sassoulas

Thank you for opening the issue.

I added the discussion label because I don't know if we want to deviate from what argparse is doing and if it's worth it.

Argpase supports this syntax. From https://docs.python.org/3/library/argparse.html#arguments-containing:

you can insert the pseudo-argument '--' which tells parse_args() that everything after that is a positional argument

So this should work out of the box, and I'm wondering why it doesn't.

In your example it's possible to use pylint file.py directly, right ?

yes, the real use case is e.g. a script that wraps Pylint and receives arbitrary filenames, which might start with a dash.

What is the standard we're talking about here, is it just something that optparse was doing or is it something well established for CLI ?

It's quite established, see e.g. the GNU Coding Standards (https://www.gnu.org/prep/standards/html_node/Command_002dLine-Interfaces.html) or POSIX (https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02, guideline 10).

phst avatar Jun 22 '22 18:06 phst

I don't think this is fixed.

$ pylint --version
pylint 2.15.2
astroid 2.12.9
Python 3.10.6 (main, Aug 30 2022, 04:58:14) [Clang 13.1.6 (clang-1316.0.21.2.5)]

$ pylint -- build.py 
************* Module --
--:1: [F0001(fatal), ] No module named --

That is, Pylint now thinks that it should lint a file named --. Instead, it should skip the -- argument.

phst avatar Sep 17 '22 10:09 phst

Hm, that's weird. My test might not be good enough.

Putting this on my TODO list.

DanielNoord avatar Sep 17 '22 11:09 DanielNoord

@Pierre-Sassoulas Based on previous knowledge I think this should be a quick fix. If possible I'd like to get this into the patch release as well. I'll tell you after fixing the open PR if I can get a quick PR in for this as well!

DanielNoord avatar Sep 18 '22 10:09 DanielNoord

I[m going to look into another solution which will take some more time. Moving this from the milestone.

DanielNoord avatar Sep 19 '22 11:09 DanielNoord

@phst https://github.com/PyCQA/pylint/pull/7551 should fix this correctly now. If you want you can check that branch out and see if it works locally for you!

DanielNoord avatar Oct 01 '22 19:10 DanielNoord

Thanks but this still doesn't work. The purpose of the -- separator is to pass positional arguments that start with dashes and not have them confused with options. This doesn't work any more:

$ pip3 install https://github.com/PyCQA/pylint/archive/refs/heads/main.zip
[...]
$ touch -- -test.py
$ pylint -- -test.py
usage: pylint [options]
pylint: error: Unrecognized option found: test.py

phst avatar Oct 02 '22 08:10 phst

Can you give an example of how you would actually use this? The issue is that pylint currently accepts "files" to be scattered around the command line invocation. So pylint file1.py --an-option file2.py --another-option=yes file3.py works. We do so be some special code that comes after the argparse parsing of arguments. This breaks your requested behaviour, but I'm not sure we can support both patterns. A good example will help me write an actually useful test to fix this.

DanielNoord avatar Oct 02 '22 08:10 DanielNoord

Can you give an example of how you would actually use this?

The concrete case is https://github.com/phst/rules_elisp/blob/b79e86bc6adfdbba2576129f0a48e120afccd1c3/check_python.py#L83, but the general thinking is that wrapper scripts in general can't and shouldn't know whether filenames start with a dash or not. For example, a wrapper script like

files=(...)
pylint --foo=bar -- "${files[@]}"

shouldn't have to parse the argument list passed by the user or fail with weird errors (or worse, silently pass undesired options) if one of the files happens to start with a dash. One reason for the -- convention is to provide for an "obviously correct" syntax where the role of each argument is already clear from the syntax.

The issue is that pylint currently accepts "files" to be scattered around the command line invocation.

Maybe I'm missing something, but doesn't ArgumentParser.parse_intermixed_args do exactly that?

>>> p = argparse.ArgumentParser()
>>> p.add_argument('files', nargs='+')
>>> p.add_argument('--an-option', action='store_true')
>>> p.add_argument('--another-option')
>>> p.parse_intermixed_args('file1.py --an-option file2.py --another-option=yes file3.py -- -test.py'.split())
Namespace(an_option=True, another_option='yes', files=['file1.py', 'file2.py', 'file3.py', '-test.py'])

phst avatar Oct 02 '22 09:10 phst

The issue is that we don't have the files argument. So we can't collect nargs='+' into it. That is also why argparse doesn't solve this automatically for us: we parse with parse_known_args but it doesn't know to collect -test.py into a files argument.

I'm not sure we can add the files argument with nargs without it breaking other workflows. https://github.com/PyCQA/pylint/pull/7496/ is a hack around it, but it still doesn't collect nargs.

DanielNoord avatar Oct 02 '22 19:10 DanielNoord