From 7a85780f7c5ff6617a215d6cffb43e9cc7630a25 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 14 Jan 2024 18:29:01 -0500 Subject: [PATCH] Use importer --- .../resources/test/fixtures/refurb/FURB167.py | 26 ++++++--- .../src/checkers/ast/analyze/expression.rs | 5 +- crates/ruff_linter/src/codes.rs | 2 +- crates/ruff_linter/src/rules/refurb/mod.rs | 2 +- .../ruff_linter/src/rules/refurb/rules/mod.rs | 4 +- ...long_regex_flag.rs => regex_flag_alias.rs} | 40 +++++++++----- ...es__refurb__tests__FURB167_FURB167.py.snap | 54 +++++++++++++------ 7 files changed, 91 insertions(+), 42 deletions(-) rename crates/ruff_linter/src/rules/refurb/rules/{long_regex_flag.rs => regex_flag_alias.rs} (60%) diff --git a/crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py b/crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py index cb1d1e26f61e3c..0903404c15b977 100644 --- a/crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py +++ b/crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py @@ -1,10 +1,22 @@ -import re +def func(): + import re -# BAD: -if re.match("^hello", "hello world", re.I): - pass + # OK + if re.match("^hello", "hello world", re.IGNORECASE): + pass -# GOOD: -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 0dbe8b3a360675..788f184c370a9c 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::long_regex_flag(checker, expr); + } // Ex) List[...] if checker.any_enabled(&[ @@ -293,7 +296,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { } } } - if checker.enabled(Rule::LongRegexFlag) { + if checker.enabled(Rule::RegexFlagAlias) { refurb::rules::long_regex_flag(checker, expr); } if checker.enabled(Rule::DatetimeTimezoneUTC) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index a3232d063fd6d3..6812870a13a32a 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -987,7 +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::LongRegexFlag), + (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 31bab75a2bbab2..fde53bda5f2c55 100644 --- a/crates/ruff_linter/src/rules/refurb/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/mod.rs @@ -28,7 +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::LongRegexFlag, Path::new("FURB167.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 e12c2a8a919aed..ea9fbceef645f4 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/mod.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/mod.rs @@ -5,11 +5,11 @@ pub(crate) use hashlib_digest_hex::*; pub(crate) use if_expr_min_max::*; pub(crate) use implicit_cwd::*; pub(crate) use isinstance_type_none::*; -pub(crate) use long_regex_flag::*; 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::*; @@ -25,11 +25,11 @@ mod hashlib_digest_hex; mod if_expr_min_max; mod implicit_cwd; mod isinstance_type_none; -mod long_regex_flag; 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/long_regex_flag.rs b/crates/ruff_linter/src/rules/refurb/rules/regex_flag_alias.rs similarity index 60% rename from crates/ruff_linter/src/rules/refurb/rules/long_regex_flag.rs rename to crates/ruff_linter/src/rules/refurb/rules/regex_flag_alias.rs index 73c52f226b14f4..f27f463bca9ca6 100644 --- a/crates/ruff_linter/src/rules/refurb/rules/long_regex_flag.rs +++ b/crates/ruff_linter/src/rules/refurb/rules/regex_flag_alias.rs @@ -4,40 +4,48 @@ use ruff_python_ast::Expr; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; +use crate::importer::ImportRequest; /// ## What it does -/// Checks for uses of shorthand regex flags such as `re.I`. +/// Checks for the use of shorthand aliases for regular expression flags +/// (e.g., `re.I` instead of `re.IGNORECASE`). /// /// ## Why is this bad? -/// These single-character flags are not as descriptive as the full names. +/// 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 LongRegexFlag { +pub struct RegexFlagAlias { short: &'static str, long: &'static str, } -impl AlwaysFixableViolation for LongRegexFlag { +impl AlwaysFixableViolation for RegexFlagAlias { #[derive_message_formats] fn message(&self) -> String { - let LongRegexFlag { short, .. } = self; + let RegexFlagAlias { short, .. } = self; format!("Use of shorthand `re.{short}`") } fn fix_title(&self) -> String { - let LongRegexFlag { long, .. } = self; + let RegexFlagAlias { long, .. } = self; format!("Replace with `re.{long}`") } } @@ -62,13 +70,17 @@ pub(crate) fn long_regex_flag(checker: &mut Checker, expr: &Expr) { return; }; - let mut diagnostic = Diagnostic::new(LongRegexFlag { short, long }, expr.range()); - - diagnostic.set_fix(Fix::safe_edit(Edit::replacement( - format!("re.{long}"), - expr.start(), - expr.end(), - ))); - + let mut diagnostic = Diagnostic::new(RegexFlagAlias { short, long }, expr.range()); + diagnostic.try_set_fix(|| { + let (edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("re", long), + expr.start(), + checker.semantic(), + )?; + Ok(Fix::safe_edits( + Edit::range_replacement(binding, expr.range()), + [edit], + )) + }); checker.diagnostics.push(diagnostic); } 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 index 10bd81b9915b4b..6f8cdbabe04ccc 100644 --- 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 @@ -1,23 +1,45 @@ --- source: crates/ruff_linter/src/rules/refurb/mod.rs --- -FURB167.py:4:38: FURB167 [*] Use of shorthand `re.I` - | -3 | # BAD: -4 | if re.match("^hello", "hello world", re.I): - | ^^^^ FURB167 -5 | pass - | - = help: Replace with `re.IGNORECASE` +FURB167.py:13:42: FURB167 [*] Use of shorthand `re.I` + | +12 | # FURB167 +13 | if re.match("^hello", "hello world", re.I): + | ^^^^ FURB167 +14 | pass + | + = help: Replace with `re.IGNORECASE` ℹ Safe fix -1 1 | import re -2 2 | -3 3 | # BAD: -4 |-if re.match("^hello", "hello world", re.I): - 4 |+if re.match("^hello", "hello world", re.IGNORECASE): -5 5 | pass -6 6 | -7 7 | +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 shorthand `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