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 7a85780
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 42 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
5 changes: 4 additions & 1 deletion 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::long_regex_flag(checker, expr);
}

// Ex) List[...]
if checker.any_enabled(&[
Expand Down Expand Up @@ -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) {
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -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}`")
}
}
Expand All @@ -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);
}
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 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


0 comments on commit 7a85780

Please sign in to comment.