mirror of
https://github.com/astral-sh/ruff.git
synced 2025-09-30 22:01:18 +00:00
Consider expression before statement when determining binding kind (#12346)
## Summary
I believe these should always bind more tightly -- e.g., in:
```python
for _ in bar(baz for foo in [1]):
pass
```
The inner `baz` and `foo` should be considered comprehension variables,
not for loop bindings.
We need to revisit this more holistically. In some of these cases,
`BindingKind` should probably be a flag, not an enum, since the values
aren't mutually exclusive. Separately, we should probably be more
precise in how we set it (e.g., by passing down from the parent rather
than sniffing in `handle_node_store`).
Closes https://github.com/astral-sh/ruff/issues/12339
This commit is contained in:
Charlie Marsh
GitHub
parent
b1487b6b4f
commit
d0c5925672
4 changed files with 111 additions and 97 deletions
|
|
@ -26,6 +26,16 @@ def func(_):
|
|||
...
|
||||
|
||||
|
||||
def func(foo):
|
||||
for _ in bar(foo for foo in [1]):
|
||||
pass
|
||||
|
||||
|
||||
def func(foo):
|
||||
for _ in bar((foo := 1) for foo in [1]):
|
||||
pass
|
||||
|
||||
|
||||
# Errors
|
||||
def func(a):
|
||||
for a in range(1):
|
||||
|
|
|
|||
|
|
@ -139,6 +139,10 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
|
|||
if checker.settings.dummy_variable_rgx.is_match(name) {
|
||||
continue;
|
||||
}
|
||||
let scope = &checker.semantic.scopes[binding.scope];
|
||||
if scope.kind.is_generator() {
|
||||
continue;
|
||||
}
|
||||
checker.diagnostics.push(Diagnostic::new(
|
||||
pylint::rules::RedefinedArgumentFromLocal {
|
||||
name: name.to_string(),
|
||||
|
|
|
|||
|
|
@ -1950,38 +1950,6 @@ impl<'a> Checker<'a> {
|
|||
flags.insert(BindingFlags::UNPACKED_ASSIGNMENT);
|
||||
}
|
||||
|
||||
// Match the left-hand side of an annotated assignment without a value,
|
||||
// like `x` in `x: int`. N.B. In stub files, these should be viewed
|
||||
// as assignments on par with statements such as `x: int = 5`.
|
||||
if matches!(
|
||||
parent,
|
||||
Stmt::AnnAssign(ast::StmtAnnAssign { value: None, .. })
|
||||
) && !self.semantic.in_annotation()
|
||||
{
|
||||
self.add_binding(id, expr.range(), BindingKind::Annotation, flags);
|
||||
return;
|
||||
}
|
||||
|
||||
// A binding within a `for` must be a loop variable, as in:
|
||||
// ```python
|
||||
// for x in range(10):
|
||||
// ...
|
||||
// ```
|
||||
if parent.is_for_stmt() {
|
||||
self.add_binding(id, expr.range(), BindingKind::LoopVar, flags);
|
||||
return;
|
||||
}
|
||||
|
||||
// A binding within a `with` must be an item, as in:
|
||||
// ```python
|
||||
// with open("file.txt") as fp:
|
||||
// ...
|
||||
// ```
|
||||
if parent.is_with_stmt() {
|
||||
self.add_binding(id, expr.range(), BindingKind::WithItemVar, flags);
|
||||
return;
|
||||
}
|
||||
|
||||
let scope = self.semantic.current_scope();
|
||||
|
||||
if scope.kind.is_module()
|
||||
|
|
@ -2051,6 +2019,38 @@ impl<'a> Checker<'a> {
|
|||
return;
|
||||
}
|
||||
|
||||
// Match the left-hand side of an annotated assignment without a value,
|
||||
// like `x` in `x: int`. N.B. In stub files, these should be viewed
|
||||
// as assignments on par with statements such as `x: int = 5`.
|
||||
if matches!(
|
||||
parent,
|
||||
Stmt::AnnAssign(ast::StmtAnnAssign { value: None, .. })
|
||||
) && !self.semantic.in_annotation()
|
||||
{
|
||||
self.add_binding(id, expr.range(), BindingKind::Annotation, flags);
|
||||
return;
|
||||
}
|
||||
|
||||
// A binding within a `for` must be a loop variable, as in:
|
||||
// ```python
|
||||
// for x in range(10):
|
||||
// ...
|
||||
// ```
|
||||
if parent.is_for_stmt() {
|
||||
self.add_binding(id, expr.range(), BindingKind::LoopVar, flags);
|
||||
return;
|
||||
}
|
||||
|
||||
// A binding within a `with` must be an item, as in:
|
||||
// ```python
|
||||
// with open("file.txt") as fp:
|
||||
// ...
|
||||
// ```
|
||||
if parent.is_with_stmt() {
|
||||
self.add_binding(id, expr.range(), BindingKind::WithItemVar, flags);
|
||||
return;
|
||||
}
|
||||
|
||||
self.add_binding(id, expr.range(), BindingKind::Assignment, flags);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,113 +1,113 @@
|
|||
---
|
||||
source: crates/ruff_linter/src/rules/pylint/mod.rs
|
||||
---
|
||||
redefined_argument_from_local.py:31:9: PLR1704 Redefining argument with the local name `a`
|
||||
redefined_argument_from_local.py:41:9: PLR1704 Redefining argument with the local name `a`
|
||||
|
|
||||
29 | # Errors
|
||||
30 | def func(a):
|
||||
31 | for a in range(1):
|
||||
39 | # Errors
|
||||
40 | def func(a):
|
||||
41 | for a in range(1):
|
||||
| ^ PLR1704
|
||||
32 | ...
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:36:9: PLR1704 Redefining argument with the local name `i`
|
||||
|
|
||||
35 | def func(i):
|
||||
36 | for i in range(10):
|
||||
| ^ PLR1704
|
||||
37 | print(i)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:43:25: PLR1704 Redefining argument with the local name `e`
|
||||
|
|
||||
41 | try:
|
||||
42 | ...
|
||||
43 | except Exception as e:
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:46:9: PLR1704 Redefining argument with the local name `i`
|
||||
|
|
||||
45 | def func(i):
|
||||
46 | for i in range(10):
|
||||
| ^ PLR1704
|
||||
47 | print(i)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:53:25: PLR1704 Redefining argument with the local name `e`
|
||||
|
|
||||
51 | try:
|
||||
52 | ...
|
||||
53 | except Exception as e:
|
||||
| ^ PLR1704
|
||||
44 | print(e)
|
||||
54 | print(e)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:48:24: PLR1704 Redefining argument with the local name `f`
|
||||
redefined_argument_from_local.py:58:24: PLR1704 Redefining argument with the local name `f`
|
||||
|
|
||||
47 | def func(f):
|
||||
48 | with open('', ) as f:
|
||||
57 | def func(f):
|
||||
58 | with open('', ) as f:
|
||||
| ^ PLR1704
|
||||
49 | print(f)
|
||||
59 | print(f)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:53:24: PLR1704 Redefining argument with the local name `a`
|
||||
redefined_argument_from_local.py:63:24: PLR1704 Redefining argument with the local name `a`
|
||||
|
|
||||
52 | def func(a, b):
|
||||
53 | with context() as (a, b, c):
|
||||
62 | def func(a, b):
|
||||
63 | with context() as (a, b, c):
|
||||
| ^ PLR1704
|
||||
54 | print(a, b, c)
|
||||
64 | print(a, b, c)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:53:27: PLR1704 Redefining argument with the local name `b`
|
||||
redefined_argument_from_local.py:63:27: PLR1704 Redefining argument with the local name `b`
|
||||
|
|
||||
52 | def func(a, b):
|
||||
53 | with context() as (a, b, c):
|
||||
62 | def func(a, b):
|
||||
63 | with context() as (a, b, c):
|
||||
| ^ PLR1704
|
||||
54 | print(a, b, c)
|
||||
64 | print(a, b, c)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:58:24: PLR1704 Redefining argument with the local name `a`
|
||||
redefined_argument_from_local.py:68:24: PLR1704 Redefining argument with the local name `a`
|
||||
|
|
||||
57 | def func(a, b):
|
||||
58 | with context() as [a, b, c]:
|
||||
67 | def func(a, b):
|
||||
68 | with context() as [a, b, c]:
|
||||
| ^ PLR1704
|
||||
59 | print(a, b, c)
|
||||
69 | print(a, b, c)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:58:27: PLR1704 Redefining argument with the local name `b`
|
||||
redefined_argument_from_local.py:68:27: PLR1704 Redefining argument with the local name `b`
|
||||
|
|
||||
57 | def func(a, b):
|
||||
58 | with context() as [a, b, c]:
|
||||
67 | def func(a, b):
|
||||
68 | with context() as [a, b, c]:
|
||||
| ^ PLR1704
|
||||
59 | print(a, b, c)
|
||||
69 | print(a, b, c)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:63:51: PLR1704 Redefining argument with the local name `a`
|
||||
redefined_argument_from_local.py:73:51: PLR1704 Redefining argument with the local name `a`
|
||||
|
|
||||
62 | def func(a):
|
||||
63 | with open('foo.py', ) as f, open('bar.py') as a:
|
||||
72 | def func(a):
|
||||
73 | with open('foo.py', ) as f, open('bar.py') as a:
|
||||
| ^ PLR1704
|
||||
64 | ...
|
||||
74 | ...
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:69:13: PLR1704 Redefining argument with the local name `a`
|
||||
redefined_argument_from_local.py:79:13: PLR1704 Redefining argument with the local name `a`
|
||||
|
|
||||
67 | def func(a):
|
||||
68 | def bar(b):
|
||||
69 | for a in range(1):
|
||||
77 | def func(a):
|
||||
78 | def bar(b):
|
||||
79 | for a in range(1):
|
||||
| ^ PLR1704
|
||||
70 | print(a)
|
||||
80 | print(a)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:75:13: PLR1704 Redefining argument with the local name `b`
|
||||
redefined_argument_from_local.py:85:13: PLR1704 Redefining argument with the local name `b`
|
||||
|
|
||||
73 | def func(a):
|
||||
74 | def bar(b):
|
||||
75 | for b in range(1):
|
||||
83 | def func(a):
|
||||
84 | def bar(b):
|
||||
85 | for b in range(1):
|
||||
| ^ PLR1704
|
||||
76 | print(b)
|
||||
86 | print(b)
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:81:13: PLR1704 Redefining argument with the local name `a`
|
||||
redefined_argument_from_local.py:91:13: PLR1704 Redefining argument with the local name `a`
|
||||
|
|
||||
79 | def func(a=1):
|
||||
80 | def bar(b=2):
|
||||
81 | for a in range(1):
|
||||
89 | def func(a=1):
|
||||
90 | def bar(b=2):
|
||||
91 | for a in range(1):
|
||||
| ^ PLR1704
|
||||
82 | print(a)
|
||||
83 | for b in range(1):
|
||||
92 | print(a)
|
||||
93 | for b in range(1):
|
||||
|
|
||||
|
||||
redefined_argument_from_local.py:83:13: PLR1704 Redefining argument with the local name `b`
|
||||
redefined_argument_from_local.py:93:13: PLR1704 Redefining argument with the local name `b`
|
||||
|
|
||||
81 | for a in range(1):
|
||||
82 | print(a)
|
||||
83 | for b in range(1):
|
||||
91 | for a in range(1):
|
||||
92 | print(a)
|
||||
93 | for b in range(1):
|
||||
| ^ PLR1704
|
||||
84 | print(b)
|
||||
94 | print(b)
|
||||
|
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue