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

unused-variable false-positive regression for argparse.Namespace #6895

Closed
The-Compiler opened this issue Jun 8, 2022 · 3 comments · Fixed by #6898
Closed

unused-variable false-positive regression for argparse.Namespace #6895

The-Compiler opened this issue Jun 8, 2022 · 3 comments · Fixed by #6898
Assignees
Labels
Astroid Related to astroid Bug 🪲 False Positive 🦟 A message is emitted but nothing is wrong with the code
Milestone

Comments

@The-Compiler
Copy link
Contributor

Bug description

With this file, there is a weird unused-variable:

# pylint: disable=missing-module-docstring,missing-class-docstring,missing-function-docstring,too-few-public-methods

import argparse

class Cls:

    def meth(self):
        return argparse.Namespace(debug=True)

seems to work fine with the function outside of the class...

Bisected to e01fa86 ("Remove the concept of checker priority (#6034)"), cc @DanielNoord

Configuration

No response

Command used

pylint x.py

Pylint output

************* Module x
x.py:8:0: W0612: Unused variable 'Namespace' (unused-variable)

Expected behavior

No errors

Pylint version

pylint 2.14.1
astroid 2.11.5
Python 3.10.5 (main, Jun  6 2022, 18:49:26) [GCC 12.1.0]

OS / Environment

Archlinux

Additional dependencies

No response

@The-Compiler The-Compiler added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jun 8, 2022
@DanielNoord
Copy link
Collaborator

DanielNoord commented Jun 8, 2022

@jacobtylerwalls This is a strange one. There seems to be inter-dependency between the variables checker and the special_methods_checker.

Reproducer with this diff:

diff --git a/pylint/checkers/classes/special_methods_checker.py b/pylint/checkers/classes/special_methods_checker.py
index 9a7c9a762..5c60e6637 100644
--- a/pylint/checkers/classes/special_methods_checker.py
+++ b/pylint/checkers/classes/special_methods_checker.py
@@ -132,7 +132,7 @@ class SpecialMethodsChecker(BaseChecker):
             "of the form tuple(tuple, dict)",
         ),
     }
-    priority = -2
+    priority = -99
 
     def __init__(self, linter=None):
         super().__init__(linter)
diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py
index ef6a5369f..4cb7059d0 100644
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -984,7 +984,7 @@ class VariablesChecker(BaseChecker):
 
     name = "variables"
     msgs = MSGS
-    priority = -1
+    priority = -999
     options = (
         (
             "init-import",
@@ -1264,6 +1264,7 @@ class VariablesChecker(BaseChecker):
                 self._store_type_annotation_node(argument_annotation)
 
         not_consumed = self._to_consume.pop().to_consume
+        print(not_consumed)
         if not (
             self.linter.is_message_enabled("unused-variable")
             or self.linter.is_message_enabled("possibly-unused-variable")
git checkout f46904ab9b058d8506b87cedd1f0c710d5c19360git apply diffpylint test.py
************* Module test
test.py:7:4: R0201: Method could be a function (no-self-use)
{'self': [<AssignName.self l.7 at 0x102644820>], 'Namespace': [<ClassDef.Namespace l.8 at 0x102710610>]}
test.py:8:0: W0612: Unused variable 'Namespace' (unused-variable)

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

Then change the priority of special_methods to -9999 so that it is lower than variables and is executed after it:

pylint test.py
************* Module test
test.py:7:4: R0201: Method could be a function (no-self-use)
{'self': [<AssignName.self l.7 at 0x107fe8820>]}

------------------------------------------------------------------
Your code has been rated at 7.50/10 (previous run: 5.00/10, +2.50)

Do you have any idea how/why these checkers are connected? I did some quick looking around but couldn't immediately find why special_methods would add Namespace to the list of consumable nodes if executed first...

@DanielNoord DanielNoord added Bug 🪲 False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jun 8, 2022
@jacobtylerwalls jacobtylerwalls added the Astroid Related to astroid label Jun 9, 2022
@jacobtylerwalls
Copy link
Member

It's another manifestation of pylint-dev/astroid#1490. I'll open a backportable PR in astroid for the argparse.Namespace brain like to quickly patch it like we've been doing elsewhere (#5982), but maybe this is enough evidence that pylint-dev/astroid#1490 is a real problem.


why special_methods would add Namespace to the list of consumable nodes if executed first...

The special methods checker uses inference, and so we hit the argparse.Namespace brain, and that chokes on pylint-dev/astroid#1490 and incorrectly adds the parent's name (argparse) to the child's locals, which is how it ends up as a consumable node.

@DanielNoord
Copy link
Collaborator

Thanks for the investigation @jacobtylerwalls!

@DanielNoord DanielNoord added this to the 2.14.2 milestone Jun 9, 2022
Pierre-Sassoulas added a commit that referenced this issue Jun 13, 2022
* Upgrade astroid to 2.11.6

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Jun 15, 2022
* Upgrade astroid to 2.11.6

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Pierre-Sassoulas added a commit that referenced this issue Jun 15, 2022
* Upgrade astroid to 2.11.6

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Astroid Related to astroid Bug 🪲 False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants