[pylint] Implement redefined-argument-from-local (R1704) (#8159)

## Summary

It implements Pylint rule R1704: redefined-argument-from-local

Problematic code:
```python
def show(host_id=10.11):
    # +1: [redefined-argument-from-local]
    for host_id, host in [[12.13, "Venus"], [14.15, "Mars"]]:
        print(host_id, host)
```

Correct code:
```python
def show(host_id=10.11):
    for inner_host_id, host in [[12.13, "Venus"], [14.15, "Mars"]]:
        print(host_id, inner_host_id, host)
```

References:
[Pylint
documentation](https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/redefined-argument-from-local.html)
[Related Issue](https://github.com/astral-sh/ruff/issues/970)

## Test Plan

`cargo test`
This commit is contained in:
Jake Park 2023-11-11 04:13:07 +09:00 committed by GitHub
parent 5a1a8bebca
commit c8edac9d2b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 268 additions and 0 deletions

View file

@ -0,0 +1,79 @@
# No Errors
def func(a):
for b in range(1):
...
def func(a):
try:
...
except ValueError:
...
except KeyError:
...
if True:
def func(a):
...
else:
for a in range(1):
print(a)
# Errors
def func(a):
for a in range(1):
...
def func(i):
for i in range(10):
print(i)
def func(e):
try:
...
except Exception as e:
print(e)
def func(f):
with open('', ) as f:
print(f)
def func(a, b):
with context() as (a, b, c):
print(a, b, c)
def func(a, b):
with context() as [a, b, c]:
print(a, b, c)
def func(a):
with open('foo.py', ) as f, open('bar.py') as a:
...
def func(a):
def bar(b):
for a in range(1):
print(a)
def func(a):
def bar(b):
for b in range(1):
print(b)
def func(a=1):
def bar(b=2):
for a in range(1):
print(a)
for b in range(1):
print(b)

View file

@ -12,6 +12,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
if !checker.any_enabled(&[ if !checker.any_enabled(&[
Rule::GlobalVariableNotAssigned, Rule::GlobalVariableNotAssigned,
Rule::ImportShadowedByLoopVar, Rule::ImportShadowedByLoopVar,
Rule::RedefinedArgumentFromLocal,
Rule::RedefinedWhileUnused, Rule::RedefinedWhileUnused,
Rule::RuntimeImportInTypeCheckingBlock, Rule::RuntimeImportInTypeCheckingBlock,
Rule::TypingOnlyFirstPartyImport, Rule::TypingOnlyFirstPartyImport,
@ -89,6 +90,32 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
} }
} }
if checker.enabled(Rule::RedefinedArgumentFromLocal) {
for (name, binding_id) in scope.bindings() {
for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) {
let binding = &checker.semantic.bindings[shadow.binding_id()];
if !matches!(
binding.kind,
BindingKind::LoopVar
| BindingKind::BoundException
| BindingKind::WithItemVar
) {
continue;
}
let shadowed = &checker.semantic.bindings[shadow.shadowed_id()];
if !shadowed.kind.is_argument() {
continue;
}
checker.diagnostics.push(Diagnostic::new(
pylint::rules::RedefinedArgumentFromLocal {
name: name.to_string(),
},
binding.range(),
));
}
}
}
if checker.enabled(Rule::ImportShadowedByLoopVar) { if checker.enabled(Rule::ImportShadowedByLoopVar) {
for (name, binding_id) in scope.bindings() { for (name, binding_id) in scope.bindings() {
for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) { for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) {

View file

@ -252,6 +252,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "R0915") => (RuleGroup::Stable, rules::pylint::rules::TooManyStatements), (Pylint, "R0915") => (RuleGroup::Stable, rules::pylint::rules::TooManyStatements),
(Pylint, "R0916") => (RuleGroup::Preview, rules::pylint::rules::TooManyBooleanExpressions), (Pylint, "R0916") => (RuleGroup::Preview, rules::pylint::rules::TooManyBooleanExpressions),
(Pylint, "R1701") => (RuleGroup::Stable, rules::pylint::rules::RepeatedIsinstanceCalls), (Pylint, "R1701") => (RuleGroup::Stable, rules::pylint::rules::RepeatedIsinstanceCalls),
(Pylint, "R1704") => (RuleGroup::Preview, rules::pylint::rules::RedefinedArgumentFromLocal),
(Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn), (Pylint, "R1711") => (RuleGroup::Stable, rules::pylint::rules::UselessReturn),
(Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison), (Pylint, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison),
(Pylint, "R1706") => (RuleGroup::Preview, rules::pylint::rules::AndOrTernary), (Pylint, "R1706") => (RuleGroup::Preview, rules::pylint::rules::AndOrTernary),

View file

@ -86,6 +86,10 @@ mod tests {
)] )]
#[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"))] #[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.py"))]
#[test_case(Rule::PropertyWithParameters, Path::new("property_with_parameters.py"))] #[test_case(Rule::PropertyWithParameters, Path::new("property_with_parameters.py"))]
#[test_case(
Rule::RedefinedArgumentFromLocal,
Path::new("redefined_argument_from_local.py")
)]
#[test_case(Rule::RedefinedLoopName, Path::new("redefined_loop_name.py"))] #[test_case(Rule::RedefinedLoopName, Path::new("redefined_loop_name.py"))]
#[test_case(Rule::ReturnInInit, Path::new("return_in_init.py"))] #[test_case(Rule::ReturnInInit, Path::new("return_in_init.py"))]
#[test_case(Rule::TooManyArguments, Path::new("too_many_arguments.py"))] #[test_case(Rule::TooManyArguments, Path::new("too_many_arguments.py"))]

View file

@ -40,6 +40,7 @@ pub(crate) use non_ascii_module_import::*;
pub(crate) use non_ascii_name::*; pub(crate) use non_ascii_name::*;
pub(crate) use nonlocal_without_binding::*; pub(crate) use nonlocal_without_binding::*;
pub(crate) use property_with_parameters::*; pub(crate) use property_with_parameters::*;
pub(crate) use redefined_argument_from_local::*;
pub(crate) use redefined_loop_name::*; pub(crate) use redefined_loop_name::*;
pub(crate) use repeated_equality_comparison::*; pub(crate) use repeated_equality_comparison::*;
pub(crate) use repeated_isinstance_calls::*; pub(crate) use repeated_isinstance_calls::*;
@ -111,6 +112,7 @@ mod non_ascii_module_import;
mod non_ascii_name; mod non_ascii_name;
mod nonlocal_without_binding; mod nonlocal_without_binding;
mod property_with_parameters; mod property_with_parameters;
mod redefined_argument_from_local;
mod redefined_loop_name; mod redefined_loop_name;
mod repeated_equality_comparison; mod repeated_equality_comparison;
mod repeated_isinstance_calls; mod repeated_isinstance_calls;

View file

@ -0,0 +1,39 @@
use ruff_diagnostics::Violation;
use ruff_macros::{derive_message_formats, violation};
/// ## What it does
/// Checks for variables defined in `for`, `try`, `with` statements
/// that redefine function parameters.
///
/// ## Why is this bad?
/// Redefined variable can cause unexpected behavior because of overridden function parameter.
/// If nested functions are declared, inner function's body can override outer function's parameter.
///
/// ## Example
/// ```python
/// def show(host_id=10.11):
/// for host_id, host in [[12.13, "Venus"], [14.15, "Mars"]]:
/// print(host_id, host)
/// ```
///
/// Use instead:
/// ```python
/// def show(host_id=10.11):
/// for inner_host_id, host in [[12.13, "Venus"], [14.15, "Mars"]]:
/// print(host_id, inner_host_id, host)
/// ```
/// ## References
/// - [Pylint documentation](https://pylint.readthedocs.io/en/latest/user_guide/messages/refactor/redefined-argument-from-local.html)
#[violation]
pub struct RedefinedArgumentFromLocal {
pub(crate) name: String,
}
impl Violation for RedefinedArgumentFromLocal {
#[derive_message_formats]
fn message(&self) -> String {
let RedefinedArgumentFromLocal { name } = self;
format!("Redefining argument with the local name `{name}`")
}
}

View file

@ -0,0 +1,115 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
redefined_argument_from_local.py:26:9: PLR1704 Redefining argument with the local name `a`
|
24 | # Errors
25 | def func(a):
26 | for a in range(1):
| ^ PLR1704
27 | ...
|
redefined_argument_from_local.py:31:9: PLR1704 Redefining argument with the local name `i`
|
30 | def func(i):
31 | for i in range(10):
| ^ PLR1704
32 | print(i)
|
redefined_argument_from_local.py:38:25: PLR1704 Redefining argument with the local name `e`
|
36 | try:
37 | ...
38 | except Exception as e:
| ^ PLR1704
39 | print(e)
|
redefined_argument_from_local.py:43:24: PLR1704 Redefining argument with the local name `f`
|
42 | def func(f):
43 | with open('', ) as f:
| ^ PLR1704
44 | print(f)
|
redefined_argument_from_local.py:48:24: PLR1704 Redefining argument with the local name `a`
|
47 | def func(a, b):
48 | with context() as (a, b, c):
| ^ PLR1704
49 | print(a, b, c)
|
redefined_argument_from_local.py:48:27: PLR1704 Redefining argument with the local name `b`
|
47 | def func(a, b):
48 | with context() as (a, b, c):
| ^ PLR1704
49 | print(a, b, c)
|
redefined_argument_from_local.py:53:24: PLR1704 Redefining argument with the local name `a`
|
52 | def func(a, b):
53 | with context() as [a, b, c]:
| ^ PLR1704
54 | print(a, b, c)
|
redefined_argument_from_local.py:53:27: PLR1704 Redefining argument with the local name `b`
|
52 | def func(a, b):
53 | with context() as [a, b, c]:
| ^ PLR1704
54 | print(a, b, c)
|
redefined_argument_from_local.py:58:51: PLR1704 Redefining argument with the local name `a`
|
57 | def func(a):
58 | with open('foo.py', ) as f, open('bar.py') as a:
| ^ PLR1704
59 | ...
|
redefined_argument_from_local.py:64:13: PLR1704 Redefining argument with the local name `a`
|
62 | def func(a):
63 | def bar(b):
64 | for a in range(1):
| ^ PLR1704
65 | print(a)
|
redefined_argument_from_local.py:70:13: PLR1704 Redefining argument with the local name `b`
|
68 | def func(a):
69 | def bar(b):
70 | for b in range(1):
| ^ PLR1704
71 | print(b)
|
redefined_argument_from_local.py:76:13: PLR1704 Redefining argument with the local name `a`
|
74 | def func(a=1):
75 | def bar(b=2):
76 | for a in range(1):
| ^ PLR1704
77 | print(a)
78 | for b in range(1):
|
redefined_argument_from_local.py:78:13: PLR1704 Redefining argument with the local name `b`
|
76 | for a in range(1):
77 | print(a)
78 | for b in range(1):
| ^ PLR1704
79 | print(b)
|

1
ruff.schema.json generated
View file

@ -3074,6 +3074,7 @@
"PLR17", "PLR17",
"PLR170", "PLR170",
"PLR1701", "PLR1701",
"PLR1704",
"PLR1706", "PLR1706",
"PLR171", "PLR171",
"PLR1711", "PLR1711",