From aefeeb9fb7df4133b3de7adf3575a61b62e2c381 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 6 Apr 2023 00:12:17 +0530 Subject: [PATCH 1/2] Do not process annotated assign target with no value --- .../fixtures/pylint/redefined_loop_name.py | 4 ++ .../rules/pylint/rules/redefined_loop_name.rs | 15 ++++- ...tests__PLW2901_redefined_loop_name.py.snap | 60 +++++++++---------- 3 files changed, 48 insertions(+), 31 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py b/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py index 13a230d50408b..65e12f54d6a2a 100644 --- a/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py +++ b/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py @@ -53,6 +53,10 @@ for i in []: i: int = 5 # error +# For -> annotated assignment without value +for i in []: + i: int # no error + # Async for -> for, variable reused async for i in []: for i in []: # error diff --git a/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs b/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs index d662811f99e95..86d17d2361491 100644 --- a/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs +++ b/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs @@ -184,7 +184,20 @@ where ), ); } - StmtKind::AugAssign { target, .. } | StmtKind::AnnAssign { target, .. } => { + StmtKind::AugAssign { target, .. } => { + self.assignment_targets.extend( + assignment_targets_from_expr(target, self.dummy_variable_rgx).map(|expr| { + ExprWithInnerBindingKind { + expr, + binding_kind: InnerBindingKind::Assignment, + } + }), + ); + } + StmtKind::AnnAssign { target, value, .. } => { + if value.is_none() { + return; + } self.assignment_targets.extend( assignment_targets_from_expr(target, self.dummy_variable_rgx).map(|expr| { ExprWithInnerBindingKind { diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap index 036ca9e2f3422..d3e4457ed3c33 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap @@ -148,10 +148,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 58 + row: 62 column: 8 end_location: - row: 58 + row: 62 column: 9 fix: edits: [] @@ -162,10 +162,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 63 + row: 67 column: 14 end_location: - row: 63 + row: 67 column: 15 fix: edits: [] @@ -176,10 +176,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 68 + row: 72 column: 8 end_location: - row: 68 + row: 72 column: 9 fix: edits: [] @@ -190,10 +190,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 73 + row: 77 column: 8 end_location: - row: 73 + row: 77 column: 9 fix: edits: [] @@ -204,10 +204,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 78 + row: 82 column: 8 end_location: - row: 78 + row: 82 column: 9 fix: edits: [] @@ -218,10 +218,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 78 + row: 82 column: 11 end_location: - row: 78 + row: 82 column: 12 fix: edits: [] @@ -232,10 +232,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 95 + row: 99 column: 8 end_location: - row: 95 + row: 99 column: 9 fix: edits: [] @@ -246,10 +246,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 112 + row: 116 column: 12 end_location: - row: 112 + row: 116 column: 13 fix: edits: [] @@ -260,10 +260,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 118 + row: 122 column: 16 end_location: - row: 118 + row: 122 column: 17 fix: edits: [] @@ -274,10 +274,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 133 + row: 137 column: 4 end_location: - row: 133 + row: 137 column: 8 fix: edits: [] @@ -288,10 +288,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 138 + row: 142 column: 4 end_location: - row: 138 + row: 142 column: 10 fix: edits: [] @@ -302,10 +302,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 143 + row: 147 column: 4 end_location: - row: 143 + row: 147 column: 7 fix: edits: [] @@ -316,10 +316,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 148 + row: 152 column: 4 end_location: - row: 148 + row: 152 column: 9 fix: edits: [] @@ -330,10 +330,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 153 + row: 157 column: 4 end_location: - row: 153 + row: 157 column: 8 fix: edits: [] @@ -344,10 +344,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 155 + row: 159 column: 4 end_location: - row: 155 + row: 159 column: 7 fix: edits: [] From ca542a9f3e59b3cc89dd8abe0c94215d95c169e6 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 6 Apr 2023 00:16:19 +0530 Subject: [PATCH 2/2] Ignore `PLW2901` when using typing cast --- .../fixtures/pylint/redefined_loop_name.py | 6 ++ .../rules/pylint/rules/redefined_loop_name.rs | 41 ++++++- ...tests__PLW2901_redefined_loop_name.py.snap | 100 +++++++++--------- 3 files changed, 96 insertions(+), 51 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py b/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py index 65e12f54d6a2a..b99bfe9323fad 100644 --- a/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py +++ b/crates/ruff/resources/test/fixtures/pylint/redefined_loop_name.py @@ -1,3 +1,6 @@ +import typing +from typing import cast + # For -> for, variable reused for i in []: for i in []: # error @@ -43,6 +46,9 @@ # For -> assignment for i in []: + # ignore typing cast + i = cast(int, i) + i = typing.cast(int, i) i = 5 # error # For -> augmented assignment diff --git a/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs b/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs index 86d17d2361491..228961dd96a2c 100644 --- a/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs +++ b/crates/ruff/src/rules/pylint/rules/redefined_loop_name.rs @@ -10,6 +10,7 @@ use ruff_python_ast::helpers::unparse_expr; use ruff_python_ast::types::{Node, Range}; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; +use ruff_python_semantic::context::Context; use crate::checkers::ast::Checker; @@ -140,6 +141,7 @@ struct ExprWithInnerBindingKind<'a> { } struct InnerForWithAssignTargetsVisitor<'a> { + context: &'a Context<'a>, dummy_variable_rgx: &'a Regex, assignment_targets: Vec>, } @@ -174,7 +176,14 @@ where ); } // Assignment, augmented assignment, and annotated assignment. - StmtKind::Assign { targets, .. } => { + StmtKind::Assign { targets, value, .. } => { + // Check for single-target assignments which are of the + // form `x = cast(..., x)`. + if targets.first().map_or(false, |target| { + assignment_is_cast_expr(self.context, value, target) + }) { + return; + } self.assignment_targets.extend( assignment_targets_from_assign_targets(targets, self.dummy_variable_rgx).map( |expr| ExprWithInnerBindingKind { @@ -222,6 +231,34 @@ where } } +/// Checks whether the given assignment value is a `typing.cast` expression +/// and that the target name is the same as the argument name. +/// +/// Example: +/// ```python +/// from typing import cast +/// +/// x = cast(int, x) +/// ``` +fn assignment_is_cast_expr(context: &Context, value: &Expr, target: &Expr) -> bool { + let ExprKind::Call { func, args, .. } = &value.node else { + return false; + }; + let ExprKind::Name { id: target_id, .. } = &target.node else { + return false; + }; + if args.len() != 2 { + return false; + } + let ExprKind::Name { id: arg_id, .. } = &args[1].node else { + return false; + }; + if arg_id != target_id { + return false; + } + context.match_typing_expr(func, "cast") +} + fn assignment_targets_from_expr<'a, U>( expr: &'a Expr, dummy_variable_rgx: &'a Regex, @@ -312,6 +349,7 @@ pub fn redefined_loop_name<'a, 'b>(checker: &'a mut Checker<'b>, node: &Node<'b> }) .collect(); let mut visitor = InnerForWithAssignTargetsVisitor { + context: &checker.ctx, dummy_variable_rgx: &checker.settings.dummy_variable_rgx, assignment_targets: vec![], }; @@ -330,6 +368,7 @@ pub fn redefined_loop_name<'a, 'b>(checker: &'a mut Checker<'b>, node: &Node<'b> }) .collect(); let mut visitor = InnerForWithAssignTargetsVisitor { + context: &checker.ctx, dummy_variable_rgx: &checker.settings.dummy_variable_rgx, assignment_targets: vec![], }; diff --git a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap index d3e4457ed3c33..11e516efb6ff7 100644 --- a/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap +++ b/crates/ruff/src/rules/pylint/snapshots/ruff__rules__pylint__tests__PLW2901_redefined_loop_name.py.snap @@ -8,10 +8,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 3 + row: 6 column: 8 end_location: - row: 3 + row: 6 column: 9 fix: edits: [] @@ -22,10 +22,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 8 + row: 11 column: 8 end_location: - row: 8 + row: 11 column: 9 fix: edits: [] @@ -36,10 +36,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 13 + row: 16 column: 17 end_location: - row: 13 + row: 16 column: 18 fix: edits: [] @@ -50,10 +50,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 18 + row: 21 column: 17 end_location: - row: 18 + row: 21 column: 18 fix: edits: [] @@ -64,10 +64,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 34 + row: 37 column: 12 end_location: - row: 34 + row: 37 column: 13 fix: edits: [] @@ -78,10 +78,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 40 + row: 43 column: 12 end_location: - row: 40 + row: 43 column: 13 fix: edits: [] @@ -92,10 +92,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 41 + row: 44 column: 16 end_location: - row: 41 + row: 44 column: 17 fix: edits: [] @@ -106,10 +106,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 46 + row: 52 column: 4 end_location: - row: 46 + row: 52 column: 5 fix: edits: [] @@ -120,10 +120,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 50 + row: 56 column: 4 end_location: - row: 50 + row: 56 column: 5 fix: edits: [] @@ -134,10 +134,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 54 + row: 60 column: 4 end_location: - row: 54 + row: 60 column: 5 fix: edits: [] @@ -148,10 +148,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 62 + row: 68 column: 8 end_location: - row: 62 + row: 68 column: 9 fix: edits: [] @@ -162,10 +162,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 67 + row: 73 column: 14 end_location: - row: 67 + row: 73 column: 15 fix: edits: [] @@ -176,10 +176,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 72 + row: 78 column: 8 end_location: - row: 72 + row: 78 column: 9 fix: edits: [] @@ -190,10 +190,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 77 + row: 83 column: 8 end_location: - row: 77 + row: 83 column: 9 fix: edits: [] @@ -204,10 +204,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 82 + row: 88 column: 8 end_location: - row: 82 + row: 88 column: 9 fix: edits: [] @@ -218,10 +218,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 82 + row: 88 column: 11 end_location: - row: 82 + row: 88 column: 12 fix: edits: [] @@ -232,10 +232,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 99 + row: 105 column: 8 end_location: - row: 99 + row: 105 column: 9 fix: edits: [] @@ -246,10 +246,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 116 + row: 122 column: 12 end_location: - row: 116 + row: 122 column: 13 fix: edits: [] @@ -260,10 +260,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 122 + row: 128 column: 16 end_location: - row: 122 + row: 128 column: 17 fix: edits: [] @@ -274,10 +274,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 137 + row: 143 column: 4 end_location: - row: 137 + row: 143 column: 8 fix: edits: [] @@ -288,10 +288,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 142 + row: 148 column: 4 end_location: - row: 142 + row: 148 column: 10 fix: edits: [] @@ -302,10 +302,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 147 + row: 153 column: 4 end_location: - row: 147 + row: 153 column: 7 fix: edits: [] @@ -316,10 +316,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 152 + row: 158 column: 4 end_location: - row: 152 + row: 158 column: 9 fix: edits: [] @@ -330,10 +330,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 157 + row: 163 column: 4 end_location: - row: 157 + row: 163 column: 8 fix: edits: [] @@ -344,10 +344,10 @@ expression: diagnostics suggestion: ~ fixable: false location: - row: 159 + row: 165 column: 4 end_location: - row: 159 + row: 165 column: 7 fix: edits: []