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.
This commit is contained in:
Jane Lewis 2024-02-02 16:21:03 -08:00 committed by GitHub
parent 25d93053da
commit e0a6034cbb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 563 additions and 1 deletions

View file

@ -0,0 +1,85 @@
val = 2
def simple_cases():
a = 4
b = "{a}" # RUF027
c = "{a} {b} f'{val}' " # RUF027
def escaped_string():
a = 4
b = "escaped string: {{ brackets surround me }}" # RUF027
def raw_string():
a = 4
b = r"raw string with formatting: {a}" # RUF027
c = r"raw string with \backslashes\ and \"escaped quotes\": {a}" # RUF027
def print_name(name: str):
a = 4
print("Hello, {name}!") # RUF027
print("The test value we're using today is {a}") # RUF027
def do_nothing(a):
return a
def nested_funcs():
a = 4
print(do_nothing(do_nothing("{a}"))) # RUF027
def tripled_quoted():
a = 4
c = a
single_line = """ {a} """ # RUF027
# RUF027
multi_line = a = """b { # comment
c} d
"""
def single_quoted_multi_line():
a = 4
# RUF027
b = " {\
a} \
"
def implicit_concat():
a = 4
b = "{a}" "+" "{b}" r" \\ " # RUF027 for the first part only
print(f"{a}" "{a}" f"{b}") # RUF027
def escaped_chars():
a = 4
b = "\"not escaped:\" \'{a}\' \"escaped:\": \'{{c}}\'" # RUF027
def alternative_formatter(src, **kwargs):
src.format(**kwargs)
def format2(src, *args):
pass
# These should not cause an RUF027 message
def negative_cases():
a = 4
positive = False
"""{a}"""
"don't format: {a}"
c = """ {b} """
d = "bad variable: {invalid}"
e = "incorrect syntax: {}"
json = "{ positive: false }"
json2 = "{ 'positive': false }"
json3 = "{ 'positive': 'false' }"
alternative_formatter("{a}", a = 5)
formatted = "{a}".fmt(a = 7)
print(do_nothing("{a}".format(a=3)))
print(do_nothing(alternative_formatter("{a}", a = 5)))
print(format(do_nothing("{a}"), a = 5))
print("{a}".to_upper())
print(do_nothing("{a}").format(a = "Test"))
print(do_nothing("{a}").format2(a))
a = 4
"always ignore this: {a}"
print("but don't ignore this: {val}") # RUF027

View file

@ -1042,6 +1042,16 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
pyupgrade::rules::unicode_kind_prefix(checker, string_literal);
}
}
if checker.enabled(Rule::MissingFStringSyntax) {
for string_literal in value.literals() {
ruff::rules::missing_fstring_syntax(
&mut checker.diagnostics,
string_literal,
checker.locator,
&checker.semantic,
);
}
}
}
Expr::BinOp(ast::ExprBinOp {
left,
@ -1313,12 +1323,22 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
refurb::rules::math_constant(checker, number_literal);
}
}
Expr::StringLiteral(ast::ExprStringLiteral { value, .. }) => {
Expr::StringLiteral(ast::ExprStringLiteral { value, range: _ }) => {
if checker.enabled(Rule::UnicodeKindPrefix) {
for string_part in value {
pyupgrade::rules::unicode_kind_prefix(checker, string_part);
}
}
if checker.enabled(Rule::MissingFStringSyntax) {
for string_literal in value.as_slice() {
ruff::rules::missing_fstring_syntax(
&mut checker.diagnostics,
string_literal,
checker.locator,
&checker.semantic,
);
}
}
}
Expr::IfExp(
if_exp @ ast::ExprIfExp {

View file

@ -937,6 +937,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "024") => (RuleGroup::Preview, rules::ruff::rules::MutableFromkeysValue),
(Ruff, "025") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryDictComprehensionForIterable),
(Ruff, "026") => (RuleGroup::Preview, rules::ruff::rules::DefaultFactoryKwarg),
(Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml),
#[cfg(feature = "test-rules")]

View file

@ -46,6 +46,7 @@ mod tests {
#[test_case(Rule::MutableFromkeysValue, Path::new("RUF024.py"))]
#[test_case(Rule::UnnecessaryDictComprehensionForIterable, Path::new("RUF025.py"))]
#[test_case(Rule::DefaultFactoryKwarg, Path::new("RUF026.py"))]
#[test_case(Rule::MissingFStringSyntax, Path::new("RUF027.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(

View file

@ -0,0 +1,157 @@
use memchr::memchr2_iter;
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast};
use ruff_python_literal::format::FormatSpec;
use ruff_python_parser::parse_expression;
use ruff_python_semantic::SemanticModel;
use ruff_source_file::Locator;
use ruff_text_size::{Ranged, TextRange};
use rustc_hash::FxHashSet;
/// ## 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.
///
/// 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.
///
/// ## Example
///
/// ```python
/// name = "Sarah"
/// dayofweek = "Tuesday"
/// msg = "Hello {name}! It is {dayofweek} today!"
/// ```
///
/// Use instead:
/// ```python
/// name = "Sarah"
/// dayofweek = "Tuesday"
/// msg = f"Hello {name}! It is {dayofweek} today!"
/// ```
#[violation]
pub struct MissingFStringSyntax;
impl AlwaysFixableViolation for MissingFStringSyntax {
#[derive_message_formats]
fn message(&self) -> String {
format!(r#"Possible f-string without an `f` prefix"#)
}
fn fix_title(&self) -> String {
"Add `f` prefix".into()
}
}
/// RUF027
pub(crate) fn missing_fstring_syntax(
diagnostics: &mut Vec<Diagnostic>,
literal: &ast::StringLiteral,
locator: &Locator,
semantic: &SemanticModel,
) {
// we want to avoid statement expressions that are just a string literal.
// there's no reason to have standalone f-strings and this lets us avoid docstrings too
if let ast::Stmt::Expr(ast::StmtExpr { value, .. }) = semantic.current_statement() {
match value.as_ref() {
ast::Expr::StringLiteral(_) | ast::Expr::FString(_) => return,
_ => {}
}
}
if should_be_fstring(literal, locator, semantic) {
let diagnostic = Diagnostic::new(MissingFStringSyntax, literal.range())
.with_fix(fix_fstring_syntax(literal.range()));
diagnostics.push(diagnostic);
}
}
/// Returns `true` if `literal` is likely an f-string with a missing `f` prefix.
/// See [`MissingFStringSyntax`] for the validation criteria.
fn should_be_fstring(
literal: &ast::StringLiteral,
locator: &Locator,
semantic: &SemanticModel,
) -> bool {
if !has_brackets(&literal.value) {
return false;
}
let Ok(ast::Expr::FString(ast::ExprFString { value, .. })) =
parse_expression(&format!("f{}", locator.slice(literal.range())))
else {
return false;
};
let mut kwargs = vec![];
for expr in semantic.current_expressions() {
match expr {
ast::Expr::Call(ast::ExprCall {
arguments: ast::Arguments { keywords, .. },
func,
..
}) => {
if let ast::Expr::Attribute(ast::ExprAttribute { .. }) = func.as_ref() {
return false;
}
kwargs.extend(keywords.iter());
}
_ => continue,
}
}
let kw_idents: FxHashSet<&str> = kwargs
.iter()
.filter_map(|k| k.arg.as_ref())
.map(ast::Identifier::as_str)
.collect();
for f_string in value.f_strings() {
let mut has_name = false;
for element in f_string
.elements
.iter()
.filter_map(|element| element.as_expression())
{
if let ast::Expr::Name(ast::ExprName { id, .. }) = element.expression.as_ref() {
if kw_idents.contains(id.as_str()) || semantic.lookup_symbol(id).is_none() {
return false;
}
has_name = true;
}
if let Some(spec) = &element.format_spec {
let spec = locator.slice(spec.range());
if FormatSpec::parse(spec).is_err() {
return false;
}
}
}
if !has_name {
return false;
}
}
true
}
// fast check to disqualify any string literal without brackets
#[inline]
fn has_brackets(possible_fstring: &str) -> bool {
// this qualifies rare false positives like "{ unclosed bracket"
// but it's faster in the general case
memchr2_iter(b'{', b'}', possible_fstring.as_bytes()).count() > 1
}
fn fix_fstring_syntax(range: TextRange) -> Fix {
Fix::unsafe_edit(Edit::insertion("f".into(), range.start()))
}

View file

@ -8,6 +8,7 @@ pub(crate) use function_call_in_dataclass_default::*;
pub(crate) use implicit_optional::*;
pub(crate) use invalid_index_type::*;
pub(crate) use invalid_pyproject_toml::*;
pub(crate) use missing_fstring_syntax::*;
pub(crate) use mutable_class_default::*;
pub(crate) use mutable_dataclass_default::*;
pub(crate) use mutable_fromkeys_value::*;
@ -37,6 +38,7 @@ mod helpers;
mod implicit_optional;
mod invalid_index_type;
mod invalid_pyproject_toml;
mod missing_fstring_syntax;
mod mutable_class_default;
mod mutable_dataclass_default;
mod mutable_fromkeys_value;

View file

@ -0,0 +1,295 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF027.py:5:9: RUF027 [*] Possible f-string without an `f` prefix
|
3 | def simple_cases():
4 | a = 4
5 | b = "{a}" # RUF027
| ^^^^^ RUF027
6 | c = "{a} {b} f'{val}' " # RUF027
|
= help: Add `f` prefix
Unsafe fix
2 2 |
3 3 | def simple_cases():
4 4 | a = 4
5 |- b = "{a}" # RUF027
5 |+ b = f"{a}" # RUF027
6 6 | c = "{a} {b} f'{val}' " # RUF027
7 7 |
8 8 | def escaped_string():
RUF027.py:6:9: RUF027 [*] Possible f-string without an `f` prefix
|
4 | a = 4
5 | b = "{a}" # RUF027
6 | c = "{a} {b} f'{val}' " # RUF027
| ^^^^^^^^^^^^^^^^^^^ RUF027
7 |
8 | def escaped_string():
|
= help: Add `f` prefix
Unsafe fix
3 3 | def simple_cases():
4 4 | a = 4
5 5 | b = "{a}" # RUF027
6 |- c = "{a} {b} f'{val}' " # RUF027
6 |+ c = f"{a} {b} f'{val}' " # RUF027
7 7 |
8 8 | def escaped_string():
9 9 | a = 4
RUF027.py:14:9: RUF027 [*] Possible f-string without an `f` prefix
|
12 | def raw_string():
13 | a = 4
14 | b = r"raw string with formatting: {a}" # RUF027
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF027
15 | c = r"raw string with \backslashes\ and \"escaped quotes\": {a}" # RUF027
|
= help: Add `f` prefix
Unsafe fix
11 11 |
12 12 | def raw_string():
13 13 | a = 4
14 |- b = r"raw string with formatting: {a}" # RUF027
14 |+ b = fr"raw string with formatting: {a}" # RUF027
15 15 | c = r"raw string with \backslashes\ and \"escaped quotes\": {a}" # RUF027
16 16 |
17 17 | def print_name(name: str):
RUF027.py:15:9: RUF027 [*] Possible f-string without an `f` prefix
|
13 | a = 4
14 | b = r"raw string with formatting: {a}" # RUF027
15 | c = r"raw string with \backslashes\ and \"escaped quotes\": {a}" # RUF027
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF027
16 |
17 | def print_name(name: str):
|
= help: Add `f` prefix
Unsafe fix
12 12 | def raw_string():
13 13 | a = 4
14 14 | b = r"raw string with formatting: {a}" # RUF027
15 |- c = r"raw string with \backslashes\ and \"escaped quotes\": {a}" # RUF027
15 |+ c = fr"raw string with \backslashes\ and \"escaped quotes\": {a}" # RUF027
16 16 |
17 17 | def print_name(name: str):
18 18 | a = 4
RUF027.py:19:11: RUF027 [*] Possible f-string without an `f` prefix
|
17 | def print_name(name: str):
18 | a = 4
19 | print("Hello, {name}!") # RUF027
| ^^^^^^^^^^^^^^^^ RUF027
20 | print("The test value we're using today is {a}") # RUF027
|
= help: Add `f` prefix
Unsafe fix
16 16 |
17 17 | def print_name(name: str):
18 18 | a = 4
19 |- print("Hello, {name}!") # RUF027
19 |+ print(f"Hello, {name}!") # RUF027
20 20 | print("The test value we're using today is {a}") # RUF027
21 21 |
22 22 | def do_nothing(a):
RUF027.py:20:11: RUF027 [*] Possible f-string without an `f` prefix
|
18 | a = 4
19 | print("Hello, {name}!") # RUF027
20 | print("The test value we're using today is {a}") # RUF027
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF027
21 |
22 | def do_nothing(a):
|
= help: Add `f` prefix
Unsafe fix
17 17 | def print_name(name: str):
18 18 | a = 4
19 19 | print("Hello, {name}!") # RUF027
20 |- print("The test value we're using today is {a}") # RUF027
20 |+ print(f"The test value we're using today is {a}") # RUF027
21 21 |
22 22 | def do_nothing(a):
23 23 | return a
RUF027.py:27:33: RUF027 [*] Possible f-string without an `f` prefix
|
25 | def nested_funcs():
26 | a = 4
27 | print(do_nothing(do_nothing("{a}"))) # RUF027
| ^^^^^ RUF027
28 |
29 | def tripled_quoted():
|
= help: Add `f` prefix
Unsafe fix
24 24 |
25 25 | def nested_funcs():
26 26 | a = 4
27 |- print(do_nothing(do_nothing("{a}"))) # RUF027
27 |+ print(do_nothing(do_nothing(f"{a}"))) # RUF027
28 28 |
29 29 | def tripled_quoted():
30 30 | a = 4
RUF027.py:32:19: RUF027 [*] Possible f-string without an `f` prefix
|
30 | a = 4
31 | c = a
32 | single_line = """ {a} """ # RUF027
| ^^^^^^^^^^^ RUF027
33 | # RUF027
34 | multi_line = a = """b { # comment
|
= help: Add `f` prefix
Unsafe fix
29 29 | def tripled_quoted():
30 30 | a = 4
31 31 | c = a
32 |- single_line = """ {a} """ # RUF027
32 |+ single_line = f""" {a} """ # RUF027
33 33 | # RUF027
34 34 | multi_line = a = """b { # comment
35 35 | c} d
RUF027.py:34:22: RUF027 [*] Possible f-string without an `f` prefix
|
32 | single_line = """ {a} """ # RUF027
33 | # RUF027
34 | multi_line = a = """b { # comment
| ______________________^
35 | | c} d
36 | | """
| |_______^ RUF027
37 |
38 | def single_quoted_multi_line():
|
= help: Add `f` prefix
Unsafe fix
31 31 | c = a
32 32 | single_line = """ {a} """ # RUF027
33 33 | # RUF027
34 |- multi_line = a = """b { # comment
34 |+ multi_line = a = f"""b { # comment
35 35 | c} d
36 36 | """
37 37 |
RUF027.py:41:9: RUF027 [*] Possible f-string without an `f` prefix
|
39 | a = 4
40 | # RUF027
41 | b = " {\
| _________^
42 | | a} \
43 | | "
| |_____^ RUF027
44 |
45 | def implicit_concat():
|
= help: Add `f` prefix
Unsafe fix
38 38 | def single_quoted_multi_line():
39 39 | a = 4
40 40 | # RUF027
41 |- b = " {\
41 |+ b = f" {\
42 42 | a} \
43 43 | "
44 44 |
RUF027.py:47:9: RUF027 [*] Possible f-string without an `f` prefix
|
45 | def implicit_concat():
46 | a = 4
47 | b = "{a}" "+" "{b}" r" \\ " # RUF027 for the first part only
| ^^^^^ RUF027
48 | print(f"{a}" "{a}" f"{b}") # RUF027
|
= help: Add `f` prefix
Unsafe fix
44 44 |
45 45 | def implicit_concat():
46 46 | a = 4
47 |- b = "{a}" "+" "{b}" r" \\ " # RUF027 for the first part only
47 |+ b = f"{a}" "+" "{b}" r" \\ " # RUF027 for the first part only
48 48 | print(f"{a}" "{a}" f"{b}") # RUF027
49 49 |
50 50 | def escaped_chars():
RUF027.py:48:18: RUF027 [*] Possible f-string without an `f` prefix
|
46 | a = 4
47 | b = "{a}" "+" "{b}" r" \\ " # RUF027 for the first part only
48 | print(f"{a}" "{a}" f"{b}") # RUF027
| ^^^^^ RUF027
49 |
50 | def escaped_chars():
|
= help: Add `f` prefix
Unsafe fix
45 45 | def implicit_concat():
46 46 | a = 4
47 47 | b = "{a}" "+" "{b}" r" \\ " # RUF027 for the first part only
48 |- print(f"{a}" "{a}" f"{b}") # RUF027
48 |+ print(f"{a}" f"{a}" f"{b}") # RUF027
49 49 |
50 50 | def escaped_chars():
51 51 | a = 4
RUF027.py:52:9: RUF027 [*] Possible f-string without an `f` prefix
|
50 | def escaped_chars():
51 | a = 4
52 | b = "\"not escaped:\" \'{a}\' \"escaped:\": \'{{c}}\'" # RUF027
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF027
53 |
54 | def alternative_formatter(src, **kwargs):
|
= help: Add `f` prefix
Unsafe fix
49 49 |
50 50 | def escaped_chars():
51 51 | a = 4
52 |- b = "\"not escaped:\" \'{a}\' \"escaped:\": \'{{c}}\'" # RUF027
52 |+ b = f"\"not escaped:\" \'{a}\' \"escaped:\": \'{{c}}\'" # RUF027
53 53 |
54 54 | def alternative_formatter(src, **kwargs):
55 55 | src.format(**kwargs)
RUF027.py:85:7: RUF027 [*] Possible f-string without an `f` prefix
|
83 | "always ignore this: {a}"
84 |
85 | print("but don't ignore this: {val}") # RUF027
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF027
|
= help: Add `f` prefix
Unsafe fix
82 82 |
83 83 | "always ignore this: {a}"
84 84 |
85 |-print("but don't ignore this: {val}") # RUF027
85 |+print(f"but don't ignore this: {val}") # RUF027

1
ruff.schema.json generated
View file

@ -3519,6 +3519,7 @@
"RUF024",
"RUF025",
"RUF026",
"RUF027",
"RUF1",
"RUF10",
"RUF100",