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

crates/ruff_linter/resources/test/fixtures/pylint/redefined_argument_from_local.py

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

crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs

@@ -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,
@@ -89,6 +90,30 @@ 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
+                    ) {
+                        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) {

crates/ruff_linter/src/codes.rs

@@ -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),

crates/ruff_linter/src/rules/pylint/mod.rs

@@ -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"))]

crates/ruff_linter/src/rules/pylint/rules/mod.rs

@@ -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::*;
@@ -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;

crates/ruff_linter/src/rules/pylint/rules/redefined_argument_from_local.rs

@@ -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}`")
+    }
+}

crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1704_redefined_argument_from_local.py.snap

@@ -0,0 +1,58 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+redefined_argument_from_local.py:28:9: PLR1704 Redefining argument with the local name `i`
+   |
+27 | def foo(i):
+28 |     for i in range(10):
+   |         ^ PLR1704
+29 |         ...
+   |
+
+redefined_argument_from_local.py:34:25: PLR1704 Redefining argument with the local name `e`
+   |
+32 |     try:
+33 |         ...
+34 |     except Exception as e:
+   |                         ^ PLR1704
+35 |         ...
+   |
+
+redefined_argument_from_local.py:51:13: PLR1704 Redefining argument with the local name `a`
+   |
+49 | def foo(a):
+50 |     def bar(b):
+51 |         for a in range(1):
+   |             ^ PLR1704
+52 |             ...
+   |
+
+redefined_argument_from_local.py:56:13: PLR1704 Redefining argument with the local name `b`
+   |
+54 | def foo(a):
+55 |     def bar(b):
+56 |         for b in range(1):
+   |             ^ PLR1704
+57 |             ...
+   |
+
+redefined_argument_from_local.py:61:13: PLR1704 Redefining argument with the local name `a`
+   |
+59 | def foo(a=1):
+60 |     def bar(b=2):
+61 |         for a in range(1):
+   |             ^ PLR1704
+62 |             ...
+63 |         for b in range(1):
+   |
+
+redefined_argument_from_local.py:63:13: PLR1704 Redefining argument with the local name `b`
+   |
+61 |         for a in range(1):
+62 |             ...
+63 |         for b in range(1):
+   |             ^ PLR1704
+64 |             ...
+   |
+
+