From eaf2a35753b8bb3777d455be8e336a3072887f16 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 5 Jun 2024 19:27:10 -0500 Subject: [PATCH] Fix shadowed cases --- .../control_var_used_after_block.py | 74 ++----------------- .../rules/control_var_used_after_block.rs | 27 +++++-- crates/ruff_python_semantic/src/model.rs | 6 ++ 3 files changed, 34 insertions(+), 73 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/wemake_python_styleguide/control_var_used_after_block.py b/crates/ruff_linter/resources/test/fixtures/wemake_python_styleguide/control_var_used_after_block.py index 5f72ae16eca12..b38843cea6bea 100644 --- a/crates/ruff_linter/resources/test/fixtures/wemake_python_styleguide/control_var_used_after_block.py +++ b/crates/ruff_linter/resources/test/fixtures/wemake_python_styleguide/control_var_used_after_block.py @@ -1,70 +1,8 @@ -import typing -from typing import cast - -for global_var in []: - _ = global_var +for x in []: + _ = x pass -_ = global_var - -def foo(): - # For control var used outside block - for event in []: - _ = event - pass - - _ = event - - for x in range(10): - pass - - # Usage in another block - for y in range(10): - # x is used outside of the loop it was defined in (meant to use y) - if x == 5: - pass - - # Tuple destructuring - for a, b, c in []: - _ = a - _ = b - _ = c - pass - - _ = a - _ = b - _ = c - - # Array destructuring - for [d, e, f] in []: - _ = d - _ = e - _ = f - pass - - _ = d - _ = e - _ = f - - # with statement - with None as w: - _ = w - pass - - _ = w - - # Nested blocks - with None as q: - _ = q - - for n in []: - _ = n - _ = q - pass - - _ = n - - _ = q - _ = n - - +# Using the same control variable name in a different loop is ok +for x in []: + _ = x + pass diff --git a/crates/ruff_linter/src/rules/wemake_python_styleguide/rules/control_var_used_after_block.rs b/crates/ruff_linter/src/rules/wemake_python_styleguide/rules/control_var_used_after_block.rs index 1864b8a888db5..de500545874c3 100644 --- a/crates/ruff_linter/src/rules/wemake_python_styleguide/rules/control_var_used_after_block.rs +++ b/crates/ruff_linter/src/rules/wemake_python_styleguide/rules/control_var_used_after_block.rs @@ -3,7 +3,7 @@ use std::fmt; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::Stmt; -use ruff_python_semantic::{NodeId, Scope}; +use ruff_python_semantic::{BindingId, NodeId, Scope}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -76,11 +76,22 @@ pub(crate) fn control_var_used_after_block( scope: &Scope, diagnostics: &mut Vec, ) { + println!("nodes {:#?}", checker.semantic().nodes()); + + // Keep track of node_ids we know are used in the same block. This helps us when + // people use the same variable name in multiple blocks. + let mut known_good_reference_node_ids: Vec = Vec::new(); + + let all_bindings: Vec<(&str, BindingId)> = scope.all_bindings().collect(); + let reversed_bindings = all_bindings.iter().rev(); + // Find for-loop and with-statement variable bindings - for (name, binding) in scope - .bindings() - .map(|(name, binding_id)| (name, checker.semantic().binding(binding_id))) + for (&name, binding) in reversed_bindings + .map(|(name, binding_id)| (name, checker.semantic().binding(*binding_id))) .filter_map(|(name, binding)| { + if !binding.kind.is_builtin() { + println!("name {:?} binding.kind {:?}", name, binding.kind); + } if binding.kind.is_loop_var() || binding.kind.is_with_item_var() { return Some((name, binding)); } @@ -108,12 +119,18 @@ pub(crate) fn control_var_used_after_block( for ancestor_node_id in statement_hierarchy { if binding_statement_id == ancestor_node_id { is_used_in_block = true; + known_good_reference_node_ids.push(reference_node_id); break; } } + println!( + "is_used_in_block {:?} binding_source_node_id {:?} binding_statement_id {:?} current_reference_node_id {:?} known_good_reference_node_ids {:?}", + is_used_in_block, binding_source_node_id, binding_statement_id, reference_node_id, known_good_reference_node_ids + ); + // If the reference wasn't used in the same block, report a violation/diagnostic - if !is_used_in_block { + if !is_used_in_block && !known_good_reference_node_ids.contains(&reference_node_id) { let block_kind = match binding_statement { Stmt::For(_) => BlockKind::For, Stmt::With(_) => BlockKind::With, diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index 06cf6f4de1f7c..b076f80537461 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -1144,6 +1144,12 @@ impl<'a> SemanticModel<'a> { None } + /// TODO + #[inline] + pub fn nodes(&self) -> &Nodes<'a> { + &self.nodes + } + /// Return the [`Stmt`] corresponding to the given [`NodeId`]. #[inline] pub fn node(&self, node_id: NodeId) -> &NodeRef<'a> {