diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/helpers.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/helpers.rs index af748a5f68..f22a8dd908 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/helpers.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/helpers.rs @@ -1,12 +1,4 @@ -use ruff_python_ast::{self as ast, Expr, Keyword}; - -pub(super) fn expr_name(func: &Expr) -> Option<&str> { - if let Expr::Name(ast::ExprName { id, .. }) = func { - Some(id) - } else { - None - } -} +use ruff_python_ast::{Expr, Keyword}; pub(super) fn exactly_one_argument_with_matching_function<'a>( name: &str, @@ -20,7 +12,8 @@ pub(super) fn exactly_one_argument_with_matching_function<'a>( if !keywords.is_empty() { return None; } - if expr_name(func)? != name { + let func = func.as_name_expr()?; + if func.id != name { return None; } Some(arg) @@ -31,8 +24,8 @@ pub(super) fn first_argument_with_matching_function<'a>( func: &Expr, args: &'a [Expr], ) -> Option<&'a Expr> { - if expr_name(func)? == name { - Some(args.first()?) + if func.as_name_expr().is_some_and(|func| func.id == name) { + args.first() } else { None } diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs index 6757c96aa3..ae09498b29 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_call_around_sorted.rs @@ -1,14 +1,11 @@ -use ruff_python_ast::{self as ast, Expr, Ranged}; - use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Expr, Ranged}; use crate::checkers::ast::Checker; use crate::registry::AsRule; use crate::rules::flake8_comprehensions::fixes; -use super::helpers; - /// ## What it does /// Checks for unnecessary `list` or `reversed` calls around `sorted` /// calls. @@ -57,10 +54,10 @@ pub(crate) fn unnecessary_call_around_sorted( func: &Expr, args: &[Expr], ) { - let Some(outer) = helpers::expr_name(func) else { + let Some(outer) = func.as_name_expr() else { return; }; - if !(outer == "list" || outer == "reversed") { + if !matches!(outer.id.as_str(), "list" | "reversed") { return; } let Some(arg) = args.first() else { @@ -69,18 +66,18 @@ pub(crate) fn unnecessary_call_around_sorted( let Expr::Call(ast::ExprCall { func, .. }) = arg else { return; }; - let Some(inner) = helpers::expr_name(func) else { + let Some(inner) = func.as_name_expr() else { return; }; - if inner != "sorted" { + if inner.id != "sorted" { return; } - if !checker.semantic().is_builtin(inner) || !checker.semantic().is_builtin(outer) { + if !checker.semantic().is_builtin(&inner.id) || !checker.semantic().is_builtin(&outer.id) { return; } let mut diagnostic = Diagnostic::new( UnnecessaryCallAroundSorted { - func: outer.to_string(), + func: outer.id.to_string(), }, expr.range(), ); @@ -91,7 +88,7 @@ pub(crate) fn unnecessary_call_around_sorted( checker.stylist(), expr, )?; - if outer == "reversed" { + if outer.id == "reversed" { Ok(Fix::suggested(edit)) } else { Ok(Fix::automatic(edit)) diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_collection_call.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_collection_call.rs index 444a1856e1..4ce85562c5 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_collection_call.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_collection_call.rs @@ -1,15 +1,12 @@ -use ruff_python_ast::{Expr, Keyword, Ranged}; - use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{Expr, Keyword, Ranged}; use crate::checkers::ast::Checker; use crate::registry::AsRule; use crate::rules::flake8_comprehensions::fixes; use crate::rules::flake8_comprehensions::settings::Settings; -use super::helpers; - /// ## What it does /// Checks for unnecessary `dict`, `list` or `tuple` calls that can be /// rewritten as empty literals. @@ -63,10 +60,10 @@ pub(crate) fn unnecessary_collection_call( if !args.is_empty() { return; } - let Some(id) = helpers::expr_name(func) else { + let Some(func) = func.as_name_expr() else { return; }; - match id { + match func.id.as_str() { "dict" if keywords.is_empty() || (!settings.allow_dict_calls_with_keyword_arguments @@ -79,12 +76,12 @@ pub(crate) fn unnecessary_collection_call( } _ => return, }; - if !checker.semantic().is_builtin(id) { + if !checker.semantic().is_builtin(func.id.as_str()) { return; } let mut diagnostic = Diagnostic::new( UnnecessaryCollectionCall { - obj_type: id.to_string(), + obj_type: func.id.to_string(), }, expr.range(), ); diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs index c9a7887dab..861dd766ce 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_comprehension.rs @@ -1,14 +1,11 @@ -use ruff_python_ast::{self as ast, Comprehension, Expr, Ranged}; - use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Comprehension, Expr, Ranged}; use crate::checkers::ast::Checker; use crate::registry::AsRule; use crate::rules::flake8_comprehensions::fixes; -use super::helpers; - /// ## What it does /// Checks for unnecessary `dict`, `list`, and `set` comprehension. /// @@ -88,28 +85,28 @@ pub(crate) fn unnecessary_dict_comprehension( if !generator.ifs.is_empty() || generator.is_async { return; } - let Some(key_id) = helpers::expr_name(key) else { + let Some(key) = key.as_name_expr() else { return; }; - let Some(value_id) = helpers::expr_name(value) else { + let Some(value) = value.as_name_expr() else { return; }; let Expr::Tuple(ast::ExprTuple { elts, .. }) = &generator.target else { return; }; - if elts.len() != 2 { - return; - } - let Some(target_key_id) = helpers::expr_name(&elts[0]) else { + let [target_key, target_value] = elts.as_slice() else { return; }; - if target_key_id != key_id { - return; - } - let Some(target_value_id) = helpers::expr_name(&elts[1]) else { + let Some(target_key) = target_key.as_name_expr() else { return; }; - if target_value_id != value_id { + let Some(target_value) = target_value.as_name_expr() else { + return; + }; + if target_key.id != key.id { + return; + } + if target_value.id != value.id { return; } add_diagnostic(checker, expr); @@ -128,13 +125,13 @@ pub(crate) fn unnecessary_list_set_comprehension( if !generator.ifs.is_empty() || generator.is_async { return; } - let Some(elt_id) = helpers::expr_name(elt) else { + let Some(elt) = elt.as_name_expr() else { return; }; - let Some(target_id) = helpers::expr_name(&generator.target) else { + let Some(target) = generator.target.as_name_expr() else { return; }; - if elt_id != target_id { + if elt.id != target.id { return; } add_diagnostic(checker, expr); diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs index 2e740c5434..489c970dce 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_double_cast_or_process.rs @@ -1,15 +1,12 @@ -use ruff_python_ast::{self as ast, Arguments, Expr, Keyword, Ranged}; - use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::comparable::ComparableKeyword; +use ruff_python_ast::{self as ast, Arguments, Expr, Keyword, Ranged}; use crate::checkers::ast::Checker; use crate::registry::AsRule; use crate::rules::flake8_comprehensions::fixes; -use super::helpers; - /// ## What it does /// Checks for unnecessary `list`, `reversed`, `set`, `sorted`, and `tuple` /// call within `list`, `set`, `sorted`, and `tuple` calls. @@ -72,15 +69,13 @@ pub(crate) fn unnecessary_double_cast_or_process( args: &[Expr], outer_kw: &[Keyword], ) { - let Some(outer) = helpers::expr_name(func) else { + let Some(outer) = func.as_name_expr() else { return; }; - if !(outer == "list" - || outer == "tuple" - || outer == "set" - || outer == "reversed" - || outer == "sorted") - { + if !matches!( + outer.id.as_str(), + "list" | "tuple" | "set" | "reversed" | "sorted" + ) { return; } let Some(arg) = args.first() else { @@ -96,16 +91,16 @@ pub(crate) fn unnecessary_double_cast_or_process( else { return; }; - let Some(inner) = helpers::expr_name(func) else { + let Some(inner) = func.as_name_expr() else { return; }; - if !checker.semantic().is_builtin(inner) || !checker.semantic().is_builtin(outer) { + if !checker.semantic().is_builtin(&inner.id) || !checker.semantic().is_builtin(&outer.id) { return; } // Avoid collapsing nested `sorted` calls with non-identical keyword arguments // (i.e., `key`, `reverse`). - if inner == "sorted" && outer == "sorted" { + if inner.id == "sorted" && outer.id == "sorted" { if inner_kw.len() != outer_kw.len() { return; } @@ -118,18 +113,19 @@ pub(crate) fn unnecessary_double_cast_or_process( } } - // Ex) set(tuple(...)) - // Ex) list(tuple(...)) - // Ex) set(set(...)) - if ((outer == "set" || outer == "sorted") - && (inner == "list" || inner == "tuple" || inner == "reversed" || inner == "sorted")) - || (outer == "set" && inner == "set") - || ((outer == "list" || outer == "tuple") && (inner == "list" || inner == "tuple")) - { + // Ex) `set(tuple(...))` + // Ex) `list(tuple(...))` + // Ex) `set(set(...))` + if matches!( + (outer.id.as_str(), inner.id.as_str()), + ("set" | "sorted", "list" | "tuple" | "reversed" | "sorted") + | ("set", "set") + | ("list" | "tuple", "list" | "tuple") + ) { let mut diagnostic = Diagnostic::new( UnnecessaryDoubleCastOrProcess { - inner: inner.to_string(), - outer: outer.to_string(), + inner: inner.id.to_string(), + outer: outer.id.to_string(), }, expr.range(), ); diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_dict.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_dict.rs index 31a30a7624..76b423facc 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_dict.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_literal_dict.rs @@ -70,7 +70,7 @@ pub(crate) fn unnecessary_literal_dict( // Accept `dict((1, 2), ...))` `dict([(1, 2), ...])`. if !elts .iter() - .all(|elt| matches!(&elt, Expr::Tuple(ast::ExprTuple { elts, .. } )if elts.len() == 2)) + .all(|elt| matches!(&elt, Expr::Tuple(ast::ExprTuple { elts, .. }) if elts.len() == 2)) { return; } diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs index 8b7e19375c..c914753be2 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_map.rs @@ -68,11 +68,11 @@ pub(crate) fn unnecessary_map( func: &Expr, args: &[Expr], ) { - let Some(id) = helpers::expr_name(func) else { + let Some(func) = func.as_name_expr() else { return; }; - let object_type = match id { + let object_type = match func.id.as_str() { "map" => ObjectType::Generator, "list" => ObjectType::List, "set" => ObjectType::Set, @@ -80,20 +80,20 @@ pub(crate) fn unnecessary_map( _ => return, }; - if !checker.semantic().is_builtin(id) { + if !checker.semantic().is_builtin(&func.id) { return; } match object_type { ObjectType::Generator => { // Exclude the parent if already matched by other arms. - if let Some(Expr::Call(ast::ExprCall { func, .. })) = parent { - if let Some(name) = helpers::expr_name(func) { - if matches!(name, "list" | "set" | "dict") { - return; - } - } - }; + if parent + .and_then(ruff_python_ast::Expr::as_call_expr) + .and_then(|call| call.func.as_name_expr()) + .is_some_and(|name| matches!(name.id.as_str(), "list" | "set" | "dict")) + { + return; + } // Only flag, e.g., `map(lambda x: x + 1, iterable)`. let [Expr::Lambda(ast::ExprLambda { diff --git a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_subscript_reversal.rs b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_subscript_reversal.rs index 60076232c4..092713ab4e 100644 --- a/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_subscript_reversal.rs +++ b/crates/ruff/src/rules/flake8_comprehensions/rules/unnecessary_subscript_reversal.rs @@ -1,13 +1,11 @@ use num_bigint::BigInt; -use ruff_python_ast::{self as ast, Constant, Expr, Ranged, UnaryOp}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Constant, Expr, Ranged, UnaryOp}; use crate::checkers::ast::Checker; -use super::helpers; - /// ## What it does /// Checks for unnecessary subscript reversal of iterable. /// @@ -52,13 +50,13 @@ pub(crate) fn unnecessary_subscript_reversal( let Some(first_arg) = args.first() else { return; }; - let Some(id) = helpers::expr_name(func) else { + let Some(func) = func.as_name_expr() else { return; }; - if !(id == "set" || id == "sorted" || id == "reversed") { + if !matches!(func.id.as_str(), "reversed" | "set" | "sorted") { return; } - if !checker.semantic().is_builtin(id) { + if !checker.semantic().is_builtin(&func.id) { return; } let Expr::Subscript(ast::ExprSubscript { slice, .. }) = first_arg else { @@ -99,7 +97,7 @@ pub(crate) fn unnecessary_subscript_reversal( }; checker.diagnostics.push(Diagnostic::new( UnnecessarySubscriptReversal { - func: id.to_string(), + func: func.id.to_string(), }, expr.range(), ));