diff --git a/crates/ruff/src/rules/flake8_boolean_trap/helpers.rs b/crates/ruff/src/rules/flake8_boolean_trap/helpers.rs index 2c397470b7..01cc630de6 100644 --- a/crates/ruff/src/rules/flake8_boolean_trap/helpers.rs +++ b/crates/ruff/src/rules/flake8_boolean_trap/helpers.rs @@ -4,50 +4,57 @@ use ruff_diagnostics::{Diagnostic, DiagnosticKind}; use crate::checkers::ast::Checker; -pub(super) const FUNC_CALL_NAME_ALLOWLIST: &[&str] = &[ - "append", - "assertEqual", - "assertEquals", - "assertNotEqual", - "assertNotEquals", - "bool", - "bytes", - "count", - "failIfEqual", - "failUnlessEqual", - "float", - "fromkeys", - "get", - "getattr", - "getboolean", - "getfloat", - "getint", - "index", - "insert", - "int", - "param", - "pop", - "remove", - "set_blocking", - "set_enabled", - "setattr", - "__setattr__", - "setdefault", - "str", -]; +/// Returns `true` if a function call is allowed to use a boolean trap. +pub(super) fn is_allowed_func_call(name: &str) -> bool { + matches!( + name, + "append" + | "assertEqual" + | "assertEquals" + | "assertNotEqual" + | "assertNotEquals" + | "bool" + | "bytes" + | "count" + | "failIfEqual" + | "failUnlessEqual" + | "float" + | "fromkeys" + | "get" + | "getattr" + | "getboolean" + | "getfloat" + | "getint" + | "index" + | "insert" + | "int" + | "param" + | "pop" + | "remove" + | "set_blocking" + | "set_enabled" + | "setattr" + | "__setattr__" + | "setdefault" + | "str" + ) +} -pub(super) const FUNC_DEF_NAME_ALLOWLIST: &[&str] = &["__setitem__"]; +/// Returns `true` if a function definition is allowed to use a boolean trap. +pub(super) fn is_allowed_func_def(name: &str) -> bool { + matches!(name, "__setitem__") +} /// Returns `true` if an argument is allowed to use a boolean trap. To return /// `true`, the function name must be explicitly allowed, and the argument must /// be either the first or second argument in the call. pub(super) fn allow_boolean_trap(func: &Expr) -> bool { if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func { - return FUNC_CALL_NAME_ALLOWLIST.contains(&attr.as_ref()); + return is_allowed_func_call(attr); } if let Expr::Name(ast::ExprName { id, .. }) = func { - return FUNC_CALL_NAME_ALLOWLIST.contains(&id.as_ref()); + return is_allowed_func_call(id); } false diff --git a/crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_default_value_in_function_definition.rs b/crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_default_value_in_function_definition.rs index fda0ff5af4..1e853abb87 100644 --- a/crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_default_value_in_function_definition.rs +++ b/crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_default_value_in_function_definition.rs @@ -1,14 +1,11 @@ use rustpython_parser::ast::{ArgWithDefault, Arguments, Decorator}; use ruff_diagnostics::Violation; - use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::collect_call_path; use crate::checkers::ast::Checker; -use crate::rules::flake8_boolean_trap::helpers::add_if_boolean; - -use super::super::helpers::FUNC_DEF_NAME_ALLOWLIST; +use crate::rules::flake8_boolean_trap::helpers::{add_if_boolean, is_allowed_func_def}; /// ## What it does /// Checks for the use of booleans as default values in function definitions. @@ -64,7 +61,7 @@ pub(crate) fn check_boolean_default_value_in_function_definition( decorator_list: &[Decorator], arguments: &Arguments, ) { - if FUNC_DEF_NAME_ALLOWLIST.contains(&name) { + if is_allowed_func_def(name) { return; } diff --git a/crates/ruff/src/rules/flake8_boolean_trap/rules/check_positional_boolean_in_def.rs b/crates/ruff/src/rules/flake8_boolean_trap/rules/check_positional_boolean_in_def.rs index 57e50e7be7..68dc43238f 100644 --- a/crates/ruff/src/rules/flake8_boolean_trap/rules/check_positional_boolean_in_def.rs +++ b/crates/ruff/src/rules/flake8_boolean_trap/rules/check_positional_boolean_in_def.rs @@ -6,7 +6,7 @@ use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::collect_call_path; use crate::checkers::ast::Checker; -use crate::rules::flake8_boolean_trap::helpers::FUNC_DEF_NAME_ALLOWLIST; +use crate::rules::flake8_boolean_trap::helpers::is_allowed_func_def; /// ## What it does /// Checks for boolean positional arguments in function definitions. @@ -82,7 +82,7 @@ pub(crate) fn check_positional_boolean_in_def( decorator_list: &[Decorator], arguments: &Arguments, ) { - if FUNC_DEF_NAME_ALLOWLIST.contains(&name) { + if is_allowed_func_def(name) { return; } diff --git a/crates/ruff/src/rules/flake8_pyi/rules/non_self_return_type.rs b/crates/ruff/src/rules/flake8_pyi/rules/non_self_return_type.rs index 04d76b4fb0..56f4cd9da0 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/non_self_return_type.rs +++ b/crates/ruff/src/rules/flake8_pyi/rules/non_self_return_type.rs @@ -155,7 +155,7 @@ pub(crate) fn non_self_return_type( } // In-place methods that are expected to return `Self`. - if INPLACE_BINOP_METHODS.contains(&name) { + if is_inplace_bin_op(name) { if !is_self(returns, checker.semantic()) { checker.diagnostics.push(Diagnostic::new( NonSelfReturnType { @@ -214,21 +214,25 @@ pub(crate) fn non_self_return_type( } } -const INPLACE_BINOP_METHODS: &[&str] = &[ - "__iadd__", - "__isub__", - "__imul__", - "__imatmul__", - "__itruediv__", - "__ifloordiv__", - "__imod__", - "__ipow__", - "__ilshift__", - "__irshift__", - "__iand__", - "__ixor__", - "__ior__", -]; +/// Returns `true` if the method is an in-place binary operator. +fn is_inplace_bin_op(name: &str) -> bool { + matches!( + name, + "__iadd__" + | "__isub__" + | "__imul__" + | "__imatmul__" + | "__itruediv__" + | "__ifloordiv__" + | "__imod__" + | "__ipow__" + | "__ilshift__" + | "__irshift__" + | "__iand__" + | "__ixor__" + | "__ior__" + ) +} /// Return `true` if the given expression resolves to the given name. fn is_name(expr: &Expr, name: &str) -> bool { diff --git a/crates/ruff/src/rules/flake8_pytest_style/rules/patch.rs b/crates/ruff/src/rules/flake8_pytest_style/rules/patch.rs index 2bc47f6952..7fb2da8f1f 100644 --- a/crates/ruff/src/rules/flake8_pytest_style/rules/patch.rs +++ b/crates/ruff/src/rules/flake8_pytest_style/rules/patch.rs @@ -3,7 +3,7 @@ use rustpython_parser::ast::{self, Expr, Keyword, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::call_path::compose_call_path; +use ruff_python_ast::call_path::collect_call_path; use ruff_python_ast::helpers::{collect_arg_names, SimpleCallArgs}; use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; @@ -18,26 +18,6 @@ impl Violation for PytestPatchWithLambda { } } -const PATCH_NAMES: &[&str] = &[ - "mocker.patch", - "class_mocker.patch", - "module_mocker.patch", - "package_mocker.patch", - "session_mocker.patch", - "mock.patch", - "unittest.mock.patch", -]; - -const PATCH_OBJECT_NAMES: &[&str] = &[ - "mocker.patch.object", - "class_mocker.patch.object", - "module_mocker.patch.object", - "package_mocker.patch.object", - "session_mocker.patch.object", - "mock.patch.object", - "unittest.mock.patch.object", -]; - #[derive(Default)] /// Visitor that checks references the argument names in the lambda body. struct LambdaBodyVisitor<'a> { @@ -98,14 +78,35 @@ pub(crate) fn patch_with_lambda( args: &[Expr], keywords: &[Keyword], ) -> Option { - if let Some(call_path) = compose_call_path(call) { - if PATCH_NAMES.contains(&call_path.as_str()) { - check_patch_call(call, args, keywords, 1) - } else if PATCH_OBJECT_NAMES.contains(&call_path.as_str()) { - check_patch_call(call, args, keywords, 2) - } else { - None - } + let call_path = collect_call_path(call)?; + + if matches!( + call_path.as_slice(), + [ + "mocker" + | "class_mocker" + | "module_mocker" + | "package_mocker" + | "session_mocker" + | "mock", + "patch" + ] | ["unittest", "mock", "patch"] + ) { + check_patch_call(call, args, keywords, 1) + } else if matches!( + call_path.as_slice(), + [ + "mocker" + | "class_mocker" + | "module_mocker" + | "package_mocker" + | "session_mocker" + | "mock", + "patch", + "object" + ] | ["unittest", "mock", "patch", "object"] + ) { + check_patch_call(call, args, keywords, 2) } else { None } diff --git a/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs b/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs index f0385b7560..05b96f3e19 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/ast_unary_op.rs @@ -119,7 +119,12 @@ impl AlwaysAutofixableViolation for DoubleNegation { } } -const DUNDER_METHODS: &[&str] = &["__eq__", "__ne__", "__lt__", "__le__", "__gt__", "__ge__"]; +fn is_dunder_method(name: &str) -> bool { + matches!( + name, + "__eq__" | "__ne__" | "__lt__" | "__le__" | "__gt__" | "__ge__" + ) +} fn is_exception_check(stmt: &Stmt) -> bool { let Stmt::If(ast::StmtIf {test: _, body, orelse: _, range: _ })= stmt else { @@ -159,7 +164,7 @@ pub(crate) fn negation_with_equal_op( | ScopeKind::AsyncFunction(ast::StmtAsyncFunctionDef { name, .. }) = &checker.semantic().scope().kind { - if DUNDER_METHODS.contains(&name.as_str()) { + if is_dunder_method(name) { return; } } @@ -211,7 +216,7 @@ pub(crate) fn negation_with_not_equal_op( | ScopeKind::AsyncFunction(ast::StmtAsyncFunctionDef { name, .. }) = &checker.semantic().scope().kind { - if DUNDER_METHODS.contains(&name.as_str()) { + if is_dunder_method(name) { return; } } diff --git a/crates/ruff/src/rules/pyflakes/rules/future_feature_not_defined.rs b/crates/ruff/src/rules/pyflakes/rules/future_feature_not_defined.rs index 11dc78d70b..a00d5267e3 100644 --- a/crates/ruff/src/rules/pyflakes/rules/future_feature_not_defined.rs +++ b/crates/ruff/src/rules/pyflakes/rules/future_feature_not_defined.rs @@ -2,7 +2,7 @@ use rustpython_parser::ast::{Alias, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_stdlib::future::ALL_FEATURE_NAMES; +use ruff_python_stdlib::future::is_feature_name; use crate::checkers::ast::Checker; @@ -30,12 +30,14 @@ impl Violation for FutureFeatureNotDefined { } pub(crate) fn future_feature_not_defined(checker: &mut Checker, alias: &Alias) { - if !ALL_FEATURE_NAMES.contains(&alias.name.as_str()) { - checker.diagnostics.push(Diagnostic::new( - FutureFeatureNotDefined { - name: alias.name.to_string(), - }, - alias.range(), - )); + if is_feature_name(&alias.name) { + return; } + + checker.diagnostics.push(Diagnostic::new( + FutureFeatureNotDefined { + name: alias.name.to_string(), + }, + alias.range(), + )); } diff --git a/crates/ruff/src/rules/pyupgrade/rules/deprecated_import.rs b/crates/ruff/src/rules/pyupgrade/rules/deprecated_import.rs index f178e3e8cb..314df7705c 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/deprecated_import.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/deprecated_import.rs @@ -94,15 +94,13 @@ impl Violation for DeprecatedImport { } } -// A list of modules that may involve import rewrites. -const RELEVANT_MODULES: &[&str] = &[ - "collections", - "pipes", - "mypy_extensions", - "typing_extensions", - "typing", - "typing.re", -]; +/// Returns `true` if the module may contain deprecated imports. +fn is_relevant_module(module: &str) -> bool { + matches!( + module, + "collections" | "pipes" | "mypy_extensions" | "typing_extensions" | "typing" | "typing.re" + ) +} // Members of `collections` that were moved to `collections.abc`. const COLLECTIONS_TO_ABC: &[&str] = &[ @@ -560,7 +558,7 @@ pub(crate) fn deprecated_import( return; }; - if !RELEVANT_MODULES.contains(&module) { + if !is_relevant_module(module) { return; } diff --git a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs index b66ed7eaaf..60d856bec0 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs +++ b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs @@ -52,37 +52,6 @@ impl AlwaysAutofixableViolation for UnnecessaryBuiltinImport { } } -const BUILTINS: &[&str] = &[ - "*", - "ascii", - "bytes", - "chr", - "dict", - "filter", - "hex", - "input", - "int", - "isinstance", - "list", - "map", - "max", - "min", - "next", - "object", - "oct", - "open", - "pow", - "range", - "round", - "str", - "super", - "zip", -]; -const IO: &[&str] = &["open"]; -const SIX_MOVES_BUILTINS: &[&str] = BUILTINS; -const SIX: &[&str] = &["callable", "next"]; -const SIX_MOVES: &[&str] = &["filter", "input", "map", "range", "zip"]; - /// UP029 pub(crate) fn unnecessary_builtin_import( checker: &mut Checker, @@ -90,26 +59,53 @@ pub(crate) fn unnecessary_builtin_import( module: &str, names: &[Alias], ) { - let deprecated_names = match module { - "builtins" => BUILTINS, - "io" => IO, - "six" => SIX, - "six.moves" => SIX_MOVES, - "six.moves.builtins" => SIX_MOVES_BUILTINS, - _ => return, - }; - - // Do this with a filter? - let mut unused_imports: Vec<&Alias> = vec![]; - for alias in names { - if alias.asname.is_some() { - continue; - } - if deprecated_names.contains(&alias.name.as_str()) { - unused_imports.push(alias); - } + // Ignore irrelevant modules. + if !matches!( + module, + "builtins" | "io" | "six" | "six.moves" | "six.moves.builtins" + ) { + return; } + // Identify unaliased, builtin imports. + let unused_imports: Vec<&Alias> = names + .iter() + .filter(|alias| alias.asname.is_none()) + .filter(|alias| { + matches!( + (module, alias.name.as_str()), + ( + "builtins" | "six.moves.builtins", + "*" | "ascii" + | "bytes" + | "chr" + | "dict" + | "filter" + | "hex" + | "input" + | "int" + | "isinstance" + | "list" + | "map" + | "max" + | "min" + | "next" + | "object" + | "oct" + | "open" + | "pow" + | "range" + | "round" + | "str" + | "super" + | "zip" + ) | ("io", "open") + | ("six", "callable" | "next") + | ("six.moves", "filter" | "input" | "map" | "range" | "zip") + ) + }) + .collect(); + if unused_imports.is_empty() { return; } diff --git a/crates/ruff_python_stdlib/src/future.rs b/crates/ruff_python_stdlib/src/future.rs index 0e7bbb3c0c..b1adaadedf 100644 --- a/crates/ruff_python_stdlib/src/future.rs +++ b/crates/ruff_python_stdlib/src/future.rs @@ -1,13 +1,17 @@ -/// A copy of `__future__.all_feature_names`. -pub const ALL_FEATURE_NAMES: &[&str] = &[ - "nested_scopes", - "generators", - "division", - "absolute_import", - "with_statement", - "print_function", - "unicode_literals", - "barry_as_FLUFL", - "generator_stop", - "annotations", -]; +/// Returns `true` if `name` is a valid `__future__` feature name, as defined by +/// `__future__.all_feature_names`. +pub fn is_feature_name(name: &str) -> bool { + matches!( + name, + "nested_scopes" + | "generators" + | "division" + | "absolute_import" + | "with_statement" + | "print_function" + | "unicode_literals" + | "barry_as_FLUFL" + | "generator_stop" + | "annotations" + ) +}