From 97e31cad2f6e0f6f07d2f03c76123c5856a38389 Mon Sep 17 00:00:00 2001
From: Harutaka Kawamura <hkawamura0130@gmail.com>
Date: Sun, 23 Jul 2023 03:42:44 +0900
Subject: [PATCH] Fix `F507` false positive (#5986)

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

F507 should not be raised when the right-hand side value is a non-tuple
object.

```python
'%s' % (1, 2, 3)  # throws
'%s' % [1, 2, 3]  # doesn't throw
'%s' % {1, 2, 3}  # doesn't throw
```
---
 .../resources/test/fixtures/pyflakes/F50x.py  |  2 +
 .../ruff/src/rules/pyflakes/rules/strings.rs  | 97 +++++++++----------
 2 files changed, 49 insertions(+), 50 deletions(-)

diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F50x.py b/crates/ruff/resources/test/fixtures/pyflakes/F50x.py
index 97e861d9a2..692bda5e19 100644
--- a/crates/ruff/resources/test/fixtures/pyflakes/F50x.py
+++ b/crates/ruff/resources/test/fixtures/pyflakes/F50x.py
@@ -23,3 +23,5 @@ a = []
 '%s %s' % (*a,)
 k = {}
 '%(k)s' % {**k}
+'%s' % [1, 2, 3]
+'%s' % {1, 2, 3}
diff --git a/crates/ruff/src/rules/pyflakes/rules/strings.rs b/crates/ruff/src/rules/pyflakes/rules/strings.rs
index a3c0820737..160304ab97 100644
--- a/crates/ruff/src/rules/pyflakes/rules/strings.rs
+++ b/crates/ruff/src/rules/pyflakes/rules/strings.rs
@@ -634,40 +634,42 @@ pub(crate) fn percent_format_missing_arguments(
         return;
     }
 
-    if let Expr::Dict(ast::ExprDict { keys, .. }) = &right {
-        if keys.iter().any(Option::is_none) {
-            return; // contains **x splat
-        }
+    let Expr::Dict(ast::ExprDict { keys, .. }) = &right else {
+        return;
+    };
 
-        let mut keywords = FxHashSet::default();
-        for key in keys.iter().flatten() {
-            match key {
-                Expr::Constant(ast::ExprConstant {
-                    value: Constant::Str(value),
-                    ..
-                }) => {
-                    keywords.insert(value);
-                }
-                _ => {
-                    return; // Dynamic keys present
-                }
+    if keys.iter().any(Option::is_none) {
+        return; // contains **x splat
+    }
+
+    let mut keywords = FxHashSet::default();
+    for key in keys.iter().flatten() {
+        match key {
+            Expr::Constant(ast::ExprConstant {
+                value: Constant::Str(value),
+                ..
+            }) => {
+                keywords.insert(value);
+            }
+            _ => {
+                return; // Dynamic keys present
             }
         }
+    }
 
-        let missing: Vec<&String> = summary
-            .keywords
-            .iter()
-            .filter(|k| !keywords.contains(k))
-            .collect();
+    let missing: Vec<&String> = summary
+        .keywords
+        .iter()
+        .filter(|k| !keywords.contains(k))
+        .collect();
 
-        if !missing.is_empty() {
-            checker.diagnostics.push(Diagnostic::new(
-                PercentFormatMissingArgument {
-                    missing: missing.iter().map(|&s| s.clone()).collect(),
-                },
-                location,
-            ));
-        }
+    if !missing.is_empty() {
+        checker.diagnostics.push(Diagnostic::new(
+            PercentFormatMissingArgument {
+                missing: missing.iter().map(|&s| s.clone()).collect(),
+            },
+            location,
+        ));
     }
 }
 
@@ -696,29 +698,24 @@ pub(crate) fn percent_format_positional_count_mismatch(
         return;
     }
 
-    match right {
-        Expr::List(ast::ExprList { elts, .. })
-        | Expr::Tuple(ast::ExprTuple { elts, .. })
-        | Expr::Set(ast::ExprSet { elts, .. }) => {
-            let mut found = 0;
-            for elt in elts {
-                if let Expr::Starred(_) = &elt {
-                    return;
-                }
-                found += 1;
-            }
-
-            if found != summary.num_positional {
-                checker.diagnostics.push(Diagnostic::new(
-                    PercentFormatPositionalCountMismatch {
-                        wanted: summary.num_positional,
-                        got: found,
-                    },
-                    location,
-                ));
+    if let Expr::Tuple(ast::ExprTuple { elts, .. }) = right {
+        let mut found = 0;
+        for elt in elts {
+            if elt.is_starred_expr() {
+                return;
             }
+            found += 1;
+        }
+
+        if found != summary.num_positional {
+            checker.diagnostics.push(Diagnostic::new(
+                PercentFormatPositionalCountMismatch {
+                    wanted: summary.num_positional,
+                    got: found,
+                },
+                location,
+            ));
         }
-        _ => {}
     }
 }