Skip to content

Commit 0af849e

Browse files
committed
Split isinstance_target into a function
1 parent d2fed1a commit 0af849e

File tree

1 file changed

+38
-34
lines changed

1 file changed

+38
-34
lines changed

crates/ruff_linter/src/rules/flake8_simplify/rules/ast_bool_op.rs

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use ruff_python_ast::comparable::ComparableExpr;
1212
use ruff_python_ast::helpers::{contains_effect, Truthiness};
1313
use ruff_python_ast::parenthesize::parenthesized_range;
1414
use ruff_python_codegen::Generator;
15+
use ruff_python_semantic::SemanticModel;
1516

1617
use crate::checkers::ast::Checker;
1718
use crate::registry::AsRule;
@@ -298,6 +299,42 @@ fn is_same_expr<'a>(a: &'a Expr, b: &'a Expr) -> Option<&'a str> {
298299
None
299300
}
300301

302+
/// If `call` is an `isinstance()` call, return its target.
303+
fn isinstance_target<'a>(call: &'a Expr, semantic: &'a SemanticModel) -> Option<&'a Expr> {
304+
// Verify that this is an `isinstance` call.
305+
let Expr::Call(ast::ExprCall {
306+
func,
307+
arguments:
308+
Arguments {
309+
args,
310+
keywords,
311+
range: _,
312+
},
313+
range: _,
314+
}) = &call
315+
else {
316+
return None;
317+
};
318+
if args.len() != 2 {
319+
return None;
320+
}
321+
if !keywords.is_empty() {
322+
return None;
323+
}
324+
let Expr::Name(ast::ExprName { id: func_name, .. }) = func.as_ref() else {
325+
return None;
326+
};
327+
if func_name != "isinstance" {
328+
return None;
329+
}
330+
if !semantic.is_builtin("isinstance") {
331+
return None;
332+
}
333+
334+
// Collect the target (e.g., `obj` in `isinstance(obj, int)`).
335+
Some(&args[0])
336+
}
337+
301338
/// SIM101
302339
pub(crate) fn duplicate_isinstance_call(checker: &mut Checker, expr: &Expr) {
303340
let Expr::BoolOp(ast::ExprBoolOp {
@@ -314,44 +351,11 @@ pub(crate) fn duplicate_isinstance_call(checker: &mut Checker, expr: &Expr) {
314351
let mut duplicates: Vec<Vec<usize>> = Vec::new();
315352
let mut last_target_option: Option<ComparableExpr> = None;
316353
for (index, call) in values.iter().enumerate() {
317-
// Verify that this is an `isinstance` call.
318-
let Expr::Call(ast::ExprCall {
319-
func,
320-
arguments:
321-
Arguments {
322-
args,
323-
keywords,
324-
range: _,
325-
},
326-
range: _,
327-
}) = &call
328-
else {
354+
let Some(target) = isinstance_target(&call, checker.semantic()) else {
329355
last_target_option = None;
330356
continue;
331357
};
332-
if args.len() != 2 {
333-
last_target_option = None;
334-
continue;
335-
}
336-
if !keywords.is_empty() {
337-
last_target_option = None;
338-
continue;
339-
}
340-
let Expr::Name(ast::ExprName { id: func_name, .. }) = func.as_ref() else {
341-
last_target_option = None;
342-
continue;
343-
};
344-
if func_name != "isinstance" {
345-
last_target_option = None;
346-
continue;
347-
}
348-
if !checker.semantic().is_builtin("isinstance") {
349-
last_target_option = None;
350-
continue;
351-
}
352358

353-
// Collect the target (e.g., `obj` in `isinstance(obj, int)`).
354-
let target = &args[0];
355359
if last_target_option
356360
.as_ref()
357361
.is_some_and(|last_target| *last_target == ComparableExpr::from(target))

0 commit comments

Comments
 (0)