diff --git a/README.md b/README.md index fc9aa1bd78099..73ee73104aa9a 100644 --- a/README.md +++ b/README.md @@ -725,6 +725,7 @@ For more, see [pyupgrade](https://pypi.org/project/pyupgrade/3.2.0/) on PyPI. | UP028 | RewriteYieldFrom | Replace `yield` over `for` loop with `yield from` | 🛠 | | UP029 | UnnecessaryBuiltinImport | Unnecessary builtin import: `...` | 🛠 | | UP030 | FormatLiterals | Use implicit references for positional format fields | 🛠 | +| UP032 | FString | Use f-string instead of `format` call | 🛠 | ### pep8-naming (N) diff --git a/resources/test/fixtures/pyupgrade/UP032.py b/resources/test/fixtures/pyupgrade/UP032.py new file mode 100644 index 0000000000000..3964f6daa6cc2 --- /dev/null +++ b/resources/test/fixtures/pyupgrade/UP032.py @@ -0,0 +1,86 @@ +### +# Errors +### + +"{} {}".format(a, b) + +"{1} {0}".format(a, b) + +"{x.y}".format(x=z) + +"{.x} {.y}".format(a, b) + +"{} {}".format(a.b, c.d) + +"{}".format(a()) + +"{}".format(a.b()) + +"{}".format(a.b().c()) + +"hello {}!".format(name) + +"{}{b}{}".format(a, c, b=b) + +"{}".format(0x0) + +"{} {}".format(a, b) + +"""{} {}""".format(a, b) + +"foo{}".format(1) + +r"foo{}".format(1) + +x = "{a}".format(a=1) + +print("foo {} ".format(x)) + +"{a[b]}".format(a=a) + +"{a.a[b]}".format(a=a) + +"{}{{}}{}".format(escaped, y) + +"{}".format(a) + +### +# Non-errors +### + +# False-negative: RustPython doesn't parse the `\N{snowman}`. +"\N{snowman} {}".format(a) + +"{".format(a) + +"}".format(a) + +"{} {}".format(*a) + +"{0} {0}".format(arg) + +"{x} {x}".format(arg) + +"{x.y} {x.z}".format(arg) + +b"{} {}".format(a, b) + +"{:{}}".format(x, y) + +"{}{}".format(a) + +"" "{}".format(a["\\"]) + +"{}".format(a["b"]) + +r'"\N{snowman} {}".format(a)' + +"{a}" "{b}".format(a=1, b=1) + + +async def c(): + return "{}".format(await 3) + + +async def c(): + return "{}".format(1 + await 3) diff --git a/ruff.schema.json b/ruff.schema.json index 821e604156efb..b1bcc896a35c5 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -1687,6 +1687,7 @@ "UP029", "UP03", "UP030", + "UP032", "W", "W2", "W29", diff --git a/src/checkers/ast.rs b/src/checkers/ast.rs index 21c7d898c33c4..cf642b9f5e65c 100644 --- a/src/checkers/ast.rs +++ b/src/checkers/ast.rs @@ -1867,6 +1867,7 @@ where || self.settings.enabled.contains(&RuleCode::F525) // pyupgrade || self.settings.enabled.contains(&RuleCode::UP030) + || self.settings.enabled.contains(&RuleCode::UP032) { if let ExprKind::Attribute { value, attr, .. } = &func.node { if let ExprKind::Constant { @@ -1890,8 +1891,8 @@ where } Ok(summary) => { if self.settings.enabled.contains(&RuleCode::F522) { - pyflakes::rules::string_dot_format_extra_named_arguments(self, - &summary, keywords, location, + pyflakes::rules::string_dot_format_extra_named_arguments( + self, &summary, keywords, location, ); } @@ -1917,6 +1918,10 @@ where if self.settings.enabled.contains(&RuleCode::UP030) { pyupgrade::rules::format_literals(self, &summary, expr); } + + if self.settings.enabled.contains(&RuleCode::UP032) { + pyupgrade::rules::f_strings(self, &summary, expr); + } } } } diff --git a/src/docstrings/constants.rs b/src/docstrings/constants.rs index efdd0a1602fd2..3473b0c841cc5 100644 --- a/src/docstrings/constants.rs +++ b/src/docstrings/constants.rs @@ -7,3 +7,7 @@ pub const TRIPLE_QUOTE_PREFIXES: &[&str] = &[ pub const SINGLE_QUOTE_PREFIXES: &[&str] = &[ "u\"", "u'", "r\"", "r'", "u\"", "u'", "r\"", "r'", "U\"", "U'", "R\"", "R'", "\"", "'", ]; + +pub const TRIPLE_QUOTE_SUFFIXES: &[&str] = &["\"\"\"", "'''"]; + +pub const SINGLE_QUOTE_SUFFIXES: &[&str] = &["\"", "'"]; diff --git a/src/registry.rs b/src/registry.rs index 96d9793fb2ab5..9878b667629ef 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -254,6 +254,7 @@ ruff_macros::define_rule_mapping!( UP028 => violations::RewriteYieldFrom, UP029 => violations::UnnecessaryBuiltinImport, UP030 => violations::FormatLiterals, + UP032 => violations::FString, // pydocstyle D100 => violations::PublicModule, D101 => violations::PublicClass, diff --git a/src/rules/pydocstyle/helpers.rs b/src/rules/pydocstyle/helpers.rs index 49138be22a670..6164b1c8ce463 100644 --- a/src/rules/pydocstyle/helpers.rs +++ b/src/rules/pydocstyle/helpers.rs @@ -30,6 +30,14 @@ pub fn leading_quote(content: &str) -> Option<&str> { None } +/// Return the trailing quote string for a docstring (e.g., `"""`). +pub fn trailing_quote(content: &str) -> Option<&&str> { + constants::TRIPLE_QUOTE_SUFFIXES + .iter() + .chain(constants::SINGLE_QUOTE_SUFFIXES) + .find(|&pattern| content.ends_with(pattern)) +} + /// Return the index of the first logical line in a string. pub fn logical_line(content: &str) -> Option { // Find the first logical line. diff --git a/src/rules/pyupgrade/mod.rs b/src/rules/pyupgrade/mod.rs index 16c3eb721bfa6..493f112640ee2 100644 --- a/src/rules/pyupgrade/mod.rs +++ b/src/rules/pyupgrade/mod.rs @@ -53,6 +53,7 @@ mod tests { #[test_case(RuleCode::UP029, Path::new("UP029.py"); "UP029")] #[test_case(RuleCode::UP030, Path::new("UP030_0.py"); "UP030_0")] #[test_case(RuleCode::UP030, Path::new("UP030_1.py"); "UP030_1")] + #[test_case(RuleCode::UP032, Path::new("UP032.py"); "UP032")] fn rules(rule_code: RuleCode, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.as_ref(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/src/rules/pyupgrade/rules/f_strings.rs b/src/rules/pyupgrade/rules/f_strings.rs new file mode 100644 index 0000000000000..76c5c33956d7f --- /dev/null +++ b/src/rules/pyupgrade/rules/f_strings.rs @@ -0,0 +1,274 @@ +use rustc_hash::FxHashMap; +use rustpython_ast::{Constant, Expr, ExprKind, KeywordData}; +use rustpython_common::format::{ + FieldName, FieldNamePart, FieldType, FormatPart, FormatString, FromTemplate, +}; +use rustpython_parser::lexer; +use rustpython_parser::lexer::Tok; + +use crate::ast::types::Range; +use crate::checkers::ast::Checker; +use crate::fix::Fix; +use crate::registry::{Diagnostic, RuleCode}; +use crate::rules::pydocstyle::helpers::{leading_quote, trailing_quote}; +use crate::rules::pyflakes::format::FormatSummary; +use crate::violations; + +/// Like [`FormatSummary`], but maps positional and keyword arguments to their +/// values. For example, given `{a} {b}".format(a=1, b=2)`, `FormatFunction` +/// would include `"a"` and `'b'` in `kwargs`, mapped to `1` and `2` +/// respectively. +#[derive(Debug)] +struct FormatSummaryValues<'a> { + args: Vec, + kwargs: FxHashMap<&'a str, String>, +} + +impl<'a> FormatSummaryValues<'a> { + fn try_from_expr(checker: &'a Checker, expr: &'a Expr) -> Option { + let mut extracted_args: Vec = Vec::new(); + let mut extracted_kwargs: FxHashMap<&str, String> = FxHashMap::default(); + if let ExprKind::Call { args, keywords, .. } = &expr.node { + for arg in args { + let arg = checker + .locator + .slice_source_code_range(&Range::from_located(arg)); + if contains_invalids(&arg) { + return None; + } + extracted_args.push(arg.to_string()); + } + for keyword in keywords { + let KeywordData { arg, value } = &keyword.node; + if let Some(key) = arg { + let kwarg = checker + .locator + .slice_source_code_range(&Range::from_located(value)); + if contains_invalids(&kwarg) { + return None; + } + extracted_kwargs.insert(key, kwarg.to_string()); + } + } + } + + if extracted_args.is_empty() && extracted_kwargs.is_empty() { + return None; + } + + Some(Self { + args: extracted_args, + kwargs: extracted_kwargs, + }) + } + + fn consume_next(&mut self) -> Option { + if self.args.is_empty() { + None + } else { + Some(self.args.remove(0)) + } + } + + fn consume_arg(&mut self, index: usize) -> Option { + if self.args.len() > index { + Some(self.args.remove(index)) + } else { + None + } + } + + fn consume_kwarg(&mut self, key: &str) -> Option { + self.kwargs.remove(key) + } +} + +/// Return `true` if the string contains characters that are forbidden in +/// argument identifier. +fn contains_invalids(string: &str) -> bool { + string.contains('*') + || string.contains('\'') + || string.contains('"') + || string.contains("await") +} + +/// Generate an f-string from an [`Expr`]. +fn try_convert_to_f_string(checker: &Checker, expr: &Expr) -> Option { + let ExprKind::Call { func, .. } = &expr.node else { + return None; + }; + let ExprKind::Attribute { value, .. } = &func.node else { + return None; + }; + if !matches!( + &value.node, + ExprKind::Constant { + value: Constant::Str(..), + .. + }, + ) { + return None; + }; + + let Some(mut summary) = FormatSummaryValues::try_from_expr(checker, expr) else { + return None; + }; + + let contents = checker + .locator + .slice_source_code_range(&Range::from_located(value)); + + // Tokenize: we need to avoid trying to fix implicit string concatenations. + if lexer::make_tokenizer(&contents) + .flatten() + .filter(|(_, tok, _)| matches!(tok, Tok::String { .. })) + .count() + > 1 + { + return None; + } + + // Strip the unicode prefix. It's redundant in Python 3, and invalid when used + // with f-strings. + let contents = if contents.starts_with('U') || contents.starts_with('u') { + &contents[1..] + } else { + &contents + }; + if contents.is_empty() { + return None; + } + + // Remove the leading and trailing quotes. + let Some(leading_quote) = leading_quote(contents) else { + return None; + }; + let Some(trailing_quote) = trailing_quote(contents) else { + return None; + }; + let contents = &contents[leading_quote.len()..contents.len() - trailing_quote.len()]; + + // Parse the format string. + let Ok(format_string) = FormatString::from_str(contents) else { + return None; + }; + + let mut converted = String::with_capacity(contents.len()); + for part in format_string.format_parts { + match part { + FormatPart::Field { + field_name, + preconversion_spec, + format_spec, + } => { + converted.push('{'); + + let field = FieldName::parse(&field_name).ok()?; + match field.field_type { + FieldType::Auto => { + let Some(arg) = summary.consume_next() else { + return None; + }; + converted.push_str(&arg); + } + FieldType::Index(index) => { + let Some(arg) = summary.consume_arg(index) else { + return None; + }; + converted.push_str(&arg); + } + FieldType::Keyword(name) => { + let Some(arg) = summary.consume_kwarg(&name) else { + return None; + }; + converted.push_str(&arg); + } + } + + for part in field.parts { + match part { + FieldNamePart::Attribute(name) => { + converted.push('.'); + converted.push_str(&name); + } + FieldNamePart::Index(index) => { + converted.push('['); + converted.push_str(index.to_string().as_str()); + converted.push(']'); + } + FieldNamePart::StringIndex(index) => { + converted.push('['); + converted.push_str(&index); + converted.push(']'); + } + } + } + + if let Some(preconversion_spec) = preconversion_spec { + converted.push('!'); + converted.push(preconversion_spec); + } + + if !format_spec.is_empty() { + converted.push(':'); + converted.push_str(&format_spec); + } + + converted.push('}'); + } + FormatPart::Literal(value) => { + if value.starts_with('{') { + converted.push('{'); + } + converted.push_str(&value); + if value.ends_with('}') { + converted.push('}'); + } + } + } + } + + // Construct the format string. + let mut contents = String::with_capacity(1 + converted.len()); + contents.push('f'); + contents.push_str(leading_quote); + contents.push_str(&converted); + contents.push_str(trailing_quote); + Some(contents) +} + +/// UP032 +pub(crate) fn f_strings(checker: &mut Checker, summary: &FormatSummary, expr: &Expr) { + if summary.has_nested_parts { + return; + } + + // Avoid refactoring multi-line strings. + if expr.location.row() != expr.end_location.unwrap().row() { + return; + } + + // Currently, the only issue we know of is in LibCST: + // https://github.com/Instagram/LibCST/issues/846 + let Some(contents) = try_convert_to_f_string(checker, expr) else { + return; + }; + + // Avoid refactors that increase the resulting string length. + let existing = checker + .locator + .slice_source_code_range(&Range::from_located(expr)); + if contents.len() > existing.len() { + return; + } + + let mut diagnostic = Diagnostic::new(violations::FString, Range::from_located(expr)); + if checker.patch(&RuleCode::UP032) { + diagnostic.amend(Fix::replacement( + contents, + expr.location, + expr.end_location.unwrap(), + )); + }; + checker.diagnostics.push(diagnostic); +} diff --git a/src/rules/pyupgrade/rules/mod.rs b/src/rules/pyupgrade/rules/mod.rs index e3af3e53377af..7c9576d720b43 100644 --- a/src/rules/pyupgrade/rules/mod.rs +++ b/src/rules/pyupgrade/rules/mod.rs @@ -2,6 +2,7 @@ pub(crate) use convert_named_tuple_functional_to_class::convert_named_tuple_func pub(crate) use convert_typed_dict_functional_to_class::convert_typed_dict_functional_to_class; pub(crate) use datetime_utc_alias::datetime_utc_alias; pub(crate) use deprecated_unittest_alias::deprecated_unittest_alias; +pub(crate) use f_strings::f_strings; pub(crate) use format_literals::format_literals; pub(crate) use native_literals::native_literals; use once_cell::sync::Lazy; @@ -31,7 +32,7 @@ pub(crate) use use_pep604_annotation::use_pep604_annotation; pub(crate) use useless_metaclass_type::useless_metaclass_type; pub(crate) use useless_object_inheritance::useless_object_inheritance; -use crate::ast::helpers::{self}; +use crate::ast::helpers; use crate::ast::types::{Range, Scope, ScopeKind}; use crate::fix::Fix; use crate::registry::Diagnostic; @@ -41,6 +42,7 @@ mod convert_named_tuple_functional_to_class; mod convert_typed_dict_functional_to_class; mod datetime_utc_alias; mod deprecated_unittest_alias; +mod f_strings; mod format_literals; mod native_literals; mod open_alias; diff --git a/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP032_UP032.py.snap b/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP032_UP032.py.snap new file mode 100644 index 0000000000000..e3e29cb71c4c4 --- /dev/null +++ b/src/rules/pyupgrade/snapshots/ruff__rules__pyupgrade__tests__UP032_UP032.py.snap @@ -0,0 +1,362 @@ +--- +source: src/rules/pyupgrade/mod.rs +expression: diagnostics +--- +- kind: + FString: ~ + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 20 + fix: + content: "f\"{a} {b}\"" + location: + row: 5 + column: 0 + end_location: + row: 5 + column: 20 + parent: ~ +- kind: + FString: ~ + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 22 + fix: + content: "f\"{b} {a}\"" + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 22 + parent: ~ +- kind: + FString: ~ + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 19 + fix: + content: "f\"{z.y}\"" + location: + row: 9 + column: 0 + end_location: + row: 9 + column: 19 + parent: ~ +- kind: + FString: ~ + location: + row: 11 + column: 0 + end_location: + row: 11 + column: 24 + fix: + content: "f\"{a.x} {b.y}\"" + location: + row: 11 + column: 0 + end_location: + row: 11 + column: 24 + parent: ~ +- kind: + FString: ~ + location: + row: 13 + column: 0 + end_location: + row: 13 + column: 24 + fix: + content: "f\"{a.b} {c.d}\"" + location: + row: 13 + column: 0 + end_location: + row: 13 + column: 24 + parent: ~ +- kind: + FString: ~ + location: + row: 15 + column: 0 + end_location: + row: 15 + column: 16 + fix: + content: "f\"{a()}\"" + location: + row: 15 + column: 0 + end_location: + row: 15 + column: 16 + parent: ~ +- kind: + FString: ~ + location: + row: 17 + column: 0 + end_location: + row: 17 + column: 18 + fix: + content: "f\"{a.b()}\"" + location: + row: 17 + column: 0 + end_location: + row: 17 + column: 18 + parent: ~ +- kind: + FString: ~ + location: + row: 19 + column: 0 + end_location: + row: 19 + column: 22 + fix: + content: "f\"{a.b().c()}\"" + location: + row: 19 + column: 0 + end_location: + row: 19 + column: 22 + parent: ~ +- kind: + FString: ~ + location: + row: 21 + column: 0 + end_location: + row: 21 + column: 24 + fix: + content: "f\"hello {name}!\"" + location: + row: 21 + column: 0 + end_location: + row: 21 + column: 24 + parent: ~ +- kind: + FString: ~ + location: + row: 23 + column: 0 + end_location: + row: 23 + column: 27 + fix: + content: "f\"{a}{b}{c}\"" + location: + row: 23 + column: 0 + end_location: + row: 23 + column: 27 + parent: ~ +- kind: + FString: ~ + location: + row: 25 + column: 0 + end_location: + row: 25 + column: 16 + fix: + content: "f\"{0x0}\"" + location: + row: 25 + column: 0 + end_location: + row: 25 + column: 16 + parent: ~ +- kind: + FString: ~ + location: + row: 27 + column: 0 + end_location: + row: 27 + column: 20 + fix: + content: "f\"{a} {b}\"" + location: + row: 27 + column: 0 + end_location: + row: 27 + column: 20 + parent: ~ +- kind: + FString: ~ + location: + row: 29 + column: 0 + end_location: + row: 29 + column: 24 + fix: + content: "f\"\"\"{a} {b}\"\"\"" + location: + row: 29 + column: 0 + end_location: + row: 29 + column: 24 + parent: ~ +- kind: + FString: ~ + location: + row: 31 + column: 0 + end_location: + row: 31 + column: 17 + fix: + content: "f\"foo{1}\"" + location: + row: 31 + column: 0 + end_location: + row: 31 + column: 17 + parent: ~ +- kind: + FString: ~ + location: + row: 33 + column: 0 + end_location: + row: 33 + column: 18 + fix: + content: "fr\"foo{1}\"" + location: + row: 33 + column: 0 + end_location: + row: 33 + column: 18 + parent: ~ +- kind: + FString: ~ + location: + row: 35 + column: 4 + end_location: + row: 35 + column: 21 + fix: + content: "f\"{1}\"" + location: + row: 35 + column: 4 + end_location: + row: 35 + column: 21 + parent: ~ +- kind: + FString: ~ + location: + row: 37 + column: 6 + end_location: + row: 37 + column: 25 + fix: + content: "f\"foo {x} \"" + location: + row: 37 + column: 6 + end_location: + row: 37 + column: 25 + parent: ~ +- kind: + FString: ~ + location: + row: 39 + column: 0 + end_location: + row: 39 + column: 20 + fix: + content: "f\"{a[b]}\"" + location: + row: 39 + column: 0 + end_location: + row: 39 + column: 20 + parent: ~ +- kind: + FString: ~ + location: + row: 41 + column: 0 + end_location: + row: 41 + column: 22 + fix: + content: "f\"{a.a[b]}\"" + location: + row: 41 + column: 0 + end_location: + row: 41 + column: 22 + parent: ~ +- kind: + FString: ~ + location: + row: 43 + column: 0 + end_location: + row: 43 + column: 29 + fix: + content: "f\"{escaped}{{}}{y}\"" + location: + row: 43 + column: 0 + end_location: + row: 43 + column: 29 + parent: ~ +- kind: + FString: ~ + location: + row: 45 + column: 0 + end_location: + row: 45 + column: 14 + fix: + content: "f\"{a}\"" + location: + row: 45 + column: 0 + end_location: + row: 45 + column: 14 + parent: ~ + diff --git a/src/source_code/stylist.rs b/src/source_code/stylist.rs index 8d20da4ea73b4..073b3d6042eeb 100644 --- a/src/source_code/stylist.rs +++ b/src/source_code/stylist.rs @@ -61,6 +61,15 @@ impl Default for Quote { } } +impl From for char { + fn from(val: Quote) -> Self { + match val { + Quote::Single => '\'', + Quote::Double => '"', + } + } +} + impl From<&Quote> for vendor::str::Quote { fn from(val: &Quote) -> Self { match val { diff --git a/src/violations.rs b/src/violations.rs index ca01eaa7f377a..a329df0bcbe00 100644 --- a/src/violations.rs +++ b/src/violations.rs @@ -3797,6 +3797,23 @@ impl AlwaysAutofixableViolation for FormatLiterals { } } +define_violation!( + pub struct FString; +); +impl AlwaysAutofixableViolation for FString { + fn message(&self) -> String { + "Use f-string instead of `format` call".to_string() + } + + fn autofix_title(&self) -> String { + "Convert to f-string".to_string() + } + + fn placeholder() -> Self { + FString + } +} + // pydocstyle define_violation!(