From 4233f6ec91b0c5db5407e1132f7e30476c3bc120 Mon Sep 17 00:00:00 2001 From: Jonathan Plasse <13716151+JonathanPlasse@users.noreply.github.com> Date: Wed, 24 May 2023 17:30:40 +0200 Subject: [PATCH] Update to the new rule architecture (#4589) --- crates/ruff/src/checkers/ast/mod.rs | 32 +- crates/ruff/src/flake8_to_ruff/converter.rs | 2 +- crates/ruff/src/registry.rs | 4 +- .../{rules.rs => rules/commented_out_code.rs} | 2 +- crates/ruff/src/rules/eradicate/rules/mod.rs | 3 + crates/ruff/src/rules/flake8_2020/helpers.rs | 8 + crates/ruff/src/rules/flake8_2020/mod.rs | 1 + .../{rules.rs => rules/compare.rs} | 117 +--- .../ruff/src/rules/flake8_2020/rules/mod.rs | 12 + .../flake8_2020/rules/name_or_attribute.rs | 29 + .../src/rules/flake8_2020/rules/subscript.rs | 96 +++ .../{rules.rs => rules/definition.rs} | 4 +- .../src/rules/flake8_annotations/rules/mod.rs | 8 + crates/ruff/src/rules/flake8_async/helpers.rs | 18 + crates/ruff/src/rules/flake8_async/mod.rs | 1 + crates/ruff/src/rules/flake8_async/rules.rs | 235 ------- .../flake8_async/rules/blocking_http_call.rs | 83 +++ .../flake8_async/rules/blocking_os_call.rs | 75 ++ .../ruff/src/rules/flake8_async/rules/mod.rs | 9 + .../rules/open_sleep_or_subprocess_call.rs | 81 +++ .../{rules.rs => rules/blind_except.rs} | 0 .../rules/flake8_blind_except/rules/mod.rs | 3 + .../src/rules/flake8_boolean_trap/helpers.rs | 66 ++ .../ruff/src/rules/flake8_boolean_trap/mod.rs | 1 + .../src/rules/flake8_boolean_trap/rules.rs | 301 -------- ...an_default_value_in_function_definition.rs | 80 +++ ...olean_positional_value_in_function_call.rs | 60 ++ .../rules/check_positional_boolean_in_def.rs | 120 ++++ .../rules/flake8_boolean_trap/rules/mod.rs | 13 + .../ruff/src/rules/flake8_builtins/helpers.rs | 45 ++ crates/ruff/src/rules/flake8_builtins/mod.rs | 1 + .../ruff/src/rules/flake8_builtins/rules.rs | 257 ------- .../rules/builtin_argument_shadowing.rs | 78 +++ .../rules/builtin_attribute_shadowing.rs | 78 +++ .../rules/builtin_variable_shadowing.rs | 74 ++ .../src/rules/flake8_builtins/rules/mod.rs | 9 + .../src/rules/flake8_builtins/rules/rules.rs | 3 + .../ruff/src/rules/flake8_commas/rules/mod.rs | 5 + .../{rules.rs => rules/trailing_commas.rs} | 0 .../ruff/src/rules/flake8_datetimez/rules.rs | 403 ----------- .../rules/call_date_fromtimestamp.rs | 40 ++ .../flake8_datetimez/rules/call_date_today.rs | 40 ++ .../rules/call_datetime_fromtimestamp.rs | 62 ++ .../rules/call_datetime_now_without_tzinfo.rs | 60 ++ .../call_datetime_strptime_without_zone.rs | 80 +++ .../rules/call_datetime_today.rs | 40 ++ .../rules/call_datetime_utcfromtimestamp.rs | 47 ++ .../rules/call_datetime_utcnow.rs | 42 ++ .../rules/call_datetime_without_tzinfo.rs | 51 ++ .../src/rules/flake8_datetimez/rules/mod.rs | 29 + .../{rules.rs => rules/debugger.rs} | 0 .../src/rules/flake8_debugger/rules/mod.rs | 3 + .../ruff/src/rules/flake8_errmsg/rules/mod.rs | 5 + .../string_in_exception.rs} | 0 .../missing_future_annotations.rs} | 0 .../flake8_future_annotations/rules/mod.rs | 5 + crates/ruff/src/rules/flake8_gettext/rules.rs | 87 --- .../rules/f_string_in_gettext_func_call.rs | 24 + .../rules/format_in_gettext_func_call.rs | 28 + .../rules/is_gettext_func_call.rs | 10 + .../src/rules/flake8_gettext/rules/mod.rs | 15 + .../rules/printf_in_gettext_func_call.rs | 35 + .../rules/explicit.rs | 70 ++ .../{rules.rs => rules/implicit.rs} | 67 -- .../flake8_implicit_str_concat/rules/mod.rs | 7 + .../{rules.rs => rules/logging_call.rs} | 0 .../rules/flake8_logging_format/rules/mod.rs | 3 + .../implicit_namespace_package.rs} | 0 .../src/rules/flake8_no_pep420/rules/mod.rs | 3 + crates/ruff/src/rules/flake8_pie/rules.rs | 652 ------------------ .../rules/duplicate_class_field_definition.rs | 114 +++ crates/ruff/src/rules/flake8_pie/rules/mod.rs | 17 + .../rules/multiple_starts_ends_with.rs | 182 +++++ .../flake8_pie/rules/no_unnecessary_pass.rs | 94 +++ .../flake8_pie/rules/non_unique_enums.rs | 102 +++ .../rules/reimplemented_list_builtin.rs | 80 +++ .../rules/unnecessary_dict_kwargs.rs | 79 +++ .../flake8_pie/rules/unnecessary_spread.rs | 52 ++ .../{rules.rs => rules/from_tokens.rs} | 2 +- .../ruff/src/rules/flake8_quotes/rules/mod.rs | 6 + .../{rules.rs => rules/function.rs} | 6 +- .../ruff/src/rules/flake8_return/rules/mod.rs | 6 + .../ruff/src/rules/flake8_tidy_imports/mod.rs | 130 +++- .../src/rules/flake8_tidy_imports/options.rs | 4 +- .../{ => rules}/banned_api.rs | 88 +-- .../rules/flake8_tidy_imports/rules/mod.rs | 7 + .../{ => rules}/relative_imports.rs | 80 +-- .../src/rules/flake8_tidy_imports/settings.rs | 29 + ...tidy_imports__tests__ban_all_imports.snap} | 2 +- ...y_imports__tests__ban_parent_imports.snap} | 2 +- ...s__tests__ban_parent_imports_package.snap} | 2 +- ...ake8_tidy_imports__tests__banned_api.snap} | 2 +- ...y_imports__tests__banned_api_package.snap} | 2 +- .../ruff/src/rules/flake8_todos/rules/mod.rs | 6 + .../flake8_todos/{rules.rs => rules/todos.rs} | 0 .../src/rules/flake8_unused_arguments/mod.rs | 1 - .../flake8_unused_arguments/rules/mod.rs | 6 + .../{rules.rs => rules/unused_arguments.rs} | 39 +- .../rules/flake8_unused_arguments/types.rs | 37 - .../ruff/src/rules/flake8_use_pathlib/mod.rs | 2 +- .../src/rules/flake8_use_pathlib/rules/mod.rs | 3 + .../replaceable_by_pathlib.rs} | 0 .../function_is_too_complex.rs} | 0 crates/ruff/src/rules/mccabe/rules/mod.rs | 3 + crates/ruff/src/settings/defaults.rs | 2 +- crates/ruff/src/settings/mod.rs | 2 +- crates/ruff/src/settings/pyproject.rs | 3 +- crates/ruff_wasm/src/lib.rs | 2 +- 108 files changed, 2742 insertions(+), 2374 deletions(-) rename crates/ruff/src/rules/eradicate/{rules.rs => rules/commented_out_code.rs} (97%) create mode 100644 crates/ruff/src/rules/eradicate/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_2020/helpers.rs rename crates/ruff/src/rules/flake8_2020/{rules.rs => rules/compare.rs} (59%) create mode 100644 crates/ruff/src/rules/flake8_2020/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_2020/rules/name_or_attribute.rs create mode 100644 crates/ruff/src/rules/flake8_2020/rules/subscript.rs rename crates/ruff/src/rules/flake8_annotations/{rules.rs => rules/definition.rs} (99%) create mode 100644 crates/ruff/src/rules/flake8_annotations/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_async/helpers.rs delete mode 100644 crates/ruff/src/rules/flake8_async/rules.rs create mode 100644 crates/ruff/src/rules/flake8_async/rules/blocking_http_call.rs create mode 100644 crates/ruff/src/rules/flake8_async/rules/blocking_os_call.rs create mode 100644 crates/ruff/src/rules/flake8_async/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_async/rules/open_sleep_or_subprocess_call.rs rename crates/ruff/src/rules/flake8_blind_except/{rules.rs => rules/blind_except.rs} (100%) create mode 100644 crates/ruff/src/rules/flake8_blind_except/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_boolean_trap/helpers.rs delete mode 100644 crates/ruff/src/rules/flake8_boolean_trap/rules.rs create mode 100644 crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_default_value_in_function_definition.rs create mode 100644 crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_positional_value_in_function_call.rs create mode 100644 crates/ruff/src/rules/flake8_boolean_trap/rules/check_positional_boolean_in_def.rs create mode 100644 crates/ruff/src/rules/flake8_boolean_trap/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_builtins/helpers.rs delete mode 100644 crates/ruff/src/rules/flake8_builtins/rules.rs create mode 100644 crates/ruff/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs create mode 100644 crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs create mode 100644 crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs create mode 100644 crates/ruff/src/rules/flake8_builtins/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_builtins/rules/rules.rs create mode 100644 crates/ruff/src/rules/flake8_commas/rules/mod.rs rename crates/ruff/src/rules/flake8_commas/{rules.rs => rules/trailing_commas.rs} (100%) delete mode 100644 crates/ruff/src/rules/flake8_datetimez/rules.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/call_date_fromtimestamp.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/call_date_today.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_fromtimestamp.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_now_without_tzinfo.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_strptime_without_zone.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_today.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_utcfromtimestamp.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_utcnow.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_without_tzinfo.rs create mode 100644 crates/ruff/src/rules/flake8_datetimez/rules/mod.rs rename crates/ruff/src/rules/flake8_debugger/{rules.rs => rules/debugger.rs} (100%) create mode 100644 crates/ruff/src/rules/flake8_debugger/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_errmsg/rules/mod.rs rename crates/ruff/src/rules/flake8_errmsg/{rules.rs => rules/string_in_exception.rs} (100%) rename crates/ruff/src/rules/flake8_future_annotations/{rules.rs => rules/missing_future_annotations.rs} (100%) create mode 100644 crates/ruff/src/rules/flake8_future_annotations/rules/mod.rs delete mode 100644 crates/ruff/src/rules/flake8_gettext/rules.rs create mode 100644 crates/ruff/src/rules/flake8_gettext/rules/f_string_in_gettext_func_call.rs create mode 100644 crates/ruff/src/rules/flake8_gettext/rules/format_in_gettext_func_call.rs create mode 100644 crates/ruff/src/rules/flake8_gettext/rules/is_gettext_func_call.rs create mode 100644 crates/ruff/src/rules/flake8_gettext/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_gettext/rules/printf_in_gettext_func_call.rs create mode 100644 crates/ruff/src/rules/flake8_implicit_str_concat/rules/explicit.rs rename crates/ruff/src/rules/flake8_implicit_str_concat/{rules.rs => rules/implicit.rs} (66%) create mode 100644 crates/ruff/src/rules/flake8_implicit_str_concat/rules/mod.rs rename crates/ruff/src/rules/flake8_logging_format/{rules.rs => rules/logging_call.rs} (100%) create mode 100644 crates/ruff/src/rules/flake8_logging_format/rules/mod.rs rename crates/ruff/src/rules/flake8_no_pep420/{rules.rs => rules/implicit_namespace_package.rs} (100%) create mode 100644 crates/ruff/src/rules/flake8_no_pep420/rules/mod.rs delete mode 100644 crates/ruff/src/rules/flake8_pie/rules.rs create mode 100644 crates/ruff/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs create mode 100644 crates/ruff/src/rules/flake8_pie/rules/mod.rs create mode 100644 crates/ruff/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs create mode 100644 crates/ruff/src/rules/flake8_pie/rules/no_unnecessary_pass.rs create mode 100644 crates/ruff/src/rules/flake8_pie/rules/non_unique_enums.rs create mode 100644 crates/ruff/src/rules/flake8_pie/rules/reimplemented_list_builtin.rs create mode 100644 crates/ruff/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs create mode 100644 crates/ruff/src/rules/flake8_pie/rules/unnecessary_spread.rs rename crates/ruff/src/rules/flake8_quotes/{rules.rs => rules/from_tokens.rs} (99%) create mode 100644 crates/ruff/src/rules/flake8_quotes/rules/mod.rs rename crates/ruff/src/rules/flake8_return/{rules.rs => rules/function.rs} (99%) create mode 100644 crates/ruff/src/rules/flake8_return/rules/mod.rs rename crates/ruff/src/rules/flake8_tidy_imports/{ => rules}/banned_api.rs (50%) create mode 100644 crates/ruff/src/rules/flake8_tidy_imports/rules/mod.rs rename crates/ruff/src/rules/flake8_tidy_imports/{ => rules}/relative_imports.rs (62%) create mode 100644 crates/ruff/src/rules/flake8_tidy_imports/settings.rs rename crates/ruff/src/rules/flake8_tidy_imports/snapshots/{ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_all_imports.snap => ruff__rules__flake8_tidy_imports__tests__ban_all_imports.snap} (98%) rename crates/ruff/src/rules/flake8_tidy_imports/snapshots/{ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports.snap => ruff__rules__flake8_tidy_imports__tests__ban_parent_imports.snap} (98%) rename crates/ruff/src/rules/flake8_tidy_imports/snapshots/{ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports_package.snap => ruff__rules__flake8_tidy_imports__tests__ban_parent_imports_package.snap} (98%) rename crates/ruff/src/rules/flake8_tidy_imports/snapshots/{ruff__rules__flake8_tidy_imports__banned_api__tests__banned_api.snap => ruff__rules__flake8_tidy_imports__tests__banned_api.snap} (97%) rename crates/ruff/src/rules/flake8_tidy_imports/snapshots/{ruff__rules__flake8_tidy_imports__banned_api__tests__banned_api_package.snap => ruff__rules__flake8_tidy_imports__tests__banned_api_package.snap} (92%) create mode 100644 crates/ruff/src/rules/flake8_todos/rules/mod.rs rename crates/ruff/src/rules/flake8_todos/{rules.rs => rules/todos.rs} (100%) create mode 100644 crates/ruff/src/rules/flake8_unused_arguments/rules/mod.rs rename crates/ruff/src/rules/flake8_unused_arguments/{rules.rs => rules/unused_arguments.rs} (92%) delete mode 100644 crates/ruff/src/rules/flake8_unused_arguments/types.rs create mode 100644 crates/ruff/src/rules/flake8_use_pathlib/rules/mod.rs rename crates/ruff/src/rules/flake8_use_pathlib/{helpers.rs => rules/replaceable_by_pathlib.rs} (100%) rename crates/ruff/src/rules/mccabe/{rules.rs => rules/function_is_too_complex.rs} (100%) create mode 100644 crates/ruff/src/rules/mccabe/rules/mod.rs diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 480edc9836..f576b0cf80 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -42,7 +42,7 @@ use crate::fs::relativize_path; use crate::importer::Importer; use crate::noqa::NoqaMapping; use crate::registry::{AsRule, Rule}; -use crate::rules::flake8_builtins::rules::AnyShadowing; +use crate::rules::flake8_builtins::helpers::AnyShadowing; use crate::rules::{ flake8_2020, flake8_annotations, flake8_async, flake8_bandit, flake8_blind_except, flake8_boolean_trap, flake8_bugbear, flake8_builtins, flake8_comprehensions, flake8_datetimez, @@ -894,7 +894,7 @@ where // flake8_tidy_imports if self.enabled(Rule::BannedApi) { - flake8_tidy_imports::banned_api::name_or_parent_is_banned( + flake8_tidy_imports::rules::name_or_parent_is_banned( self, &alias.name, alias, @@ -1057,15 +1057,13 @@ where if let Some(module) = helpers::resolve_imported_module_path(level, module, self.module_path) { - flake8_tidy_imports::banned_api::name_or_parent_is_banned( - self, &module, stmt, - ); + flake8_tidy_imports::rules::name_or_parent_is_banned(self, &module, stmt); for alias in names { if &alias.name == "*" { continue; } - flake8_tidy_imports::banned_api::name_is_banned( + flake8_tidy_imports::rules::name_is_banned( self, format!("{module}.{}", alias.name), alias, @@ -1179,16 +1177,14 @@ where } if self.enabled(Rule::RelativeImports) { - if let Some(diagnostic) = - flake8_tidy_imports::relative_imports::banned_relative_import( - self, - stmt, - level, - module, - self.module_path, - self.settings.flake8_tidy_imports.ban_relative_imports, - ) - { + if let Some(diagnostic) = flake8_tidy_imports::rules::banned_relative_import( + self, + stmt, + level, + module, + self.module_path, + self.settings.flake8_tidy_imports.ban_relative_imports, + ) { self.diagnostics.push(diagnostic); } } @@ -2438,7 +2434,7 @@ where flake8_2020::rules::name_or_attribute(self, expr); } if self.enabled(Rule::BannedApi) { - flake8_tidy_imports::banned_api::banned_attribute_access(self, expr); + flake8_tidy_imports::rules::banned_attribute_access(self, expr); } if self.enabled(Rule::PrivateMemberAccess) { flake8_self::rules::private_member_access(self, expr); @@ -3024,7 +3020,7 @@ where Rule::BuiltinOpen, Rule::PyPath, ]) { - flake8_use_pathlib::helpers::replaceable_by_pathlib(self, func); + flake8_use_pathlib::rules::replaceable_by_pathlib(self, func); } // numpy diff --git a/crates/ruff/src/flake8_to_ruff/converter.rs b/crates/ruff/src/flake8_to_ruff/converter.rs index f205ccaeb5..5bccf63691 100644 --- a/crates/ruff/src/flake8_to_ruff/converter.rs +++ b/crates/ruff/src/flake8_to_ruff/converter.rs @@ -10,7 +10,7 @@ use crate::rules::flake8_pytest_style::types::{ ParametrizeNameType, ParametrizeValuesRowType, ParametrizeValuesType, }; use crate::rules::flake8_quotes::settings::Quote; -use crate::rules::flake8_tidy_imports::relative_imports::Strictness; +use crate::rules::flake8_tidy_imports::settings::Strictness; use crate::rules::pydocstyle::settings::Convention; use crate::rules::{ flake8_annotations, flake8_bugbear, flake8_builtins, flake8_errmsg, flake8_pytest_style, diff --git a/crates/ruff/src/registry.rs b/crates/ruff/src/registry.rs index 315d66ec66..bc7c6cdea2 100644 --- a/crates/ruff/src/registry.rs +++ b/crates/ruff/src/registry.rs @@ -230,8 +230,8 @@ ruff_macros::register_rules!( // mccabe rules::mccabe::rules::ComplexStructure, // flake8-tidy-imports - rules::flake8_tidy_imports::banned_api::BannedApi, - rules::flake8_tidy_imports::relative_imports::RelativeImports, + rules::flake8_tidy_imports::rules::BannedApi, + rules::flake8_tidy_imports::rules::RelativeImports, // flake8-return rules::flake8_return::rules::UnnecessaryReturnNone, rules::flake8_return::rules::ImplicitReturnValue, diff --git a/crates/ruff/src/rules/eradicate/rules.rs b/crates/ruff/src/rules/eradicate/rules/commented_out_code.rs similarity index 97% rename from crates/ruff/src/rules/eradicate/rules.rs rename to crates/ruff/src/rules/eradicate/rules/commented_out_code.rs index 69765453eb..cc757b6c10 100644 --- a/crates/ruff/src/rules/eradicate/rules.rs +++ b/crates/ruff/src/rules/eradicate/rules/commented_out_code.rs @@ -5,7 +5,7 @@ use ruff_python_ast::source_code::{Indexer, Locator}; use crate::registry::Rule; use crate::settings::Settings; -use super::detection::comment_contains_code; +use super::super::detection::comment_contains_code; /// ## What it does /// Checks for commented-out Python code. diff --git a/crates/ruff/src/rules/eradicate/rules/mod.rs b/crates/ruff/src/rules/eradicate/rules/mod.rs new file mode 100644 index 0000000000..8ec37813d9 --- /dev/null +++ b/crates/ruff/src/rules/eradicate/rules/mod.rs @@ -0,0 +1,3 @@ +pub(crate) use commented_out_code::{commented_out_code, CommentedOutCode}; + +mod commented_out_code; diff --git a/crates/ruff/src/rules/flake8_2020/helpers.rs b/crates/ruff/src/rules/flake8_2020/helpers.rs new file mode 100644 index 0000000000..e5bb8fd4b6 --- /dev/null +++ b/crates/ruff/src/rules/flake8_2020/helpers.rs @@ -0,0 +1,8 @@ +use ruff_python_semantic::model::SemanticModel; +use rustpython_parser::ast::Expr; + +pub(super) fn is_sys(model: &SemanticModel, expr: &Expr, target: &str) -> bool { + model + .resolve_call_path(expr) + .map_or(false, |call_path| call_path.as_slice() == ["sys", target]) +} diff --git a/crates/ruff/src/rules/flake8_2020/mod.rs b/crates/ruff/src/rules/flake8_2020/mod.rs index 3bf933c433..35b09c2da8 100644 --- a/crates/ruff/src/rules/flake8_2020/mod.rs +++ b/crates/ruff/src/rules/flake8_2020/mod.rs @@ -1,4 +1,5 @@ //! Rules from [flake8-2020](https://pypi.org/project/flake8-2020/). +mod helpers; pub(crate) mod rules; #[cfg(test)] diff --git a/crates/ruff/src/rules/flake8_2020/rules.rs b/crates/ruff/src/rules/flake8_2020/rules/compare.rs similarity index 59% rename from crates/ruff/src/rules/flake8_2020/rules.rs rename to crates/ruff/src/rules/flake8_2020/rules/compare.rs index f4917de161..9877be4fae 100644 --- a/crates/ruff/src/rules/flake8_2020/rules.rs +++ b/crates/ruff/src/rules/flake8_2020/rules/compare.rs @@ -3,30 +3,11 @@ use rustpython_parser::ast::{self, Cmpop, Constant, Expr, Ranged}; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_semantic::model::SemanticModel; use crate::checkers::ast::Checker; use crate::registry::Rule; -#[violation] -pub struct SysVersionSlice3; - -impl Violation for SysVersionSlice3 { - #[derive_message_formats] - fn message(&self) -> String { - format!("`sys.version[:3]` referenced (python3.10), use `sys.version_info`") - } -} - -#[violation] -pub struct SysVersion2; - -impl Violation for SysVersion2 { - #[derive_message_formats] - fn message(&self) -> String { - format!("`sys.version[2]` referenced (python3.10), use `sys.version_info`") - } -} +use super::super::helpers::is_sys; #[violation] pub struct SysVersionCmpStr3; @@ -48,16 +29,6 @@ impl Violation for SysVersionInfo0Eq3 { } } -#[violation] -pub struct SixPY3; - -impl Violation for SixPY3 { - #[derive_message_formats] - fn message(&self) -> String { - format!("`six.PY3` referenced (python4), use `not six.PY2`") - } -} - #[violation] pub struct SysVersionInfo1CmpInt; @@ -84,16 +55,6 @@ impl Violation for SysVersionInfoMinorCmpInt { } } -#[violation] -pub struct SysVersion0; - -impl Violation for SysVersion0 { - #[derive_message_formats] - fn message(&self) -> String { - format!("`sys.version[0]` referenced (python10), use `sys.version_info`") - } -} - #[violation] pub struct SysVersionCmpStr10; @@ -104,69 +65,6 @@ impl Violation for SysVersionCmpStr10 { } } -#[violation] -pub struct SysVersionSlice1; - -impl Violation for SysVersionSlice1 { - #[derive_message_formats] - fn message(&self) -> String { - format!("`sys.version[:1]` referenced (python10), use `sys.version_info`") - } -} - -fn is_sys(model: &SemanticModel, expr: &Expr, target: &str) -> bool { - model - .resolve_call_path(expr) - .map_or(false, |call_path| call_path.as_slice() == ["sys", target]) -} - -/// YTT101, YTT102, YTT301, YTT303 -pub(crate) fn subscript(checker: &mut Checker, value: &Expr, slice: &Expr) { - if is_sys(checker.semantic_model(), value, "version") { - match slice { - Expr::Slice(ast::ExprSlice { - lower: None, - upper: Some(upper), - step: None, - range: _, - }) => { - if let Expr::Constant(ast::ExprConstant { - value: Constant::Int(i), - .. - }) = upper.as_ref() - { - if *i == BigInt::from(1) && checker.enabled(Rule::SysVersionSlice1) { - checker - .diagnostics - .push(Diagnostic::new(SysVersionSlice1, value.range())); - } else if *i == BigInt::from(3) && checker.enabled(Rule::SysVersionSlice3) { - checker - .diagnostics - .push(Diagnostic::new(SysVersionSlice3, value.range())); - } - } - } - - Expr::Constant(ast::ExprConstant { - value: Constant::Int(i), - .. - }) => { - if *i == BigInt::from(2) && checker.enabled(Rule::SysVersion2) { - checker - .diagnostics - .push(Diagnostic::new(SysVersion2, value.range())); - } else if *i == BigInt::from(0) && checker.enabled(Rule::SysVersion0) { - checker - .diagnostics - .push(Diagnostic::new(SysVersion0, value.range())); - } - } - - _ => {} - } - } -} - /// YTT103, YTT201, YTT203, YTT204, YTT302 pub(crate) fn compare(checker: &mut Checker, left: &Expr, ops: &[Cmpop], comparators: &[Expr]) { match left { @@ -257,16 +155,3 @@ pub(crate) fn compare(checker: &mut Checker, left: &Expr, ops: &[Cmpop], compara } } } - -/// YTT202 -pub(crate) fn name_or_attribute(checker: &mut Checker, expr: &Expr) { - if checker - .semantic_model() - .resolve_call_path(expr) - .map_or(false, |call_path| call_path.as_slice() == ["six", "PY3"]) - { - checker - .diagnostics - .push(Diagnostic::new(SixPY3, expr.range())); - } -} diff --git a/crates/ruff/src/rules/flake8_2020/rules/mod.rs b/crates/ruff/src/rules/flake8_2020/rules/mod.rs new file mode 100644 index 0000000000..cb77bcc0dd --- /dev/null +++ b/crates/ruff/src/rules/flake8_2020/rules/mod.rs @@ -0,0 +1,12 @@ +pub(crate) use compare::{ + compare, SysVersionCmpStr10, SysVersionCmpStr3, SysVersionInfo0Eq3, SysVersionInfo1CmpInt, + SysVersionInfoMinorCmpInt, +}; +pub(crate) use name_or_attribute::{name_or_attribute, SixPY3}; +pub(crate) use subscript::{ + subscript, SysVersion0, SysVersion2, SysVersionSlice1, SysVersionSlice3, +}; + +mod compare; +mod name_or_attribute; +mod subscript; diff --git a/crates/ruff/src/rules/flake8_2020/rules/name_or_attribute.rs b/crates/ruff/src/rules/flake8_2020/rules/name_or_attribute.rs new file mode 100644 index 0000000000..d861abd262 --- /dev/null +++ b/crates/ruff/src/rules/flake8_2020/rules/name_or_attribute.rs @@ -0,0 +1,29 @@ +use rustpython_parser::ast::{Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct SixPY3; + +impl Violation for SixPY3 { + #[derive_message_formats] + fn message(&self) -> String { + format!("`six.PY3` referenced (python4), use `not six.PY2`") + } +} + +/// YTT202 +pub(crate) fn name_or_attribute(checker: &mut Checker, expr: &Expr) { + if checker + .semantic_model() + .resolve_call_path(expr) + .map_or(false, |call_path| call_path.as_slice() == ["six", "PY3"]) + { + checker + .diagnostics + .push(Diagnostic::new(SixPY3, expr.range())); + } +} diff --git a/crates/ruff/src/rules/flake8_2020/rules/subscript.rs b/crates/ruff/src/rules/flake8_2020/rules/subscript.rs new file mode 100644 index 0000000000..b55a602423 --- /dev/null +++ b/crates/ruff/src/rules/flake8_2020/rules/subscript.rs @@ -0,0 +1,96 @@ +use num_bigint::BigInt; +use rustpython_parser::ast::{self, Constant, Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; +use crate::registry::Rule; +use crate::rules::flake8_2020::helpers::is_sys; + +#[violation] +pub struct SysVersionSlice3; + +impl Violation for SysVersionSlice3 { + #[derive_message_formats] + fn message(&self) -> String { + format!("`sys.version[:3]` referenced (python3.10), use `sys.version_info`") + } +} + +#[violation] +pub struct SysVersion2; + +impl Violation for SysVersion2 { + #[derive_message_formats] + fn message(&self) -> String { + format!("`sys.version[2]` referenced (python3.10), use `sys.version_info`") + } +} + +#[violation] +pub struct SysVersion0; + +impl Violation for SysVersion0 { + #[derive_message_formats] + fn message(&self) -> String { + format!("`sys.version[0]` referenced (python10), use `sys.version_info`") + } +} + +#[violation] +pub struct SysVersionSlice1; + +impl Violation for SysVersionSlice1 { + #[derive_message_formats] + fn message(&self) -> String { + format!("`sys.version[:1]` referenced (python10), use `sys.version_info`") + } +} + +/// YTT101, YTT102, YTT301, YTT303 +pub(crate) fn subscript(checker: &mut Checker, value: &Expr, slice: &Expr) { + if is_sys(checker.semantic_model(), value, "version") { + match slice { + Expr::Slice(ast::ExprSlice { + lower: None, + upper: Some(upper), + step: None, + range: _, + }) => { + if let Expr::Constant(ast::ExprConstant { + value: Constant::Int(i), + .. + }) = upper.as_ref() + { + if *i == BigInt::from(1) && checker.enabled(Rule::SysVersionSlice1) { + checker + .diagnostics + .push(Diagnostic::new(SysVersionSlice1, value.range())); + } else if *i == BigInt::from(3) && checker.enabled(Rule::SysVersionSlice3) { + checker + .diagnostics + .push(Diagnostic::new(SysVersionSlice3, value.range())); + } + } + } + + Expr::Constant(ast::ExprConstant { + value: Constant::Int(i), + .. + }) => { + if *i == BigInt::from(2) && checker.enabled(Rule::SysVersion2) { + checker + .diagnostics + .push(Diagnostic::new(SysVersion2, value.range())); + } else if *i == BigInt::from(0) && checker.enabled(Rule::SysVersion0) { + checker + .diagnostics + .push(Diagnostic::new(SysVersion0, value.range())); + } + } + + _ => {} + } + } +} diff --git a/crates/ruff/src/rules/flake8_annotations/rules.rs b/crates/ruff/src/rules/flake8_annotations/rules/definition.rs similarity index 99% rename from crates/ruff/src/rules/flake8_annotations/rules.rs rename to crates/ruff/src/rules/flake8_annotations/rules/definition.rs index 4f81ce1e04..11bfd64781 100644 --- a/crates/ruff/src/rules/flake8_annotations/rules.rs +++ b/crates/ruff/src/rules/flake8_annotations/rules/definition.rs @@ -14,8 +14,8 @@ use ruff_python_stdlib::typing::SIMPLE_MAGIC_RETURN_TYPES; use crate::checkers::ast::Checker; use crate::registry::{AsRule, Rule}; -use super::fixes; -use super::helpers::match_function_def; +use super::super::fixes; +use super::super::helpers::match_function_def; /// ## What it does /// Checks that function arguments have type annotations. diff --git a/crates/ruff/src/rules/flake8_annotations/rules/mod.rs b/crates/ruff/src/rules/flake8_annotations/rules/mod.rs new file mode 100644 index 0000000000..b57c156b18 --- /dev/null +++ b/crates/ruff/src/rules/flake8_annotations/rules/mod.rs @@ -0,0 +1,8 @@ +pub(crate) use definition::{ + definition, AnyType, MissingReturnTypeClassMethod, MissingReturnTypePrivateFunction, + MissingReturnTypeSpecialMethod, MissingReturnTypeStaticMethod, + MissingReturnTypeUndocumentedPublicFunction, MissingTypeArgs, MissingTypeCls, + MissingTypeFunctionArgument, MissingTypeKwargs, MissingTypeSelf, +}; + +mod definition; diff --git a/crates/ruff/src/rules/flake8_async/helpers.rs b/crates/ruff/src/rules/flake8_async/helpers.rs new file mode 100644 index 0000000000..5bd8b0f2c5 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/helpers.rs @@ -0,0 +1,18 @@ +use ruff_python_semantic::{ + model::SemanticModel, + scope::{FunctionDef, ScopeKind}, +}; + +/// Return `true` if the [`SemanticModel`] is inside an async function definition. +pub(crate) fn in_async_function(model: &SemanticModel) -> bool { + model + .scopes() + .find_map(|scope| { + if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { + Some(*async_) + } else { + None + } + }) + .unwrap_or(false) +} diff --git a/crates/ruff/src/rules/flake8_async/mod.rs b/crates/ruff/src/rules/flake8_async/mod.rs index 8842526400..c5293dc043 100644 --- a/crates/ruff/src/rules/flake8_async/mod.rs +++ b/crates/ruff/src/rules/flake8_async/mod.rs @@ -1,4 +1,5 @@ //! Rules from [flake8-async](https://pypi.org/project/flake8-async/). +mod helpers; pub(crate) mod rules; #[cfg(test)] diff --git a/crates/ruff/src/rules/flake8_async/rules.rs b/crates/ruff/src/rules/flake8_async/rules.rs deleted file mode 100644 index 88b0b98123..0000000000 --- a/crates/ruff/src/rules/flake8_async/rules.rs +++ /dev/null @@ -1,235 +0,0 @@ -use rustpython_parser::ast; -use rustpython_parser::ast::{Expr, Ranged}; - -use ruff_diagnostics::{Diagnostic, Violation}; -use ruff_macros::{derive_message_formats, violation}; -use ruff_python_semantic::model::SemanticModel; -use ruff_python_semantic::scope::{FunctionDef, ScopeKind}; - -use crate::checkers::ast::Checker; - -/// ## What it does -/// Checks that async functions do not contain blocking HTTP calls. -/// -/// ## Why is this bad? -/// Blocking an async function via a blocking HTTP call will block the entire -/// event loop, preventing it from executing other tasks while waiting for the -/// HTTP response, negating the benefits of asynchronous programming. -/// -/// Instead of making a blocking HTTP call, use an asynchronous HTTP client -/// library such as `aiohttp` or `httpx`. -/// -/// ## Example -/// ```python -/// async def fetch(): -/// urllib.request.urlopen("https://example.com/foo/bar").read() -/// ``` -/// -/// Use instead: -/// ```python -/// async def fetch(): -/// async with aiohttp.ClientSession() as session: -/// async with session.get("https://example.com/foo/bar") as resp: -/// ... -/// ``` -#[violation] -pub struct BlockingHttpCallInAsyncFunction; - -impl Violation for BlockingHttpCallInAsyncFunction { - #[derive_message_formats] - fn message(&self) -> String { - format!("Async functions should not call blocking HTTP methods") - } -} - -const BLOCKING_HTTP_CALLS: &[&[&str]] = &[ - &["urllib", "request", "urlopen"], - &["httpx", "get"], - &["httpx", "post"], - &["httpx", "delete"], - &["httpx", "patch"], - &["httpx", "put"], - &["httpx", "head"], - &["httpx", "connect"], - &["httpx", "options"], - &["httpx", "trace"], - &["requests", "get"], - &["requests", "post"], - &["requests", "delete"], - &["requests", "patch"], - &["requests", "put"], - &["requests", "head"], - &["requests", "connect"], - &["requests", "options"], - &["requests", "trace"], -]; - -/// ASYNC100 -pub(crate) fn blocking_http_call(checker: &mut Checker, expr: &Expr) { - if in_async_function(checker.semantic_model()) { - if let Expr::Call(ast::ExprCall { func, .. }) = expr { - let call_path = checker.semantic_model().resolve_call_path(func); - let is_blocking = - call_path.map_or(false, |path| BLOCKING_HTTP_CALLS.contains(&path.as_slice())); - - if is_blocking { - checker.diagnostics.push(Diagnostic::new( - BlockingHttpCallInAsyncFunction, - func.range(), - )); - } - } - } -} - -/// ## What it does -/// Checks that async functions do not contain calls to `open`, `time.sleep`, -/// or `subprocess` methods. -/// -/// ## Why is this bad? -/// Blocking an async function via a blocking call will block the entire -/// event loop, preventing it from executing other tasks while waiting for the -/// call to complete, negating the benefits of asynchronous programming. -/// -/// Instead of making a blocking call, use an equivalent asynchronous library -/// or function. -/// -/// ## Example -/// ```python -/// async def foo(): -/// time.sleep(1000) -/// ``` -/// -/// Use instead: -/// ```python -/// async def foo(): -/// await asyncio.sleep(1000) -/// ``` -#[violation] -pub struct OpenSleepOrSubprocessInAsyncFunction; - -impl Violation for OpenSleepOrSubprocessInAsyncFunction { - #[derive_message_formats] - fn message(&self) -> String { - format!("Async functions should not call `open`, `time.sleep`, or `subprocess` methods") - } -} - -const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[&[&str]] = &[ - &["", "open"], - &["time", "sleep"], - &["subprocess", "run"], - &["subprocess", "Popen"], - // Deprecated subprocess calls: - &["subprocess", "call"], - &["subprocess", "check_call"], - &["subprocess", "check_output"], - &["subprocess", "getoutput"], - &["subprocess", "getstatusoutput"], - &["os", "wait"], - &["os", "wait3"], - &["os", "wait4"], - &["os", "waitid"], - &["os", "waitpid"], -]; - -/// ASYNC101 -pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, expr: &Expr) { - if in_async_function(checker.semantic_model()) { - if let Expr::Call(ast::ExprCall { func, .. }) = expr { - let is_open_sleep_or_subprocess_call = checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |path| { - OPEN_SLEEP_OR_SUBPROCESS_CALL.contains(&path.as_slice()) - }); - - if is_open_sleep_or_subprocess_call { - checker.diagnostics.push(Diagnostic::new( - OpenSleepOrSubprocessInAsyncFunction, - func.range(), - )); - } - } - } -} - -/// ## What it does -/// Checks that async functions do not contain calls to blocking synchronous -/// process calls via the `os` module. -/// -/// ## Why is this bad? -/// Blocking an async function via a blocking call will block the entire -/// event loop, preventing it from executing other tasks while waiting for the -/// call to complete, negating the benefits of asynchronous programming. -/// -/// Instead of making a blocking call, use an equivalent asynchronous library -/// or function. -/// -/// ## Example -/// ```python -/// async def foo(): -/// os.popen() -/// ``` -/// -/// Use instead: -/// ```python -/// def foo(): -/// os.popen() -/// ``` -#[violation] -pub struct BlockingOsCallInAsyncFunction; - -impl Violation for BlockingOsCallInAsyncFunction { - #[derive_message_formats] - fn message(&self) -> String { - format!("Async functions should not call synchronous `os` methods") - } -} - -const UNSAFE_OS_METHODS: &[&[&str]] = &[ - &["os", "popen"], - &["os", "posix_spawn"], - &["os", "posix_spawnp"], - &["os", "spawnl"], - &["os", "spawnle"], - &["os", "spawnlp"], - &["os", "spawnlpe"], - &["os", "spawnv"], - &["os", "spawnve"], - &["os", "spawnvp"], - &["os", "spawnvpe"], - &["os", "system"], -]; - -/// ASYNC102 -pub(crate) fn blocking_os_call(checker: &mut Checker, expr: &Expr) { - if in_async_function(checker.semantic_model()) { - if let Expr::Call(ast::ExprCall { func, .. }) = expr { - let is_unsafe_os_method = checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |path| UNSAFE_OS_METHODS.contains(&path.as_slice())); - - if is_unsafe_os_method { - checker - .diagnostics - .push(Diagnostic::new(BlockingOsCallInAsyncFunction, func.range())); - } - } - } -} - -/// Return `true` if the [`SemanticModel`] is inside an async function definition. -fn in_async_function(model: &SemanticModel) -> bool { - model - .scopes() - .find_map(|scope| { - if let ScopeKind::Function(FunctionDef { async_, .. }) = &scope.kind { - Some(*async_) - } else { - None - } - }) - .unwrap_or(false) -} diff --git a/crates/ruff/src/rules/flake8_async/rules/blocking_http_call.rs b/crates/ruff/src/rules/flake8_async/rules/blocking_http_call.rs new file mode 100644 index 0000000000..457f696d4e --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/rules/blocking_http_call.rs @@ -0,0 +1,83 @@ +use rustpython_parser::ast; +use rustpython_parser::ast::{Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +use super::super::helpers::in_async_function; + +/// ## What it does +/// Checks that async functions do not contain blocking HTTP calls. +/// +/// ## Why is this bad? +/// Blocking an async function via a blocking HTTP call will block the entire +/// event loop, preventing it from executing other tasks while waiting for the +/// HTTP response, negating the benefits of asynchronous programming. +/// +/// Instead of making a blocking HTTP call, use an asynchronous HTTP client +/// library such as `aiohttp` or `httpx`. +/// +/// ## Example +/// ```python +/// async def fetch(): +/// urllib.request.urlopen("https://example.com/foo/bar").read() +/// ``` +/// +/// Use instead: +/// ```python +/// async def fetch(): +/// async with aiohttp.ClientSession() as session: +/// async with session.get("https://example.com/foo/bar") as resp: +/// ... +/// ``` +#[violation] +pub struct BlockingHttpCallInAsyncFunction; + +impl Violation for BlockingHttpCallInAsyncFunction { + #[derive_message_formats] + fn message(&self) -> String { + format!("Async functions should not call blocking HTTP methods") + } +} + +const BLOCKING_HTTP_CALLS: &[&[&str]] = &[ + &["urllib", "request", "urlopen"], + &["httpx", "get"], + &["httpx", "post"], + &["httpx", "delete"], + &["httpx", "patch"], + &["httpx", "put"], + &["httpx", "head"], + &["httpx", "connect"], + &["httpx", "options"], + &["httpx", "trace"], + &["requests", "get"], + &["requests", "post"], + &["requests", "delete"], + &["requests", "patch"], + &["requests", "put"], + &["requests", "head"], + &["requests", "connect"], + &["requests", "options"], + &["requests", "trace"], +]; + +/// ASYNC100 +pub(crate) fn blocking_http_call(checker: &mut Checker, expr: &Expr) { + if in_async_function(checker.semantic_model()) { + if let Expr::Call(ast::ExprCall { func, .. }) = expr { + let call_path = checker.semantic_model().resolve_call_path(func); + let is_blocking = + call_path.map_or(false, |path| BLOCKING_HTTP_CALLS.contains(&path.as_slice())); + + if is_blocking { + checker.diagnostics.push(Diagnostic::new( + BlockingHttpCallInAsyncFunction, + func.range(), + )); + } + } + } +} diff --git a/crates/ruff/src/rules/flake8_async/rules/blocking_os_call.rs b/crates/ruff/src/rules/flake8_async/rules/blocking_os_call.rs new file mode 100644 index 0000000000..3d0fbcf567 --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/rules/blocking_os_call.rs @@ -0,0 +1,75 @@ +use rustpython_parser::ast; +use rustpython_parser::ast::{Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +use super::super::helpers::in_async_function; + +/// ## What it does +/// Checks that async functions do not contain calls to blocking synchronous +/// process calls via the `os` module. +/// +/// ## Why is this bad? +/// Blocking an async function via a blocking call will block the entire +/// event loop, preventing it from executing other tasks while waiting for the +/// call to complete, negating the benefits of asynchronous programming. +/// +/// Instead of making a blocking call, use an equivalent asynchronous library +/// or function. +/// +/// ## Example +/// ```python +/// async def foo(): +/// os.popen() +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(): +/// os.popen() +/// ``` +#[violation] +pub struct BlockingOsCallInAsyncFunction; + +impl Violation for BlockingOsCallInAsyncFunction { + #[derive_message_formats] + fn message(&self) -> String { + format!("Async functions should not call synchronous `os` methods") + } +} + +const UNSAFE_OS_METHODS: &[&[&str]] = &[ + &["os", "popen"], + &["os", "posix_spawn"], + &["os", "posix_spawnp"], + &["os", "spawnl"], + &["os", "spawnle"], + &["os", "spawnlp"], + &["os", "spawnlpe"], + &["os", "spawnv"], + &["os", "spawnve"], + &["os", "spawnvp"], + &["os", "spawnvpe"], + &["os", "system"], +]; + +/// ASYNC102 +pub(crate) fn blocking_os_call(checker: &mut Checker, expr: &Expr) { + if in_async_function(checker.semantic_model()) { + if let Expr::Call(ast::ExprCall { func, .. }) = expr { + let is_unsafe_os_method = checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |path| UNSAFE_OS_METHODS.contains(&path.as_slice())); + + if is_unsafe_os_method { + checker + .diagnostics + .push(Diagnostic::new(BlockingOsCallInAsyncFunction, func.range())); + } + } + } +} diff --git a/crates/ruff/src/rules/flake8_async/rules/mod.rs b/crates/ruff/src/rules/flake8_async/rules/mod.rs new file mode 100644 index 0000000000..0f6e8faaca --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/rules/mod.rs @@ -0,0 +1,9 @@ +pub(crate) use blocking_http_call::{blocking_http_call, BlockingHttpCallInAsyncFunction}; +pub(crate) use blocking_os_call::{blocking_os_call, BlockingOsCallInAsyncFunction}; +pub(crate) use open_sleep_or_subprocess_call::{ + open_sleep_or_subprocess_call, OpenSleepOrSubprocessInAsyncFunction, +}; + +mod blocking_http_call; +mod blocking_os_call; +mod open_sleep_or_subprocess_call; diff --git a/crates/ruff/src/rules/flake8_async/rules/open_sleep_or_subprocess_call.rs b/crates/ruff/src/rules/flake8_async/rules/open_sleep_or_subprocess_call.rs new file mode 100644 index 0000000000..6935a5a45b --- /dev/null +++ b/crates/ruff/src/rules/flake8_async/rules/open_sleep_or_subprocess_call.rs @@ -0,0 +1,81 @@ +use rustpython_parser::ast; +use rustpython_parser::ast::{Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +use super::super::helpers::in_async_function; + +/// ## What it does +/// Checks that async functions do not contain calls to `open`, `time.sleep`, +/// or `subprocess` methods. +/// +/// ## Why is this bad? +/// Blocking an async function via a blocking call will block the entire +/// event loop, preventing it from executing other tasks while waiting for the +/// call to complete, negating the benefits of asynchronous programming. +/// +/// Instead of making a blocking call, use an equivalent asynchronous library +/// or function. +/// +/// ## Example +/// ```python +/// async def foo(): +/// time.sleep(1000) +/// ``` +/// +/// Use instead: +/// ```python +/// async def foo(): +/// await asyncio.sleep(1000) +/// ``` +#[violation] +pub struct OpenSleepOrSubprocessInAsyncFunction; + +impl Violation for OpenSleepOrSubprocessInAsyncFunction { + #[derive_message_formats] + fn message(&self) -> String { + format!("Async functions should not call `open`, `time.sleep`, or `subprocess` methods") + } +} + +const OPEN_SLEEP_OR_SUBPROCESS_CALL: &[&[&str]] = &[ + &["", "open"], + &["time", "sleep"], + &["subprocess", "run"], + &["subprocess", "Popen"], + // Deprecated subprocess calls: + &["subprocess", "call"], + &["subprocess", "check_call"], + &["subprocess", "check_output"], + &["subprocess", "getoutput"], + &["subprocess", "getstatusoutput"], + &["os", "wait"], + &["os", "wait3"], + &["os", "wait4"], + &["os", "waitid"], + &["os", "waitpid"], +]; + +/// ASYNC101 +pub(crate) fn open_sleep_or_subprocess_call(checker: &mut Checker, expr: &Expr) { + if in_async_function(checker.semantic_model()) { + if let Expr::Call(ast::ExprCall { func, .. }) = expr { + let is_open_sleep_or_subprocess_call = checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |path| { + OPEN_SLEEP_OR_SUBPROCESS_CALL.contains(&path.as_slice()) + }); + + if is_open_sleep_or_subprocess_call { + checker.diagnostics.push(Diagnostic::new( + OpenSleepOrSubprocessInAsyncFunction, + func.range(), + )); + } + } + } +} diff --git a/crates/ruff/src/rules/flake8_blind_except/rules.rs b/crates/ruff/src/rules/flake8_blind_except/rules/blind_except.rs similarity index 100% rename from crates/ruff/src/rules/flake8_blind_except/rules.rs rename to crates/ruff/src/rules/flake8_blind_except/rules/blind_except.rs diff --git a/crates/ruff/src/rules/flake8_blind_except/rules/mod.rs b/crates/ruff/src/rules/flake8_blind_except/rules/mod.rs new file mode 100644 index 0000000000..520b3ece06 --- /dev/null +++ b/crates/ruff/src/rules/flake8_blind_except/rules/mod.rs @@ -0,0 +1,3 @@ +pub(crate) use blind_except::{blind_except, BlindExcept}; + +mod blind_except; diff --git a/crates/ruff/src/rules/flake8_boolean_trap/helpers.rs b/crates/ruff/src/rules/flake8_boolean_trap/helpers.rs new file mode 100644 index 0000000000..463ad690ab --- /dev/null +++ b/crates/ruff/src/rules/flake8_boolean_trap/helpers.rs @@ -0,0 +1,66 @@ +use rustpython_parser::ast::{self, Constant, Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, DiagnosticKind}; + +use crate::checkers::ast::Checker; + +pub(super) const FUNC_CALL_NAME_ALLOWLIST: &[&str] = &[ + "append", + "assertEqual", + "assertEquals", + "assertNotEqual", + "assertNotEquals", + "bytes", + "count", + "failIfEqual", + "failUnlessEqual", + "float", + "fromkeys", + "get", + "getattr", + "getboolean", + "getfloat", + "getint", + "index", + "insert", + "int", + "param", + "pop", + "remove", + "setattr", + "setdefault", + "str", +]; + +pub(super) const FUNC_DEF_NAME_ALLOWLIST: &[&str] = &["__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()); + } + + if let Expr::Name(ast::ExprName { id, .. }) = func { + return FUNC_CALL_NAME_ALLOWLIST.contains(&id.as_ref()); + } + + false +} + +const fn is_boolean_arg(arg: &Expr) -> bool { + matches!( + &arg, + Expr::Constant(ast::ExprConstant { + value: Constant::Bool(_), + .. + }) + ) +} + +pub(super) fn add_if_boolean(checker: &mut Checker, arg: &Expr, kind: DiagnosticKind) { + if is_boolean_arg(arg) { + checker.diagnostics.push(Diagnostic::new(kind, arg.range())); + } +} diff --git a/crates/ruff/src/rules/flake8_boolean_trap/mod.rs b/crates/ruff/src/rules/flake8_boolean_trap/mod.rs index 84ad7a7caf..1e86c51cbf 100644 --- a/crates/ruff/src/rules/flake8_boolean_trap/mod.rs +++ b/crates/ruff/src/rules/flake8_boolean_trap/mod.rs @@ -1,4 +1,5 @@ //! Rules from [flake8-boolean-trap](https://pypi.org/project/flake8-boolean-trap/). +mod helpers; pub(crate) mod rules; #[cfg(test)] diff --git a/crates/ruff/src/rules/flake8_boolean_trap/rules.rs b/crates/ruff/src/rules/flake8_boolean_trap/rules.rs deleted file mode 100644 index e50375d7b6..0000000000 --- a/crates/ruff/src/rules/flake8_boolean_trap/rules.rs +++ /dev/null @@ -1,301 +0,0 @@ -use rustpython_parser::ast::{self, Arguments, Constant, Expr, Ranged}; - -use ruff_diagnostics::Violation; -use ruff_diagnostics::{Diagnostic, DiagnosticKind}; -use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::call_path::collect_call_path; - -use crate::checkers::ast::Checker; - -/// ## What it does -/// Checks for boolean positional arguments in function definitions. -/// -/// ## Why is this bad? -/// Calling a function with boolean positional arguments is confusing as the -/// meaning of the boolean value is not clear to the caller, and to future -/// readers of the code. -/// -/// The use of a boolean will also limit the function to only two possible -/// behaviors, which makes the function difficult to extend in the future. -/// -/// ## Example -/// ```python -/// from math import ceil, floor -/// -/// -/// def round_number(number: float, up: bool) -> int: -/// return ceil(number) if up else floor(number) -/// -/// -/// round_number(1.5, True) # What does `True` mean? -/// round_number(1.5, False) # What does `False` mean? -/// ``` -/// -/// Instead, refactor into separate implementations: -/// ```python -/// from math import ceil, floor -/// -/// -/// def round_up(number: float) -> int: -/// return ceil(number) -/// -/// -/// def round_down(number: float) -> int: -/// return floor(number) -/// -/// -/// round_up(1.5) -/// round_down(1.5) -/// ``` -/// -/// Or, refactor to use an `Enum`: -/// ```python -/// from enum import Enum -/// -/// -/// class RoundingMethod(Enum): -/// UP = 1 -/// DOWN = 2 -/// -/// -/// def round_number(value: float, method: RoundingMethod) -> float: -/// ... -/// ``` -/// -/// ## References -/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#calls) -/// - [_How to Avoid “The Boolean Trap”_ by Adam Johnson](https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/) -#[violation] -pub struct BooleanPositionalArgInFunctionDefinition; - -impl Violation for BooleanPositionalArgInFunctionDefinition { - #[derive_message_formats] - fn message(&self) -> String { - format!("Boolean positional arg in function definition") - } -} - -/// ## What it does -/// Checks for the use of booleans as default values in function definitions. -/// -/// ## Why is this bad? -/// Calling a function with boolean default means that the keyword argument -/// argument can be omitted, which makes the function call ambiguous. -/// -/// Instead, define the relevant argument as keyword-only. -/// -/// ## Example -/// ```python -/// from math import ceil, floor -/// -/// -/// def round_number(number: float, *, up: bool = True) -> int: -/// return ceil(number) if up else floor(number) -/// -/// -/// round_number(1.5) -/// round_number(1.5, up=False) -/// ``` -/// -/// Use instead: -/// ```python -/// from math import ceil, floor -/// -/// -/// def round_number(number: float, *, up: bool) -> int: -/// return ceil(number) if up else floor(number) -/// -/// -/// round_number(1.5, up=True) -/// round_number(1.5, up=False) -/// ``` -/// -/// ## References -/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#calls) -/// - [_How to Avoid “The Boolean Trap”_ by Adam Johnson](https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/) -#[violation] -pub struct BooleanDefaultValueInFunctionDefinition; - -impl Violation for BooleanDefaultValueInFunctionDefinition { - #[derive_message_formats] - fn message(&self) -> String { - format!("Boolean default value in function definition") - } -} - -/// ## What it does -/// Checks for boolean positional arguments in function calls. -/// -/// ## Why is this bad? -/// Calling a function with boolean positional arguments is confusing as the -/// meaning of the boolean value is not clear to the caller, and to future -/// readers of the code. -/// -/// ## Example -/// ```python -/// def foo(flag: bool) -> None: -/// ... -/// -/// -/// foo(True) -/// ``` -/// -/// Use instead: -/// ```python -/// def foo(flag: bool) -> None: -/// ... -/// -/// -/// foo(flag=True) -/// ``` -/// -/// ## References -/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#calls) -/// - [_How to Avoid “The Boolean Trap”_ by Adam Johnson](https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/) -#[violation] -pub struct BooleanPositionalValueInFunctionCall; - -impl Violation for BooleanPositionalValueInFunctionCall { - #[derive_message_formats] - fn message(&self) -> String { - format!("Boolean positional value in function call") - } -} - -const FUNC_CALL_NAME_ALLOWLIST: &[&str] = &[ - "append", - "assertEqual", - "assertEquals", - "assertNotEqual", - "assertNotEquals", - "bytes", - "count", - "failIfEqual", - "failUnlessEqual", - "float", - "fromkeys", - "get", - "getattr", - "getboolean", - "getfloat", - "getint", - "index", - "insert", - "int", - "param", - "pop", - "remove", - "setattr", - "setdefault", - "str", -]; - -const FUNC_DEF_NAME_ALLOWLIST: &[&str] = &["__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. -fn allow_boolean_trap(func: &Expr) -> bool { - if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func { - return FUNC_CALL_NAME_ALLOWLIST.contains(&attr.as_ref()); - } - - if let Expr::Name(ast::ExprName { id, .. }) = func { - return FUNC_CALL_NAME_ALLOWLIST.contains(&id.as_ref()); - } - - false -} - -const fn is_boolean_arg(arg: &Expr) -> bool { - matches!( - &arg, - Expr::Constant(ast::ExprConstant { - value: Constant::Bool(_), - .. - }) - ) -} - -fn add_if_boolean(checker: &mut Checker, arg: &Expr, kind: DiagnosticKind) { - if is_boolean_arg(arg) { - checker.diagnostics.push(Diagnostic::new(kind, arg.range())); - } -} - -pub(crate) fn check_positional_boolean_in_def( - checker: &mut Checker, - name: &str, - decorator_list: &[Expr], - arguments: &Arguments, -) { - if FUNC_DEF_NAME_ALLOWLIST.contains(&name) { - return; - } - - if decorator_list.iter().any(|expr| { - collect_call_path(expr).map_or(false, |call_path| call_path.as_slice() == [name, "setter"]) - }) { - return; - } - - for arg in arguments.posonlyargs.iter().chain(arguments.args.iter()) { - if arg.annotation.is_none() { - continue; - } - let Some(expr) = &arg.annotation else { - continue; - }; - - // check for both bool (python class) and 'bool' (string annotation) - let hint = match expr.as_ref() { - Expr::Name(name) => &name.id == "bool", - Expr::Constant(ast::ExprConstant { - value: Constant::Str(value), - .. - }) => value == "bool", - _ => false, - }; - if !hint { - continue; - } - checker.diagnostics.push(Diagnostic::new( - BooleanPositionalArgInFunctionDefinition, - arg.range(), - )); - } -} - -pub(crate) fn check_boolean_default_value_in_function_definition( - checker: &mut Checker, - name: &str, - decorator_list: &[Expr], - arguments: &Arguments, -) { - if FUNC_DEF_NAME_ALLOWLIST.contains(&name) { - return; - } - - if decorator_list.iter().any(|expr| { - collect_call_path(expr).map_or(false, |call_path| call_path.as_slice() == [name, "setter"]) - }) { - return; - } - - for arg in &arguments.defaults { - add_if_boolean(checker, arg, BooleanDefaultValueInFunctionDefinition.into()); - } -} - -pub(crate) fn check_boolean_positional_value_in_function_call( - checker: &mut Checker, - args: &[Expr], - func: &Expr, -) { - if allow_boolean_trap(func) { - return; - } - for arg in args { - add_if_boolean(checker, arg, BooleanPositionalValueInFunctionCall.into()); - } -} 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 new file mode 100644 index 0000000000..610a6c8491 --- /dev/null +++ b/crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_default_value_in_function_definition.rs @@ -0,0 +1,80 @@ +use rustpython_parser::ast::{Arguments, Expr}; + +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; + +/// ## What it does +/// Checks for the use of booleans as default values in function definitions. +/// +/// ## Why is this bad? +/// Calling a function with boolean default means that the keyword argument +/// argument can be omitted, which makes the function call ambiguous. +/// +/// Instead, define the relevant argument as keyword-only. +/// +/// ## Example +/// ```python +/// from math import ceil, floor +/// +/// +/// def round_number(number: float, *, up: bool = True) -> int: +/// return ceil(number) if up else floor(number) +/// +/// +/// round_number(1.5) +/// round_number(1.5, up=False) +/// ``` +/// +/// Use instead: +/// ```python +/// from math import ceil, floor +/// +/// +/// def round_number(number: float, *, up: bool) -> int: +/// return ceil(number) if up else floor(number) +/// +/// +/// round_number(1.5, up=True) +/// round_number(1.5, up=False) +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#calls) +/// - [_How to Avoid “The Boolean Trap”_ by Adam Johnson](https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/) +#[violation] +pub struct BooleanDefaultValueInFunctionDefinition; + +impl Violation for BooleanDefaultValueInFunctionDefinition { + #[derive_message_formats] + fn message(&self) -> String { + format!("Boolean default value in function definition") + } +} + +pub(crate) fn check_boolean_default_value_in_function_definition( + checker: &mut Checker, + name: &str, + decorator_list: &[Expr], + arguments: &Arguments, +) { + if FUNC_DEF_NAME_ALLOWLIST.contains(&name) { + return; + } + + if decorator_list.iter().any(|expr| { + collect_call_path(expr).map_or(false, |call_path| call_path.as_slice() == [name, "setter"]) + }) { + return; + } + + for arg in &arguments.defaults { + add_if_boolean(checker, arg, BooleanDefaultValueInFunctionDefinition.into()); + } +} diff --git a/crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_positional_value_in_function_call.rs b/crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_positional_value_in_function_call.rs new file mode 100644 index 0000000000..5b39f0f24a --- /dev/null +++ b/crates/ruff/src/rules/flake8_boolean_trap/rules/check_boolean_positional_value_in_function_call.rs @@ -0,0 +1,60 @@ +use rustpython_parser::ast::Expr; + +use ruff_diagnostics::Violation; + +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; +use crate::rules::flake8_boolean_trap::helpers::{add_if_boolean, allow_boolean_trap}; + +/// ## What it does +/// Checks for boolean positional arguments in function calls. +/// +/// ## Why is this bad? +/// Calling a function with boolean positional arguments is confusing as the +/// meaning of the boolean value is not clear to the caller, and to future +/// readers of the code. +/// +/// ## Example +/// ```python +/// def foo(flag: bool) -> None: +/// ... +/// +/// +/// foo(True) +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(flag: bool) -> None: +/// ... +/// +/// +/// foo(flag=True) +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#calls) +/// - [_How to Avoid “The Boolean Trap”_ by Adam Johnson](https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/) +#[violation] +pub struct BooleanPositionalValueInFunctionCall; + +impl Violation for BooleanPositionalValueInFunctionCall { + #[derive_message_formats] + fn message(&self) -> String { + format!("Boolean positional value in function call") + } +} + +pub(crate) fn check_boolean_positional_value_in_function_call( + checker: &mut Checker, + args: &[Expr], + func: &Expr, +) { + if allow_boolean_trap(func) { + return; + } + for arg in args { + add_if_boolean(checker, arg, BooleanPositionalValueInFunctionCall.into()); + } +} 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 new file mode 100644 index 0000000000..3a6f2d4df3 --- /dev/null +++ b/crates/ruff/src/rules/flake8_boolean_trap/rules/check_positional_boolean_in_def.rs @@ -0,0 +1,120 @@ +use rustpython_parser::ast::{self, Arguments, Constant, Expr, Ranged}; + +use ruff_diagnostics::Diagnostic; +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::FUNC_DEF_NAME_ALLOWLIST; + +/// ## What it does +/// Checks for boolean positional arguments in function definitions. +/// +/// ## Why is this bad? +/// Calling a function with boolean positional arguments is confusing as the +/// meaning of the boolean value is not clear to the caller, and to future +/// readers of the code. +/// +/// The use of a boolean will also limit the function to only two possible +/// behaviors, which makes the function difficult to extend in the future. +/// +/// ## Example +/// ```python +/// from math import ceil, floor +/// +/// +/// def round_number(number: float, up: bool) -> int: +/// return ceil(number) if up else floor(number) +/// +/// +/// round_number(1.5, True) # What does `True` mean? +/// round_number(1.5, False) # What does `False` mean? +/// ``` +/// +/// Instead, refactor into separate implementations: +/// ```python +/// from math import ceil, floor +/// +/// +/// def round_up(number: float) -> int: +/// return ceil(number) +/// +/// +/// def round_down(number: float) -> int: +/// return floor(number) +/// +/// +/// round_up(1.5) +/// round_down(1.5) +/// ``` +/// +/// Or, refactor to use an `Enum`: +/// ```python +/// from enum import Enum +/// +/// +/// class RoundingMethod(Enum): +/// UP = 1 +/// DOWN = 2 +/// +/// +/// def round_number(value: float, method: RoundingMethod) -> float: +/// ... +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#calls) +/// - [_How to Avoid “The Boolean Trap”_ by Adam Johnson](https://adamj.eu/tech/2021/07/10/python-type-hints-how-to-avoid-the-boolean-trap/) +#[violation] +pub struct BooleanPositionalArgInFunctionDefinition; + +impl Violation for BooleanPositionalArgInFunctionDefinition { + #[derive_message_formats] + fn message(&self) -> String { + format!("Boolean positional arg in function definition") + } +} + +pub(crate) fn check_positional_boolean_in_def( + checker: &mut Checker, + name: &str, + decorator_list: &[Expr], + arguments: &Arguments, +) { + if FUNC_DEF_NAME_ALLOWLIST.contains(&name) { + return; + } + + if decorator_list.iter().any(|expr| { + collect_call_path(expr).map_or(false, |call_path| call_path.as_slice() == [name, "setter"]) + }) { + return; + } + + for arg in arguments.posonlyargs.iter().chain(arguments.args.iter()) { + if arg.annotation.is_none() { + continue; + } + let Some(expr) = &arg.annotation else { + continue; + }; + + // check for both bool (python class) and 'bool' (string annotation) + let hint = match expr.as_ref() { + Expr::Name(name) => &name.id == "bool", + Expr::Constant(ast::ExprConstant { + value: Constant::Str(value), + .. + }) => value == "bool", + _ => false, + }; + if !hint { + continue; + } + checker.diagnostics.push(Diagnostic::new( + BooleanPositionalArgInFunctionDefinition, + arg.range(), + )); + } +} diff --git a/crates/ruff/src/rules/flake8_boolean_trap/rules/mod.rs b/crates/ruff/src/rules/flake8_boolean_trap/rules/mod.rs new file mode 100644 index 0000000000..a0e9b8bd66 --- /dev/null +++ b/crates/ruff/src/rules/flake8_boolean_trap/rules/mod.rs @@ -0,0 +1,13 @@ +pub(crate) use check_boolean_default_value_in_function_definition::{ + check_boolean_default_value_in_function_definition, BooleanDefaultValueInFunctionDefinition, +}; +pub(crate) use check_boolean_positional_value_in_function_call::{ + check_boolean_positional_value_in_function_call, BooleanPositionalValueInFunctionCall, +}; +pub(crate) use check_positional_boolean_in_def::{ + check_positional_boolean_in_def, BooleanPositionalArgInFunctionDefinition, +}; + +mod check_boolean_default_value_in_function_definition; +mod check_boolean_positional_value_in_function_call; +mod check_positional_boolean_in_def; diff --git a/crates/ruff/src/rules/flake8_builtins/helpers.rs b/crates/ruff/src/rules/flake8_builtins/helpers.rs new file mode 100644 index 0000000000..1f1eb0f3ba --- /dev/null +++ b/crates/ruff/src/rules/flake8_builtins/helpers.rs @@ -0,0 +1,45 @@ +use rustpython_parser::ast::{Excepthandler, Expr, Ranged, Stmt}; + +use ruff_python_ast::helpers::identifier_range; +use ruff_python_ast::source_code::Locator; +use ruff_python_stdlib::builtins::BUILTINS; +use ruff_text_size::TextRange; + +pub(super) fn shadows_builtin(name: &str, ignorelist: &[String]) -> bool { + BUILTINS.contains(&name) && ignorelist.iter().all(|ignore| ignore != name) +} + +#[derive(Debug, Copy, Clone, PartialEq)] +pub(crate) enum AnyShadowing<'a> { + Expression(&'a Expr), + Statement(&'a Stmt), + ExceptHandler(&'a Excepthandler), +} + +impl AnyShadowing<'_> { + pub(crate) fn range(self, locator: &Locator) -> TextRange { + match self { + AnyShadowing::Expression(expr) => expr.range(), + AnyShadowing::Statement(stmt) => identifier_range(stmt, locator), + AnyShadowing::ExceptHandler(handler) => handler.range(), + } + } +} + +impl<'a> From<&'a Stmt> for AnyShadowing<'a> { + fn from(value: &'a Stmt) -> Self { + AnyShadowing::Statement(value) + } +} + +impl<'a> From<&'a Expr> for AnyShadowing<'a> { + fn from(value: &'a Expr) -> Self { + AnyShadowing::Expression(value) + } +} + +impl<'a> From<&'a Excepthandler> for AnyShadowing<'a> { + fn from(value: &'a Excepthandler) -> Self { + AnyShadowing::ExceptHandler(value) + } +} diff --git a/crates/ruff/src/rules/flake8_builtins/mod.rs b/crates/ruff/src/rules/flake8_builtins/mod.rs index 5229d00171..58aea5fba4 100644 --- a/crates/ruff/src/rules/flake8_builtins/mod.rs +++ b/crates/ruff/src/rules/flake8_builtins/mod.rs @@ -1,4 +1,5 @@ //! Rules from [flake8-builtins](https://pypi.org/project/flake8-builtins/). +pub(crate) mod helpers; pub(crate) mod rules; pub mod settings; diff --git a/crates/ruff/src/rules/flake8_builtins/rules.rs b/crates/ruff/src/rules/flake8_builtins/rules.rs deleted file mode 100644 index 9879ef66fd..0000000000 --- a/crates/ruff/src/rules/flake8_builtins/rules.rs +++ /dev/null @@ -1,257 +0,0 @@ -use ruff_text_size::TextRange; -use rustpython_parser::ast::{Arg, Excepthandler, Expr, Ranged, Stmt}; - -use ruff_diagnostics::Diagnostic; -use ruff_diagnostics::Violation; -use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::identifier_range; -use ruff_python_ast::source_code::Locator; -use ruff_python_stdlib::builtins::BUILTINS; - -use crate::checkers::ast::Checker; - -/// ## What it does -/// Checks for variable (and function) assignments that use the same name -/// as a builtin. -/// -/// ## Why is this bad? -/// Reusing a builtin name for the name of a variable increases the -/// difficulty of reading and maintaining the code, and can cause -/// non-obvious errors, as readers may mistake the variable for the -/// builtin and vice versa. -/// -/// Builtins can be marked as exceptions to this rule via the -/// [`flake8-builtins.builtins-ignorelist`] configuration option. -/// -/// ## Options -/// -/// - `flake8-builtins.builtins-ignorelist` -/// -/// ## Example -/// ```python -/// def find_max(list_of_lists): -/// max = 0 -/// for flat_list in list_of_lists: -/// for value in flat_list: -/// max = max(max, value) # TypeError: 'int' object is not callable -/// return max -/// ``` -/// -/// Use instead: -/// ```python -/// def find_max(list_of_lists): -/// result = 0 -/// for flat_list in list_of_lists: -/// for value in flat_list: -/// result = max(result, value) -/// return result -/// ``` -/// -/// - [_Why is it a bad idea to name a variable `id` in Python?_](https://stackoverflow.com/questions/77552/id-is-a-bad-variable-name-in-python) -#[violation] -pub struct BuiltinVariableShadowing { - name: String, -} - -impl Violation for BuiltinVariableShadowing { - #[derive_message_formats] - fn message(&self) -> String { - let BuiltinVariableShadowing { name } = self; - format!("Variable `{name}` is shadowing a Python builtin") - } -} - -/// ## What it does -/// Checks for any function arguments that use the same name as a builtin. -/// -/// ## Why is this bad? -/// Reusing a builtin name for the name of an argument increases the -/// difficulty of reading and maintaining the code, and can cause -/// non-obvious errors, as readers may mistake the argument for the -/// builtin and vice versa. -/// -/// Builtins can be marked as exceptions to this rule via the -/// [`flake8-builtins.builtins-ignorelist`] configuration option. -/// -/// ## Options -/// -/// - `flake8-builtins.builtins-ignorelist` -/// -/// ## Example -/// ```python -/// def remove_duplicates(list, list2): -/// result = set() -/// for value in list: -/// result.add(value) -/// for value in list2: -/// result.add(value) -/// return list(result) # TypeError: 'list' object is not callable -/// ``` -/// -/// Use instead: -/// ```python -/// def remove_duplicates(list1, list2): -/// result = set() -/// for value in list1: -/// result.add(value) -/// for value in list2: -/// result.add(value) -/// return list(result) -/// ``` -/// -/// ## References -/// - [_Is it bad practice to use a built-in function name as an attribute or method identifier?_](https://stackoverflow.com/questions/9109333/is-it-bad-practice-to-use-a-built-in-function-name-as-an-attribute-or-method-ide) -/// - [_Why is it a bad idea to name a variable `id` in Python?_](https://stackoverflow.com/questions/77552/id-is-a-bad-variable-name-in-python) -#[violation] -pub struct BuiltinArgumentShadowing { - name: String, -} - -impl Violation for BuiltinArgumentShadowing { - #[derive_message_formats] - fn message(&self) -> String { - let BuiltinArgumentShadowing { name } = self; - format!("Argument `{name}` is shadowing a Python builtin") - } -} - -/// ## What it does -/// Checks for any class attributes that use the same name as a builtin. -/// -/// ## Why is this bad? -/// Reusing a builtin name for the name of an attribute increases the -/// difficulty of reading and maintaining the code, and can cause -/// non-obvious errors, as readers may mistake the attribute for the -/// builtin and vice versa. -/// -/// Builtins can be marked as exceptions to this rule via the -/// [`flake8-builtins.builtins-ignorelist`] configuration option, or -/// converted to the appropriate dunder method. -/// -/// ## Options -/// -/// - `flake8-builtins.builtins-ignorelist` -/// -/// ## Example -/// ```python -/// class Shadow: -/// def int(): -/// return 0 -/// ``` -/// -/// Use instead: -/// ```python -/// class Shadow: -/// def to_int(): -/// return 0 -/// ``` -/// -/// Or: -/// ```python -/// class Shadow: -/// # Callable as `int(shadow)` -/// def __int__(): -/// return 0 -/// ``` -/// -/// ## References -/// - [_Is it bad practice to use a built-in function name as an attribute or method identifier?_](https://stackoverflow.com/questions/9109333/is-it-bad-practice-to-use-a-built-in-function-name-as-an-attribute-or-method-ide) -/// - [_Why is it a bad idea to name a variable `id` in Python?_](https://stackoverflow.com/questions/77552/id-is-a-bad-variable-name-in-python) -#[violation] -pub struct BuiltinAttributeShadowing { - name: String, -} - -impl Violation for BuiltinAttributeShadowing { - #[derive_message_formats] - fn message(&self) -> String { - let BuiltinAttributeShadowing { name } = self; - format!("Class attribute `{name}` is shadowing a Python builtin") - } -} - -fn shadows_builtin(name: &str, ignorelist: &[String]) -> bool { - BUILTINS.contains(&name) && ignorelist.iter().all(|ignore| ignore != name) -} - -/// A001 -pub(crate) fn builtin_variable_shadowing( - checker: &mut Checker, - name: &str, - shadowing: AnyShadowing, -) { - if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { - checker.diagnostics.push(Diagnostic::new( - BuiltinVariableShadowing { - name: name.to_string(), - }, - shadowing.range(checker.locator), - )); - } -} - -/// A002 -pub(crate) fn builtin_argument_shadowing(checker: &mut Checker, argument: &Arg) { - if shadows_builtin( - argument.arg.as_str(), - &checker.settings.flake8_builtins.builtins_ignorelist, - ) { - checker.diagnostics.push(Diagnostic::new( - BuiltinArgumentShadowing { - name: argument.arg.to_string(), - }, - argument.range(), - )); - } -} - -/// A003 -pub(crate) fn builtin_attribute_shadowing( - checker: &mut Checker, - name: &str, - shadowing: AnyShadowing, -) { - if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { - checker.diagnostics.push(Diagnostic::new( - BuiltinAttributeShadowing { - name: name.to_string(), - }, - shadowing.range(checker.locator), - )); - } -} - -#[derive(Debug, Copy, Clone, PartialEq)] -pub(crate) enum AnyShadowing<'a> { - Expression(&'a Expr), - Statement(&'a Stmt), - ExceptHandler(&'a Excepthandler), -} - -impl AnyShadowing<'_> { - fn range(self, locator: &Locator) -> TextRange { - match self { - AnyShadowing::Expression(expr) => expr.range(), - AnyShadowing::Statement(stmt) => identifier_range(stmt, locator), - AnyShadowing::ExceptHandler(handler) => handler.range(), - } - } -} - -impl<'a> From<&'a Stmt> for AnyShadowing<'a> { - fn from(value: &'a Stmt) -> Self { - AnyShadowing::Statement(value) - } -} - -impl<'a> From<&'a Expr> for AnyShadowing<'a> { - fn from(value: &'a Expr) -> Self { - AnyShadowing::Expression(value) - } -} - -impl<'a> From<&'a Excepthandler> for AnyShadowing<'a> { - fn from(value: &'a Excepthandler) -> Self { - AnyShadowing::ExceptHandler(value) - } -} diff --git a/crates/ruff/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs b/crates/ruff/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs new file mode 100644 index 0000000000..e6aea40d3d --- /dev/null +++ b/crates/ruff/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs @@ -0,0 +1,78 @@ +use rustpython_parser::ast::{Arg, Ranged}; + +use ruff_diagnostics::Diagnostic; +use ruff_diagnostics::Violation; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +use super::super::helpers::shadows_builtin; + +/// ## What it does +/// Checks for any function arguments that use the same name as a builtin. +/// +/// ## Why is this bad? +/// Reusing a builtin name for the name of an argument increases the +/// difficulty of reading and maintaining the code, and can cause +/// non-obvious errors, as readers may mistake the argument for the +/// builtin and vice versa. +/// +/// Builtins can be marked as exceptions to this rule via the +/// [`flake8-builtins.builtins-ignorelist`] configuration option. +/// +/// ## Options +/// +/// - `flake8-builtins.builtins-ignorelist` +/// +/// ## Example +/// ```python +/// def remove_duplicates(list, list2): +/// result = set() +/// for value in list: +/// result.add(value) +/// for value in list2: +/// result.add(value) +/// return list(result) # TypeError: 'list' object is not callable +/// ``` +/// +/// Use instead: +/// ```python +/// def remove_duplicates(list1, list2): +/// result = set() +/// for value in list1: +/// result.add(value) +/// for value in list2: +/// result.add(value) +/// return list(result) +/// ``` +/// +/// ## References +/// - [_Is it bad practice to use a built-in function name as an attribute or method identifier?_](https://stackoverflow.com/questions/9109333/is-it-bad-practice-to-use-a-built-in-function-name-as-an-attribute-or-method-ide) +/// - [_Why is it a bad idea to name a variable `id` in Python?_](https://stackoverflow.com/questions/77552/id-is-a-bad-variable-name-in-python) +#[violation] +pub struct BuiltinArgumentShadowing { + name: String, +} + +impl Violation for BuiltinArgumentShadowing { + #[derive_message_formats] + fn message(&self) -> String { + let BuiltinArgumentShadowing { name } = self; + format!("Argument `{name}` is shadowing a Python builtin") + } +} + +/// A002 +pub(crate) fn builtin_argument_shadowing(checker: &mut Checker, argument: &Arg) { + if shadows_builtin( + argument.arg.as_str(), + &checker.settings.flake8_builtins.builtins_ignorelist, + ) { + checker.diagnostics.push(Diagnostic::new( + BuiltinArgumentShadowing { + name: argument.arg.to_string(), + }, + argument.range(), + )); + } +} diff --git a/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs b/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs new file mode 100644 index 0000000000..701fa4edd3 --- /dev/null +++ b/crates/ruff/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs @@ -0,0 +1,78 @@ +use ruff_diagnostics::Diagnostic; +use ruff_diagnostics::Violation; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +use super::super::helpers::{shadows_builtin, AnyShadowing}; + +/// ## What it does +/// Checks for any class attributes that use the same name as a builtin. +/// +/// ## Why is this bad? +/// Reusing a builtin name for the name of an attribute increases the +/// difficulty of reading and maintaining the code, and can cause +/// non-obvious errors, as readers may mistake the attribute for the +/// builtin and vice versa. +/// +/// Builtins can be marked as exceptions to this rule via the +/// [`flake8-builtins.builtins-ignorelist`] configuration option, or +/// converted to the appropriate dunder method. +/// +/// ## Options +/// +/// - `flake8-builtins.builtins-ignorelist` +/// +/// ## Example +/// ```python +/// class Shadow: +/// def int(): +/// return 0 +/// ``` +/// +/// Use instead: +/// ```python +/// class Shadow: +/// def to_int(): +/// return 0 +/// ``` +/// +/// Or: +/// ```python +/// class Shadow: +/// # Callable as `int(shadow)` +/// def __int__(): +/// return 0 +/// ``` +/// +/// ## References +/// - [_Is it bad practice to use a built-in function name as an attribute or method identifier?_](https://stackoverflow.com/questions/9109333/is-it-bad-practice-to-use-a-built-in-function-name-as-an-attribute-or-method-ide) +/// - [_Why is it a bad idea to name a variable `id` in Python?_](https://stackoverflow.com/questions/77552/id-is-a-bad-variable-name-in-python) +#[violation] +pub struct BuiltinAttributeShadowing { + name: String, +} + +impl Violation for BuiltinAttributeShadowing { + #[derive_message_formats] + fn message(&self) -> String { + let BuiltinAttributeShadowing { name } = self; + format!("Class attribute `{name}` is shadowing a Python builtin") + } +} + +/// A003 +pub(crate) fn builtin_attribute_shadowing( + checker: &mut Checker, + name: &str, + shadowing: AnyShadowing, +) { + if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { + checker.diagnostics.push(Diagnostic::new( + BuiltinAttributeShadowing { + name: name.to_string(), + }, + shadowing.range(checker.locator), + )); + } +} diff --git a/crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs b/crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs new file mode 100644 index 0000000000..a965af53ca --- /dev/null +++ b/crates/ruff/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs @@ -0,0 +1,74 @@ +use ruff_diagnostics::Diagnostic; +use ruff_diagnostics::Violation; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +use super::super::helpers::{shadows_builtin, AnyShadowing}; + +/// ## What it does +/// Checks for variable (and function) assignments that use the same name +/// as a builtin. +/// +/// ## Why is this bad? +/// Reusing a builtin name for the name of a variable increases the +/// difficulty of reading and maintaining the code, and can cause +/// non-obvious errors, as readers may mistake the variable for the +/// builtin and vice versa. +/// +/// Builtins can be marked as exceptions to this rule via the +/// [`flake8-builtins.builtins-ignorelist`] configuration option. +/// +/// ## Options +/// +/// - `flake8-builtins.builtins-ignorelist` +/// +/// ## Example +/// ```python +/// def find_max(list_of_lists): +/// max = 0 +/// for flat_list in list_of_lists: +/// for value in flat_list: +/// max = max(max, value) # TypeError: 'int' object is not callable +/// return max +/// ``` +/// +/// Use instead: +/// ```python +/// def find_max(list_of_lists): +/// result = 0 +/// for flat_list in list_of_lists: +/// for value in flat_list: +/// result = max(result, value) +/// return result +/// ``` +/// +/// - [_Why is it a bad idea to name a variable `id` in Python?_](https://stackoverflow.com/questions/77552/id-is-a-bad-variable-name-in-python) +#[violation] +pub struct BuiltinVariableShadowing { + name: String, +} + +impl Violation for BuiltinVariableShadowing { + #[derive_message_formats] + fn message(&self) -> String { + let BuiltinVariableShadowing { name } = self; + format!("Variable `{name}` is shadowing a Python builtin") + } +} + +/// A001 +pub(crate) fn builtin_variable_shadowing( + checker: &mut Checker, + name: &str, + shadowing: AnyShadowing, +) { + if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { + checker.diagnostics.push(Diagnostic::new( + BuiltinVariableShadowing { + name: name.to_string(), + }, + shadowing.range(checker.locator), + )); + } +} diff --git a/crates/ruff/src/rules/flake8_builtins/rules/mod.rs b/crates/ruff/src/rules/flake8_builtins/rules/mod.rs new file mode 100644 index 0000000000..f9b8c3c3d7 --- /dev/null +++ b/crates/ruff/src/rules/flake8_builtins/rules/mod.rs @@ -0,0 +1,9 @@ +pub(crate) use builtin_argument_shadowing::{builtin_argument_shadowing, BuiltinArgumentShadowing}; +pub(crate) use builtin_attribute_shadowing::{ + builtin_attribute_shadowing, BuiltinAttributeShadowing, +}; +pub(crate) use builtin_variable_shadowing::{builtin_variable_shadowing, BuiltinVariableShadowing}; + +mod builtin_argument_shadowing; +mod builtin_attribute_shadowing; +mod builtin_variable_shadowing; diff --git a/crates/ruff/src/rules/flake8_builtins/rules/rules.rs b/crates/ruff/src/rules/flake8_builtins/rules/rules.rs new file mode 100644 index 0000000000..b28b04f643 --- /dev/null +++ b/crates/ruff/src/rules/flake8_builtins/rules/rules.rs @@ -0,0 +1,3 @@ + + + diff --git a/crates/ruff/src/rules/flake8_commas/rules/mod.rs b/crates/ruff/src/rules/flake8_commas/rules/mod.rs new file mode 100644 index 0000000000..0286278d8c --- /dev/null +++ b/crates/ruff/src/rules/flake8_commas/rules/mod.rs @@ -0,0 +1,5 @@ +pub(crate) use trailing_commas::{ + trailing_commas, MissingTrailingComma, ProhibitedTrailingComma, TrailingCommaOnBareTuple, +}; + +mod trailing_commas; diff --git a/crates/ruff/src/rules/flake8_commas/rules.rs b/crates/ruff/src/rules/flake8_commas/rules/trailing_commas.rs similarity index 100% rename from crates/ruff/src/rules/flake8_commas/rules.rs rename to crates/ruff/src/rules/flake8_commas/rules/trailing_commas.rs diff --git a/crates/ruff/src/rules/flake8_datetimez/rules.rs b/crates/ruff/src/rules/flake8_datetimez/rules.rs deleted file mode 100644 index 2a7393ff65..0000000000 --- a/crates/ruff/src/rules/flake8_datetimez/rules.rs +++ /dev/null @@ -1,403 +0,0 @@ -use ruff_text_size::TextRange; -use rustpython_parser::ast::{self, Constant, Expr, Keyword}; - -use ruff_diagnostics::{Diagnostic, Violation}; -use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::helpers::{has_non_none_keyword, is_const_none}; - -use crate::checkers::ast::Checker; - -#[violation] -pub struct CallDatetimeWithoutTzinfo; - -impl Violation for CallDatetimeWithoutTzinfo { - #[derive_message_formats] - fn message(&self) -> String { - format!("The use of `datetime.datetime()` without `tzinfo` argument is not allowed") - } -} - -#[violation] -pub struct CallDatetimeToday; - -impl Violation for CallDatetimeToday { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "The use of `datetime.datetime.today()` is not allowed, use \ - `datetime.datetime.now(tz=)` instead" - ) - } -} - -#[violation] -pub struct CallDatetimeUtcnow; - -impl Violation for CallDatetimeUtcnow { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "The use of `datetime.datetime.utcnow()` is not allowed, use \ - `datetime.datetime.now(tz=)` instead" - ) - } -} - -#[violation] -pub struct CallDatetimeUtcfromtimestamp; - -impl Violation for CallDatetimeUtcfromtimestamp { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "The use of `datetime.datetime.utcfromtimestamp()` is not allowed, use \ - `datetime.datetime.fromtimestamp(ts, tz=)` instead" - ) - } -} - -#[violation] -pub struct CallDatetimeNowWithoutTzinfo; - -impl Violation for CallDatetimeNowWithoutTzinfo { - #[derive_message_formats] - fn message(&self) -> String { - format!("The use of `datetime.datetime.now()` without `tz` argument is not allowed") - } -} - -#[violation] -pub struct CallDatetimeFromtimestamp; - -impl Violation for CallDatetimeFromtimestamp { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "The use of `datetime.datetime.fromtimestamp()` without `tz` argument is not allowed" - ) - } -} - -#[violation] -pub struct CallDatetimeStrptimeWithoutZone; - -impl Violation for CallDatetimeStrptimeWithoutZone { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "The use of `datetime.datetime.strptime()` without %z must be followed by \ - `.replace(tzinfo=)` or `.astimezone()`" - ) - } -} - -#[violation] -pub struct CallDateToday; - -impl Violation for CallDateToday { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "The use of `datetime.date.today()` is not allowed, use \ - `datetime.datetime.now(tz=).date()` instead" - ) - } -} - -#[violation] -pub struct CallDateFromtimestamp; - -impl Violation for CallDateFromtimestamp { - #[derive_message_formats] - fn message(&self) -> String { - format!( - "The use of `datetime.date.fromtimestamp()` is not allowed, use \ - `datetime.datetime.fromtimestamp(ts, tz=).date()` instead" - ) - } -} - -pub(crate) fn call_datetime_without_tzinfo( - checker: &mut Checker, - func: &Expr, - args: &[Expr], - keywords: &[Keyword], - location: TextRange, -) { - if !checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| { - call_path.as_slice() == ["datetime", "datetime"] - }) - { - return; - } - - // No positional arg: keyword is missing or constant None. - if args.len() < 8 && !has_non_none_keyword(keywords, "tzinfo") { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeWithoutTzinfo, location)); - return; - } - - // Positional arg: is constant None. - if args.len() >= 8 && is_const_none(&args[7]) { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeWithoutTzinfo, location)); - } -} - -/// Checks for `datetime.datetime.today()`. (DTZ002) -/// -/// ## Why is this bad? -/// -/// It uses the system local timezone. -/// Use `datetime.datetime.now(tz=)` instead. -pub(crate) fn call_datetime_today(checker: &mut Checker, func: &Expr, location: TextRange) { - if checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| { - call_path.as_slice() == ["datetime", "datetime", "today"] - }) - { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeToday, location)); - } -} - -/// Checks for `datetime.datetime.today()`. (DTZ003) -/// -/// ## Why is this bad? -/// -/// Because naive `datetime` objects are treated by many `datetime` methods as -/// local times, it is preferred to use aware datetimes to represent times in -/// UTC. As such, the recommended way to create an object representing the -/// current time in UTC is by calling `datetime.now(timezone.utc)`. -pub(crate) fn call_datetime_utcnow(checker: &mut Checker, func: &Expr, location: TextRange) { - if checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| { - call_path.as_slice() == ["datetime", "datetime", "utcnow"] - }) - { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeUtcnow, location)); - } -} - -/// Checks for `datetime.datetime.utcfromtimestamp()`. (DTZ004) -/// -/// ## Why is this bad? -/// -/// Because naive `datetime` objects are treated by many `datetime` methods as -/// local times, it is preferred to use aware datetimes to represent times in -/// UTC. As such, the recommended way to create an object representing a -/// specific timestamp in UTC is by calling `datetime.fromtimestamp(timestamp, -/// tz=timezone.utc)`. -pub(crate) fn call_datetime_utcfromtimestamp( - checker: &mut Checker, - func: &Expr, - location: TextRange, -) { - if checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| { - call_path.as_slice() == ["datetime", "datetime", "utcfromtimestamp"] - }) - { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeUtcfromtimestamp, location)); - } -} - -/// DTZ005 -pub(crate) fn call_datetime_now_without_tzinfo( - checker: &mut Checker, - func: &Expr, - args: &[Expr], - keywords: &[Keyword], - location: TextRange, -) { - if !checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| { - call_path.as_slice() == ["datetime", "datetime", "now"] - }) - { - return; - } - - // no args / no args unqualified - if args.is_empty() && keywords.is_empty() { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, location)); - return; - } - - // none args - if !args.is_empty() && is_const_none(&args[0]) { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, location)); - return; - } - - // wrong keywords / none keyword - if !keywords.is_empty() && !has_non_none_keyword(keywords, "tz") { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, location)); - } -} - -/// DTZ006 -pub(crate) fn call_datetime_fromtimestamp( - checker: &mut Checker, - func: &Expr, - args: &[Expr], - keywords: &[Keyword], - location: TextRange, -) { - if !checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| { - call_path.as_slice() == ["datetime", "datetime", "fromtimestamp"] - }) - { - return; - } - - // no args / no args unqualified - if args.len() < 2 && keywords.is_empty() { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeFromtimestamp, location)); - return; - } - - // none args - if args.len() > 1 && is_const_none(&args[1]) { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeFromtimestamp, location)); - return; - } - - // wrong keywords / none keyword - if !keywords.is_empty() && !has_non_none_keyword(keywords, "tz") { - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeFromtimestamp, location)); - } -} - -/// DTZ007 -pub(crate) fn call_datetime_strptime_without_zone( - checker: &mut Checker, - func: &Expr, - args: &[Expr], - location: TextRange, -) { - if !checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| { - call_path.as_slice() == ["datetime", "datetime", "strptime"] - }) - { - return; - } - - // Does the `strptime` call contain a format string with a timezone specifier? - if let Some(Expr::Constant(ast::ExprConstant { - value: Constant::Str(format), - kind: None, - range: _, - })) = args.get(1).as_ref() - { - if format.contains("%z") { - return; - } - }; - - let (Some(grandparent), Some(parent)) = (checker.semantic_model().expr_grandparent(), checker.semantic_model().expr_parent()) else { - checker.diagnostics.push(Diagnostic::new( - CallDatetimeStrptimeWithoutZone, - location, - )); - return; - }; - - if let Expr::Call(ast::ExprCall { keywords, .. }) = grandparent { - if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = parent { - let attr = attr.as_str(); - // Ex) `datetime.strptime(...).astimezone()` - if attr == "astimezone" { - return; - } - - // Ex) `datetime.strptime(...).replace(tzinfo=UTC)` - if attr == "replace" { - if has_non_none_keyword(keywords, "tzinfo") { - return; - } - } - } - } - - checker - .diagnostics - .push(Diagnostic::new(CallDatetimeStrptimeWithoutZone, location)); -} - -/// Checks for `datetime.date.today()`. (DTZ011) -/// -/// ## Why is this bad? -/// -/// It uses the system local timezone. -/// Use `datetime.datetime.now(tz=).date()` instead. -pub(crate) fn call_date_today(checker: &mut Checker, func: &Expr, location: TextRange) { - if checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| { - call_path.as_slice() == ["datetime", "date", "today"] - }) - { - checker - .diagnostics - .push(Diagnostic::new(CallDateToday, location)); - } -} - -/// Checks for `datetime.date.fromtimestamp()`. (DTZ012) -/// -/// ## Why is this bad? -/// -/// It uses the system local timezone. -/// Use `datetime.datetime.fromtimestamp(, tz=).date()` instead. -pub(crate) fn call_date_fromtimestamp(checker: &mut Checker, func: &Expr, location: TextRange) { - if checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| { - call_path.as_slice() == ["datetime", "date", "fromtimestamp"] - }) - { - checker - .diagnostics - .push(Diagnostic::new(CallDateFromtimestamp, location)); - } -} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/call_date_fromtimestamp.rs b/crates/ruff/src/rules/flake8_datetimez/rules/call_date_fromtimestamp.rs new file mode 100644 index 0000000000..3029801216 --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/call_date_fromtimestamp.rs @@ -0,0 +1,40 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::Expr; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct CallDateFromtimestamp; + +impl Violation for CallDateFromtimestamp { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "The use of `datetime.date.fromtimestamp()` is not allowed, use \ + `datetime.datetime.fromtimestamp(ts, tz=).date()` instead" + ) + } +} + +/// Checks for `datetime.date.fromtimestamp()`. (DTZ012) +/// +/// ## Why is this bad? +/// +/// It uses the system local timezone. +/// Use `datetime.datetime.fromtimestamp(, tz=).date()` instead. +pub(crate) fn call_date_fromtimestamp(checker: &mut Checker, func: &Expr, location: TextRange) { + if checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "date", "fromtimestamp"] + }) + { + checker + .diagnostics + .push(Diagnostic::new(CallDateFromtimestamp, location)); + } +} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/call_date_today.rs b/crates/ruff/src/rules/flake8_datetimez/rules/call_date_today.rs new file mode 100644 index 0000000000..461fa06761 --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/call_date_today.rs @@ -0,0 +1,40 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::Expr; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct CallDateToday; + +impl Violation for CallDateToday { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "The use of `datetime.date.today()` is not allowed, use \ + `datetime.datetime.now(tz=).date()` instead" + ) + } +} + +/// Checks for `datetime.date.today()`. (DTZ011) +/// +/// ## Why is this bad? +/// +/// It uses the system local timezone. +/// Use `datetime.datetime.now(tz=).date()` instead. +pub(crate) fn call_date_today(checker: &mut Checker, func: &Expr, location: TextRange) { + if checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "date", "today"] + }) + { + checker + .diagnostics + .push(Diagnostic::new(CallDateToday, location)); + } +} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_fromtimestamp.rs b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_fromtimestamp.rs new file mode 100644 index 0000000000..4ff08ba885 --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_fromtimestamp.rs @@ -0,0 +1,62 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::{Expr, Keyword}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::{has_non_none_keyword, is_const_none}; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct CallDatetimeFromtimestamp; + +impl Violation for CallDatetimeFromtimestamp { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "The use of `datetime.datetime.fromtimestamp()` without `tz` argument is not allowed" + ) + } +} + +/// DTZ006 +pub(crate) fn call_datetime_fromtimestamp( + checker: &mut Checker, + func: &Expr, + args: &[Expr], + keywords: &[Keyword], + location: TextRange, +) { + if !checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "datetime", "fromtimestamp"] + }) + { + return; + } + + // no args / no args unqualified + if args.len() < 2 && keywords.is_empty() { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeFromtimestamp, location)); + return; + } + + // none args + if args.len() > 1 && is_const_none(&args[1]) { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeFromtimestamp, location)); + return; + } + + // wrong keywords / none keyword + if !keywords.is_empty() && !has_non_none_keyword(keywords, "tz") { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeFromtimestamp, location)); + } +} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_now_without_tzinfo.rs b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_now_without_tzinfo.rs new file mode 100644 index 0000000000..46202fb6a2 --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_now_without_tzinfo.rs @@ -0,0 +1,60 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::{Expr, Keyword}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::{has_non_none_keyword, is_const_none}; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct CallDatetimeNowWithoutTzinfo; + +impl Violation for CallDatetimeNowWithoutTzinfo { + #[derive_message_formats] + fn message(&self) -> String { + format!("The use of `datetime.datetime.now()` without `tz` argument is not allowed") + } +} + +/// DTZ005 +pub(crate) fn call_datetime_now_without_tzinfo( + checker: &mut Checker, + func: &Expr, + args: &[Expr], + keywords: &[Keyword], + location: TextRange, +) { + if !checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "datetime", "now"] + }) + { + return; + } + + // no args / no args unqualified + if args.is_empty() && keywords.is_empty() { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, location)); + return; + } + + // none args + if !args.is_empty() && is_const_none(&args[0]) { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, location)); + return; + } + + // wrong keywords / none keyword + if !keywords.is_empty() && !has_non_none_keyword(keywords, "tz") { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeNowWithoutTzinfo, location)); + } +} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_strptime_without_zone.rs b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_strptime_without_zone.rs new file mode 100644 index 0000000000..2cc4fb0b99 --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_strptime_without_zone.rs @@ -0,0 +1,80 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::{self, Constant, Expr}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::has_non_none_keyword; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct CallDatetimeStrptimeWithoutZone; + +impl Violation for CallDatetimeStrptimeWithoutZone { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "The use of `datetime.datetime.strptime()` without %z must be followed by \ + `.replace(tzinfo=)` or `.astimezone()`" + ) + } +} + +/// DTZ007 +pub(crate) fn call_datetime_strptime_without_zone( + checker: &mut Checker, + func: &Expr, + args: &[Expr], + location: TextRange, +) { + if !checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "datetime", "strptime"] + }) + { + return; + } + + // Does the `strptime` call contain a format string with a timezone specifier? + if let Some(Expr::Constant(ast::ExprConstant { + value: Constant::Str(format), + kind: None, + range: _, + })) = args.get(1).as_ref() + { + if format.contains("%z") { + return; + } + }; + + let (Some(grandparent), Some(parent)) = (checker.semantic_model().expr_grandparent(), checker.semantic_model().expr_parent()) else { + checker.diagnostics.push(Diagnostic::new( + CallDatetimeStrptimeWithoutZone, + location, + )); + return; + }; + + if let Expr::Call(ast::ExprCall { keywords, .. }) = grandparent { + if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = parent { + let attr = attr.as_str(); + // Ex) `datetime.strptime(...).astimezone()` + if attr == "astimezone" { + return; + } + + // Ex) `datetime.strptime(...).replace(tzinfo=UTC)` + if attr == "replace" { + if has_non_none_keyword(keywords, "tzinfo") { + return; + } + } + } + } + + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeStrptimeWithoutZone, location)); +} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_today.rs b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_today.rs new file mode 100644 index 0000000000..5c42c7c176 --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_today.rs @@ -0,0 +1,40 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::Expr; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct CallDatetimeToday; + +impl Violation for CallDatetimeToday { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "The use of `datetime.datetime.today()` is not allowed, use \ + `datetime.datetime.now(tz=)` instead" + ) + } +} + +/// Checks for `datetime.datetime.today()`. (DTZ002) +/// +/// ## Why is this bad? +/// +/// It uses the system local timezone. +/// Use `datetime.datetime.now(tz=)` instead. +pub(crate) fn call_datetime_today(checker: &mut Checker, func: &Expr, location: TextRange) { + if checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "datetime", "today"] + }) + { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeToday, location)); + } +} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_utcfromtimestamp.rs b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_utcfromtimestamp.rs new file mode 100644 index 0000000000..ffa73e5416 --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_utcfromtimestamp.rs @@ -0,0 +1,47 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::Expr; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct CallDatetimeUtcfromtimestamp; + +impl Violation for CallDatetimeUtcfromtimestamp { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "The use of `datetime.datetime.utcfromtimestamp()` is not allowed, use \ + `datetime.datetime.fromtimestamp(ts, tz=)` instead" + ) + } +} + +/// Checks for `datetime.datetime.utcfromtimestamp()`. (DTZ004) +/// +/// ## Why is this bad? +/// +/// Because naive `datetime` objects are treated by many `datetime` methods as +/// local times, it is preferred to use aware datetimes to represent times in +/// UTC. As such, the recommended way to create an object representing a +/// specific timestamp in UTC is by calling `datetime.fromtimestamp(timestamp, +/// tz=timezone.utc)`. +pub(crate) fn call_datetime_utcfromtimestamp( + checker: &mut Checker, + func: &Expr, + location: TextRange, +) { + if checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "datetime", "utcfromtimestamp"] + }) + { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeUtcfromtimestamp, location)); + } +} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_utcnow.rs b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_utcnow.rs new file mode 100644 index 0000000000..33d89f8673 --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_utcnow.rs @@ -0,0 +1,42 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::Expr; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct CallDatetimeUtcnow; + +impl Violation for CallDatetimeUtcnow { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "The use of `datetime.datetime.utcnow()` is not allowed, use \ + `datetime.datetime.now(tz=)` instead" + ) + } +} + +/// Checks for `datetime.datetime.today()`. (DTZ003) +/// +/// ## Why is this bad? +/// +/// Because naive `datetime` objects are treated by many `datetime` methods as +/// local times, it is preferred to use aware datetimes to represent times in +/// UTC. As such, the recommended way to create an object representing the +/// current time in UTC is by calling `datetime.now(timezone.utc)`. +pub(crate) fn call_datetime_utcnow(checker: &mut Checker, func: &Expr, location: TextRange) { + if checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "datetime", "utcnow"] + }) + { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeUtcnow, location)); + } +} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_without_tzinfo.rs b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_without_tzinfo.rs new file mode 100644 index 0000000000..af7eabf837 --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/call_datetime_without_tzinfo.rs @@ -0,0 +1,51 @@ +use ruff_text_size::TextRange; +use rustpython_parser::ast::{Expr, Keyword}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::{has_non_none_keyword, is_const_none}; + +use crate::checkers::ast::Checker; + +#[violation] +pub struct CallDatetimeWithoutTzinfo; + +impl Violation for CallDatetimeWithoutTzinfo { + #[derive_message_formats] + fn message(&self) -> String { + format!("The use of `datetime.datetime()` without `tzinfo` argument is not allowed") + } +} + +pub(crate) fn call_datetime_without_tzinfo( + checker: &mut Checker, + func: &Expr, + args: &[Expr], + keywords: &[Keyword], + location: TextRange, +) { + if !checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| { + call_path.as_slice() == ["datetime", "datetime"] + }) + { + return; + } + + // No positional arg: keyword is missing or constant None. + if args.len() < 8 && !has_non_none_keyword(keywords, "tzinfo") { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeWithoutTzinfo, location)); + return; + } + + // Positional arg: is constant None. + if args.len() >= 8 && is_const_none(&args[7]) { + checker + .diagnostics + .push(Diagnostic::new(CallDatetimeWithoutTzinfo, location)); + } +} diff --git a/crates/ruff/src/rules/flake8_datetimez/rules/mod.rs b/crates/ruff/src/rules/flake8_datetimez/rules/mod.rs new file mode 100644 index 0000000000..8cfa09cdcc --- /dev/null +++ b/crates/ruff/src/rules/flake8_datetimez/rules/mod.rs @@ -0,0 +1,29 @@ +pub(crate) use call_date_fromtimestamp::{call_date_fromtimestamp, CallDateFromtimestamp}; +pub(crate) use call_date_today::{call_date_today, CallDateToday}; +pub(crate) use call_datetime_fromtimestamp::{ + call_datetime_fromtimestamp, CallDatetimeFromtimestamp, +}; +pub(crate) use call_datetime_now_without_tzinfo::{ + call_datetime_now_without_tzinfo, CallDatetimeNowWithoutTzinfo, +}; +pub(crate) use call_datetime_strptime_without_zone::{ + call_datetime_strptime_without_zone, CallDatetimeStrptimeWithoutZone, +}; +pub(crate) use call_datetime_today::{call_datetime_today, CallDatetimeToday}; +pub(crate) use call_datetime_utcfromtimestamp::{ + call_datetime_utcfromtimestamp, CallDatetimeUtcfromtimestamp, +}; +pub(crate) use call_datetime_utcnow::{call_datetime_utcnow, CallDatetimeUtcnow}; +pub(crate) use call_datetime_without_tzinfo::{ + call_datetime_without_tzinfo, CallDatetimeWithoutTzinfo, +}; + +mod call_date_fromtimestamp; +mod call_date_today; +mod call_datetime_fromtimestamp; +mod call_datetime_now_without_tzinfo; +mod call_datetime_strptime_without_zone; +mod call_datetime_today; +mod call_datetime_utcfromtimestamp; +mod call_datetime_utcnow; +mod call_datetime_without_tzinfo; diff --git a/crates/ruff/src/rules/flake8_debugger/rules.rs b/crates/ruff/src/rules/flake8_debugger/rules/debugger.rs similarity index 100% rename from crates/ruff/src/rules/flake8_debugger/rules.rs rename to crates/ruff/src/rules/flake8_debugger/rules/debugger.rs diff --git a/crates/ruff/src/rules/flake8_debugger/rules/mod.rs b/crates/ruff/src/rules/flake8_debugger/rules/mod.rs new file mode 100644 index 0000000000..1dda738e6a --- /dev/null +++ b/crates/ruff/src/rules/flake8_debugger/rules/mod.rs @@ -0,0 +1,3 @@ +pub(crate) use debugger::{debugger_call, debugger_import, Debugger}; + +mod debugger; diff --git a/crates/ruff/src/rules/flake8_errmsg/rules/mod.rs b/crates/ruff/src/rules/flake8_errmsg/rules/mod.rs new file mode 100644 index 0000000000..8740e30113 --- /dev/null +++ b/crates/ruff/src/rules/flake8_errmsg/rules/mod.rs @@ -0,0 +1,5 @@ +pub(crate) use string_in_exception::{ + string_in_exception, DotFormatInException, FStringInException, RawStringInException, +}; + +mod string_in_exception; diff --git a/crates/ruff/src/rules/flake8_errmsg/rules.rs b/crates/ruff/src/rules/flake8_errmsg/rules/string_in_exception.rs similarity index 100% rename from crates/ruff/src/rules/flake8_errmsg/rules.rs rename to crates/ruff/src/rules/flake8_errmsg/rules/string_in_exception.rs diff --git a/crates/ruff/src/rules/flake8_future_annotations/rules.rs b/crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations.rs similarity index 100% rename from crates/ruff/src/rules/flake8_future_annotations/rules.rs rename to crates/ruff/src/rules/flake8_future_annotations/rules/missing_future_annotations.rs diff --git a/crates/ruff/src/rules/flake8_future_annotations/rules/mod.rs b/crates/ruff/src/rules/flake8_future_annotations/rules/mod.rs new file mode 100644 index 0000000000..5a845f4b8a --- /dev/null +++ b/crates/ruff/src/rules/flake8_future_annotations/rules/mod.rs @@ -0,0 +1,5 @@ +pub(crate) use missing_future_annotations::{ + missing_future_annotations, MissingFutureAnnotationsImport, +}; + +mod missing_future_annotations; diff --git a/crates/ruff/src/rules/flake8_gettext/rules.rs b/crates/ruff/src/rules/flake8_gettext/rules.rs deleted file mode 100644 index a9df874c77..0000000000 --- a/crates/ruff/src/rules/flake8_gettext/rules.rs +++ /dev/null @@ -1,87 +0,0 @@ -use rustpython_parser::ast::{self, Constant, Expr, Operator, Ranged}; - -use ruff_diagnostics::{Diagnostic, Violation}; -use ruff_macros::{derive_message_formats, violation}; - -#[violation] -pub struct FStringInGetTextFuncCall; - -impl Violation for FStringInGetTextFuncCall { - #[derive_message_formats] - fn message(&self) -> String { - format!("f-string is resolved before function call; consider `_(\"string %s\") % arg`") - } -} - -#[violation] -pub struct FormatInGetTextFuncCall; - -impl Violation for FormatInGetTextFuncCall { - #[derive_message_formats] - fn message(&self) -> String { - format!("`format` method argument is resolved before function call; consider `_(\"string %s\") % arg`") - } -} -#[violation] -pub struct PrintfInGetTextFuncCall; - -impl Violation for PrintfInGetTextFuncCall { - #[derive_message_formats] - fn message(&self) -> String { - format!("printf-style format is resolved before function call; consider `_(\"string %s\") % arg`") - } -} - -/// Returns true if the [`Expr`] is an internationalization function call. -pub(crate) fn is_gettext_func_call(func: &Expr, functions_names: &[String]) -> bool { - if let Expr::Name(ast::ExprName { id, .. }) = func { - functions_names.contains(id.as_ref()) - } else { - false - } -} - -/// INT001 -pub(crate) fn f_string_in_gettext_func_call(args: &[Expr]) -> Option { - if let Some(first) = args.first() { - if first.is_joined_str_expr() { - return Some(Diagnostic::new(FStringInGetTextFuncCall {}, first.range())); - } - } - None -} - -/// INT002 -pub(crate) fn format_in_gettext_func_call(args: &[Expr]) -> Option { - if let Some(first) = args.first() { - if let Expr::Call(ast::ExprCall { func, .. }) = &first { - if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func.as_ref() { - if attr == "format" { - return Some(Diagnostic::new(FormatInGetTextFuncCall {}, first.range())); - } - } - } - } - None -} - -/// INT003 -pub(crate) fn printf_in_gettext_func_call(args: &[Expr]) -> Option { - if let Some(first) = args.first() { - if let Expr::BinOp(ast::ExprBinOp { - op: Operator::Mod { .. }, - left, - .. - }) = &first - { - if let Expr::Constant(ast::ExprConstant { - value: Constant::Str(_), - .. - }) = left.as_ref() - { - return Some(Diagnostic::new(PrintfInGetTextFuncCall {}, first.range())); - } - } - } - None -} diff --git a/crates/ruff/src/rules/flake8_gettext/rules/f_string_in_gettext_func_call.rs b/crates/ruff/src/rules/flake8_gettext/rules/f_string_in_gettext_func_call.rs new file mode 100644 index 0000000000..0f6b8c6a60 --- /dev/null +++ b/crates/ruff/src/rules/flake8_gettext/rules/f_string_in_gettext_func_call.rs @@ -0,0 +1,24 @@ +use rustpython_parser::ast::{Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +#[violation] +pub struct FStringInGetTextFuncCall; + +impl Violation for FStringInGetTextFuncCall { + #[derive_message_formats] + fn message(&self) -> String { + format!("f-string is resolved before function call; consider `_(\"string %s\") % arg`") + } +} + +/// INT001 +pub(crate) fn f_string_in_gettext_func_call(args: &[Expr]) -> Option { + if let Some(first) = args.first() { + if first.is_joined_str_expr() { + return Some(Diagnostic::new(FStringInGetTextFuncCall {}, first.range())); + } + } + None +} diff --git a/crates/ruff/src/rules/flake8_gettext/rules/format_in_gettext_func_call.rs b/crates/ruff/src/rules/flake8_gettext/rules/format_in_gettext_func_call.rs new file mode 100644 index 0000000000..ec159d0337 --- /dev/null +++ b/crates/ruff/src/rules/flake8_gettext/rules/format_in_gettext_func_call.rs @@ -0,0 +1,28 @@ +use rustpython_parser::ast::{self, Expr, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +#[violation] +pub struct FormatInGetTextFuncCall; + +impl Violation for FormatInGetTextFuncCall { + #[derive_message_formats] + fn message(&self) -> String { + format!("`format` method argument is resolved before function call; consider `_(\"string %s\") % arg`") + } +} + +/// INT002 +pub(crate) fn format_in_gettext_func_call(args: &[Expr]) -> Option { + if let Some(first) = args.first() { + if let Expr::Call(ast::ExprCall { func, .. }) = &first { + if let Expr::Attribute(ast::ExprAttribute { attr, .. }) = func.as_ref() { + if attr == "format" { + return Some(Diagnostic::new(FormatInGetTextFuncCall {}, first.range())); + } + } + } + } + None +} diff --git a/crates/ruff/src/rules/flake8_gettext/rules/is_gettext_func_call.rs b/crates/ruff/src/rules/flake8_gettext/rules/is_gettext_func_call.rs new file mode 100644 index 0000000000..0e539ead9a --- /dev/null +++ b/crates/ruff/src/rules/flake8_gettext/rules/is_gettext_func_call.rs @@ -0,0 +1,10 @@ +use rustpython_parser::ast::{self, Expr}; + +/// Returns true if the [`Expr`] is an internationalization function call. +pub(crate) fn is_gettext_func_call(func: &Expr, functions_names: &[String]) -> bool { + if let Expr::Name(ast::ExprName { id, .. }) = func { + functions_names.contains(id.as_ref()) + } else { + false + } +} diff --git a/crates/ruff/src/rules/flake8_gettext/rules/mod.rs b/crates/ruff/src/rules/flake8_gettext/rules/mod.rs new file mode 100644 index 0000000000..1a80938fd1 --- /dev/null +++ b/crates/ruff/src/rules/flake8_gettext/rules/mod.rs @@ -0,0 +1,15 @@ +pub(crate) use f_string_in_gettext_func_call::{ + f_string_in_gettext_func_call, FStringInGetTextFuncCall, +}; +pub(crate) use format_in_gettext_func_call::{ + format_in_gettext_func_call, FormatInGetTextFuncCall, +}; +pub(crate) use is_gettext_func_call::is_gettext_func_call; +pub(crate) use printf_in_gettext_func_call::{ + printf_in_gettext_func_call, PrintfInGetTextFuncCall, +}; + +mod f_string_in_gettext_func_call; +mod format_in_gettext_func_call; +mod is_gettext_func_call; +mod printf_in_gettext_func_call; diff --git a/crates/ruff/src/rules/flake8_gettext/rules/printf_in_gettext_func_call.rs b/crates/ruff/src/rules/flake8_gettext/rules/printf_in_gettext_func_call.rs new file mode 100644 index 0000000000..088eaa60f8 --- /dev/null +++ b/crates/ruff/src/rules/flake8_gettext/rules/printf_in_gettext_func_call.rs @@ -0,0 +1,35 @@ +use rustpython_parser::ast::{self, Constant, Expr, Operator, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +#[violation] +pub struct PrintfInGetTextFuncCall; + +impl Violation for PrintfInGetTextFuncCall { + #[derive_message_formats] + fn message(&self) -> String { + format!("printf-style format is resolved before function call; consider `_(\"string %s\") % arg`") + } +} + +/// INT003 +pub(crate) fn printf_in_gettext_func_call(args: &[Expr]) -> Option { + if let Some(first) = args.first() { + if let Expr::BinOp(ast::ExprBinOp { + op: Operator::Mod { .. }, + left, + .. + }) = &first + { + if let Expr::Constant(ast::ExprConstant { + value: Constant::Str(_), + .. + }) = left.as_ref() + { + return Some(Diagnostic::new(PrintfInGetTextFuncCall {}, first.range())); + } + } + } + None +} diff --git a/crates/ruff/src/rules/flake8_implicit_str_concat/rules/explicit.rs b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/explicit.rs new file mode 100644 index 0000000000..e6b706519e --- /dev/null +++ b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/explicit.rs @@ -0,0 +1,70 @@ +use rustpython_parser::ast::{self, Constant, Expr, Operator, Ranged}; + +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; + +/// ## What it does +/// Checks for string literals that are explicitly concatenated (using the +/// `+` operator). +/// +/// ## Why is this bad? +/// For string literals that wrap across multiple lines, implicit string +/// concatenation within parentheses is preferred over explicit +/// concatenation using the `+` operator, as the former is more readable. +/// +/// ## Example +/// ```python +/// z = ( +/// "The quick brown fox jumps over the lazy " +/// + "dog" +/// ) +/// ``` +/// +/// Use instead: +/// ```python +/// z = ( +/// "The quick brown fox jumps over the lazy " +/// "dog" +/// ) +/// ``` +#[violation] +pub struct ExplicitStringConcatenation; + +impl Violation for ExplicitStringConcatenation { + #[derive_message_formats] + fn message(&self) -> String { + format!("Explicitly concatenated string should be implicitly concatenated") + } +} + +/// ISC003 +pub(crate) fn explicit(expr: &Expr) -> Option { + if let Expr::BinOp(ast::ExprBinOp { + left, + op, + right, + range: _, + }) = expr + { + if matches!(op, Operator::Add) { + if matches!( + left.as_ref(), + Expr::JoinedStr(_) + | Expr::Constant(ast::ExprConstant { + value: Constant::Str(..) | Constant::Bytes(..), + .. + }) + ) && matches!( + right.as_ref(), + Expr::JoinedStr(_) + | Expr::Constant(ast::ExprConstant { + value: Constant::Str(..) | Constant::Bytes(..), + .. + }) + ) { + return Some(Diagnostic::new(ExplicitStringConcatenation, expr.range())); + } + } + } + None +} diff --git a/crates/ruff/src/rules/flake8_implicit_str_concat/rules.rs b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/implicit.rs similarity index 66% rename from crates/ruff/src/rules/flake8_implicit_str_concat/rules.rs rename to crates/ruff/src/rules/flake8_implicit_str_concat/rules/implicit.rs index 49350098c0..43dac7b3e5 100644 --- a/crates/ruff/src/rules/flake8_implicit_str_concat/rules.rs +++ b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/implicit.rs @@ -1,6 +1,5 @@ use itertools::Itertools; use ruff_text_size::TextRange; -use rustpython_parser::ast::{self, Constant, Expr, Operator, Ranged}; use rustpython_parser::lexer::LexResult; use rustpython_parser::Tok; @@ -84,40 +83,6 @@ impl Violation for MultiLineImplicitStringConcatenation { } } -/// ## What it does -/// Checks for string literals that are explicitly concatenated (using the -/// `+` operator). -/// -/// ## Why is this bad? -/// For string literals that wrap across multiple lines, implicit string -/// concatenation within parentheses is preferred over explicit -/// concatenation using the `+` operator, as the former is more readable. -/// -/// ## Example -/// ```python -/// z = ( -/// "The quick brown fox jumps over the lazy " -/// + "dog" -/// ) -/// ``` -/// -/// Use instead: -/// ```python -/// z = ( -/// "The quick brown fox jumps over the lazy " -/// "dog" -/// ) -/// ``` -#[violation] -pub struct ExplicitStringConcatenation; - -impl Violation for ExplicitStringConcatenation { - #[derive_message_formats] - fn message(&self) -> String { - format!("Explicitly concatenated string should be implicitly concatenated") - } -} - /// ISC001, ISC002 pub(crate) fn implicit( tokens: &[LexResult], @@ -150,35 +115,3 @@ pub(crate) fn implicit( } diagnostics } - -/// ISC003 -pub(crate) fn explicit(expr: &Expr) -> Option { - if let Expr::BinOp(ast::ExprBinOp { - left, - op, - right, - range: _, - }) = expr - { - if matches!(op, Operator::Add) { - if matches!( - left.as_ref(), - Expr::JoinedStr(_) - | Expr::Constant(ast::ExprConstant { - value: Constant::Str(..) | Constant::Bytes(..), - .. - }) - ) && matches!( - right.as_ref(), - Expr::JoinedStr(_) - | Expr::Constant(ast::ExprConstant { - value: Constant::Str(..) | Constant::Bytes(..), - .. - }) - ) { - return Some(Diagnostic::new(ExplicitStringConcatenation, expr.range())); - } - } - } - None -} diff --git a/crates/ruff/src/rules/flake8_implicit_str_concat/rules/mod.rs b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/mod.rs new file mode 100644 index 0000000000..605341dc27 --- /dev/null +++ b/crates/ruff/src/rules/flake8_implicit_str_concat/rules/mod.rs @@ -0,0 +1,7 @@ +pub(crate) use explicit::{explicit, ExplicitStringConcatenation}; +pub(crate) use implicit::{ + implicit, MultiLineImplicitStringConcatenation, SingleLineImplicitStringConcatenation, +}; + +mod explicit; +mod implicit; diff --git a/crates/ruff/src/rules/flake8_logging_format/rules.rs b/crates/ruff/src/rules/flake8_logging_format/rules/logging_call.rs similarity index 100% rename from crates/ruff/src/rules/flake8_logging_format/rules.rs rename to crates/ruff/src/rules/flake8_logging_format/rules/logging_call.rs diff --git a/crates/ruff/src/rules/flake8_logging_format/rules/mod.rs b/crates/ruff/src/rules/flake8_logging_format/rules/mod.rs new file mode 100644 index 0000000000..c0cce2dd57 --- /dev/null +++ b/crates/ruff/src/rules/flake8_logging_format/rules/mod.rs @@ -0,0 +1,3 @@ +pub(crate) use logging_call::logging_call; + +mod logging_call; diff --git a/crates/ruff/src/rules/flake8_no_pep420/rules.rs b/crates/ruff/src/rules/flake8_no_pep420/rules/implicit_namespace_package.rs similarity index 100% rename from crates/ruff/src/rules/flake8_no_pep420/rules.rs rename to crates/ruff/src/rules/flake8_no_pep420/rules/implicit_namespace_package.rs diff --git a/crates/ruff/src/rules/flake8_no_pep420/rules/mod.rs b/crates/ruff/src/rules/flake8_no_pep420/rules/mod.rs new file mode 100644 index 0000000000..6b3762cae3 --- /dev/null +++ b/crates/ruff/src/rules/flake8_no_pep420/rules/mod.rs @@ -0,0 +1,3 @@ +pub(crate) use implicit_namespace_package::{implicit_namespace_package, ImplicitNamespacePackage}; + +mod implicit_namespace_package; diff --git a/crates/ruff/src/rules/flake8_pie/rules.rs b/crates/ruff/src/rules/flake8_pie/rules.rs deleted file mode 100644 index 282bfa6d17..0000000000 --- a/crates/ruff/src/rules/flake8_pie/rules.rs +++ /dev/null @@ -1,652 +0,0 @@ -use std::collections::BTreeMap; -use std::iter; - -use itertools::Either::{Left, Right}; -use log::error; -use ruff_text_size::TextRange; -use rustc_hash::FxHashSet; -use rustpython_parser::ast::{ - self, Boolop, Constant, Expr, ExprContext, ExprLambda, Keyword, Ranged, Stmt, -}; - -use ruff_diagnostics::{AlwaysAutofixableViolation, Violation}; -use ruff_diagnostics::{Diagnostic, Edit, Fix}; -use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::comparable::ComparableExpr; -use ruff_python_ast::helpers::trailing_comment_start_offset; -use ruff_python_ast::types::RefEquality; -use ruff_python_stdlib::identifiers::is_identifier; - -use crate::autofix::actions::delete_stmt; -use crate::checkers::ast::Checker; -use crate::registry::AsRule; - -/// ## What it does -/// Checks for unnecessary `pass` statements in class and function bodies. -/// where it is not needed syntactically (e.g., when an indented docstring is -/// present). -/// -/// ## Why is this bad? -/// When a function or class definition contains a docstring, an additional -/// `pass` statement is redundant. -/// -/// ## Example -/// ```python -/// def foo(): -/// """Placeholder docstring.""" -/// pass -/// ``` -/// -/// Use instead: -/// ```python -/// def foo(): -/// """Placeholder docstring.""" -/// ``` -/// -/// ## References -/// - [Python documentation](https://docs.python.org/3/reference/simple_stmts.html#the-pass-statement) -#[violation] -pub struct UnnecessaryPass; - -impl AlwaysAutofixableViolation for UnnecessaryPass { - #[derive_message_formats] - fn message(&self) -> String { - format!("Unnecessary `pass` statement") - } - - fn autofix_title(&self) -> String { - "Remove unnecessary `pass`".to_string() - } -} - -/// ## What it does -/// Checks for duplicate field definitions in classes. -/// -/// ## Why is this bad? -/// Defining a field multiple times in a class body is redundant and likely a -/// mistake. -/// -/// ## Example -/// ```python -/// class Person: -/// name = Tom -/// ... -/// name = Ben -/// ``` -/// -/// Use instead: -/// ```python -/// class Person: -/// name = Tom -/// ... -/// ``` -#[violation] -pub struct DuplicateClassFieldDefinition(pub String); - -impl AlwaysAutofixableViolation for DuplicateClassFieldDefinition { - #[derive_message_formats] - fn message(&self) -> String { - let DuplicateClassFieldDefinition(name) = self; - format!("Class field `{name}` is defined multiple times") - } - - fn autofix_title(&self) -> String { - let DuplicateClassFieldDefinition(name) = self; - format!("Remove duplicate field definition for `{name}`") - } -} - -/// ## What it does -/// Checks for enums that contain duplicate values. -/// -/// ## Why is this bad? -/// Enum values should be unique. Non-unique values are redundant and likely a -/// mistake. -/// -/// ## Example -/// ```python -/// from enum import Enum -/// -/// -/// class Foo(Enum): -/// A = 1 -/// B = 2 -/// C = 1 -/// ``` -/// -/// Use instead: -/// ```python -/// from enum import Enum -/// -/// -/// class Foo(Enum): -/// A = 1 -/// B = 2 -/// C = 3 -/// ``` -/// -/// ## References -/// - [Python documentation](https://docs.python.org/3/library/enum.html#enum.Enum) -#[violation] -pub struct NonUniqueEnums { - value: String, -} - -impl Violation for NonUniqueEnums { - #[derive_message_formats] - fn message(&self) -> String { - let NonUniqueEnums { value } = self; - format!("Enum contains duplicate value: `{value}`") - } -} - -/// ## What it does -/// Checks for unnecessary dictionary unpacking operators (`**`). -/// -/// ## Why is this bad? -/// Unpacking a dictionary into another dictionary is redundant. The unpacking -/// operator can be removed, making the code more readable. -/// -/// ## Example -/// ```python -/// foo = {"A": 1, "B": 2} -/// bar = {**foo, **{"C": 3}} -/// ``` -/// -/// Use instead: -/// ```python -/// foo = {"A": 1, "B": 2} -/// bar = {**foo, "C": 3} -/// ``` -/// -/// ## References -/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#dictionary-displays) -#[violation] -pub struct UnnecessarySpread; - -impl Violation for UnnecessarySpread { - #[derive_message_formats] - fn message(&self) -> String { - format!("Unnecessary spread `**`") - } -} - -/// ## What it does -/// Checks for `startswith` or `endswith` calls on the same value with -/// different prefixes or suffixes. -/// -/// ## Why is this bad? -/// The `startswith` and `endswith` methods accept tuples of prefixes or -/// suffixes respectively. Passing a tuple of prefixes or suffixes is more -/// more efficient and readable than calling the method multiple times. -/// -/// ## Example -/// ```python -/// msg = "Hello, world!" -/// if msg.startswith("Hello") or msg.startswith("Hi"): -/// print("Greetings!") -/// ``` -/// -/// Use instead: -/// ```python -/// msg = "Hello, world!" -/// if msg.startswith(("Hello", "Hi")): -/// print("Greetings!") -/// ``` -/// -/// ## References -/// - [Python documentation](https://docs.python.org/3/library/stdtypes.html#str.startswith) -/// - [Python documentation](https://docs.python.org/3/library/stdtypes.html#str.endswith) -#[violation] -pub struct MultipleStartsEndsWith { - attr: String, -} - -impl AlwaysAutofixableViolation for MultipleStartsEndsWith { - #[derive_message_formats] - fn message(&self) -> String { - let MultipleStartsEndsWith { attr } = self; - format!("Call `{attr}` once with a `tuple`") - } - - fn autofix_title(&self) -> String { - let MultipleStartsEndsWith { attr } = self; - format!("Merge into a single `{attr}` call") - } -} - -/// ## What it does -/// Checks for unnecessary `dict` kwargs. -/// -/// ## Why is this bad? -/// If the `dict` keys are valid identifiers, they can be passed as keyword -/// arguments directly. -/// -/// ## Example -/// ```python -/// def foo(bar): -/// return bar + 1 -/// -/// -/// print(foo(**{"bar": 2})) # prints 3 -/// ``` -/// -/// Use instead: -/// ```python -/// def foo(bar): -/// return bar + 1 -/// -/// -/// print(foo(bar=2)) # prints 3 -/// ``` -/// -/// ## References -/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#dictionary-displays) -/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#calls) -#[violation] -pub struct UnnecessaryDictKwargs; - -impl Violation for UnnecessaryDictKwargs { - #[derive_message_formats] - fn message(&self) -> String { - format!("Unnecessary `dict` kwargs") - } -} - -/// ## What it does -/// Checks for lambdas that can be replaced with the `list` builtin. -/// -/// ## Why is this bad? -/// Using `list` builtin is more readable. -/// -/// ## Example -/// ```python -/// from dataclasses import dataclass, field -/// -/// -/// @dataclass -/// class Foo: -/// bar: list[int] = field(default_factory=lambda: []) -/// ``` -/// -/// Use instead: -/// ```python -/// from dataclasses import dataclass, field -/// -/// -/// @dataclass -/// class Foo: -/// bar: list[int] = field(default_factory=list) -/// ``` -/// -/// ## References -/// - [Python documentation](https://docs.python.org/3/library/functions.html#func-list) -#[violation] -pub struct ReimplementedListBuiltin; - -impl AlwaysAutofixableViolation for ReimplementedListBuiltin { - #[derive_message_formats] - fn message(&self) -> String { - format!("Prefer `list` over useless lambda") - } - - fn autofix_title(&self) -> String { - "Replace with `list`".to_string() - } -} - -/// PIE790 -pub(crate) fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) { - if body.len() > 1 { - // This only catches the case in which a docstring makes a `pass` statement - // redundant. Consider removing all `pass` statements instead. - let docstring_stmt = &body[0]; - let pass_stmt = &body[1]; - let Stmt::Expr(ast::StmtExpr { value, range: _ } )= docstring_stmt else { - return; - }; - if matches!( - value.as_ref(), - Expr::Constant(ast::ExprConstant { - value: Constant::Str(..), - .. - }) - ) { - if pass_stmt.is_pass_stmt() { - let mut diagnostic = Diagnostic::new(UnnecessaryPass, pass_stmt.range()); - if checker.patch(diagnostic.kind.rule()) { - if let Some(index) = trailing_comment_start_offset(pass_stmt, checker.locator) { - #[allow(deprecated)] - diagnostic.set_fix(Fix::unspecified(Edit::range_deletion( - pass_stmt.range().add_end(index), - ))); - } else { - #[allow(deprecated)] - diagnostic.try_set_fix_from_edit(|| { - delete_stmt( - pass_stmt, - None, - &[], - checker.locator, - checker.indexer, - checker.stylist, - ) - }); - } - } - checker.diagnostics.push(diagnostic); - } - } - } -} - -/// PIE794 -pub(crate) fn duplicate_class_field_definition<'a, 'b>( - checker: &mut Checker<'a>, - parent: &'b Stmt, - body: &'b [Stmt], -) where - 'b: 'a, -{ - let mut seen_targets: FxHashSet<&str> = FxHashSet::default(); - for stmt in body { - // Extract the property name from the assignment statement. - let target = match stmt { - Stmt::Assign(ast::StmtAssign { targets, .. }) => { - if targets.len() != 1 { - continue; - } - if let Expr::Name(ast::ExprName { id, .. }) = &targets[0] { - id - } else { - continue; - } - } - Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => { - if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() { - id - } else { - continue; - } - } - _ => continue, - }; - - if !seen_targets.insert(target) { - let mut diagnostic = Diagnostic::new( - DuplicateClassFieldDefinition(target.to_string()), - stmt.range(), - ); - if checker.patch(diagnostic.kind.rule()) { - let deleted: Vec<&Stmt> = checker.deletions.iter().map(Into::into).collect(); - let locator = checker.locator; - match delete_stmt( - stmt, - Some(parent), - &deleted, - locator, - checker.indexer, - checker.stylist, - ) { - Ok(fix) => { - checker.deletions.insert(RefEquality(stmt)); - #[allow(deprecated)] - diagnostic.set_fix_from_edit(fix); - } - Err(err) => { - error!("Failed to remove duplicate class definition: {}", err); - } - } - } - checker.diagnostics.push(diagnostic); - } - } -} - -/// PIE796 -pub(crate) fn non_unique_enums<'a, 'b>( - checker: &mut Checker<'a>, - parent: &'b Stmt, - body: &'b [Stmt], -) where - 'b: 'a, -{ - let Stmt::ClassDef(ast::StmtClassDef { bases, .. }) = parent else { - return; - }; - - if !bases.iter().any(|expr| { - checker - .semantic_model() - .resolve_call_path(expr) - .map_or(false, |call_path| call_path.as_slice() == ["enum", "Enum"]) - }) { - return; - } - - let mut seen_targets: FxHashSet = FxHashSet::default(); - for stmt in body { - let Stmt::Assign(ast::StmtAssign { value, .. }) = stmt else { - continue; - }; - - if let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() { - if checker - .semantic_model() - .resolve_call_path(func) - .map_or(false, |call_path| call_path.as_slice() == ["enum", "auto"]) - { - continue; - } - } - - if !seen_targets.insert(ComparableExpr::from(value)) { - let diagnostic = Diagnostic::new( - NonUniqueEnums { - value: checker.generator().expr(value), - }, - stmt.range(), - ); - checker.diagnostics.push(diagnostic); - } - } -} - -/// PIE800 -pub(crate) fn unnecessary_spread(checker: &mut Checker, keys: &[Option], values: &[Expr]) { - for item in keys.iter().zip(values.iter()) { - if let (None, value) = item { - // We only care about when the key is None which indicates a spread `**` - // inside a dict. - if let Expr::Dict(_) = value { - let diagnostic = Diagnostic::new(UnnecessarySpread, value.range()); - checker.diagnostics.push(diagnostic); - } - } - } -} - -/// Return `true` if a key is a valid keyword argument name. -fn is_valid_kwarg_name(key: &Expr) -> bool { - if let Expr::Constant(ast::ExprConstant { - value: Constant::Str(value), - .. - }) = key - { - is_identifier(value) - } else { - false - } -} - -/// PIE804 -pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, expr: &Expr, kwargs: &[Keyword]) { - for kw in kwargs { - // keyword is a spread operator (indicated by None) - if kw.arg.is_none() { - if let Expr::Dict(ast::ExprDict { keys, .. }) = &kw.value { - // ensure foo(**{"bar-bar": 1}) doesn't error - if keys.iter().all(|expr| expr.as_ref().map_or(false, is_valid_kwarg_name)) || - // handle case of foo(**{**bar}) - (keys.len() == 1 && keys[0].is_none()) - { - let diagnostic = Diagnostic::new(UnnecessaryDictKwargs, expr.range()); - checker.diagnostics.push(diagnostic); - } - } - } - } -} - -/// PIE810 -pub(crate) fn multiple_starts_ends_with(checker: &mut Checker, expr: &Expr) { - let Expr::BoolOp(ast::ExprBoolOp { op: Boolop::Or, values, range: _ }) = expr else { - return; - }; - - let mut duplicates = BTreeMap::new(); - for (index, call) in values.iter().enumerate() { - let Expr::Call(ast::ExprCall { - func, - args, - keywords, - range: _ - }) = &call else { - continue - }; - - if !(args.len() == 1 && keywords.is_empty()) { - continue; - } - - let Expr::Attribute(ast::ExprAttribute { value, attr, .. } )= func.as_ref() else { - continue - }; - if attr != "startswith" && attr != "endswith" { - continue; - } - - let Expr::Name(ast::ExprName { id: arg_name, .. } )= value.as_ref() else { - continue - }; - - duplicates - .entry((attr.as_str(), arg_name.as_str())) - .or_insert_with(Vec::new) - .push(index); - } - - // Generate a `Diagnostic` for each duplicate. - for ((attr_name, arg_name), indices) in duplicates { - if indices.len() > 1 { - let mut diagnostic = Diagnostic::new( - MultipleStartsEndsWith { - attr: attr_name.to_string(), - }, - expr.range(), - ); - if checker.patch(diagnostic.kind.rule()) { - let words: Vec<&Expr> = indices - .iter() - .map(|index| &values[*index]) - .map(|expr| { - let Expr::Call(ast::ExprCall { func: _, args, keywords: _, range: _}) = expr else { - unreachable!("{}", format!("Indices should only contain `{attr_name}` calls")) - }; - args.get(0) - .unwrap_or_else(|| panic!("`{attr_name}` should have one argument")) - }) - .collect(); - - let node = Expr::Tuple(ast::ExprTuple { - elts: words - .iter() - .flat_map(|value| { - if let Expr::Tuple(ast::ExprTuple { elts, .. }) = value { - Left(elts.iter()) - } else { - Right(iter::once(*value)) - } - }) - .map(Clone::clone) - .collect(), - ctx: ExprContext::Load, - range: TextRange::default(), - }); - let node1 = Expr::Name(ast::ExprName { - id: arg_name.into(), - ctx: ExprContext::Load, - range: TextRange::default(), - }); - let node2 = Expr::Attribute(ast::ExprAttribute { - value: Box::new(node1), - attr: attr_name.into(), - ctx: ExprContext::Load, - range: TextRange::default(), - }); - let node3 = Expr::Call(ast::ExprCall { - func: Box::new(node2), - args: vec![node], - keywords: vec![], - range: TextRange::default(), - }); - let call = node3; - - // Generate the combined `BoolOp`. - let mut call = Some(call); - let node = Expr::BoolOp(ast::ExprBoolOp { - op: Boolop::Or, - values: values - .iter() - .enumerate() - .filter_map(|(index, elt)| { - if indices.contains(&index) { - std::mem::take(&mut call) - } else { - Some(elt.clone()) - } - }) - .collect(), - range: TextRange::default(), - }); - let bool_op = node; - #[allow(deprecated)] - diagnostic.set_fix(Fix::unspecified(Edit::range_replacement( - checker.generator().expr(&bool_op), - expr.range(), - ))); - } - checker.diagnostics.push(diagnostic); - } - } -} - -/// PIE807 -pub(crate) fn reimplemented_list_builtin(checker: &mut Checker, expr: &ExprLambda) { - let ExprLambda { - args, - body, - range: _, - } = expr; - - if args.args.is_empty() - && args.kwonlyargs.is_empty() - && args.posonlyargs.is_empty() - && args.vararg.is_none() - && args.kwarg.is_none() - { - if let Expr::List(ast::ExprList { elts, .. }) = body.as_ref() { - if elts.is_empty() { - let mut diagnostic = Diagnostic::new(ReimplementedListBuiltin, expr.range()); - if checker.patch(diagnostic.kind.rule()) { - #[allow(deprecated)] - diagnostic.set_fix(Fix::unspecified(Edit::range_replacement( - "list".to_string(), - expr.range(), - ))); - } - checker.diagnostics.push(diagnostic); - } - } - } -} diff --git a/crates/ruff/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs b/crates/ruff/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs new file mode 100644 index 0000000000..f75d9928d2 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs @@ -0,0 +1,114 @@ +use log::error; + +use rustc_hash::FxHashSet; +use rustpython_parser::ast::{self, Expr, Ranged, Stmt}; + +use ruff_diagnostics::AlwaysAutofixableViolation; +use ruff_diagnostics::Diagnostic; +use ruff_macros::{derive_message_formats, violation}; + +use ruff_python_ast::types::RefEquality; + +use crate::autofix::actions::delete_stmt; +use crate::checkers::ast::Checker; +use crate::registry::AsRule; + +/// ## What it does +/// Checks for duplicate field definitions in classes. +/// +/// ## Why is this bad? +/// Defining a field multiple times in a class body is redundant and likely a +/// mistake. +/// +/// ## Example +/// ```python +/// class Person: +/// name = Tom +/// ... +/// name = Ben +/// ``` +/// +/// Use instead: +/// ```python +/// class Person: +/// name = Tom +/// ... +/// ``` +#[violation] +pub struct DuplicateClassFieldDefinition(pub String); + +impl AlwaysAutofixableViolation for DuplicateClassFieldDefinition { + #[derive_message_formats] + fn message(&self) -> String { + let DuplicateClassFieldDefinition(name) = self; + format!("Class field `{name}` is defined multiple times") + } + + fn autofix_title(&self) -> String { + let DuplicateClassFieldDefinition(name) = self; + format!("Remove duplicate field definition for `{name}`") + } +} + +/// PIE794 +pub(crate) fn duplicate_class_field_definition<'a, 'b>( + checker: &mut Checker<'a>, + parent: &'b Stmt, + body: &'b [Stmt], +) where + 'b: 'a, +{ + let mut seen_targets: FxHashSet<&str> = FxHashSet::default(); + for stmt in body { + // Extract the property name from the assignment statement. + let target = match stmt { + Stmt::Assign(ast::StmtAssign { targets, .. }) => { + if targets.len() != 1 { + continue; + } + if let Expr::Name(ast::ExprName { id, .. }) = &targets[0] { + id + } else { + continue; + } + } + Stmt::AnnAssign(ast::StmtAnnAssign { target, .. }) => { + if let Expr::Name(ast::ExprName { id, .. }) = target.as_ref() { + id + } else { + continue; + } + } + _ => continue, + }; + + if !seen_targets.insert(target) { + let mut diagnostic = Diagnostic::new( + DuplicateClassFieldDefinition(target.to_string()), + stmt.range(), + ); + if checker.patch(diagnostic.kind.rule()) { + let deleted: Vec<&Stmt> = checker.deletions.iter().map(Into::into).collect(); + let locator = checker.locator; + match delete_stmt( + stmt, + Some(parent), + &deleted, + locator, + checker.indexer, + checker.stylist, + ) { + Ok(fix) => { + checker.deletions.insert(RefEquality(stmt)); + #[allow(deprecated)] + diagnostic.set_fix_from_edit(fix); + } + Err(err) => { + error!("Failed to remove duplicate class definition: {}", err); + } + } + } + checker.diagnostics.push(diagnostic); + } + } +} diff --git a/crates/ruff/src/rules/flake8_pie/rules/mod.rs b/crates/ruff/src/rules/flake8_pie/rules/mod.rs new file mode 100644 index 0000000000..75a94605ee --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/rules/mod.rs @@ -0,0 +1,17 @@ +pub(crate) use duplicate_class_field_definition::{ + duplicate_class_field_definition, DuplicateClassFieldDefinition, +}; +pub(crate) use multiple_starts_ends_with::{multiple_starts_ends_with, MultipleStartsEndsWith}; +pub(crate) use no_unnecessary_pass::{no_unnecessary_pass, UnnecessaryPass}; +pub(crate) use non_unique_enums::{non_unique_enums, NonUniqueEnums}; +pub(crate) use reimplemented_list_builtin::{reimplemented_list_builtin, ReimplementedListBuiltin}; +pub(crate) use unnecessary_dict_kwargs::{unnecessary_dict_kwargs, UnnecessaryDictKwargs}; +pub(crate) use unnecessary_spread::{unnecessary_spread, UnnecessarySpread}; + +mod duplicate_class_field_definition; +mod multiple_starts_ends_with; +mod no_unnecessary_pass; +mod non_unique_enums; +mod reimplemented_list_builtin; +mod unnecessary_dict_kwargs; +mod unnecessary_spread; diff --git a/crates/ruff/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs b/crates/ruff/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs new file mode 100644 index 0000000000..737c55e6a1 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/rules/multiple_starts_ends_with.rs @@ -0,0 +1,182 @@ +use std::collections::BTreeMap; +use std::iter; + +use itertools::Either::{Left, Right}; + +use ruff_text_size::TextRange; + +use rustpython_parser::ast::{self, Boolop, Expr, ExprContext, Ranged}; + +use ruff_diagnostics::AlwaysAutofixableViolation; +use ruff_diagnostics::{Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; +use crate::registry::AsRule; + +/// ## What it does +/// Checks for `startswith` or `endswith` calls on the same value with +/// different prefixes or suffixes. +/// +/// ## Why is this bad? +/// The `startswith` and `endswith` methods accept tuples of prefixes or +/// suffixes respectively. Passing a tuple of prefixes or suffixes is more +/// more efficient and readable than calling the method multiple times. +/// +/// ## Example +/// ```python +/// msg = "Hello, world!" +/// if msg.startswith("Hello") or msg.startswith("Hi"): +/// print("Greetings!") +/// ``` +/// +/// Use instead: +/// ```python +/// msg = "Hello, world!" +/// if msg.startswith(("Hello", "Hi")): +/// print("Greetings!") +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/stdtypes.html#str.startswith) +/// - [Python documentation](https://docs.python.org/3/library/stdtypes.html#str.endswith) +#[violation] +pub struct MultipleStartsEndsWith { + attr: String, +} + +impl AlwaysAutofixableViolation for MultipleStartsEndsWith { + #[derive_message_formats] + fn message(&self) -> String { + let MultipleStartsEndsWith { attr } = self; + format!("Call `{attr}` once with a `tuple`") + } + + fn autofix_title(&self) -> String { + let MultipleStartsEndsWith { attr } = self; + format!("Merge into a single `{attr}` call") + } +} + +/// PIE810 +pub(crate) fn multiple_starts_ends_with(checker: &mut Checker, expr: &Expr) { + let Expr::BoolOp(ast::ExprBoolOp { op: Boolop::Or, values, range: _ }) = expr else { + return; + }; + + let mut duplicates = BTreeMap::new(); + for (index, call) in values.iter().enumerate() { + let Expr::Call(ast::ExprCall { + func, + args, + keywords, + range: _ + }) = &call else { + continue + }; + + if !(args.len() == 1 && keywords.is_empty()) { + continue; + } + + let Expr::Attribute(ast::ExprAttribute { value, attr, .. } )= func.as_ref() else { + continue + }; + if attr != "startswith" && attr != "endswith" { + continue; + } + + let Expr::Name(ast::ExprName { id: arg_name, .. } )= value.as_ref() else { + continue + }; + + duplicates + .entry((attr.as_str(), arg_name.as_str())) + .or_insert_with(Vec::new) + .push(index); + } + + // Generate a `Diagnostic` for each duplicate. + for ((attr_name, arg_name), indices) in duplicates { + if indices.len() > 1 { + let mut diagnostic = Diagnostic::new( + MultipleStartsEndsWith { + attr: attr_name.to_string(), + }, + expr.range(), + ); + if checker.patch(diagnostic.kind.rule()) { + let words: Vec<&Expr> = indices + .iter() + .map(|index| &values[*index]) + .map(|expr| { + let Expr::Call(ast::ExprCall { func: _, args, keywords: _, range: _}) = expr else { + unreachable!("{}", format!("Indices should only contain `{attr_name}` calls")) + }; + args.get(0) + .unwrap_or_else(|| panic!("`{attr_name}` should have one argument")) + }) + .collect(); + + let node = Expr::Tuple(ast::ExprTuple { + elts: words + .iter() + .flat_map(|value| { + if let Expr::Tuple(ast::ExprTuple { elts, .. }) = value { + Left(elts.iter()) + } else { + Right(iter::once(*value)) + } + }) + .map(Clone::clone) + .collect(), + ctx: ExprContext::Load, + range: TextRange::default(), + }); + let node1 = Expr::Name(ast::ExprName { + id: arg_name.into(), + ctx: ExprContext::Load, + range: TextRange::default(), + }); + let node2 = Expr::Attribute(ast::ExprAttribute { + value: Box::new(node1), + attr: attr_name.into(), + ctx: ExprContext::Load, + range: TextRange::default(), + }); + let node3 = Expr::Call(ast::ExprCall { + func: Box::new(node2), + args: vec![node], + keywords: vec![], + range: TextRange::default(), + }); + let call = node3; + + // Generate the combined `BoolOp`. + let mut call = Some(call); + let node = Expr::BoolOp(ast::ExprBoolOp { + op: Boolop::Or, + values: values + .iter() + .enumerate() + .filter_map(|(index, elt)| { + if indices.contains(&index) { + std::mem::take(&mut call) + } else { + Some(elt.clone()) + } + }) + .collect(), + range: TextRange::default(), + }); + let bool_op = node; + #[allow(deprecated)] + diagnostic.set_fix(Fix::unspecified(Edit::range_replacement( + checker.generator().expr(&bool_op), + expr.range(), + ))); + } + checker.diagnostics.push(diagnostic); + } + } +} diff --git a/crates/ruff/src/rules/flake8_pie/rules/no_unnecessary_pass.rs b/crates/ruff/src/rules/flake8_pie/rules/no_unnecessary_pass.rs new file mode 100644 index 0000000000..70befdbe47 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/rules/no_unnecessary_pass.rs @@ -0,0 +1,94 @@ +use rustpython_parser::ast::{self, Constant, Expr, Ranged, Stmt}; + +use ruff_diagnostics::AlwaysAutofixableViolation; +use ruff_diagnostics::{Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; + +use ruff_python_ast::helpers::trailing_comment_start_offset; + +use crate::autofix::actions::delete_stmt; +use crate::checkers::ast::Checker; +use crate::registry::AsRule; + +/// ## What it does +/// Checks for unnecessary `pass` statements in class and function bodies. +/// where it is not needed syntactically (e.g., when an indented docstring is +/// present). +/// +/// ## Why is this bad? +/// When a function or class definition contains a docstring, an additional +/// `pass` statement is redundant. +/// +/// ## Example +/// ```python +/// def foo(): +/// """Placeholder docstring.""" +/// pass +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(): +/// """Placeholder docstring.""" +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/reference/simple_stmts.html#the-pass-statement) +#[violation] +pub struct UnnecessaryPass; + +impl AlwaysAutofixableViolation for UnnecessaryPass { + #[derive_message_formats] + fn message(&self) -> String { + format!("Unnecessary `pass` statement") + } + + fn autofix_title(&self) -> String { + "Remove unnecessary `pass`".to_string() + } +} + +/// PIE790 +pub(crate) fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) { + if body.len() > 1 { + // This only catches the case in which a docstring makes a `pass` statement + // redundant. Consider removing all `pass` statements instead. + let docstring_stmt = &body[0]; + let pass_stmt = &body[1]; + let Stmt::Expr(ast::StmtExpr { value, range: _ } )= docstring_stmt else { + return; + }; + if matches!( + value.as_ref(), + Expr::Constant(ast::ExprConstant { + value: Constant::Str(..), + .. + }) + ) { + if pass_stmt.is_pass_stmt() { + let mut diagnostic = Diagnostic::new(UnnecessaryPass, pass_stmt.range()); + if checker.patch(diagnostic.kind.rule()) { + if let Some(index) = trailing_comment_start_offset(pass_stmt, checker.locator) { + #[allow(deprecated)] + diagnostic.set_fix(Fix::unspecified(Edit::range_deletion( + pass_stmt.range().add_end(index), + ))); + } else { + #[allow(deprecated)] + diagnostic.try_set_fix_from_edit(|| { + delete_stmt( + pass_stmt, + None, + &[], + checker.locator, + checker.indexer, + checker.stylist, + ) + }); + } + } + checker.diagnostics.push(diagnostic); + } + } + } +} diff --git a/crates/ruff/src/rules/flake8_pie/rules/non_unique_enums.rs b/crates/ruff/src/rules/flake8_pie/rules/non_unique_enums.rs new file mode 100644 index 0000000000..47e1a427a5 --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/rules/non_unique_enums.rs @@ -0,0 +1,102 @@ +use rustc_hash::FxHashSet; +use rustpython_parser::ast::{self, Expr, Ranged, Stmt}; + +use ruff_diagnostics::Diagnostic; +use ruff_diagnostics::Violation; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::comparable::ComparableExpr; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for enums that contain duplicate values. +/// +/// ## Why is this bad? +/// Enum values should be unique. Non-unique values are redundant and likely a +/// mistake. +/// +/// ## Example +/// ```python +/// from enum import Enum +/// +/// +/// class Foo(Enum): +/// A = 1 +/// B = 2 +/// C = 1 +/// ``` +/// +/// Use instead: +/// ```python +/// from enum import Enum +/// +/// +/// class Foo(Enum): +/// A = 1 +/// B = 2 +/// C = 3 +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/enum.html#enum.Enum) +#[violation] +pub struct NonUniqueEnums { + value: String, +} + +impl Violation for NonUniqueEnums { + #[derive_message_formats] + fn message(&self) -> String { + let NonUniqueEnums { value } = self; + format!("Enum contains duplicate value: `{value}`") + } +} + +/// PIE796 +pub(crate) fn non_unique_enums<'a, 'b>( + checker: &mut Checker<'a>, + parent: &'b Stmt, + body: &'b [Stmt], +) where + 'b: 'a, +{ + let Stmt::ClassDef(ast::StmtClassDef { bases, .. }) = parent else { + return; + }; + + if !bases.iter().any(|expr| { + checker + .semantic_model() + .resolve_call_path(expr) + .map_or(false, |call_path| call_path.as_slice() == ["enum", "Enum"]) + }) { + return; + } + + let mut seen_targets: FxHashSet = FxHashSet::default(); + for stmt in body { + let Stmt::Assign(ast::StmtAssign { value, .. }) = stmt else { + continue; + }; + + if let Expr::Call(ast::ExprCall { func, .. }) = value.as_ref() { + if checker + .semantic_model() + .resolve_call_path(func) + .map_or(false, |call_path| call_path.as_slice() == ["enum", "auto"]) + { + continue; + } + } + + if !seen_targets.insert(ComparableExpr::from(value)) { + let diagnostic = Diagnostic::new( + NonUniqueEnums { + value: checker.generator().expr(value), + }, + stmt.range(), + ); + checker.diagnostics.push(diagnostic); + } + } +} diff --git a/crates/ruff/src/rules/flake8_pie/rules/reimplemented_list_builtin.rs b/crates/ruff/src/rules/flake8_pie/rules/reimplemented_list_builtin.rs new file mode 100644 index 0000000000..8c7f180c3a --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/rules/reimplemented_list_builtin.rs @@ -0,0 +1,80 @@ +use rustpython_parser::ast::{self, Expr, ExprLambda, Ranged}; + +use ruff_diagnostics::AlwaysAutofixableViolation; +use ruff_diagnostics::{Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; +use crate::registry::AsRule; + +/// ## What it does +/// Checks for lambdas that can be replaced with the `list` builtin. +/// +/// ## Why is this bad? +/// Using `list` builtin is more readable. +/// +/// ## Example +/// ```python +/// from dataclasses import dataclass, field +/// +/// +/// @dataclass +/// class Foo: +/// bar: list[int] = field(default_factory=lambda: []) +/// ``` +/// +/// Use instead: +/// ```python +/// from dataclasses import dataclass, field +/// +/// +/// @dataclass +/// class Foo: +/// bar: list[int] = field(default_factory=list) +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/library/functions.html#func-list) +#[violation] +pub struct ReimplementedListBuiltin; + +impl AlwaysAutofixableViolation for ReimplementedListBuiltin { + #[derive_message_formats] + fn message(&self) -> String { + format!("Prefer `list` over useless lambda") + } + + fn autofix_title(&self) -> String { + "Replace with `list`".to_string() + } +} + +/// PIE807 +pub(crate) fn reimplemented_list_builtin(checker: &mut Checker, expr: &ExprLambda) { + let ExprLambda { + args, + body, + range: _, + } = expr; + + if args.args.is_empty() + && args.kwonlyargs.is_empty() + && args.posonlyargs.is_empty() + && args.vararg.is_none() + && args.kwarg.is_none() + { + if let Expr::List(ast::ExprList { elts, .. }) = body.as_ref() { + if elts.is_empty() { + let mut diagnostic = Diagnostic::new(ReimplementedListBuiltin, expr.range()); + if checker.patch(diagnostic.kind.rule()) { + #[allow(deprecated)] + diagnostic.set_fix(Fix::unspecified(Edit::range_replacement( + "list".to_string(), + expr.range(), + ))); + } + checker.diagnostics.push(diagnostic); + } + } + } +} diff --git a/crates/ruff/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs b/crates/ruff/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs new file mode 100644 index 0000000000..b621d1ff5f --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/rules/unnecessary_dict_kwargs.rs @@ -0,0 +1,79 @@ +use rustpython_parser::ast::{self, Constant, Expr, Keyword, Ranged}; + +use ruff_diagnostics::Diagnostic; +use ruff_diagnostics::Violation; +use ruff_macros::{derive_message_formats, violation}; + +use ruff_python_stdlib::identifiers::is_identifier; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for unnecessary `dict` kwargs. +/// +/// ## Why is this bad? +/// If the `dict` keys are valid identifiers, they can be passed as keyword +/// arguments directly. +/// +/// ## Example +/// ```python +/// def foo(bar): +/// return bar + 1 +/// +/// +/// print(foo(**{"bar": 2})) # prints 3 +/// ``` +/// +/// Use instead: +/// ```python +/// def foo(bar): +/// return bar + 1 +/// +/// +/// print(foo(bar=2)) # prints 3 +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#dictionary-displays) +/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#calls) +#[violation] +pub struct UnnecessaryDictKwargs; + +impl Violation for UnnecessaryDictKwargs { + #[derive_message_formats] + fn message(&self) -> String { + format!("Unnecessary `dict` kwargs") + } +} + +/// PIE804 +pub(crate) fn unnecessary_dict_kwargs(checker: &mut Checker, expr: &Expr, kwargs: &[Keyword]) { + for kw in kwargs { + // keyword is a spread operator (indicated by None) + if kw.arg.is_none() { + if let Expr::Dict(ast::ExprDict { keys, .. }) = &kw.value { + // ensure foo(**{"bar-bar": 1}) doesn't error + if keys.iter().all(|expr| expr.as_ref().map_or(false, is_valid_kwarg_name)) || + // handle case of foo(**{**bar}) + (keys.len() == 1 && keys[0].is_none()) + { + let diagnostic = Diagnostic::new(UnnecessaryDictKwargs, expr.range()); + checker.diagnostics.push(diagnostic); + } + } + } + } +} + +/// Return `true` if a key is a valid keyword argument name. +fn is_valid_kwarg_name(key: &Expr) -> bool { + if let Expr::Constant(ast::ExprConstant { + value: Constant::Str(value), + .. + }) = key + { + is_identifier(value) + } else { + false + } +} diff --git a/crates/ruff/src/rules/flake8_pie/rules/unnecessary_spread.rs b/crates/ruff/src/rules/flake8_pie/rules/unnecessary_spread.rs new file mode 100644 index 0000000000..855d4ea26d --- /dev/null +++ b/crates/ruff/src/rules/flake8_pie/rules/unnecessary_spread.rs @@ -0,0 +1,52 @@ +use rustpython_parser::ast::{Expr, Ranged}; + +use ruff_diagnostics::Diagnostic; +use ruff_diagnostics::Violation; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for unnecessary dictionary unpacking operators (`**`). +/// +/// ## Why is this bad? +/// Unpacking a dictionary into another dictionary is redundant. The unpacking +/// operator can be removed, making the code more readable. +/// +/// ## Example +/// ```python +/// foo = {"A": 1, "B": 2} +/// bar = {**foo, **{"C": 3}} +/// ``` +/// +/// Use instead: +/// ```python +/// foo = {"A": 1, "B": 2} +/// bar = {**foo, "C": 3} +/// ``` +/// +/// ## References +/// - [Python documentation](https://docs.python.org/3/reference/expressions.html#dictionary-displays) +#[violation] +pub struct UnnecessarySpread; + +impl Violation for UnnecessarySpread { + #[derive_message_formats] + fn message(&self) -> String { + format!("Unnecessary spread `**`") + } +} + +/// PIE800 +pub(crate) fn unnecessary_spread(checker: &mut Checker, keys: &[Option], values: &[Expr]) { + for item in keys.iter().zip(values.iter()) { + if let (None, value) = item { + // We only care about when the key is None which indicates a spread `**` + // inside a dict. + if let Expr::Dict(_) = value { + let diagnostic = Diagnostic::new(UnnecessarySpread, value.range()); + checker.diagnostics.push(diagnostic); + } + } + } +} diff --git a/crates/ruff/src/rules/flake8_quotes/rules.rs b/crates/ruff/src/rules/flake8_quotes/rules/from_tokens.rs similarity index 99% rename from crates/ruff/src/rules/flake8_quotes/rules.rs rename to crates/ruff/src/rules/flake8_quotes/rules/from_tokens.rs index b9f5efc6b0..7c6f990dd3 100644 --- a/crates/ruff/src/rules/flake8_quotes/rules.rs +++ b/crates/ruff/src/rules/flake8_quotes/rules/from_tokens.rs @@ -10,7 +10,7 @@ use crate::lex::docstring_detection::StateMachine; use crate::registry::Rule; use crate::settings::Settings; -use super::settings::Quote; +use super::super::settings::Quote; /// ## What it does /// Checks for inline strings that use single quotes or double quotes, diff --git a/crates/ruff/src/rules/flake8_quotes/rules/mod.rs b/crates/ruff/src/rules/flake8_quotes/rules/mod.rs new file mode 100644 index 0000000000..7f04e8908f --- /dev/null +++ b/crates/ruff/src/rules/flake8_quotes/rules/mod.rs @@ -0,0 +1,6 @@ +pub(crate) use from_tokens::{ + from_tokens, AvoidableEscapedQuote, BadQuotesDocstring, BadQuotesInlineString, + BadQuotesMultilineString, +}; + +mod from_tokens; diff --git a/crates/ruff/src/rules/flake8_return/rules.rs b/crates/ruff/src/rules/flake8_return/rules/function.rs similarity index 99% rename from crates/ruff/src/rules/flake8_return/rules.rs rename to crates/ruff/src/rules/flake8_return/rules/function.rs index 7251f656bc..cc84657be0 100644 --- a/crates/ruff/src/rules/flake8_return/rules.rs +++ b/crates/ruff/src/rules/flake8_return/rules/function.rs @@ -15,9 +15,9 @@ use crate::checkers::ast::Checker; use crate::registry::{AsRule, Rule}; use crate::rules::flake8_return::helpers::end_of_last_statement; -use super::branch::Branch; -use super::helpers::result_exists; -use super::visitor::{ReturnVisitor, Stack}; +use super::super::branch::Branch; +use super::super::helpers::result_exists; +use super::super::visitor::{ReturnVisitor, Stack}; /// ## What it does /// Checks for the presence of a `return None` statement when `None` is the only diff --git a/crates/ruff/src/rules/flake8_return/rules/mod.rs b/crates/ruff/src/rules/flake8_return/rules/mod.rs new file mode 100644 index 0000000000..7e6a3fbcf7 --- /dev/null +++ b/crates/ruff/src/rules/flake8_return/rules/mod.rs @@ -0,0 +1,6 @@ +pub(crate) use function::{ + function, ImplicitReturn, ImplicitReturnValue, SuperfluousElseBreak, SuperfluousElseContinue, + SuperfluousElseRaise, SuperfluousElseReturn, UnnecessaryAssign, UnnecessaryReturnNone, +}; + +mod function; diff --git a/crates/ruff/src/rules/flake8_tidy_imports/mod.rs b/crates/ruff/src/rules/flake8_tidy_imports/mod.rs index f9dc34868e..8686f3971c 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/mod.rs +++ b/crates/ruff/src/rules/flake8_tidy_imports/mod.rs @@ -1,13 +1,127 @@ //! Rules from [flake8-tidy-imports](https://pypi.org/project/flake8-tidy-imports/). -use ruff_macros::CacheKey; - pub mod options; +pub(crate) mod rules; +pub mod settings; -pub mod banned_api; -pub mod relative_imports; +#[cfg(test)] +mod tests { + use std::path::Path; -#[derive(Debug, CacheKey, Default)] -pub struct Settings { - pub ban_relative_imports: relative_imports::Settings, - pub banned_api: banned_api::Settings, + use anyhow::Result; + use rustc_hash::FxHashMap; + + use crate::assert_messages; + use crate::registry::Rule; + use crate::rules::flake8_tidy_imports; + use crate::rules::flake8_tidy_imports::settings::{ApiBan, Strictness}; + use crate::settings::Settings; + use crate::test::test_path; + + #[test] + fn banned_api() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_tidy_imports/TID251.py"), + &Settings { + flake8_tidy_imports: flake8_tidy_imports::settings::Settings { + banned_api: FxHashMap::from_iter([ + ( + "cgi".to_string(), + ApiBan { + msg: "The cgi module is deprecated.".to_string(), + }, + ), + ( + "typing.TypedDict".to_string(), + ApiBan { + msg: "Use typing_extensions.TypedDict instead.".to_string(), + }, + ), + ]), + ..Default::default() + }, + ..Settings::for_rules(vec![Rule::BannedApi]) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } + + #[test] + fn banned_api_package() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_tidy_imports/TID/my_package/sublib/api/application.py"), + &Settings { + flake8_tidy_imports: flake8_tidy_imports::settings::Settings { + banned_api: FxHashMap::from_iter([ + ( + "attrs".to_string(), + ApiBan { + msg: "The attrs module is deprecated.".to_string(), + }, + ), + ( + "my_package.sublib.protocol".to_string(), + ApiBan { + msg: "The protocol module is deprecated.".to_string(), + }, + ), + ]), + ..Default::default() + }, + namespace_packages: vec![Path::new("my_package").to_path_buf()], + ..Settings::for_rules(vec![Rule::BannedApi]) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } + + #[test] + fn ban_parent_imports() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_tidy_imports/TID252.py"), + &Settings { + flake8_tidy_imports: flake8_tidy_imports::settings::Settings { + ban_relative_imports: Strictness::Parents, + ..Default::default() + }, + ..Settings::for_rules(vec![Rule::RelativeImports]) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } + + #[test] + fn ban_all_imports() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_tidy_imports/TID252.py"), + &Settings { + flake8_tidy_imports: flake8_tidy_imports::settings::Settings { + ban_relative_imports: Strictness::All, + ..Default::default() + }, + ..Settings::for_rules(vec![Rule::RelativeImports]) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } + + #[test] + fn ban_parent_imports_package() -> Result<()> { + let diagnostics = test_path( + Path::new("flake8_tidy_imports/TID/my_package/sublib/api/application.py"), + &Settings { + flake8_tidy_imports: flake8_tidy_imports::settings::Settings { + ban_relative_imports: Strictness::Parents, + ..Default::default() + }, + namespace_packages: vec![Path::new("my_package").to_path_buf()], + ..Settings::for_rules(vec![Rule::RelativeImports]) + }, + )?; + assert_messages!(diagnostics); + Ok(()) + } } diff --git a/crates/ruff/src/rules/flake8_tidy_imports/options.rs b/crates/ruff/src/rules/flake8_tidy_imports/options.rs index f180974a49..6f3eb99fcb 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/options.rs +++ b/crates/ruff/src/rules/flake8_tidy_imports/options.rs @@ -5,9 +5,7 @@ use serde::{Deserialize, Serialize}; use ruff_macros::{CombineOptions, ConfigurationOptions}; -use super::banned_api::ApiBan; -use super::relative_imports::Strictness; -use super::Settings; +use super::settings::{ApiBan, Settings, Strictness}; #[derive( Debug, PartialEq, Eq, Serialize, Deserialize, Default, ConfigurationOptions, CombineOptions, diff --git a/crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs b/crates/ruff/src/rules/flake8_tidy_imports/rules/banned_api.rs similarity index 50% rename from crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs rename to crates/ruff/src/rules/flake8_tidy_imports/rules/banned_api.rs index 04343bd8ab..668f764354 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs +++ b/crates/ruff/src/rules/flake8_tidy_imports/rules/banned_api.rs @@ -1,23 +1,11 @@ -use rustc_hash::FxHashMap; use rustpython_parser::ast::{Expr, Ranged}; -use serde::{Deserialize, Serialize}; use ruff_diagnostics::{Diagnostic, Violation}; -use ruff_macros::{derive_message_formats, violation, CacheKey}; +use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::call_path::from_qualified_name; use crate::checkers::ast::Checker; -pub type Settings = FxHashMap; - -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, CacheKey)] -#[serde(deny_unknown_fields, rename_all = "kebab-case")] -#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] -pub struct ApiBan { - /// The message to display when the API is used. - pub msg: String, -} - /// ## What it does /// Checks for banned imports. /// @@ -115,77 +103,3 @@ pub(crate) fn banned_attribute_access(checker: &mut Checker, expr: &Expr) { )); } } - -#[cfg(test)] -mod tests { - use std::path::Path; - - use anyhow::Result; - use rustc_hash::FxHashMap; - - use crate::assert_messages; - use crate::registry::Rule; - use crate::settings::Settings; - use crate::test::test_path; - - use super::ApiBan; - - #[test] - fn banned_api() -> Result<()> { - let diagnostics = test_path( - Path::new("flake8_tidy_imports/TID251.py"), - &Settings { - flake8_tidy_imports: super::super::Settings { - banned_api: FxHashMap::from_iter([ - ( - "cgi".to_string(), - ApiBan { - msg: "The cgi module is deprecated.".to_string(), - }, - ), - ( - "typing.TypedDict".to_string(), - ApiBan { - msg: "Use typing_extensions.TypedDict instead.".to_string(), - }, - ), - ]), - ..Default::default() - }, - ..Settings::for_rules(vec![Rule::BannedApi]) - }, - )?; - assert_messages!(diagnostics); - Ok(()) - } - - #[test] - fn banned_api_package() -> Result<()> { - let diagnostics = test_path( - Path::new("flake8_tidy_imports/TID/my_package/sublib/api/application.py"), - &Settings { - flake8_tidy_imports: super::super::Settings { - banned_api: FxHashMap::from_iter([ - ( - "attrs".to_string(), - ApiBan { - msg: "The attrs module is deprecated.".to_string(), - }, - ), - ( - "my_package.sublib.protocol".to_string(), - ApiBan { - msg: "The protocol module is deprecated.".to_string(), - }, - ), - ]), - ..Default::default() - }, - namespace_packages: vec![Path::new("my_package").to_path_buf()], - ..Settings::for_rules(vec![Rule::BannedApi]) - }, - )?; - assert_messages!(diagnostics); - Ok(()) - } -} diff --git a/crates/ruff/src/rules/flake8_tidy_imports/rules/mod.rs b/crates/ruff/src/rules/flake8_tidy_imports/rules/mod.rs new file mode 100644 index 0000000000..8efbdfd6ae --- /dev/null +++ b/crates/ruff/src/rules/flake8_tidy_imports/rules/mod.rs @@ -0,0 +1,7 @@ +pub(crate) use banned_api::{ + banned_attribute_access, name_is_banned, name_or_parent_is_banned, BannedApi, +}; +pub(crate) use relative_imports::{banned_relative_import, RelativeImports}; + +mod banned_api; +mod relative_imports; diff --git a/crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs b/crates/ruff/src/rules/flake8_tidy_imports/rules/relative_imports.rs similarity index 62% rename from crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs rename to crates/ruff/src/rules/flake8_tidy_imports/rules/relative_imports.rs index 79ccdd8fdb..9bf9bc41a8 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs +++ b/crates/ruff/src/rules/flake8_tidy_imports/rules/relative_imports.rs @@ -1,28 +1,15 @@ use ruff_text_size::TextRange; use rustpython_parser::ast::{self, Int, Ranged, Stmt}; -use serde::{Deserialize, Serialize}; use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; -use ruff_macros::{derive_message_formats, violation, CacheKey}; +use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::helpers::resolve_imported_module_path; use ruff_python_ast::source_code::Generator; use ruff_python_stdlib::identifiers::is_identifier; use crate::checkers::ast::Checker; use crate::registry::AsRule; - -pub type Settings = Strictness; - -#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, CacheKey, Default)] -#[serde(deny_unknown_fields, rename_all = "kebab-case")] -#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] -pub enum Strictness { - /// Ban imports that extend into the parent module or beyond. - #[default] - Parents, - /// Ban all relative imports. - All, -} +use crate::rules::flake8_tidy_imports::settings::Strictness; /// ## What it does /// Checks for relative imports. @@ -147,66 +134,3 @@ pub(crate) fn banned_relative_import( None } } - -#[cfg(test)] -mod tests { - use std::path::Path; - - use anyhow::Result; - - use crate::assert_messages; - use crate::registry::Rule; - use crate::settings::Settings; - use crate::test::test_path; - - use super::Strictness; - - #[test] - fn ban_parent_imports() -> Result<()> { - let diagnostics = test_path( - Path::new("flake8_tidy_imports/TID252.py"), - &Settings { - flake8_tidy_imports: super::super::Settings { - ban_relative_imports: Strictness::Parents, - ..Default::default() - }, - ..Settings::for_rules(vec![Rule::RelativeImports]) - }, - )?; - assert_messages!(diagnostics); - Ok(()) - } - - #[test] - fn ban_all_imports() -> Result<()> { - let diagnostics = test_path( - Path::new("flake8_tidy_imports/TID252.py"), - &Settings { - flake8_tidy_imports: super::super::Settings { - ban_relative_imports: Strictness::All, - ..Default::default() - }, - ..Settings::for_rules(vec![Rule::RelativeImports]) - }, - )?; - assert_messages!(diagnostics); - Ok(()) - } - - #[test] - fn ban_parent_imports_package() -> Result<()> { - let diagnostics = test_path( - Path::new("flake8_tidy_imports/TID/my_package/sublib/api/application.py"), - &Settings { - flake8_tidy_imports: super::super::Settings { - ban_relative_imports: Strictness::Parents, - ..Default::default() - }, - namespace_packages: vec![Path::new("my_package").to_path_buf()], - ..Settings::for_rules(vec![Rule::RelativeImports]) - }, - )?; - assert_messages!(diagnostics); - Ok(()) - } -} diff --git a/crates/ruff/src/rules/flake8_tidy_imports/settings.rs b/crates/ruff/src/rules/flake8_tidy_imports/settings.rs new file mode 100644 index 0000000000..90b2843280 --- /dev/null +++ b/crates/ruff/src/rules/flake8_tidy_imports/settings.rs @@ -0,0 +1,29 @@ +use rustc_hash::FxHashMap; +use serde::{Deserialize, Serialize}; + +use ruff_macros::CacheKey; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, CacheKey)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub struct ApiBan { + /// The message to display when the API is used. + pub msg: String, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize, Deserialize, CacheKey, Default)] +#[serde(deny_unknown_fields, rename_all = "kebab-case")] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] +pub enum Strictness { + /// Ban imports that extend into the parent module or beyond. + #[default] + Parents, + /// Ban all relative imports. + All, +} + +#[derive(Debug, CacheKey, Default)] +pub struct Settings { + pub ban_relative_imports: Strictness, + pub banned_api: FxHashMap, +} diff --git a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_all_imports.snap b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__ban_all_imports.snap similarity index 98% rename from crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_all_imports.snap rename to crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__ban_all_imports.snap index 30f082000a..d3b952fa33 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_all_imports.snap +++ b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__ban_all_imports.snap @@ -1,5 +1,5 @@ --- -source: crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs +source: crates/ruff/src/rules/flake8_tidy_imports/mod.rs --- TID252.py:7:1: TID252 Relative imports are banned | diff --git a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports.snap b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__ban_parent_imports.snap similarity index 98% rename from crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports.snap rename to crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__ban_parent_imports.snap index 05b6d83503..39c572fb0f 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports.snap +++ b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__ban_parent_imports.snap @@ -1,5 +1,5 @@ --- -source: crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs +source: crates/ruff/src/rules/flake8_tidy_imports/mod.rs --- TID252.py:9:1: TID252 Relative imports from parent modules are banned | diff --git a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports_package.snap b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__ban_parent_imports_package.snap similarity index 98% rename from crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports_package.snap rename to crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__ban_parent_imports_package.snap index 4d0d27311a..d3c132753c 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__relative_imports__tests__ban_parent_imports_package.snap +++ b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__ban_parent_imports_package.snap @@ -1,5 +1,5 @@ --- -source: crates/ruff/src/rules/flake8_tidy_imports/relative_imports.rs +source: crates/ruff/src/rules/flake8_tidy_imports/mod.rs --- application.py:5:1: TID252 Relative imports from parent modules are banned | diff --git a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__banned_api__tests__banned_api.snap b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__banned_api.snap similarity index 97% rename from crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__banned_api__tests__banned_api.snap rename to crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__banned_api.snap index 0fcbbaf94d..442a0838f9 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__banned_api__tests__banned_api.snap +++ b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__banned_api.snap @@ -1,5 +1,5 @@ --- -source: crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs +source: crates/ruff/src/rules/flake8_tidy_imports/mod.rs --- TID251.py:2:8: TID251 `cgi` is banned: The cgi module is deprecated. | diff --git a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__banned_api__tests__banned_api_package.snap b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__banned_api_package.snap similarity index 92% rename from crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__banned_api__tests__banned_api_package.snap rename to crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__banned_api_package.snap index d11d98610f..bceabe72a2 100644 --- a/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__banned_api__tests__banned_api_package.snap +++ b/crates/ruff/src/rules/flake8_tidy_imports/snapshots/ruff__rules__flake8_tidy_imports__tests__banned_api_package.snap @@ -1,5 +1,5 @@ --- -source: crates/ruff/src/rules/flake8_tidy_imports/banned_api.rs +source: crates/ruff/src/rules/flake8_tidy_imports/mod.rs --- application.py:3:8: TID251 `attrs` is banned: The attrs module is deprecated. | diff --git a/crates/ruff/src/rules/flake8_todos/rules/mod.rs b/crates/ruff/src/rules/flake8_todos/rules/mod.rs new file mode 100644 index 0000000000..dd10c6bc3b --- /dev/null +++ b/crates/ruff/src/rules/flake8_todos/rules/mod.rs @@ -0,0 +1,6 @@ +pub(crate) use todos::{ + todos, InvalidTodoCapitalization, InvalidTodoTag, MissingSpaceAfterTodoColon, + MissingTodoAuthor, MissingTodoColon, MissingTodoDescription, MissingTodoLink, +}; + +mod todos; diff --git a/crates/ruff/src/rules/flake8_todos/rules.rs b/crates/ruff/src/rules/flake8_todos/rules/todos.rs similarity index 100% rename from crates/ruff/src/rules/flake8_todos/rules.rs rename to crates/ruff/src/rules/flake8_todos/rules/todos.rs diff --git a/crates/ruff/src/rules/flake8_unused_arguments/mod.rs b/crates/ruff/src/rules/flake8_unused_arguments/mod.rs index d2543b31d9..528a001a69 100644 --- a/crates/ruff/src/rules/flake8_unused_arguments/mod.rs +++ b/crates/ruff/src/rules/flake8_unused_arguments/mod.rs @@ -2,7 +2,6 @@ mod helpers; pub(crate) mod rules; pub mod settings; -mod types; #[cfg(test)] mod tests { diff --git a/crates/ruff/src/rules/flake8_unused_arguments/rules/mod.rs b/crates/ruff/src/rules/flake8_unused_arguments/rules/mod.rs new file mode 100644 index 0000000000..5cf115d8bf --- /dev/null +++ b/crates/ruff/src/rules/flake8_unused_arguments/rules/mod.rs @@ -0,0 +1,6 @@ +pub(crate) use unused_arguments::{ + unused_arguments, UnusedClassMethodArgument, UnusedFunctionArgument, UnusedLambdaArgument, + UnusedMethodArgument, UnusedStaticMethodArgument, +}; + +mod unused_arguments; diff --git a/crates/ruff/src/rules/flake8_unused_arguments/rules.rs b/crates/ruff/src/rules/flake8_unused_arguments/rules/unused_arguments.rs similarity index 92% rename from crates/ruff/src/rules/flake8_unused_arguments/rules.rs rename to crates/ruff/src/rules/flake8_unused_arguments/rules/unused_arguments.rs index 4188f8d39f..7212b37836 100644 --- a/crates/ruff/src/rules/flake8_unused_arguments/rules.rs +++ b/crates/ruff/src/rules/flake8_unused_arguments/rules/unused_arguments.rs @@ -3,6 +3,7 @@ use std::iter; use regex::Regex; use rustpython_parser::ast::{Arg, Arguments}; +use ruff_diagnostics::DiagnosticKind; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_semantic::analyze::function_type; @@ -11,10 +12,42 @@ use ruff_python_semantic::analyze::visibility; use ruff_python_semantic::binding::Bindings; use ruff_python_semantic::scope::{FunctionDef, Lambda, Scope, ScopeKind}; -use crate::checkers::ast::Checker; +use super::super::helpers; -use super::helpers; -use super::types::Argumentable; +use crate::checkers::ast::Checker; +use crate::registry::Rule; + +/// An AST node that can contain arguments. +#[derive(Copy, Clone)] +enum Argumentable { + Function, + Method, + ClassMethod, + StaticMethod, + Lambda, +} + +impl Argumentable { + pub(crate) fn check_for(self, name: String) -> DiagnosticKind { + match self { + Self::Function => UnusedFunctionArgument { name }.into(), + Self::Method => UnusedMethodArgument { name }.into(), + Self::ClassMethod => UnusedClassMethodArgument { name }.into(), + Self::StaticMethod => UnusedStaticMethodArgument { name }.into(), + Self::Lambda => UnusedLambdaArgument { name }.into(), + } + } + + pub(crate) const fn rule_code(self) -> Rule { + match self { + Self::Function => Rule::UnusedFunctionArgument, + Self::Method => Rule::UnusedMethodArgument, + Self::ClassMethod => Rule::UnusedClassMethodArgument, + Self::StaticMethod => Rule::UnusedStaticMethodArgument, + Self::Lambda => Rule::UnusedLambdaArgument, + } + } +} /// ## What it does /// Checks for the presence of unused arguments in function definitions. diff --git a/crates/ruff/src/rules/flake8_unused_arguments/types.rs b/crates/ruff/src/rules/flake8_unused_arguments/types.rs deleted file mode 100644 index 778e53c556..0000000000 --- a/crates/ruff/src/rules/flake8_unused_arguments/types.rs +++ /dev/null @@ -1,37 +0,0 @@ -use ruff_diagnostics::DiagnosticKind; - -use crate::registry::Rule; - -use super::rules; - -/// An AST node that can contain arguments. -#[derive(Copy, Clone)] -pub(crate) enum Argumentable { - Function, - Method, - ClassMethod, - StaticMethod, - Lambda, -} - -impl Argumentable { - pub(crate) fn check_for(self, name: String) -> DiagnosticKind { - match self { - Self::Function => rules::UnusedFunctionArgument { name }.into(), - Self::Method => rules::UnusedMethodArgument { name }.into(), - Self::ClassMethod => rules::UnusedClassMethodArgument { name }.into(), - Self::StaticMethod => rules::UnusedStaticMethodArgument { name }.into(), - Self::Lambda => rules::UnusedLambdaArgument { name }.into(), - } - } - - pub(crate) const fn rule_code(self) -> Rule { - match self { - Self::Function => Rule::UnusedFunctionArgument, - Self::Method => Rule::UnusedMethodArgument, - Self::ClassMethod => Rule::UnusedClassMethodArgument, - Self::StaticMethod => Rule::UnusedStaticMethodArgument, - Self::Lambda => Rule::UnusedLambdaArgument, - } - } -} diff --git a/crates/ruff/src/rules/flake8_use_pathlib/mod.rs b/crates/ruff/src/rules/flake8_use_pathlib/mod.rs index dc2f6ba566..99f4aca16c 100644 --- a/crates/ruff/src/rules/flake8_use_pathlib/mod.rs +++ b/crates/ruff/src/rules/flake8_use_pathlib/mod.rs @@ -1,5 +1,5 @@ //! Rules from [flake8-use-pathlib](https://pypi.org/project/flake8-use-pathlib/). -pub(crate) mod helpers; +pub(crate) mod rules; pub(crate) mod violations; #[cfg(test)] diff --git a/crates/ruff/src/rules/flake8_use_pathlib/rules/mod.rs b/crates/ruff/src/rules/flake8_use_pathlib/rules/mod.rs new file mode 100644 index 0000000000..33eac782e4 --- /dev/null +++ b/crates/ruff/src/rules/flake8_use_pathlib/rules/mod.rs @@ -0,0 +1,3 @@ +pub(crate) use replaceable_by_pathlib::replaceable_by_pathlib; + +mod replaceable_by_pathlib; diff --git a/crates/ruff/src/rules/flake8_use_pathlib/helpers.rs b/crates/ruff/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs similarity index 100% rename from crates/ruff/src/rules/flake8_use_pathlib/helpers.rs rename to crates/ruff/src/rules/flake8_use_pathlib/rules/replaceable_by_pathlib.rs diff --git a/crates/ruff/src/rules/mccabe/rules.rs b/crates/ruff/src/rules/mccabe/rules/function_is_too_complex.rs similarity index 100% rename from crates/ruff/src/rules/mccabe/rules.rs rename to crates/ruff/src/rules/mccabe/rules/function_is_too_complex.rs diff --git a/crates/ruff/src/rules/mccabe/rules/mod.rs b/crates/ruff/src/rules/mccabe/rules/mod.rs new file mode 100644 index 0000000000..b6898b7da2 --- /dev/null +++ b/crates/ruff/src/rules/mccabe/rules/mod.rs @@ -0,0 +1,3 @@ +pub(crate) use function_is_too_complex::{function_is_too_complex, ComplexStructure}; + +mod function_is_too_complex; diff --git a/crates/ruff/src/settings/defaults.rs b/crates/ruff/src/settings/defaults.rs index 2aac4fa44f..ae7dbc962d 100644 --- a/crates/ruff/src/settings/defaults.rs +++ b/crates/ruff/src/settings/defaults.rs @@ -97,7 +97,7 @@ impl Default for Settings { flake8_quotes: flake8_quotes::settings::Settings::default(), flake8_gettext: flake8_gettext::settings::Settings::default(), flake8_self: flake8_self::settings::Settings::default(), - flake8_tidy_imports: flake8_tidy_imports::Settings::default(), + flake8_tidy_imports: flake8_tidy_imports::settings::Settings::default(), flake8_type_checking: flake8_type_checking::settings::Settings::default(), flake8_unused_arguments: flake8_unused_arguments::settings::Settings::default(), isort: isort::settings::Settings::default(), diff --git a/crates/ruff/src/settings/mod.rs b/crates/ruff/src/settings/mod.rs index f604098fd4..2f31b6d221 100644 --- a/crates/ruff/src/settings/mod.rs +++ b/crates/ruff/src/settings/mod.rs @@ -118,7 +118,7 @@ pub struct Settings { pub flake8_pytest_style: flake8_pytest_style::settings::Settings, pub flake8_quotes: flake8_quotes::settings::Settings, pub flake8_self: flake8_self::settings::Settings, - pub flake8_tidy_imports: flake8_tidy_imports::Settings, + pub flake8_tidy_imports: flake8_tidy_imports::settings::Settings, pub flake8_type_checking: flake8_type_checking::settings::Settings, pub flake8_unused_arguments: flake8_unused_arguments::settings::Settings, pub isort: isort::settings::Settings, diff --git a/crates/ruff/src/settings/pyproject.rs b/crates/ruff/src/settings/pyproject.rs index 76337502cf..6577dedd5c 100644 --- a/crates/ruff/src/settings/pyproject.rs +++ b/crates/ruff/src/settings/pyproject.rs @@ -153,8 +153,7 @@ mod tests { use crate::codes::{self, RuleCodePrefix}; use crate::line_width::LineLength; use crate::rules::flake8_quotes::settings::Quote; - use crate::rules::flake8_tidy_imports::banned_api::ApiBan; - use crate::rules::flake8_tidy_imports::relative_imports::Strictness; + use crate::rules::flake8_tidy_imports::settings::{ApiBan, Strictness}; use crate::rules::{ flake8_bugbear, flake8_builtins, flake8_errmsg, flake8_import_conventions, flake8_pytest_style, flake8_quotes, flake8_tidy_imports, mccabe, pep8_naming, diff --git a/crates/ruff_wasm/src/lib.rs b/crates/ruff_wasm/src/lib.rs index 41a9feef5b..9aeb656786 100644 --- a/crates/ruff_wasm/src/lib.rs +++ b/crates/ruff_wasm/src/lib.rs @@ -148,7 +148,7 @@ pub fn defaultSettings() -> Result { flake8_import_conventions: Some( flake8_import_conventions::settings::Settings::default().into(), ), - flake8_tidy_imports: Some(flake8_tidy_imports::Settings::default().into()), + flake8_tidy_imports: Some(flake8_tidy_imports::settings::Settings::default().into()), flake8_type_checking: Some(flake8_type_checking::settings::Settings::default().into()), flake8_unused_arguments: Some( flake8_unused_arguments::settings::Settings::default().into(),