Move string formatting checks to plugins (#1185)

This commit is contained in:
Charlie Marsh
2022-12-10 16:43:21 -05:00
committed by GitHub
parent 06ed125771
commit fb681c614a
4 changed files with 405 additions and 429 deletions

View File

@@ -1481,36 +1481,28 @@ where
}
Ok(summary) => {
if self.settings.enabled.contains(&CheckCode::F522) {
pyflakes::checks::string_dot_format_extra_named_arguments(self,
pyflakes::plugins::string_dot_format_extra_named_arguments(self,
&summary, keywords, location,
);
}
if self.settings.enabled.contains(&CheckCode::F523) {
pyflakes::checks::string_dot_format_extra_positional_arguments(
pyflakes::plugins::string_dot_format_extra_positional_arguments(
self,
&summary, args, location,
);
}
if self.settings.enabled.contains(&CheckCode::F524) {
if let Some(check) =
pyflakes::checks::string_dot_format_missing_argument(
&summary, args, keywords, location,
)
{
self.add_check(check);
}
pyflakes::plugins::string_dot_format_missing_argument(
self, &summary, args, keywords, location,
);
}
if self.settings.enabled.contains(&CheckCode::F525) {
if let Some(check) =
pyflakes::checks::string_dot_format_mixing_automatic(
&summary, location,
)
{
self.add_check(check);
}
pyflakes::plugins::string_dot_format_mixing_automatic(
self, &summary, location,
);
}
}
}
@@ -1961,63 +1953,39 @@ where
}
Ok(summary) => {
if self.settings.enabled.contains(&CheckCode::F502) {
if let Some(check) =
pyflakes::checks::percent_format_expected_mapping(
&summary, right, location,
)
{
self.add_check(check);
}
pyflakes::plugins::percent_format_expected_mapping(
self, &summary, right, location,
);
}
if self.settings.enabled.contains(&CheckCode::F503) {
if let Some(check) =
pyflakes::checks::percent_format_expected_sequence(
&summary, right, location,
)
{
self.add_check(check);
}
pyflakes::plugins::percent_format_expected_sequence(
self, &summary, right, location,
);
}
if self.settings.enabled.contains(&CheckCode::F504) {
pyflakes::checks::percent_format_extra_named_arguments(
pyflakes::plugins::percent_format_extra_named_arguments(
self, &summary, right, location,
);
}
if self.settings.enabled.contains(&CheckCode::F505) {
if let Some(check) =
pyflakes::checks::percent_format_missing_arguments(
&summary, right, location,
)
{
self.add_check(check);
}
pyflakes::plugins::percent_format_missing_arguments(
self, &summary, right, location,
);
}
if self.settings.enabled.contains(&CheckCode::F506) {
if let Some(check) =
pyflakes::checks::percent_format_mixed_positional_and_named(
&summary, location,
)
{
self.add_check(check);
}
pyflakes::plugins::percent_format_mixed_positional_and_named(
self, &summary, location,
);
}
if self.settings.enabled.contains(&CheckCode::F507) {
if let Some(check) =
pyflakes::checks::percent_format_positional_count_mismatch(
&summary, right, location,
)
{
self.add_check(check);
}
pyflakes::plugins::percent_format_positional_count_mismatch(
self, &summary, right, location,
);
}
if self.settings.enabled.contains(&CheckCode::F508) {
if let Some(check) =
pyflakes::checks::percent_format_star_requires_sequence(
&summary, right, location,
)
{
self.add_check(check);
}
pyflakes::plugins::percent_format_star_requires_sequence(
self, &summary, right, location,
);
}
}
}

View File

@@ -2,382 +2,12 @@ use std::string::ToString;
use regex::Regex;
use rustc_hash::FxHashSet;
use rustpython_ast::{Keyword, KeywordData};
use rustpython_parser::ast::{
Arg, Arguments, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Stmt, StmtKind,
};
use crate::ast::types::{Binding, BindingKind, Range, Scope, ScopeKind};
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
use crate::pyflakes::cformat::CFormatSummary;
use crate::pyflakes::fixes::{
remove_unused_format_arguments_from_dict, remove_unused_keyword_arguments_from_format_call,
};
use crate::pyflakes::format::FormatSummary;
fn has_star_star_kwargs(keywords: &[Keyword]) -> bool {
keywords.iter().any(|k| {
let KeywordData { arg, .. } = &k.node;
arg.is_none()
})
}
fn has_star_args(args: &[Expr]) -> bool {
args.iter()
.any(|arg| matches!(&arg.node, ExprKind::Starred { .. }))
}
/// F502
pub(crate) fn percent_format_expected_mapping(
summary: &CFormatSummary,
right: &Expr,
location: Range,
) -> Option<Check> {
if summary.keywords.is_empty() {
None
} else {
// Tuple, List, Set (+comprehensions)
match right.node {
ExprKind::List { .. }
| ExprKind::Tuple { .. }
| ExprKind::Set { .. }
| ExprKind::ListComp { .. }
| ExprKind::SetComp { .. }
| ExprKind::GeneratorExp { .. } => Some(Check::new(
CheckKind::PercentFormatExpectedMapping,
location,
)),
_ => None,
}
}
}
/// F503
pub(crate) fn percent_format_expected_sequence(
summary: &CFormatSummary,
right: &Expr,
location: Range,
) -> Option<Check> {
if summary.num_positional <= 1 {
None
} else {
match right.node {
ExprKind::Dict { .. } | ExprKind::DictComp { .. } => Some(Check::new(
CheckKind::PercentFormatExpectedSequence,
location,
)),
_ => None,
}
}
}
/// F504
pub(crate) fn percent_format_extra_named_arguments(
checker: &mut Checker,
summary: &CFormatSummary,
right: &Expr,
location: Range,
) {
if summary.num_positional > 0 {
return;
}
let ExprKind::Dict { keys, values } = &right.node else {
return;
};
if values.len() > keys.len() {
return; // contains **x splat
}
let missing: Vec<&str> = keys
.iter()
.filter_map(|k| match &k.node {
// We can only check that string literals exist
ExprKind::Constant {
value: Constant::Str(value),
..
} => {
if summary.keywords.contains(value) {
None
} else {
Some(value.as_str())
}
}
_ => None,
})
.collect();
if missing.is_empty() {
return;
}
let mut check = Check::new(
CheckKind::PercentFormatExtraNamedArguments(
missing.iter().map(|&arg| arg.to_string()).collect(),
),
location,
);
if checker.patch(check.kind.code()) {
if let Ok(fix) = remove_unused_format_arguments_from_dict(checker.locator, &missing, right)
{
check.amend(fix);
}
}
checker.add_check(check);
}
/// F505
pub(crate) fn percent_format_missing_arguments(
summary: &CFormatSummary,
right: &Expr,
location: Range,
) -> Option<Check> {
if summary.num_positional > 0 {
return None;
}
if let ExprKind::Dict { keys, values } = &right.node {
if values.len() > keys.len() {
return None; // contains **x splat
}
let mut keywords = FxHashSet::default();
for key in keys {
match &key.node {
ExprKind::Constant {
value: Constant::Str(value),
..
} => {
keywords.insert(value);
}
_ => {
return None; // Dynamic keys present
}
}
}
let missing: Vec<&String> = summary
.keywords
.iter()
.filter(|k| !keywords.contains(k))
.collect();
if missing.is_empty() {
None
} else {
Some(Check::new(
CheckKind::PercentFormatMissingArgument(
missing.iter().map(|&s| s.clone()).collect(),
),
location,
))
}
} else {
None
}
}
/// F506
pub(crate) fn percent_format_mixed_positional_and_named(
summary: &CFormatSummary,
location: Range,
) -> Option<Check> {
if summary.num_positional == 0 || summary.keywords.is_empty() {
None
} else {
Some(Check::new(
CheckKind::PercentFormatMixedPositionalAndNamed,
location,
))
}
}
/// F507
pub(crate) fn percent_format_positional_count_mismatch(
summary: &CFormatSummary,
right: &Expr,
location: Range,
) -> Option<Check> {
if !summary.keywords.is_empty() {
return None;
}
match &right.node {
ExprKind::List { elts, .. } | ExprKind::Tuple { elts, .. } | ExprKind::Set { elts, .. } => {
let mut found = 0;
for elt in elts {
if let ExprKind::Starred { .. } = &elt.node {
return None;
}
found += 1;
}
if found == summary.num_positional {
None
} else {
Some(Check::new(
CheckKind::PercentFormatPositionalCountMismatch(summary.num_positional, found),
location,
))
}
}
_ => None,
}
}
/// F508
pub(crate) fn percent_format_star_requires_sequence(
summary: &CFormatSummary,
right: &Expr,
location: Range,
) -> Option<Check> {
if summary.starred {
match &right.node {
ExprKind::Dict { .. } | ExprKind::DictComp { .. } => Some(Check::new(
CheckKind::PercentFormatStarRequiresSequence,
location,
)),
_ => None,
}
} else {
None
}
}
/// F522
pub(crate) fn string_dot_format_extra_named_arguments(
checker: &mut Checker,
summary: &FormatSummary,
keywords: &[Keyword],
location: Range,
) {
if has_star_star_kwargs(keywords) {
return;
}
let keywords = keywords.iter().filter_map(|k| {
let KeywordData { arg, .. } = &k.node;
arg.as_ref()
});
let missing: Vec<&str> = keywords
.filter_map(|arg| {
if summary.keywords.contains(arg) {
None
} else {
Some(arg.as_str())
}
})
.collect();
if missing.is_empty() {
return;
}
let mut check = Check::new(
CheckKind::StringDotFormatExtraNamedArguments(
missing.iter().map(|&arg| arg.to_string()).collect(),
),
location,
);
if checker.patch(check.kind.code()) {
if let Ok(fix) =
remove_unused_keyword_arguments_from_format_call(checker.locator, &missing, location)
{
check.amend(fix);
}
}
checker.add_check(check);
}
/// F523
pub(crate) fn string_dot_format_extra_positional_arguments(
checker: &mut Checker,
summary: &FormatSummary,
args: &[Expr],
location: Range,
) {
if has_star_args(args) {
return;
}
let missing: Vec<usize> = (0..args.len())
.filter(|i| !(summary.autos.contains(i) || summary.indexes.contains(i)))
.collect();
if missing.is_empty() {
return;
}
checker.add_check(Check::new(
CheckKind::StringDotFormatExtraPositionalArguments(
missing
.iter()
.map(std::string::ToString::to_string)
.collect::<Vec<String>>(),
),
location,
));
}
/// F524
pub(crate) fn string_dot_format_missing_argument(
summary: &FormatSummary,
args: &[Expr],
keywords: &[Keyword],
location: Range,
) -> Option<Check> {
if has_star_args(args) || has_star_star_kwargs(keywords) {
return None;
}
let keywords: FxHashSet<_> = keywords
.iter()
.filter_map(|k| {
let KeywordData { arg, .. } = &k.node;
arg.as_ref()
})
.collect();
let missing: Vec<String> = summary
.autos
.iter()
.chain(summary.indexes.iter())
.filter(|&&i| i >= args.len())
.map(ToString::to_string)
.chain(
summary
.keywords
.iter()
.filter(|k| !keywords.contains(k))
.cloned(),
)
.collect();
if missing.is_empty() {
None
} else {
Some(Check::new(
CheckKind::StringDotFormatMissingArguments(missing),
location,
))
}
}
/// F525
pub(crate) fn string_dot_format_mixing_automatic(
summary: &FormatSummary,
location: Range,
) -> Option<Check> {
if summary.autos.is_empty() || summary.indexes.is_empty() {
None
} else {
Some(Check::new(
CheckKind::StringDotFormatMixingAutomatic,
location,
))
}
}
/// F631
pub fn assert_tuple(test: &Expr, location: Range) -> Option<Check> {

View File

@@ -3,9 +3,18 @@ pub use if_tuple::if_tuple;
pub use invalid_literal_comparisons::invalid_literal_comparison;
pub use invalid_print_syntax::invalid_print_syntax;
pub use raise_not_implemented::raise_not_implemented;
pub(crate) use strings::{
percent_format_expected_mapping, percent_format_expected_sequence,
percent_format_extra_named_arguments, percent_format_missing_arguments,
percent_format_mixed_positional_and_named, percent_format_positional_count_mismatch,
percent_format_star_requires_sequence, string_dot_format_extra_named_arguments,
string_dot_format_extra_positional_arguments, string_dot_format_missing_argument,
string_dot_format_mixing_automatic,
};
mod assert_tuple;
mod if_tuple;
mod invalid_literal_comparisons;
mod invalid_print_syntax;
mod raise_not_implemented;
mod strings;

View File

@@ -0,0 +1,369 @@
use std::string::ToString;
use rustc_hash::FxHashSet;
use rustpython_ast::{Keyword, KeywordData};
use rustpython_parser::ast::{Constant, Expr, ExprKind};
use crate::ast::types::Range;
use crate::check_ast::Checker;
use crate::checks::{Check, CheckKind};
use crate::pyflakes::cformat::CFormatSummary;
use crate::pyflakes::fixes::{
remove_unused_format_arguments_from_dict, remove_unused_keyword_arguments_from_format_call,
};
use crate::pyflakes::format::FormatSummary;
fn has_star_star_kwargs(keywords: &[Keyword]) -> bool {
keywords.iter().any(|k| {
let KeywordData { arg, .. } = &k.node;
arg.is_none()
})
}
fn has_star_args(args: &[Expr]) -> bool {
args.iter()
.any(|arg| matches!(&arg.node, ExprKind::Starred { .. }))
}
/// F502
pub(crate) fn percent_format_expected_mapping(
checker: &mut Checker,
summary: &CFormatSummary,
right: &Expr,
location: Range,
) {
if !summary.keywords.is_empty() {
// Tuple, List, Set (+comprehensions)
match right.node {
ExprKind::List { .. }
| ExprKind::Tuple { .. }
| ExprKind::Set { .. }
| ExprKind::ListComp { .. }
| ExprKind::SetComp { .. }
| ExprKind::GeneratorExp { .. } => checker.add_check(Check::new(
CheckKind::PercentFormatExpectedMapping,
location,
)),
_ => {}
}
}
}
/// F503
pub(crate) fn percent_format_expected_sequence(
checker: &mut Checker,
summary: &CFormatSummary,
right: &Expr,
location: Range,
) {
if summary.num_positional > 1
&& matches!(
right.node,
ExprKind::Dict { .. } | ExprKind::DictComp { .. }
)
{
checker.add_check(Check::new(
CheckKind::PercentFormatExpectedSequence,
location,
));
}
}
/// F504
pub(crate) fn percent_format_extra_named_arguments(
checker: &mut Checker,
summary: &CFormatSummary,
right: &Expr,
location: Range,
) {
if summary.num_positional > 0 {
return;
}
let ExprKind::Dict { keys, values } = &right.node else {
return;
};
if values.len() > keys.len() {
return; // contains **x splat
}
let missing: Vec<&str> = keys
.iter()
.filter_map(|k| match &k.node {
// We can only check that string literals exist
ExprKind::Constant {
value: Constant::Str(value),
..
} => {
if summary.keywords.contains(value) {
None
} else {
Some(value.as_str())
}
}
_ => None,
})
.collect();
if missing.is_empty() {
return;
}
let mut check = Check::new(
CheckKind::PercentFormatExtraNamedArguments(
missing.iter().map(|&arg| arg.to_string()).collect(),
),
location,
);
if checker.patch(check.kind.code()) {
if let Ok(fix) = remove_unused_format_arguments_from_dict(checker.locator, &missing, right)
{
check.amend(fix);
}
}
checker.add_check(check);
}
/// F505
pub(crate) fn percent_format_missing_arguments(
checker: &mut Checker,
summary: &CFormatSummary,
right: &Expr,
location: Range,
) {
if summary.num_positional > 0 {
return;
}
if let ExprKind::Dict { keys, values } = &right.node {
if values.len() > keys.len() {
return; // contains **x splat
}
let mut keywords = FxHashSet::default();
for key in keys {
match &key.node {
ExprKind::Constant {
value: Constant::Str(value),
..
} => {
keywords.insert(value);
}
_ => {
return; // Dynamic keys present
}
}
}
let missing: Vec<&String> = summary
.keywords
.iter()
.filter(|k| !keywords.contains(k))
.collect();
if !missing.is_empty() {
checker.add_check(Check::new(
CheckKind::PercentFormatMissingArgument(
missing.iter().map(|&s| s.clone()).collect(),
),
location,
));
}
}
}
/// F506
pub(crate) fn percent_format_mixed_positional_and_named(
checker: &mut Checker,
summary: &CFormatSummary,
location: Range,
) {
if !(summary.num_positional == 0 || summary.keywords.is_empty()) {
checker.add_check(Check::new(
CheckKind::PercentFormatMixedPositionalAndNamed,
location,
));
}
}
/// F507
pub(crate) fn percent_format_positional_count_mismatch(
checker: &mut Checker,
summary: &CFormatSummary,
right: &Expr,
location: Range,
) {
if !summary.keywords.is_empty() {
return;
}
match &right.node {
ExprKind::List { elts, .. } | ExprKind::Tuple { elts, .. } | ExprKind::Set { elts, .. } => {
let mut found = 0;
for elt in elts {
if let ExprKind::Starred { .. } = &elt.node {
return;
}
found += 1;
}
if found != summary.num_positional {
checker.add_check(Check::new(
CheckKind::PercentFormatPositionalCountMismatch(summary.num_positional, found),
location,
));
}
}
_ => {}
}
}
/// F508
pub(crate) fn percent_format_star_requires_sequence(
checker: &mut Checker,
summary: &CFormatSummary,
right: &Expr,
location: Range,
) {
if summary.starred {
match &right.node {
ExprKind::Dict { .. } | ExprKind::DictComp { .. } => checker.add_check(Check::new(
CheckKind::PercentFormatStarRequiresSequence,
location,
)),
_ => {}
}
}
}
/// F522
pub(crate) fn string_dot_format_extra_named_arguments(
checker: &mut Checker,
summary: &FormatSummary,
keywords: &[Keyword],
location: Range,
) {
if has_star_star_kwargs(keywords) {
return;
}
let keywords = keywords.iter().filter_map(|k| {
let KeywordData { arg, .. } = &k.node;
arg.as_ref()
});
let missing: Vec<&str> = keywords
.filter_map(|arg| {
if summary.keywords.contains(arg) {
None
} else {
Some(arg.as_str())
}
})
.collect();
if missing.is_empty() {
return;
}
let mut check = Check::new(
CheckKind::StringDotFormatExtraNamedArguments(
missing.iter().map(|&arg| arg.to_string()).collect(),
),
location,
);
if checker.patch(check.kind.code()) {
if let Ok(fix) =
remove_unused_keyword_arguments_from_format_call(checker.locator, &missing, location)
{
check.amend(fix);
}
}
checker.add_check(check);
}
/// F523
pub(crate) fn string_dot_format_extra_positional_arguments(
checker: &mut Checker,
summary: &FormatSummary,
args: &[Expr],
location: Range,
) {
if has_star_args(args) {
return;
}
let missing: Vec<usize> = (0..args.len())
.filter(|i| !(summary.autos.contains(i) || summary.indexes.contains(i)))
.collect();
if missing.is_empty() {
return;
}
checker.add_check(Check::new(
CheckKind::StringDotFormatExtraPositionalArguments(
missing
.iter()
.map(std::string::ToString::to_string)
.collect::<Vec<String>>(),
),
location,
));
}
/// F524
pub(crate) fn string_dot_format_missing_argument(
checker: &mut Checker,
summary: &FormatSummary,
args: &[Expr],
keywords: &[Keyword],
location: Range,
) {
if has_star_args(args) || has_star_star_kwargs(keywords) {
return;
}
let keywords: FxHashSet<_> = keywords
.iter()
.filter_map(|k| {
let KeywordData { arg, .. } = &k.node;
arg.as_ref()
})
.collect();
let missing: Vec<String> = summary
.autos
.iter()
.chain(summary.indexes.iter())
.filter(|&&i| i >= args.len())
.map(ToString::to_string)
.chain(
summary
.keywords
.iter()
.filter(|k| !keywords.contains(k))
.cloned(),
)
.collect();
if !missing.is_empty() {
checker.add_check(Check::new(
CheckKind::StringDotFormatMissingArguments(missing),
location,
));
}
}
/// F525
pub(crate) fn string_dot_format_mixing_automatic(
checker: &mut Checker,
summary: &FormatSummary,
location: Range,
) {
if !(summary.autos.is_empty() || summary.indexes.is_empty()) {
checker.add_check(Check::new(
CheckKind::StringDotFormatMixingAutomatic,
location,
));
}
}