Skip to content

Commit

Permalink
Fix shadowed cases
Browse files Browse the repository at this point in the history
  • Loading branch information
MadLittleMods committed Jun 6, 2024
1 parent 0ad0df0 commit eaf2a35
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 73 deletions.
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -76,11 +76,22 @@ pub(crate) fn control_var_used_after_block(
scope: &Scope,
diagnostics: &mut Vec<Diagnostic>,
) {
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<NodeId> = 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));
}
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions crates/ruff_python_semantic/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down

0 comments on commit eaf2a35

Please sign in to comment.