Remove unnecessary expr_name function (#6544)

This commit is contained in:
Charlie Marsh 2023-08-13 23:51:36 -04:00 committed by GitHub
parent 768686148f
commit bf4c6473c8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 69 additions and 91 deletions

View file

@ -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
}

View file

@ -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))

View file

@ -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(),
);

View file

@ -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);

View file

@ -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(),
);

View file

@ -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;
}

View file

@ -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 {

View file

@ -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(),
));