Skip to content

Commit

Permalink
Use importer
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 14, 2024
1 parent 6644a9b commit de5f663
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 103 deletions.
26 changes: 19 additions & 7 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py
Original file line number Diff line number Diff line change
@@ -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
7 changes: 5 additions & 2 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(&[
Expand Down Expand Up @@ -293,8 +296,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
}
if checker.enabled(Rule::LongRegexFlag) {
refurb::rules::long_regex_flag(checker, expr);
if checker.enabled(Rule::RegexFlagAlias) {
refurb::rules::regex_flag_alias(checker, expr);
}
if checker.enabled(Rule::DatetimeTimezoneUTC) {
if checker.settings.target_version >= PythonVersion::Py311 {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
74 changes: 0 additions & 74 deletions crates/ruff_linter/src/rules/refurb/rules/long_regex_flag.rs

This file was deleted.

4 changes: 2 additions & 2 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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;
Expand Down
133 changes: 133 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/regex_flag_alias.rs
Original file line number Diff line number Diff line change
@@ -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",
}
}
}
Original file line number Diff line number Diff line change
@@ -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 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
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 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


0 comments on commit de5f663

Please sign in to comment.