Skip to content

Commit

Permalink
Implement F541
Browse files Browse the repository at this point in the history
  • Loading branch information
Charles Marsh committed Aug 16, 2022
1 parent 3b1b53d commit 41449cf
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 7 deletions.
9 changes: 9 additions & 0 deletions resources/test/src/f_string_missing_placeholders.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
a = "abc"
b = f"ghi{'jkl'}"

c = f"def"
d = f"def" + "ghi"
e = (
f"def" +
"ghi"
)
25 changes: 21 additions & 4 deletions src/check_ast.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -66,7 +83,7 @@ impl Visitor for Checker {
idents.insert(ident.clone());
}

walk_arguments(self, arguments);
visitor::walk_arguments(self, arguments);
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ use serde::{Deserialize, Serialize};
#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum CheckKind {
DuplicateArgumentName,
ImportStarUsage,
FStringMissingPlaceholders,
IfTuple,
ImportStarUsage,
LineTooLong,
}

Expand All @@ -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",
Expand All @@ -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",
Expand Down
35 changes: 34 additions & 1 deletion src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion src/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 41449cf

Please sign in to comment.