From 6183b8e98bdfc8403c8770c2cf669dd2ead3230f Mon Sep 17 00:00:00 2001 From: Steve C Date: Sun, 14 Jan 2024 18:40:17 -0500 Subject: [PATCH] [`refurb`] Implement `regex-flag-alias` with fix (`FURB167`) (#9516) ## Summary add [`FURB167`/`use-long-regex-flag`](https://github.com/dosisod/refurb/blob/master/refurb/checks/regex/use_long_flag.py) with autofix See: #1348 ## Test Plan `cargo test` --------- Co-authored-by: Charlie Marsh --- .../resources/test/fixtures/refurb/FURB167.py | 22 +++ .../src/checkers/ast/analyze/expression.rs | 6 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/refurb/mod.rs | 1 + .../ruff_linter/src/rules/refurb/rules/mod.rs | 2 + .../rules/refurb/rules/regex_flag_alias.rs | 133 ++++++++++++++++++ ...es__refurb__tests__FURB167_FURB167.py.snap | 45 ++++++ ruff.schema.json | 1 + 8 files changed, 211 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py create mode 100644 crates/ruff_linter/src/rules/refurb/rules/regex_flag_alias.rs create mode 100644 crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB167_FURB167.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py new file mode 100644 index 0000000000000..0903404c15b97 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py @@ -0,0 +1,22 @@ +def func(): + import re + + # OK + if re.match("^hello", "hello world", re.IGNORECASE): + pass + + +def func(): + import re + + # FURB167 + if re.match("^hello", "hello world", re.I): + pass + + +def func(): + from re import match, I + + # FURB167 + if match("^hello", "hello world", I): + pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index bc2bbeaecc8c1..f8214c6c8924d 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -170,6 +170,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::CollectionsNamedTuple) { flake8_pyi::rules::collections_named_tuple(checker, expr); } + if checker.enabled(Rule::RegexFlagAlias) { + refurb::rules::regex_flag_alias(checker, expr); + } // Ex) List[...] if checker.any_enabled(&[ @@ -293,6 +296,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { } } } + if checker.enabled(Rule::RegexFlagAlias) { + refurb::rules::regex_flag_alias(checker, expr); + } if checker.enabled(Rule::DatetimeTimezoneUTC) { if checker.settings.target_version >= PythonVersion::Py311 { pyupgrade::rules::datetime_utc_alias(checker, expr); diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index d417ef3af9cb5..6812870a13a32 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -987,6 +987,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Refurb, "152") => (RuleGroup::Preview, rules::refurb::rules::MathConstant), (Refurb, "161") => (RuleGroup::Preview, rules::refurb::rules::BitCount), (Refurb, "163") => (RuleGroup::Preview, rules::refurb::rules::RedundantLogBase), + (Refurb, "167") => (RuleGroup::Preview, rules::refurb::rules::RegexFlagAlias), (Refurb, "168") => (RuleGroup::Preview, rules::refurb::rules::IsinstanceTypeNone), (Refurb, "169") => (RuleGroup::Preview, rules::refurb::rules::TypeNoneComparison), (Refurb, "171") => (RuleGroup::Preview, rules::refurb::rules::SingleItemMembershipTest), diff --git a/crates/ruff_linter/src/rules/refurb/mod.rs b/crates/ruff_linter/src/rules/refurb/mod.rs index 35950e2a72b09..fde53bda5f2c5 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -28,6 +28,7 @@ mod tests { #[test_case(Rule::ImplicitCwd, Path::new("FURB177.py"))] #[test_case(Rule::SingleItemMembershipTest, Path::new("FURB171.py"))] #[test_case(Rule::BitCount, Path::new("FURB161.py"))] + #[test_case(Rule::RegexFlagAlias, Path::new("FURB167.py"))] #[test_case(Rule::IsinstanceTypeNone, Path::new("FURB168.py"))] #[test_case(Rule::TypeNoneComparison, Path::new("FURB169.py"))] #[test_case(Rule::RedundantLogBase, Path::new("FURB163.py"))] diff --git a/crates/ruff_linter/src/rules/refurb/rules/mod.rs b/crates/ruff_linter/src/rules/refurb/rules/mod.rs index ff4c02360f7f4..ea9fbceef645f 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -9,6 +9,7 @@ pub(crate) use math_constant::*; pub(crate) use print_empty_string::*; pub(crate) use read_whole_file::*; pub(crate) use redundant_log_base::*; +pub(crate) use regex_flag_alias::*; pub(crate) use reimplemented_operator::*; pub(crate) use reimplemented_starmap::*; pub(crate) use repeated_append::*; @@ -28,6 +29,7 @@ mod math_constant; mod print_empty_string; mod read_whole_file; mod redundant_log_base; +mod regex_flag_alias; mod reimplemented_operator; mod reimplemented_starmap; mod repeated_append; diff --git a/crates/ruff_linter/src/rules/refurb/rules/regex_flag_alias.rs b/crates/ruff_linter/src/rules/refurb/rules/regex_flag_alias.rs new file mode 100644 index 0000000000000..bd3fcdd56c64a --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/rules/regex_flag_alias.rs @@ -0,0 +1,133 @@ +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::Expr; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; +use crate::importer::ImportRequest; + +/// ## What it does +/// Checks for the use of shorthand aliases for regular expression flags +/// (e.g., `re.I` instead of `re.IGNORECASE`). +/// +/// ## Why is this bad? +/// The regular expression module provides descriptive names for each flag, +/// along with single-letter aliases. Prefer the descriptive names, as they +/// are more readable and self-documenting. +/// +/// ## Example +/// ```python +/// import re +/// +/// if re.match("^hello", "hello world", re.I): +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// import re +/// +/// if re.match("^hello", "hello world", re.IGNORECASE): +/// ... +/// ``` +/// +#[violation] +pub struct RegexFlagAlias { + alias: &'static str, + full_name: &'static str, +} + +impl AlwaysFixableViolation for RegexFlagAlias { + #[derive_message_formats] + fn message(&self) -> String { + let RegexFlagAlias { alias, .. } = self; + format!("Use of regular expression alias `re.{alias}`") + } + + fn fix_title(&self) -> String { + let RegexFlagAlias { full_name, .. } = self; + format!("Replace with `re.{full_name}`") + } +} + +/// FURB167 +pub(crate) fn regex_flag_alias(checker: &mut Checker, expr: &Expr) { + let Some(flag) = + checker + .semantic() + .resolve_call_path(expr) + .and_then(|call_path| match call_path.as_slice() { + ["re", "A"] => Some(RegexFlag::Ascii), + ["re", "I"] => Some(RegexFlag::IgnoreCase), + ["re", "L"] => Some(RegexFlag::Locale), + ["re", "M"] => Some(RegexFlag::Multiline), + ["re", "S"] => Some(RegexFlag::DotAll), + ["re", "T"] => Some(RegexFlag::Template), + ["re", "U"] => Some(RegexFlag::Unicode), + ["re", "X"] => Some(RegexFlag::Verbose), + _ => None, + }) + else { + return; + }; + + let mut diagnostic = Diagnostic::new( + RegexFlagAlias { + alias: flag.alias(), + full_name: flag.full_name(), + }, + expr.range(), + ); + diagnostic.try_set_fix(|| { + let (edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("re", flag.full_name()), + expr.start(), + checker.semantic(), + )?; + Ok(Fix::safe_edits( + Edit::range_replacement(binding, expr.range()), + [edit], + )) + }); + checker.diagnostics.push(diagnostic); +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum RegexFlag { + Ascii, + IgnoreCase, + Locale, + Multiline, + DotAll, + Template, + Unicode, + Verbose, +} + +impl RegexFlag { + fn alias(self) -> &'static str { + match self { + Self::Ascii => "A", + Self::IgnoreCase => "I", + Self::Locale => "L", + Self::Multiline => "M", + Self::DotAll => "S", + Self::Template => "T", + Self::Unicode => "U", + Self::Verbose => "X", + } + } + + fn full_name(self) -> &'static str { + match self { + Self::Ascii => "ASCII", + Self::IgnoreCase => "IGNORECASE", + Self::Locale => "LOCALE", + Self::Multiline => "MULTILINE", + Self::DotAll => "DOTALL", + Self::Template => "TEMPLATE", + Self::Unicode => "UNICODE", + Self::Verbose => "VERBOSE", + } + } +} diff --git a/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB167_FURB167.py.snap b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB167_FURB167.py.snap new file mode 100644 index 0000000000000..a95fff1e098b8 --- /dev/null +++ b/crates/ruff_linter/src/rules/refurb/snapshots/ruff_linter__rules__refurb__tests__FURB167_FURB167.py.snap @@ -0,0 +1,45 @@ +--- +source: crates/ruff_linter/src/rules/refurb/mod.rs +--- +FURB167.py:13:42: FURB167 [*] Use of regular expression alias `re.I` + | +12 | # FURB167 +13 | if re.match("^hello", "hello world", re.I): + | ^^^^ FURB167 +14 | pass + | + = help: Replace with `re.IGNORECASE` + +ℹ Safe fix +10 10 | import re +11 11 | +12 12 | # FURB167 +13 |- if re.match("^hello", "hello world", re.I): + 13 |+ if re.match("^hello", "hello world", re.IGNORECASE): +14 14 | pass +15 15 | +16 16 | + +FURB167.py:21:39: FURB167 [*] Use of regular expression alias `re.I` + | +20 | # FURB167 +21 | if match("^hello", "hello world", I): + | ^ FURB167 +22 | pass + | + = help: Replace with `re.IGNORECASE` + +ℹ Safe fix + 1 |+import re +1 2 | def func(): +2 3 | import re +3 4 | +-------------------------------------------------------------------------------- +18 19 | from re import match, I +19 20 | +20 21 | # FURB167 +21 |- if match("^hello", "hello world", I): + 22 |+ if match("^hello", "hello world", re.IGNORECASE): +22 23 | pass + + diff --git a/ruff.schema.json b/ruff.schema.json index cd6bcee1270e9..c85044300ba12 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -2926,6 +2926,7 @@ "FURB16", "FURB161", "FURB163", + "FURB167", "FURB168", "FURB169", "FURB17",