Add visit_format_spec to avoid false positives for F541 in f-string format specifier (#1528)

This commit is contained in:
Harutaka Kawamura 2023-01-02 03:03:32 +09:00 committed by GitHub
parent 6695988b59
commit 509c6d5ec7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 64 additions and 18 deletions

View file

@ -20,7 +20,8 @@ v = 23.234234
# OK # OK
f"{v:0.2f}" f"{v:0.2f}"
f"{f'{v:0.2f}'}"
# OK (false negatives) # Errors
f"{v:{f'0.2f'}}" f"{v:{f'0.2f'}}"
f"{f''}" f"{f''}"

View file

@ -89,7 +89,8 @@ A = (
f'B' f'B'
f'{B}' f'{B}'
) )
C = f'{A:{B}}'
C = f'{A:{f"{B}"}}'
from typing import Annotated, Literal from typing import Annotated, Literal

View file

@ -38,6 +38,9 @@ pub trait Visitor<'a> {
fn visit_excepthandler(&mut self, excepthandler: &'a Excepthandler) { fn visit_excepthandler(&mut self, excepthandler: &'a Excepthandler) {
walk_excepthandler(self, excepthandler); walk_excepthandler(self, excepthandler);
} }
fn visit_format_spec(&mut self, format_spec: &'a Expr) {
walk_expr(self, format_spec);
}
fn visit_arguments(&mut self, arguments: &'a Arguments) { fn visit_arguments(&mut self, arguments: &'a Arguments) {
walk_arguments(self, arguments); walk_arguments(self, arguments);
} }
@ -387,7 +390,7 @@ pub fn walk_expr<'a, V: Visitor<'a> + ?Sized>(visitor: &mut V, expr: &'a Expr) {
} => { } => {
visitor.visit_expr(value); visitor.visit_expr(value);
if let Some(expr) = format_spec { if let Some(expr) = format_spec {
visitor.visit_expr(expr); visitor.visit_format_spec(expr);
} }
} }
ExprKind::JoinedStr { values } => { ExprKind::JoinedStr { values } => {

View file

@ -91,7 +91,6 @@ pub struct Checker<'a> {
in_type_definition: bool, in_type_definition: bool,
in_deferred_string_type_definition: bool, in_deferred_string_type_definition: bool,
in_deferred_type_definition: bool, in_deferred_type_definition: bool,
in_f_string: bool,
in_literal: bool, in_literal: bool,
in_subscript: bool, in_subscript: bool,
seen_import_boundary: bool, seen_import_boundary: bool,
@ -148,7 +147,6 @@ impl<'a> Checker<'a> {
in_type_definition: false, in_type_definition: false,
in_deferred_string_type_definition: false, in_deferred_string_type_definition: false,
in_deferred_type_definition: false, in_deferred_type_definition: false,
in_f_string: false,
in_literal: false, in_literal: false,
in_subscript: false, in_subscript: false,
seen_import_boundary: false, seen_import_boundary: false,
@ -1486,7 +1484,6 @@ where
self.push_expr(expr); self.push_expr(expr);
let prev_in_f_string = self.in_f_string;
let prev_in_literal = self.in_literal; let prev_in_literal = self.in_literal;
let prev_in_type_definition = self.in_type_definition; let prev_in_type_definition = self.in_type_definition;
@ -2176,9 +2173,7 @@ where
} }
} }
ExprKind::JoinedStr { values } => { ExprKind::JoinedStr { values } => {
// Conversion flags are parsed as f-strings without placeholders, so skip if self.settings.enabled.contains(&CheckCode::F541) {
// nested f-strings, which would lead to false positives.
if !self.in_f_string && self.settings.enabled.contains(&CheckCode::F541) {
if !values if !values
.iter() .iter()
.any(|value| matches!(value.node, ExprKind::FormattedValue { .. })) .any(|value| matches!(value.node, ExprKind::FormattedValue { .. }))
@ -2682,9 +2677,7 @@ where
self.in_subscript = prev_in_subscript; self.in_subscript = prev_in_subscript;
} }
ExprKind::JoinedStr { .. } => { ExprKind::JoinedStr { .. } => {
self.in_f_string = true;
visitor::walk_expr(self, expr); visitor::walk_expr(self, expr);
self.in_f_string = prev_in_f_string;
} }
_ => visitor::walk_expr(self, expr), _ => visitor::walk_expr(self, expr),
} }
@ -2703,7 +2696,6 @@ where
self.in_type_definition = prev_in_type_definition; self.in_type_definition = prev_in_type_definition;
self.in_literal = prev_in_literal; self.in_literal = prev_in_literal;
self.in_f_string = prev_in_f_string;
self.pop_expr(); self.pop_expr();
} }
@ -2807,6 +2799,17 @@ where
} }
} }
fn visit_format_spec(&mut self, format_spec: &'b Expr) {
match &format_spec.node {
ExprKind::JoinedStr { values } => {
for value in values {
self.visit_expr(value);
}
}
_ => unreachable!("Unexpected expression for format_spec"),
}
}
fn visit_arguments(&mut self, arguments: &'b Arguments) { fn visit_arguments(&mut self, arguments: &'b Arguments) {
if self.settings.enabled.contains(&CheckCode::B006) { if self.settings.enabled.contains(&CheckCode::B006) {
flake8_bugbear::plugins::mutable_argument_default(self, arguments); flake8_bugbear::plugins::mutable_argument_default(self, arguments);

View file

@ -38,4 +38,22 @@ expression: checks
column: 16 column: 16
fix: ~ fix: ~
parent: ~ parent: ~
- kind: FStringMissingPlaceholders
location:
row: 26
column: 6
end_location:
row: 26
column: 13
fix: ~
parent: ~
- kind: FStringMissingPlaceholders
location:
row: 27
column: 3
end_location:
row: 27
column: 6
fix: ~
parent: ~

View file

@ -82,33 +82,53 @@ expression: checks
column: 8 column: 8
fix: ~ fix: ~
parent: ~ parent: ~
- kind:
UndefinedName: B
location:
row: 92
column: 10
end_location:
row: 92
column: 11
fix: ~
parent: ~
- kind:
UndefinedName: B
location:
row: 93
column: 13
end_location:
row: 93
column: 14
fix: ~
parent: ~
- kind: - kind:
UndefinedName: PEP593Test123 UndefinedName: PEP593Test123
location: location:
row: 114 row: 115
column: 8 column: 8
end_location: end_location:
row: 114 row: 115
column: 23 column: 23
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
UndefinedName: foo UndefinedName: foo
location: location:
row: 122 row: 123
column: 13 column: 13
end_location: end_location:
row: 122 row: 123
column: 18 column: 18
fix: ~ fix: ~
parent: ~ parent: ~
- kind: - kind:
UndefinedName: bar UndefinedName: bar
location: location:
row: 122 row: 123
column: 20 column: 20
end_location: end_location:
row: 122 row: 123
column: 25 column: 25
fix: ~ fix: ~
parent: ~ parent: ~