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

False positive used-before-assignment for filtered comprehensions in try blocks #5586

Closed
jacobtylerwalls opened this issue Dec 22, 2021 · 3 comments · Fixed by #5666
Closed
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code
Milestone

Comments

@jacobtylerwalls
Copy link
Member

Bug description

def func():
    try:
        print(value for value in range(1 / 0) if isinstance(value, int))  # BUG: emits used-before-assignment
    except ZeroDivisionError:
        value = 1

Configuration

No response

Command used

pylint a.py

Pylint output

************* Module a
/Users/.../a.py:3:60: E0601: Using variable 'value' before assignment (used-before-assignment)

Expected behavior

No message.

Here, found_nodes for the node inside the generator expression includes the assignment of value in the except block.
https://github.com/PyCQA/pylint/blob/c0fbbb7adbc39cbd67bbd0e0b1ca4e9421c12643/pylint/checkers/variables.py#L611-L620

I'm not sure if the responsibility for filtering that out lies in to_consume(); if not, I suppose the new used-before-assignment logic will have to deal with it.

Pylint version

Caused in bd55b27d41542e3ca1f031f986b6151f6cac457f (2.13.0 unreleased)

OS / Environment

No response

Additional dependencies

No response

@jacobtylerwalls jacobtylerwalls added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Dec 22, 2021
@Pierre-Sassoulas Pierre-Sassoulas added 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 Dec 22, 2021
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Dec 22, 2021
@DanielNoord
Copy link
Collaborator

@jacobtylerwalls This is one of the final issues in 2.13 without an associated PR. Would you consider taking a look at it before we release it? Feel free to say no obviously, I'm just going through any remaining open issues and PRs 😉

@jacobtylerwalls
Copy link
Member Author

I'll let you know this week if I think I can. Meanwhile, I was wondering if you had a gut feeling about this part without having to do much investigation straight away:

I'm not sure if the responsibility for filtering that out lies in to_consume()

And if so, is that in astroid? Or other related issues you know of with comprehension variables not being isolated enough by the variables checker?

@DanielNoord
Copy link
Collaborator

I'll let you know this week if I think I can.

Thanks!

Meanwhile, I was wondering if you had a gut feeling about this part without having to do much investigation straight away:

I'm not sure if the responsibility for filtering that out lies in to_consume()

And if so, is that in astroid? Or other related issues you know of with comprehension variables not being isolated enough by the variables checker?

diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py
index 66af3ec94..db9c812a7 100644
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -1316,6 +1316,7 @@ class VariablesChecker(BaseChecker):
             if utils.is_func_decorator(current_consumer.node) or not (
                 current_consumer.scope_type == "comprehension"
                 and self._has_homonym_in_upper_function_scope(node, consumer_level)
+                and not node.parent in node.parent.parent.ifs
             ):
                 self._check_late_binding_closure(node)
                 self._loopvar_name(node)

This raise unused-variable but I think could be a start. A little investigation showed that we do check the comprehension scope for a value name but we don't seem to find it. Then we move up to function and see that is defined after we try and access it.
The fix should thus probably be in _check_consumer and how it handles the ifs of comprehensions.

jacobtylerwalls added a commit to jacobtylerwalls/pylint that referenced this issue Jan 11, 2022
… homonyms in filtered comprehensions and except blocks
Pierre-Sassoulas added a commit that referenced this issue Jan 12, 2022
…in filtered comprehensions and except blocks (#5666)

Co-authored-by: Pierre Sassoulas <pierre.sassoulas@gmail.com>
jacobtylerwalls added a commit to jacobtylerwalls/pylint that referenced this issue Mar 29, 2022
…ltered comprehensions

The prior fixes in pylint-dev#5586 and pylint-dev#5817 still relied on assumptions of direct parentage.
jacobtylerwalls added a commit that referenced this issue Mar 31, 2022
…onym handling

The previous fixes for false positives involving homonyms with variables in comprehension tests in
#5586 and #5817 still relied on assumptions of direct parentage.
Pierre-Sassoulas pushed a commit that referenced this issue Mar 31, 2022
…onym handling

The previous fixes for false positives involving homonyms with variables in comprehension tests in
#5586 and #5817 still relied on assumptions of direct parentage.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
3 participants