mirror of https://github.com/astral-sh/ruff
Merge a95fe58d8f into b0bc990cbf
This commit is contained in:
commit
94f87ed94f
|
|
@ -183,3 +183,53 @@ for val in range(3):
|
||||||
|
|
||||||
|
|
||||||
funcs.append(make_func())
|
funcs.append(make_func())
|
||||||
|
|
||||||
|
|
||||||
|
# Test cases for issue #15716 - false positives with lambda parameters
|
||||||
|
for _ in range(3):
|
||||||
|
[x for x in []]
|
||||||
|
def func():
|
||||||
|
lambda x: x # Should not trigger B023 - x is a lambda parameter
|
||||||
|
|
||||||
|
|
||||||
|
# Test case from issue comment - pandas apply (should NOT trigger B023 - apply is safe like map)
|
||||||
|
import pandas as pd
|
||||||
|
|
||||||
|
data = pd.DataFrame()
|
||||||
|
data.loc[0, "hex"] = "72756666"
|
||||||
|
data.loc[1, "hex"] = "75767576"
|
||||||
|
|
||||||
|
def modifier(value):
|
||||||
|
return chr(int(value, 16))
|
||||||
|
|
||||||
|
|
||||||
|
for _i in range(0, 4):
|
||||||
|
data[f"v{_i}"] = data["hex"].apply(
|
||||||
|
lambda x: modifier(x[2 * _i : 2 * _i + 2]), # Should NOT trigger B023 - apply is safe
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# Test case from issue comment - nested function (should NOT trigger B023 - value is function parameter)
|
||||||
|
for _ in range(2):
|
||||||
|
|
||||||
|
def add_one():
|
||||||
|
def _add_one_inner(value):
|
||||||
|
return value + 1 # Should NOT trigger B023 - value is function parameter
|
||||||
|
|
||||||
|
return _add_one_inner
|
||||||
|
|
||||||
|
for value in range(5):
|
||||||
|
result = add_one()(value)
|
||||||
|
print(result)
|
||||||
|
|
||||||
|
|
||||||
|
# nested function that captures loop variable (SHOULD trigger B023)
|
||||||
|
lst = []
|
||||||
|
for value in range(2):
|
||||||
|
def add_one():
|
||||||
|
def _add_one_inner():
|
||||||
|
return value + 1 # Should trigger B023 - value is loop variable, not bound
|
||||||
|
|
||||||
|
return _add_one_inner
|
||||||
|
|
||||||
|
lst.append(add_one())
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,7 @@ use ruff_python_ast::types::Node;
|
||||||
use ruff_python_ast::visitor;
|
use ruff_python_ast::visitor;
|
||||||
use ruff_python_ast::visitor::Visitor;
|
use ruff_python_ast::visitor::Visitor;
|
||||||
use ruff_python_ast::{self as ast, Comprehension, Expr, ExprContext, Stmt};
|
use ruff_python_ast::{self as ast, Comprehension, Expr, ExprContext, Stmt};
|
||||||
|
use ruff_python_semantic::Modules;
|
||||||
use ruff_text_size::Ranged;
|
use ruff_text_size::Ranged;
|
||||||
|
|
||||||
use crate::Violation;
|
use crate::Violation;
|
||||||
|
|
@ -63,6 +64,14 @@ struct LoadedNamesVisitor<'a> {
|
||||||
|
|
||||||
/// `Visitor` to collect all used identifiers in a statement.
|
/// `Visitor` to collect all used identifiers in a statement.
|
||||||
impl<'a> Visitor<'a> for LoadedNamesVisitor<'a> {
|
impl<'a> Visitor<'a> for LoadedNamesVisitor<'a> {
|
||||||
|
fn visit_stmt(&mut self, stmt: &'a Stmt) {
|
||||||
|
// Skip nested function definitions - they are handled separately by `SuspiciousVariablesVisitor`
|
||||||
|
if stmt.is_function_def_stmt() {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
visitor::walk_stmt(self, stmt);
|
||||||
|
}
|
||||||
|
|
||||||
fn visit_expr(&mut self, expr: &'a Expr) {
|
fn visit_expr(&mut self, expr: &'a Expr) {
|
||||||
match expr {
|
match expr {
|
||||||
Expr::Name(name) => match &name.ctx {
|
Expr::Name(name) => match &name.ctx {
|
||||||
|
|
@ -70,15 +79,17 @@ impl<'a> Visitor<'a> for LoadedNamesVisitor<'a> {
|
||||||
ExprContext::Store => self.stored.push(name),
|
ExprContext::Store => self.stored.push(name),
|
||||||
_ => {}
|
_ => {}
|
||||||
},
|
},
|
||||||
|
Expr::Lambda(ast::ExprLambda { parameters: _, .. }) => {}
|
||||||
_ => visitor::walk_expr(self, expr),
|
_ => visitor::walk_expr(self, expr),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Default)]
|
|
||||||
struct SuspiciousVariablesVisitor<'a> {
|
struct SuspiciousVariablesVisitor<'a> {
|
||||||
names: Vec<&'a ast::ExprName>,
|
names: Vec<&'a ast::ExprName>,
|
||||||
safe_functions: Vec<&'a Expr>,
|
safe_functions: Vec<&'a Expr>,
|
||||||
|
pandas_imported: bool,
|
||||||
|
outer_parameters: Vec<&'a ast::Parameters>,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// `Visitor` to collect all suspicious variables (those referenced in
|
/// `Visitor` to collect all suspicious variables (those referenced in
|
||||||
|
|
@ -89,7 +100,7 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
|
||||||
Stmt::FunctionDef(ast::StmtFunctionDef {
|
Stmt::FunctionDef(ast::StmtFunctionDef {
|
||||||
parameters, body, ..
|
parameters, body, ..
|
||||||
}) => {
|
}) => {
|
||||||
// Collect all loaded variable names.
|
// Collect all loaded variable names and lambda parameters.
|
||||||
let mut visitor = LoadedNamesVisitor::default();
|
let mut visitor = LoadedNamesVisitor::default();
|
||||||
visitor.visit_body(body);
|
visitor.visit_body(body);
|
||||||
|
|
||||||
|
|
@ -100,13 +111,27 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check if variable is bound in current function parameters
|
||||||
if parameters.includes(&loaded.id) {
|
if parameters.includes(&loaded.id) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Check if variable is bound in outer function parameters
|
||||||
|
if self
|
||||||
|
.outer_parameters
|
||||||
|
.iter()
|
||||||
|
.any(|params| params.includes(&loaded.id))
|
||||||
|
{
|
||||||
|
return false;
|
||||||
|
}
|
||||||
true
|
true
|
||||||
}));
|
}));
|
||||||
|
|
||||||
|
// Recursively visit nested functions with updated parameter stack
|
||||||
|
self.outer_parameters.push(parameters);
|
||||||
|
visitor::walk_body(self, body);
|
||||||
|
self.outer_parameters.pop();
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
Stmt::Return(ast::StmtReturn {
|
Stmt::Return(ast::StmtReturn {
|
||||||
|
|
@ -153,6 +178,15 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
} else if attr == "apply" {
|
||||||
|
// If pandas is imported, apply is safe like map
|
||||||
|
if self.pandas_imported {
|
||||||
|
for arg in &*arguments.args {
|
||||||
|
if arg.is_lambda_expr() {
|
||||||
|
self.safe_functions.push(arg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
_ => {}
|
_ => {}
|
||||||
|
|
@ -173,10 +207,16 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
|
||||||
node_index: _,
|
node_index: _,
|
||||||
}) => {
|
}) => {
|
||||||
if !self.safe_functions.contains(&expr) {
|
if !self.safe_functions.contains(&expr) {
|
||||||
// Collect all loaded variable names.
|
// Collect all loaded variable names from the lambda body.
|
||||||
let mut visitor = LoadedNamesVisitor::default();
|
let mut visitor = LoadedNamesVisitor::default();
|
||||||
visitor.visit_expr(body);
|
visitor.visit_expr(body);
|
||||||
|
|
||||||
|
// Collect lambda parameter names
|
||||||
|
let lambda_param_names: Vec<&str> = parameters
|
||||||
|
.as_ref()
|
||||||
|
.map(|params| params.iter().map(|param| param.name().as_str()).collect())
|
||||||
|
.unwrap_or_default();
|
||||||
|
|
||||||
// Treat any non-arguments as "suspicious".
|
// Treat any non-arguments as "suspicious".
|
||||||
self.names
|
self.names
|
||||||
.extend(visitor.loaded.into_iter().filter(|loaded| {
|
.extend(visitor.loaded.into_iter().filter(|loaded| {
|
||||||
|
|
@ -184,10 +224,8 @@ impl<'a> Visitor<'a> for SuspiciousVariablesVisitor<'a> {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
if parameters
|
// Check if the variable is a lambda parameter
|
||||||
.as_ref()
|
if lambda_param_names.contains(&loaded.id.as_str()) {
|
||||||
.is_some_and(|parameters| parameters.includes(&loaded.id))
|
|
||||||
{
|
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -285,7 +323,12 @@ pub(crate) fn function_uses_loop_variable(checker: &Checker, node: &Node) {
|
||||||
// Identify any "suspicious" variables. These are defined as variables that are
|
// Identify any "suspicious" variables. These are defined as variables that are
|
||||||
// referenced in a function or lambda body, but aren't bound as arguments.
|
// referenced in a function or lambda body, but aren't bound as arguments.
|
||||||
let suspicious_variables = {
|
let suspicious_variables = {
|
||||||
let mut visitor = SuspiciousVariablesVisitor::default();
|
let mut visitor = SuspiciousVariablesVisitor {
|
||||||
|
names: Vec::new(),
|
||||||
|
safe_functions: Vec::new(),
|
||||||
|
pandas_imported: checker.semantic().seen_module(Modules::PANDAS),
|
||||||
|
outer_parameters: Vec::new(),
|
||||||
|
};
|
||||||
match node {
|
match node {
|
||||||
Node::Stmt(stmt) => visitor.visit_stmt(stmt),
|
Node::Stmt(stmt) => visitor.visit_stmt(stmt),
|
||||||
Node::Expr(expr) => visitor.visit_expr(expr),
|
Node::Expr(expr) => visitor.visit_expr(expr),
|
||||||
|
|
|
||||||
|
|
@ -244,3 +244,14 @@ B023 Function definition does not bind loop variable `i`
|
||||||
174 | return [lambda: i for i in range(3)] # error
|
174 | return [lambda: i for i in range(3)] # error
|
||||||
| ^
|
| ^
|
||||||
|
|
|
|
||||||
|
|
||||||
|
B023 Function definition does not bind loop variable `value`
|
||||||
|
--> B023.py:231:20
|
||||||
|
|
|
||||||
|
229 | def add_one():
|
||||||
|
230 | def _add_one_inner():
|
||||||
|
231 | return value + 1 # Should trigger B023 - value is loop variable, not bound
|
||||||
|
| ^^^^^
|
||||||
|
232 |
|
||||||
|
233 | return _add_one_inner
|
||||||
|
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue