mirror of https://github.com/astral-sh/ruff
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
```
This commit is contained in:
parent
ed7d2b8a3d
commit
97e31cad2f
|
|
@ -23,3 +23,5 @@ a = []
|
|||
'%s %s' % (*a,)
|
||||
k = {}
|
||||
'%(k)s' % {**k}
|
||||
'%s' % [1, 2, 3]
|
||||
'%s' % {1, 2, 3}
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
));
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue