Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sys.argv is inferred wrongly as pylint's own values for sys.argv instead of Uninferrable #7710

Closed
MarcoGorelli opened this issue Nov 4, 2022 · 7 comments · Fixed by #9041 or #9093
Closed
Assignees
Labels
Astroid Related to astroid False Positive 🦟 A message is emitted but nothing is wrong with the code Needs astroid update Needs an astroid update (probably a release too) before being mergable
Milestone

Comments

@MarcoGorelli
Copy link
Contributor

MarcoGorelli commented Nov 4, 2022

Bug description

Make file t.py

import sys

a, b, c = sys.argv

print(a, b, c)

and run python t.py 1 2

Configuration

No response

Command used

pylint t.py

Pylint output

$ pylint t.py
************* Module t
t.py:1:0: C0114: Missing module docstring (missing-module-docstring)
t.py:3:0: W0632: Possible unbalanced tuple unpacking with sequence: left side has 3 label(s), right side has 2 value(s) (unbalanced-tuple-unpacking)

------------------------------------------------------------------
Your code has been rated at 3.33/10 (previous run: 0.00/10, +3.33)

Expected behavior

The W0632 on line 3 looks like a bug, as in this case sys.argv is ['t.py', '1', '2']

Pylint version

$ pylint --version
pylint 2.15.5
astroid 2.12.12
Python 3.10.6 | packaged by conda-forge | (main, Aug 22 2022, 20:35:26) [GCC 10.4.0]

OS / Environment

$ uname -a
Linux DESKTOP-U8OKFP3 5.10.102.1-microsoft-standard-WSL2 #1 SMP Wed Mar 2 00:30:59 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Additional dependencies

No response

@MarcoGorelli MarcoGorelli added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Nov 4, 2022
@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Nov 4, 2022

pylint a.py --enable=all --enable-all-extensions on the same code gives: left side has 3 label(s), right side has 4 value(s), so we infer the right value of sys.argv with the value given to pylint but it make no sense as we can't know what will be inside sys.argv when the code will be launched. We need to do a special case for this, and we might still raise a message like "Possible unbalanced tuple unpacking: we can't know what will be in sys.argv in advance".

@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Nov 4, 2022
@clavedeluna
Copy link
Collaborator

We need to do a special case for this, and we might still raise a message like "Possible unbalanced tuple unpacking: we can't know what will be in sys.argv in advance".

This can still be a unbalanced-tuple-unpacking msg but we can somehow pass the extra msg "we can't know what will be in sys.argv in advance" as an arg, right?

@Pierre-Sassoulas
Copy link
Member

Maybe astroid needs to raise an inference error on sys.argv. That would be more accurate and also probably fix pylint in this particular case because if we can't infer we don't raise to avoid false positives.

@eli-schwartz
Copy link

I think it definitely makes sense to not report an error here. The workaround to quell the error is replacing a, b, c = sys.argv with a = sys.argv[0]; b = sys.argv[1]; c = sys.argv[2], and there's no error for accessing indexes that may not exist...

Maybe related: my first attempt to solve this was: foo, bar = sys.argv[1:3], which is quite explicit that there must be enough values there. Pylint doesn't detect that the right side has two values (or else will ValueError: not enough values to unpack at runtime). Instead it still tells me that "right side has 1 value". :D I would have expected the explicit slicing to result in a defined sequence with known length.

@Pierre-Sassoulas Pierre-Sassoulas changed the title Incorrect W0632 when unpacking sys.argv sys.argv is inferred wrongly as pylint's own values for sys.argv instead of Uninferrable Mar 1, 2023
@Pierre-Sassoulas Pierre-Sassoulas added Astroid Related to astroid Needs astroid update Needs an astroid update (probably a release too) before being mergable and removed Needs specification 🔐 Accepted as a potential improvement, and needs to specify edge cases, message names, etc. labels Mar 1, 2023
@Pierre-Sassoulas
Copy link
Member

This require a change in astroid to infer correctly sys.argv as uninferrable.

Using argv directly instead of letting argparse or click or a new fancy parsing library I never heard about do the parsing could be another check in itself imo : if sys.argv is accessed directly it means you're not going to have the same level of support for errors than a dedicated library, and argparse is a builtin lib since python 3.2.

@eli-schwartz
Copy link

Yup, agreed.

That aligns with my use case: if I was writing a tool that needed to do parsing of user arguments I'd use argparse or optparse because stdlib, but in this case I actually want to generate a script at runtime and then execute it with a different python interpreter than the current one. It's not user facing, I always know exactly how many arguments are internally passed, so I feel confident in not bothering with even checking the length, let alone using named flag-arguments.

@Timmmm
Copy link

Timmmm commented Jul 7, 2023

This was a very frustrating to debug. Adding an extra Python file to my project caused this error in a completely different Python file that previously was fine.

I would disable it by default until this is fixed.

Pierre-Sassoulas added a commit to pylint-dev/astroid that referenced this issue Jul 8, 2023
It's impossible to infer the value it will have outside of static analysis where it's our own value.

See pylint-dev/pylint#7710
Pierre-Sassoulas added a commit to pylint-dev/astroid that referenced this issue Jul 8, 2023
It's impossible to infer the value it will have outside of static analysis where it's our own value.

See pylint-dev/pylint#7710
schopin-pro added a commit to schopin-pro/apport that referenced this issue Jul 12, 2023
This line bugs out once in a while in pylint ever since we enabled
parallelism. Since the parallel gains are substantial, let's just
disable the check there.

See pylint-dev/pylint#7710
bdrung pushed a commit to canonical/apport that referenced this issue Jul 12, 2023
This line bugs out once in a while in pylint ever since we enabled
parallelism. Since the parallel gains are substantial, let's just
disable the check there.

See pylint-dev/pylint#7710
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 17, 2023
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 17, 2023
@Pierre-Sassoulas Pierre-Sassoulas self-assigned this Sep 17, 2023
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.17.6 milestone Sep 17, 2023
jacobtylerwalls pushed a commit to pylint-dev/astroid that referenced this issue Sep 23, 2023
It's impossible to infer the value it will have outside of static analysis where it's our own value.

See pylint-dev/pylint#7710
jacobtylerwalls pushed a commit to jacobtylerwalls/astroid that referenced this issue Sep 23, 2023
It's impossible to infer the value it will have outside of static analysis where it's our own value.

See pylint-dev/pylint#7710

(cherry picked from commit ea78827)
jacobtylerwalls added a commit to pylint-dev/astroid that referenced this issue Sep 23, 2023
…never is (#2244) (#2297)

* Make `sys.argv` uninferable because it never is (#2244)

It's impossible to infer the value it will have outside of static analysis where it's our own value.

See pylint-dev/pylint#7710

(cherry picked from commit ea78827)
Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 24, 2023
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Astroid Related to astroid False Positive 🦟 A message is emitted but nothing is wrong with the code Needs astroid update Needs an astroid update (probably a release too) before being mergable
Projects
None yet
5 participants