Skip to content

Commit

Permalink
Implement RUF010 to detect explicit type conversions within f-strin…
Browse files Browse the repository at this point in the history
…gs (#4387)
  • Loading branch information
LotemAm committed May 12, 2023
1 parent a6176d2 commit 52f6663
Show file tree
Hide file tree
Showing 9 changed files with 218 additions and 0 deletions.
23 changes: 23 additions & 0 deletions crates/ruff/resources/test/fixtures/ruff/RUF010.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
bla = b"bla"


def foo(one_arg):
pass


f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010

f"{foo(bla)}" # OK

f"{str(bla, 'ascii')}, {str(bla, encoding='cp1255')}" # OK

f"{bla!s} {[]!r} {'bar'!a}" # OK

"Not an f-string {str(bla)}, {repr(bla)}, {ascii(bla)}" # OK


def ascii(arg):
pass


f"{ascii(bla)}" # OK
11 changes: 11 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3643,6 +3643,17 @@ where
flake8_simplify::rules::expr_and_false(self, expr);
}
}
ExprKind::FormattedValue(ast::ExprFormattedValue {
value, conversion, ..
}) => {
if self
.settings
.rules
.enabled(Rule::ExplicitFStringTypeConversion)
{
ruff::rules::explicit_f_string_type_conversion(self, expr, value, *conversion);
}
}
_ => {}
};

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Ruff, "007") => Rule::PairwiseOverZipped,
(Ruff, "008") => Rule::MutableDataclassDefault,
(Ruff, "009") => Rule::FunctionCallInDataclassDefaultArgument,
(Ruff, "010") => Rule::ExplicitFStringTypeConversion,
(Ruff, "100") => Rule::UnusedNOQA,

// flake8-django
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,7 @@ ruff_macros::register_rules!(
rules::ruff::rules::PairwiseOverZipped,
rules::ruff::rules::MutableDataclassDefault,
rules::ruff::rules::FunctionCallInDataclassDefaultArgument,
rules::ruff::rules::ExplicitFStringTypeConversion,
// flake8-django
rules::flake8_django::rules::DjangoNullableModelStringField,
rules::flake8_django::rules::DjangoLocalsInRenderFunction,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ mod tests {
use crate::test::test_path;
use crate::{assert_messages, settings};

#[test_case(Rule::ExplicitFStringTypeConversion, Path::new("RUF010.py"); "RUF010")]
#[test_case(Rule::CollectionLiteralConcatenation, Path::new("RUF005.py"); "RUF005")]
#[test_case(Rule::AsyncioDanglingTask, Path::new("RUF006.py"); "RUF006")]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
Expand Down
113 changes: 113 additions & 0 deletions crates/ruff/src/rules/ruff/rules/explicit_f_string_type_conversion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
use ruff_text_size::TextSize;
use rustpython_parser::ast::{self, Expr, ExprKind};

use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::helpers::unparse_expr;

use crate::checkers::ast::Checker;
use crate::registry::AsRule;

/// ## What it does
/// Checks for usages of `str()`, `repr()`, and `ascii()` as explicit type
/// conversions within f-strings.
///
/// ## Why is this bad?
/// f-strings support dedicated conversion flags for these types, which are
/// more succinct and idiomatic.
///
/// ## Example
/// ```python
/// a = "some string"
/// f"{repr(a)}"
/// ```
///
/// Use instead:
/// ```python
/// a = "some string"
/// f"{a!r}"
/// ```
#[violation]
pub struct ExplicitFStringTypeConversion;

impl AlwaysAutofixableViolation for ExplicitFStringTypeConversion {
#[derive_message_formats]
fn message(&self) -> String {
format!("Use conversion in f-string")
}

fn autofix_title(&self) -> String {
"Replace f-string function call with conversion".to_string()
}
}

/// RUF010
pub(crate) fn explicit_f_string_type_conversion(
checker: &mut Checker,
expr: &Expr,
formatted_value: &Expr,
conversion: ast::Int,
) {
// Skip if there's already a conversion flag.
if conversion != ast::ConversionFlag::None as u32 {
return;
}

let ExprKind::Call(ast::ExprCall {
func,
args,
keywords,
}) = &formatted_value.node else {
return;
};

// Can't be a conversion otherwise.
if args.len() != 1 || !keywords.is_empty() {
return;
}

let ExprKind::Name(ast::ExprName { id, .. }) = &func.node else {
return;
};

if !matches!(id.as_str(), "str" | "repr" | "ascii") {
return;
};

if !checker.ctx.is_builtin(id) {
return;
}

let mut diagnostic = Diagnostic::new(ExplicitFStringTypeConversion, formatted_value.range());

if checker.patch(diagnostic.kind.rule()) {
// Replace the call node with its argument and a conversion flag.
let mut conv_expr = expr.clone();
let ExprKind::FormattedValue(ast::ExprFormattedValue {
ref mut conversion,
ref mut value,
..
}) = conv_expr.node else {
return;
};

*conversion = match id.as_str() {
"ascii" => ast::Int::new(ast::ConversionFlag::Ascii as u32),
"str" => ast::Int::new(ast::ConversionFlag::Str as u32),
"repr" => ast::Int::new(ast::ConversionFlag::Repr as u32),
&_ => unreachable!(),
};

value.node = args[0].node.clone();

diagnostic.set_fix(Fix::automatic(Edit::range_replacement(
unparse_expr(&conv_expr, checker.stylist),
formatted_value
.range()
.sub_start(TextSize::from(1))
.add_end(TextSize::from(1)),
)));
}

checker.diagnostics.push(diagnostic);
}
5 changes: 5 additions & 0 deletions crates/ruff/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ mod ambiguous_unicode_character;
mod asyncio_dangling_task;
mod collection_literal_concatenation;
mod confusables;
mod explicit_f_string_type_conversion;
mod mutable_defaults_in_dataclass_fields;
mod pairwise_over_zipped;
mod unused_noqa;
Expand All @@ -21,6 +22,10 @@ pub(crate) use mutable_defaults_in_dataclass_fields::{
pub(crate) use pairwise_over_zipped::{pairwise_over_zipped, PairwiseOverZipped};
pub(crate) use unused_noqa::{UnusedCodes, UnusedNOQA};

pub(crate) use explicit_f_string_type_conversion::{
explicit_f_string_type_conversion, ExplicitFStringTypeConversion,
};

#[derive(Clone, Copy)]
pub(crate) enum Context {
String,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
---
source: crates/ruff/src/rules/ruff/mod.rs
---
RUF010.py:8:4: RUF010 [*] Use conversion in f-string
|
8 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010
| ^^^^^^^^ RUF010
9 |
10 | f"{foo(bla)}" # OK
|
= help: Replace f-string function call with conversion

ℹ Fix
5 5 | pass
6 6 |
7 7 |
8 |-f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010
8 |+f"{bla!s}, {repr(bla)}, {ascii(bla)}" # RUF010
9 9 |
10 10 | f"{foo(bla)}" # OK
11 11 |

RUF010.py:8:16: RUF010 [*] Use conversion in f-string
|
8 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010
| ^^^^^^^^^ RUF010
9 |
10 | f"{foo(bla)}" # OK
|
= help: Replace f-string function call with conversion

ℹ Fix
5 5 | pass
6 6 |
7 7 |
8 |-f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010
8 |+f"{str(bla)}, {bla!r}, {ascii(bla)}" # RUF010
9 9 |
10 10 | f"{foo(bla)}" # OK
11 11 |

RUF010.py:8:29: RUF010 [*] Use conversion in f-string
|
8 | f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010
| ^^^^^^^^^^ RUF010
9 |
10 | f"{foo(bla)}" # OK
|
= help: Replace f-string function call with conversion

ℹ Fix
5 5 | pass
6 6 |
7 7 |
8 |-f"{str(bla)}, {repr(bla)}, {ascii(bla)}" # RUF010
8 |+f"{str(bla)}, {repr(bla)}, {bla!a}" # RUF010
9 9 |
10 10 | f"{foo(bla)}" # OK
11 11 |


2 changes: 2 additions & 0 deletions ruff.schema.json

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

0 comments on commit 52f6663

Please sign in to comment.