Fix undefined-variable false positive with metaclass in nested class#10853
Conversation
Pierre-Sassoulas
added
False Positive 🦟
Pierre-Sassoulas
left a comment
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
The two commits seems atomic, we could open a PR for each one, or the description needs to be adapted.
| Fix ``used-before-assignment`` false negative when a variable has a bare type | ||
| annotation (without a value) and is only assigned inside ``except`` blocks. | ||
|
|
||
| Closes #10847 |
There was a problem hiding this comment.
This issue isn't going to be closed because it doesn't appear in the PR description.
There was a problem hiding this comment.
The #10847 fix was split out into #10852 (the consumed_uncertain approach for bare type annotations). This PR now only addresses #10823 — the metaclass false positive. The changelog fragment here is 10823.false_positive, which matches the PR description. The 10847.false_negative fragment you're seeing is from the old two-commit version before the rebase.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10853 +/- ##
=======================================
Coverage 96.03% 96.04%
=======================================
Files 177 177
Lines 19621 19622 +1
=======================================
+ Hits 18844 18845 +1
Misses 777 777
🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
When an imported name was used as a `metaclass=` argument inside a nested class within a method, pylint incorrectly flagged subsequent uses of that name at module level as `undefined-variable`. Root cause: `_check_classdef_metaclasses()` found the name in enclosing scopes and `_check_metaclasses()` popped it directly from `to_consume`, making it completely unresolvable. Subsequent references to the same name triggered `undefined-variable`. Fix: use `NamesConsumer.mark_as_consumed()` instead of raw dict pop. This properly moves the name from `to_consume` to `consumed`, so: 1. The import is not flagged as unused (original intent) 2. Later references find the name in `consumed` and resolve correctly Closes pylint-dev#10823
worksbyfriday
force-pushed
the
fix-undefined-variable-metaclass-nested
branch
from
5dd2a04 to
bdc4575
Compare
|
The primer results show new emissions in ansible's This is likely because my fix allows pylint to successfully process files that previously triggered a false positive The new emissions are all legitimate findings, not regressions from this change. |
|
@jacobtylerwalls I think you're the expert on this part of the codebase. Perhaps you can review this? We can also take the code out into a separate PR to not have to deal with this overly talkative AI bot if the code itself is sound. |
jacobtylerwalls
left a comment
jacobtylerwalls
left a comment
There was a problem hiding this comment.
Thanks, looks correct ⭐
Pierre-Sassoulas
temporarily deployed
to
Backport
GitHub Actions
Inactive
…10853) When an imported name was used as a `metaclass=` argument inside a nested class within a method, pylint incorrectly flagged subsequent uses of that name at module level as `undefined-variable`. Root cause: `_check_classdef_metaclasses()` found the name in enclosing scopes and `_check_metaclasses()` popped it directly from `to_consume`, making it completely unresolvable. Subsequent references to the same name triggered `undefined-variable`. Fix: use `NamesConsumer.mark_as_consumed()` instead of raw dict pop. This properly moves the name from `to_consume` to `consumed`, so: 1. The import is not flagged as unused (original intent) 2. Later references find the name in `consumed` and resolve correctly Closes #10823 (cherry picked from commit f56e41a)
…10853) When an imported name was used as a `metaclass=` argument inside a nested class within a method, pylint incorrectly flagged subsequent uses of that name at module level as `undefined-variable`. Root cause: `_check_classdef_metaclasses()` found the name in enclosing scopes and `_check_metaclasses()` popped it directly from `to_consume`, making it completely unresolvable. Subsequent references to the same name triggered `undefined-variable`. Fix: use `NamesConsumer.mark_as_consumed()` instead of raw dict pop. This properly moves the name from `to_consume` to `consumed`, so: 1. The import is not flagged as unused (original intent) 2. Later references find the name in `consumed` and resolve correctly Closes #10823 (cherry picked from commit f56e41a)
PR: pylint-dev#10863 Issue: pylint-dev#10853 Base commit: 88e1ab7 Changed lines: 42
PR: pylint-dev#10853 Issue: pylint-dev#10823 Base commit: d2dc5df Changed lines: 42
4 participants
Description
Fixes #10823
Closes #10823
When an imported name is used as a
metaclass=argument inside a nested class within a method, pylint incorrectly flags subsequent uses of that name at module level asundefined-variable.Reproducer
Root cause
_check_classdef_metaclasses()walks enclosing scopes to find the name used as a metaclass and collects(scope_locals_dict, name)tuples. Then_check_metaclasses()callsscope_locals.pop(name, None)— removing the name entirely fromto_consumein the enclosing scope.This was intended to prevent unused-import/unused-variable false positives: the metaclass usage should "count" as using the name. But
pop()removes the name without moving it to the consumer'sconsumeddict. Later references to the same name can't find it in eitherto_consumeorconsumed, so pylint reportsundefined-variable.Fix
Replace the raw
scope_locals.pop()withNamesConsumer.mark_as_consumed(). This properly moves the name fromto_consumetoconsumed, preserving both behaviors:to_consume)consumed(noundefined-variable)The function signatures change to pass the
NamesConsumerandfound_nodesneeded bymark_as_consumed().Test
Added
tests/functional/u/undefined/undefined_variable_metaclass_nested.py— reproducer from the issue.