Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[refurb] Implement regex-flag-alias with fix (FURB167) #9516

Merged
merged 2 commits into from
Jan 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/refurb/FURB167.py
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 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,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);
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/refurb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"))]
Expand Down
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/refurb/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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;
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
@@ -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


1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading