Skip to content

Commit

Permalink
[pylint] Implement redefined-argument-from-local (R1704) (#8159)
Browse files Browse the repository at this point in the history
## 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](#970)

## Test Plan

`cargo test`
  • Loading branch information
danbi2990 authored Nov 10, 2023
1 parent 5a1a8be commit c8edac9
Show file tree
Hide file tree
Showing 8 changed files with 268 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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)
27 changes: 27 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) {
if !checker.any_enabled(&[
Rule::GlobalVariableNotAssigned,
Rule::ImportShadowedByLoopVar,
Rule::RedefinedArgumentFromLocal,
Rule::RedefinedWhileUnused,
Rule::RuntimeImportInTypeCheckingBlock,
Rule::TypingOnlyFirstPartyImport,
Expand Down Expand Up @@ -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) {
for (name, binding_id) in scope.bindings() {
for shadow in checker.semantic.shadowed_bindings(scope_id, binding_id) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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, "R0916") => (RuleGroup::Preview, rules::pylint::rules::TooManyBooleanExpressions),
(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, "R1714") => (RuleGroup::Stable, rules::pylint::rules::RepeatedEqualityComparison),
(Pylint, "R1706") => (RuleGroup::Preview, rules::pylint::rules::AndOrTernary),
Expand Down
4 changes: 4 additions & 0 deletions crates/ruff_linter/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ mod tests {
)]
#[test_case(Rule::NonlocalWithoutBinding, Path::new("nonlocal_without_binding.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::ReturnInInit, Path::new("return_in_init.py"))]
#[test_case(Rule::TooManyArguments, Path::new("too_many_arguments.py"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ pub(crate) use non_ascii_module_import::*;
pub(crate) use non_ascii_name::*;
pub(crate) use nonlocal_without_binding::*;
pub(crate) use property_with_parameters::*;
pub(crate) use redefined_argument_from_local::*;
pub(crate) use redefined_loop_name::*;
pub(crate) use repeated_equality_comparison::*;
pub(crate) use repeated_isinstance_calls::*;
Expand Down Expand Up @@ -111,6 +112,7 @@ mod non_ascii_module_import;
mod non_ascii_name;
mod nonlocal_without_binding;
mod property_with_parameters;
mod redefined_argument_from_local;
mod redefined_loop_name;
mod repeated_equality_comparison;
mod repeated_isinstance_calls;
Expand Down
Original file line number Diff line number Diff line change
@@ -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}`")
}
}
Original file line number Diff line number Diff line change
@@ -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 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c8edac9

Please sign in to comment.