RUF027 no longer has false negatives with string literals inside of m…

…ethod calls (#9865)

Fixes #9857.

## Summary

Statements like `logging.info("Today it is: {day}")` will no longer be
ignored by RUF027. As before, statements like `"Today it is:
{day}".format(day="Tuesday")` will continue to be ignored.

## Test Plan

The snapshot tests were expanded to include new cases. Additionally, the
snapshot tests have been split in two to separate positive cases from
negative cases.

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

@@ -0,0 +1,70 @@
+val = 2
+
+"always ignore this: {val}"
+
+print("but don't ignore this: {val}")  # RUF027
+
+
+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 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 method_calls():
+    value = {}
+    value.method = print_name
+    first = "Wendy"
+    last = "Appleseed"
+    value.method("{first} {last}")  # RUF027

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

@@ -0,0 +1,36 @@
+def do_nothing(a):
+    return a
+
+
+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: {}"
+    f = "uses a builtin: {max}"
+    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))
+    print(("{a}" "{c}").format(a=1, c=2))
+    print("{a}".attribute.chaining.call(a=2))
+    print("{a} {c}".format(a))

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

@@ -46,7 +46,8 @@ 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"))]
+    #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_0.py"))]
+    #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_1.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

@@ -20,7 +20,7 @@ use rustc_hash::FxHashSet;
 /// 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")`)
+/// 2. The literal is part of a function call with argument names 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.
@@ -94,29 +94,51 @@ fn should_be_fstring(
         return false;
     };
 
-    let mut kwargs = vec![];
+    let mut arg_names = FxHashSet::default();
+    let mut last_expr: Option<&ast::Expr> = None;
     for expr in semantic.current_expressions() {
         match expr {
             ast::Expr::Call(ast::ExprCall {
-                arguments: ast::Arguments { keywords, .. },
+                arguments: ast::Arguments { keywords, args, .. },
                 func,
                 ..
             }) => {
-                if let ast::Expr::Attribute(ast::ExprAttribute { .. }) = func.as_ref() {
-                    return false;
+                if let ast::Expr::Attribute(ast::ExprAttribute { value, .. }) = func.as_ref() {
+                    match value.as_ref() {
+                        // if the first part of the attribute is the string literal,
+                        // we want to ignore this literal from the lint.
+                        // for example: `"{x}".some_method(...)`
+                        ast::Expr::StringLiteral(expr_literal)
+                            if expr_literal.value.as_slice().contains(literal) =>
+                        {
+                            return false;
+                        }
+                        // if the first part of the attribute was the expression we
+                        // just went over in the last iteration, then we also want to pass
+                        // this over in the lint.
+                        // for example: `some_func("{x}").some_method(...)`
+                        value if last_expr == Some(value) => {
+                            return false;
+                        }
+                        _ => {}
+                    }
+                }
+                for keyword in keywords {
+                    if let Some(ident) = keyword.arg.as_ref() {
+                        arg_names.insert(ident.as_str());
+                    }
+                }
+                for arg in args {
+                    if let ast::Expr::Name(ast::ExprName { id, .. }) = arg {
+                        arg_names.insert(id.as_str());
+                    }
                 }
-                kwargs.extend(keywords.iter());
             }
             _ => continue,
         }
+        last_expr.replace(expr);
     }
 
-    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
@@ -125,7 +147,7 @@ fn should_be_fstring(
             .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()) {
+                if arg_names.contains(id.as_str()) {
                     return false;
                 }
                 if semantic