From ddd554f9de39a5f5876ba4cf7e479b1111f9c6f5 Mon Sep 17 00:00:00 2001
From: Charlie Marsh <charlie.r.marsh@gmail.com>
Date: Tue, 16 Aug 2022 10:47:13 -0400
Subject: [PATCH] Implement F541 (#12)

---
 .../test/src/f_string_missing_placeholders.py |  9 +++++
 src/check_ast.rs                              | 25 ++++++++++---
 src/checks.rs                                 |  5 ++-
 src/linter.rs                                 | 35 ++++++++++++++++++-
 src/visitor.rs                                |  2 +-
 5 files changed, 69 insertions(+), 7 deletions(-)
 create mode 100644 resources/test/src/f_string_missing_placeholders.py

diff --git a/resources/test/src/f_string_missing_placeholders.py b/resources/test/src/f_string_missing_placeholders.py
new file mode 100644
index 0000000000..c9b6c43d41
--- /dev/null
+++ b/resources/test/src/f_string_missing_placeholders.py
@@ -0,0 +1,9 @@
+a = "abc"
+b = f"ghi{'jkl'}"
+
+c = f"def"
+d = f"def" + "ghi"
+e = (
+    f"def" +
+    "ghi"
+)
diff --git a/src/check_ast.rs b/src/check_ast.rs
index 26a213f207..d8aee9f174 100644
--- a/src/check_ast.rs
+++ b/src/check_ast.rs
@@ -1,9 +1,10 @@
 use std::collections::HashSet;
 
-use rustpython_parser::ast::{Arg, Arguments, ExprKind, Stmt, StmtKind, Suite};
+use rustpython_parser::ast::{Arg, Arguments, Expr, ExprKind, Stmt, StmtKind, Suite};
 
 use crate::checks::{Check, CheckKind};
-use crate::visitor::{walk_arguments, walk_stmt, Visitor};
+use crate::visitor;
+use crate::visitor::Visitor;
 
 #[derive(Default)]
 struct Checker {
@@ -34,7 +35,23 @@ impl Visitor for Checker {
             _ => {}
         }
 
-        walk_stmt(self, stmt);
+        visitor::walk_stmt(self, stmt);
+    }
+
+    fn visit_expr(&mut self, expr: &Expr) {
+        if let ExprKind::JoinedStr { values } = &expr.node {
+            if !values
+                .iter()
+                .any(|value| matches!(value.node, ExprKind::FormattedValue { .. }))
+            {
+                self.checks.push(Check {
+                    kind: CheckKind::FStringMissingPlaceholders,
+                    location: expr.location,
+                });
+            }
+        }
+
+        visitor::walk_expr(self, expr);
     }
 
     fn visit_arguments(&mut self, arguments: &Arguments) {
@@ -66,7 +83,7 @@ impl Visitor for Checker {
             idents.insert(ident.clone());
         }
 
-        walk_arguments(self, arguments);
+        visitor::walk_arguments(self, arguments);
     }
 }
 
diff --git a/src/checks.rs b/src/checks.rs
index bd278b9216..9b024fae68 100644
--- a/src/checks.rs
+++ b/src/checks.rs
@@ -4,8 +4,9 @@ use serde::{Deserialize, Serialize};
 #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
 pub enum CheckKind {
     DuplicateArgumentName,
-    ImportStarUsage,
+    FStringMissingPlaceholders,
     IfTuple,
+    ImportStarUsage,
     LineTooLong,
 }
 
@@ -14,6 +15,7 @@ impl CheckKind {
     pub fn code(&self) -> &'static str {
         match self {
             CheckKind::DuplicateArgumentName => "F831",
+            CheckKind::FStringMissingPlaceholders => "F541",
             CheckKind::IfTuple => "F634",
             CheckKind::ImportStarUsage => "F403",
             CheckKind::LineTooLong => "E501",
@@ -24,6 +26,7 @@ impl CheckKind {
     pub fn body(&self) -> &'static str {
         match self {
             CheckKind::DuplicateArgumentName => "Duplicate argument name in function definition",
+            CheckKind::FStringMissingPlaceholders => "f-string without any placeholders",
             CheckKind::IfTuple => "If test is a tuple, which is always `True`",
             CheckKind::ImportStarUsage => "Unable to detect undefined names",
             CheckKind::LineTooLong => "Line too long",
diff --git a/src/linter.rs b/src/linter.rs
index 89b2707d0f..46ac4709c1 100644
--- a/src/linter.rs
+++ b/src/linter.rs
@@ -44,7 +44,9 @@ mod tests {
     use rustpython_parser::ast::Location;
 
     use crate::cache;
-    use crate::checks::CheckKind::{DuplicateArgumentName, IfTuple, ImportStarUsage, LineTooLong};
+    use crate::checks::CheckKind::{
+        DuplicateArgumentName, FStringMissingPlaceholders, IfTuple, ImportStarUsage, LineTooLong,
+    };
     use crate::linter::check_path;
     use crate::message::Message;
 
@@ -79,6 +81,37 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn f_string_missing_placeholders() -> Result<()> {
+        let actual = check_path(
+            &Path::new("./resources/test/src/f_string_missing_placeholders.py"),
+            &cache::Mode::None,
+        )?;
+        let expected = vec![
+            Message {
+                kind: FStringMissingPlaceholders,
+                location: Location::new(4, 7),
+                filename: "./resources/test/src/f_string_missing_placeholders.py".to_string(),
+            },
+            Message {
+                kind: FStringMissingPlaceholders,
+                location: Location::new(5, 7),
+                filename: "./resources/test/src/f_string_missing_placeholders.py".to_string(),
+            },
+            Message {
+                kind: FStringMissingPlaceholders,
+                location: Location::new(7, 7),
+                filename: "./resources/test/src/f_string_missing_placeholders.py".to_string(),
+            },
+        ];
+        assert_eq!(actual.len(), expected.len());
+        for i in 1..actual.len() {
+            assert_eq!(actual[i], expected[i]);
+        }
+
+        Ok(())
+    }
+
     #[test]
     fn if_tuple() -> Result<()> {
         let actual = check_path(
diff --git a/src/visitor.rs b/src/visitor.rs
index bb3cd6d630..0cbddba3eb 100644
--- a/src/visitor.rs
+++ b/src/visitor.rs
@@ -295,7 +295,7 @@ pub fn walk_stmt<V: Visitor + ?Sized>(visitor: &mut V, stmt: &Stmt) {
 }
 
 #[allow(unused_variables)]
-fn walk_expr<V: Visitor + ?Sized>(visitor: &mut V, expr: &Expr) {
+pub fn walk_expr<V: Visitor + ?Sized>(visitor: &mut V, expr: &Expr) {
     match &expr.node {
         ExprKind::BoolOp { op, values } => {
             visitor.visit_boolop(op);