Implement RUF027: Missing F-String Syntax lint

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

@@ -0,0 +1,81 @@
+val = 2
+
+def simple_cases():
+    a = 4
+    b = "{a}" # RUF027
+    c = "{a} {b} f'{val}' " # RUF027
+    uppercase = "{a}".to_upper() # 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)
+
+# 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(do_nothing("{a}").format(a = "Test"))
+
+a = 4
+
+"always ignore this: {a}"
+
+print("but don't ignore this: {val}")

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

@@ -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 {

crates/ruff_linter/src/codes.rs

@@ -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")]

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

@@ -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(

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

@@ -0,0 +1,177 @@
+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 applies the following criteria to the string literal:
+/// 1. The string literal is not a standalone expression: it needs to be part of an assignment, function call, and so on.
+/// 2. If it's an argument in a function call that takes keyword arguments, no keywords can be used as identifiers within the formatting syntax.
+/// 3. Every identifier in a {...} section needs to be a valid, in-scope reference, and the string should have at least one identifier.
+/// 4. All format specifiers should be valid.
+///
+/// ## 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!"
+/// ```
+#[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 an `f` prefix to the f-string".into()
+    }
+}
+
+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 `source` is valid f-string syntax with qualified, bound variables.
+/// `kwargs` should be the keyword arguments that were passed to function if the string literal is also
+/// being passed to the same function.
+/// If a identifier from `kwargs` is used in `source`'s formatting, this will return `false`,
+/// since it's possible the function could be formatting the literal in question.
+/// Here's a example case where we don't want to suggest turning a literal into an
+/// f-string:
+/// ```python
+/// def alternative_formatter(string, **kwargs):
+///     format(string, **kwargs)
+///
+/// print(alternative_formatter("{fmt}", fmt = "Hello World"))
+/// ```
+/// In general, if the literal is passed to a function which is also being
+/// passed at least one keyword argument with an identifier that also exists in the literal,
+/// that literal should return `false`.
+/// ```
+pub(super) 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 kwargs = get_func_keywords(semantic);
+
+    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 get_func_keywords<'a>(semantic: &'a SemanticModel) -> Vec<&'a ast::Keyword> {
+    let mut total_keywords = vec![];
+    for expr in semantic.current_expressions() {
+        match expr {
+            ast::Expr::Call(ast::ExprCall {
+                arguments: ast::Arguments { keywords, .. },
+                ..
+            }) => total_keywords.extend(keywords.iter()),
+            ast::Expr::Attribute(ast::ExprAttribute { value, .. }) => {
+                if let ast::Expr::Call(ast::ExprCall {
+                    arguments: ast::Arguments { keywords, .. },
+                    ..
+                }) = value.as_ref()
+                {
+                    total_keywords.extend(keywords.iter());
+                }
+            }
+            _ => continue,
+        }
+    }
+    total_keywords
+}
+
+fn fix_fstring_syntax(range: TextRange) -> Fix {
+    Fix::unsafe_edit(Edit::insertion("f".into(), range.start()))
+}

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

@@ -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;