Skip to content

Commit e0a6034

Browse files
snowsignalsnowsignal
authored
Implement RUF027: Missing F-String Syntax lint (#9728)
<!-- Thank you for contributing to Ruff! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary Fixes #8151 This PR implements a new rule, `RUF027`. ## What it does Checks for strings that contain f-string syntax but are not f-strings. ### Why is this bad? An f-string missing an `f` at the beginning won't format anything, and instead treat the interpolation syntax as literal. ### Example ```python name = "Sarah" dayofweek = "Tuesday" msg = "Hello {name}! It is {dayofweek} today!" ``` It should instead be: ```python name = "Sarah" dayofweek = "Tuesday" msg = f"Hello {name}! It is {dayofweek} today!" ``` ## Heuristics Since there are many possible string literals which contain syntax similar to f-strings yet are not intended to be, this lint will disqualify any literal that satisfies any of the following conditions: 1. The string literal is a standalone expression. For example, a docstring. 2. The literal is part of a function call with keyword arguments that match at least one variable (for example: `format("Message: {value}", value = "Hello World")`) 3. The literal (or a parent expression of the literal) has a direct method call on it (for example: `"{value}".format(...)`) 4. The string has no `{...}` expression sections, or uses invalid f-string syntax. 5. The string references variables that are not in scope, or it doesn't capture variables at all. 6. Any format specifiers in the potential f-string are invalid. ## Test Plan I created a new test file, `RUF027.py`, which is both an example of what the lint should catch and a way to test edge cases that may trigger false positives.
1 parent 25d9305 commit e0a6034

8 files changed

Lines changed: 563 additions & 1 deletion

File tree

‎crates/ruff_linter/resources/test/fixtures/ruff/RUF027.py‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
val = 2
2+
3+
def simple_cases():
4+
a = 4
5+
b = "{a}" # RUF027
6+
c = "{a} {b} f'{val}' " # RUF027
7+
8+
def escaped_string():
9+
a = 4
10+
b = "escaped string: {{ brackets surround me }}" # RUF027
11+
12+
def raw_string():
13+
a = 4
14+
b = r"raw string with formatting: {a}" # RUF027
15+
c = r"raw string with \backslashes\ and \"escaped quotes\": {a}" # RUF027
16+
17+
def print_name(name: str):
18+
a = 4
19+
print("Hello, {name}!") # RUF027
20+
print("The test value we're using today is {a}") # RUF027
21+
22+
def do_nothing(a):
23+
return a
24+
25+
def nested_funcs():
26+
a = 4
27+
print(do_nothing(do_nothing("{a}"))) # RUF027
28+
29+
def tripled_quoted():
30+
a = 4
31+
c = a
32+
single_line = """ {a} """ # RUF027
33+
# RUF027
34+
multi_line = a = """b { # comment
35+
c} d
36+
"""
37+
38+
def single_quoted_multi_line():
39+
a = 4
40+
# RUF027
41+
b = " {\
42+
a} \
43+
"
44+
45+
def implicit_concat():
46+
a = 4
47+
b = "{a}" "+" "{b}" r" \\ " # RUF027 for the first part only
48+
print(f"{a}" "{a}" f"{b}") # RUF027
49+
50+
def escaped_chars():
51+
a = 4
52+
b = "\"not escaped:\" \'{a}\' \"escaped:\": \'{{c}}\'" # RUF027
53+
54+
def alternative_formatter(src, **kwargs):
55+
src.format(**kwargs)
56+
57+
def format2(src, *args):
58+
pass
59+
60+
# These should not cause an RUF027 message
61+
def negative_cases():
62+
a = 4
63+
positive = False
64+
"""{a}"""
65+
"don't format: {a}"
66+
c = """ {b} """
67+
d = "bad variable: {invalid}"
68+
e = "incorrect syntax: {}"
69+
json = "{ positive: false }"
70+
json2 = "{ 'positive': false }"
71+
json3 = "{ 'positive': 'false' }"
72+
alternative_formatter("{a}", a = 5)
73+
formatted = "{a}".fmt(a = 7)
74+
print(do_nothing("{a}".format(a=3)))
75+
print(do_nothing(alternative_formatter("{a}", a = 5)))
76+
print(format(do_nothing("{a}"), a = 5))
77+
print("{a}".to_upper())
78+
print(do_nothing("{a}").format(a = "Test"))
79+
print(do_nothing("{a}").format2(a))
80+
81+
a = 4
82+
83+
"always ignore this: {a}"
84+
85+
print("but don't ignore this: {val}") # RUF027

‎crates/ruff_linter/src/checkers/ast/analyze/expression.rs‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1042,6 +1042,16 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
10421042
pyupgrade::rules::unicode_kind_prefix(checker, string_literal);
10431043
}
10441044
}
1045+
if checker.enabled(Rule::MissingFStringSyntax) {
1046+
for string_literal in value.literals() {
1047+
ruff::rules::missing_fstring_syntax(
1048+
&mut checker.diagnostics,
1049+
string_literal,
1050+
checker.locator,
1051+
&checker.semantic,
1052+
);
1053+
}
1054+
}
10451055
}
10461056
Expr::BinOp(ast::ExprBinOp {
10471057
left,
@@ -1313,12 +1323,22 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
13131323
refurb::rules::math_constant(checker, number_literal);
13141324
}
13151325
}
1316-
Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {
1326+
Expr::StringLiteral(ast::ExprStringLiteral { value, range: _ }) => {
13171327
if checker.enabled(Rule::UnicodeKindPrefix) {
13181328
for string_part in value {
13191329
pyupgrade::rules::unicode_kind_prefix(checker, string_part);
13201330
}
13211331
}
1332+
if checker.enabled(Rule::MissingFStringSyntax) {
1333+
for string_literal in value.as_slice() {
1334+
ruff::rules::missing_fstring_syntax(
1335+
&mut checker.diagnostics,
1336+
string_literal,
1337+
checker.locator,
1338+
&checker.semantic,
1339+
);
1340+
}
1341+
}
13221342
}
13231343
Expr::IfExp(
13241344
if_exp @ ast::ExprIfExp {

‎crates/ruff_linter/src/codes.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -937,6 +937,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
937937
(Ruff, "024") => (RuleGroup::Preview, rules::ruff::rules::MutableFromkeysValue),
938938
(Ruff, "025") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryDictComprehensionForIterable),
939939
(Ruff, "026") => (RuleGroup::Preview, rules::ruff::rules::DefaultFactoryKwarg),
940+
(Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax),
940941
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
941942
(Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),
942943
#[cfg(feature = "test-rules")]

‎crates/ruff_linter/src/rules/ruff/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ mod tests {
4646
#[test_case(Rule::MutableFromkeysValue, Path::new("RUF024.py"))]
4747
#[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("RUF025.py"))]
4848
#[test_case(Rule::DefaultFactoryKwarg, Path::new("RUF026.py"))]
49+
#[test_case(Rule::MissingFStringSyntax, Path::new("RUF027.py"))]
4950
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
5051
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
5152
let diagnostics = test_path(

‎crates/ruff_linter/src/rules/ruff/rules/missing_fstring_syntax.rs‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
use memchr::memchr2_iter;
2+
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
3+
use ruff_macros::{derive_message_formats, violation};
4+
use ruff_python_ast::{self as ast};
5+
use ruff_python_literal::format::FormatSpec;
6+
use ruff_python_parser::parse_expression;
7+
use ruff_python_semantic::SemanticModel;
8+
use ruff_source_file::Locator;
9+
use ruff_text_size::{Ranged, TextRange};
10+
use rustc_hash::FxHashSet;
11+
12+
/// ## What it does
13+
/// Checks for strings that contain f-string syntax but are not f-strings.
14+
///
15+
/// ## Why is this bad?
16+
/// An f-string missing an `f` at the beginning won't format anything, and instead
17+
/// treat the interpolation syntax as literal.
18+
///
19+
/// Since there are many possible string literals which contain syntax similar to f-strings yet are not intended to be,
20+
/// this lint will disqualify any literal that satisfies any of the following conditions:
21+
/// 1. The string literal is a standalone expression. For example, a docstring.
22+
/// 2. The literal is part of a function call with keyword arguments that match at least one variable (for example: `format("Message: {value}", value = "Hello World")`)
23+
/// 3. The literal (or a parent expression of the literal) has a direct method call on it (for example: `"{value}".format(...)`)
24+
/// 4. The string has no `{...}` expression sections, or uses invalid f-string syntax.
25+
/// 5. The string references variables that are not in scope, or it doesn't capture variables at all.
26+
/// 6. Any format specifiers in the potential f-string are invalid.
27+
///
28+
/// ## Example
29+
///
30+
/// ```python
31+
/// name = "Sarah"
32+
/// dayofweek = "Tuesday"
33+
/// msg = "Hello {name}! It is {dayofweek} today!"
34+
/// ```
35+
///
36+
/// Use instead:
37+
/// ```python
38+
/// name = "Sarah"
39+
/// dayofweek = "Tuesday"
40+
/// msg = f"Hello {name}! It is {dayofweek} today!"
41+
/// ```
42+
#[violation]
43+
pub struct MissingFStringSyntax;
44+
45+
impl AlwaysFixableViolation for MissingFStringSyntax {
46+
#[derive_message_formats]
47+
fn message(&self) -> String {
48+
format!(r#"Possible f-string without an `f` prefix"#)
49+
}
50+
51+
fn fix_title(&self) -> String {
52+
"Add `f` prefix".into()
53+
}
54+
}
55+
56+
/// RUF027
57+
pub(crate) fn missing_fstring_syntax(
58+
diagnostics: &mut Vec<Diagnostic>,
59+
literal: &ast::StringLiteral,
60+
locator: &Locator,
61+
semantic: &SemanticModel,
62+
) {
63+
// we want to avoid statement expressions that are just a string literal.
64+
// there's no reason to have standalone f-strings and this lets us avoid docstrings too
65+
if let ast::Stmt::Expr(ast::StmtExpr { value, .. }) = semantic.current_statement() {
66+
match value.as_ref() {
67+
ast::Expr::StringLiteral(_) | ast::Expr::FString(_) => return,
68+
_ => {}
69+
}
70+
}
71+
72+
if should_be_fstring(literal, locator, semantic) {
73+
let diagnostic = Diagnostic::new(MissingFStringSyntax, literal.range())
74+
.with_fix(fix_fstring_syntax(literal.range()));
75+
diagnostics.push(diagnostic);
76+
}
77+
}
78+
79+
/// Returns `true` if `literal` is likely an f-string with a missing `f` prefix.
80+
/// See [`MissingFStringSyntax`] for the validation criteria.
81+
fn should_be_fstring(
82+
literal: &ast::StringLiteral,
83+
locator: &Locator,
84+
semantic: &SemanticModel,
85+
) -> bool {
86+
if !has_brackets(&literal.value) {
87+
return false;
88+
}
89+
90+
let Ok(ast::Expr::FString(ast::ExprFString { value, .. })) =
91+
parse_expression(&format!("f{}", locator.slice(literal.range())))
92+
else {
93+
return false;
94+
};
95+
96+
let mut kwargs = vec![];
97+
for expr in semantic.current_expressions() {
98+
match expr {
99+
ast::Expr::Call(ast::ExprCall {
100+
arguments: ast::Arguments { keywords, .. },
101+
func,
102+
..
103+
}) => {
104+
if let ast::Expr::Attribute(ast::ExprAttribute { .. }) = func.as_ref() {
105+
return false;
106+
}
107+
kwargs.extend(keywords.iter());
108+
}
109+
_ => continue,
110+
}
111+
}
112+
113+
let kw_idents: FxHashSet<&str> = kwargs
114+
.iter()
115+
.filter_map(|k| k.arg.as_ref())
116+
.map(ast::Identifier::as_str)
117+
.collect();
118+
119+
for f_string in value.f_strings() {
120+
let mut has_name = false;
121+
for element in f_string
122+
.elements
123+
.iter()
124+
.filter_map(|element| element.as_expression())
125+
{
126+
if let ast::Expr::Name(ast::ExprName { id, .. }) = element.expression.as_ref() {
127+
if kw_idents.contains(id.as_str()) || semantic.lookup_symbol(id).is_none() {
128+
return false;
129+
}
130+
has_name = true;
131+
}
132+
if let Some(spec) = &element.format_spec {
133+
let spec = locator.slice(spec.range());
134+
if FormatSpec::parse(spec).is_err() {
135+
return false;
136+
}
137+
}
138+
}
139+
if !has_name {
140+
return false;
141+
}
142+
}
143+
144+
true
145+
}
146+
147+
// fast check to disqualify any string literal without brackets
148+
#[inline]
149+
fn has_brackets(possible_fstring: &str) -> bool {
150+
// this qualifies rare false positives like "{ unclosed bracket"
151+
// but it's faster in the general case
152+
memchr2_iter(b'{', b'}', possible_fstring.as_bytes()).count() > 1
153+
}
154+
155+
fn fix_fstring_syntax(range: TextRange) -> Fix {
156+
Fix::unsafe_edit(Edit::insertion("f".into(), range.start()))
157+
}

‎crates/ruff_linter/src/rules/ruff/rules/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ pub(crate) use function_call_in_dataclass_default::*;
88
pub(crate) use implicit_optional::*;
99
pub(crate) use invalid_index_type::*;
1010
pub(crate) use invalid_pyproject_toml::*;
11+
pub(crate) use missing_fstring_syntax::*;
1112
pub(crate) use mutable_class_default::*;
1213
pub(crate) use mutable_dataclass_default::*;
1314
pub(crate) use mutable_fromkeys_value::*;
@@ -37,6 +38,7 @@ mod helpers;
3738
mod implicit_optional;
3839
mod invalid_index_type;
3940
mod invalid_pyproject_toml;
41+
mod missing_fstring_syntax;
4042
mod mutable_class_default;
4143
mod mutable_dataclass_default;
4244
mod mutable_fromkeys_value;

0 commit comments

Comments
 (0)