diff --git a/src/check_ast.rs b/src/check_ast.rs index bc6c1c7151..48afebedb0 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -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, + ); } } } diff --git a/src/pyflakes/checks.rs b/src/pyflakes/checks.rs index a586c5bd64..290b27e7eb 100644 --- a/src/pyflakes/checks.rs +++ b/src/pyflakes/checks.rs @@ -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 { - 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 { - 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 { - 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 { - 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 { - 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 { - 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 = (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::>(), - ), - location, - )); -} - -/// F524 -pub(crate) fn string_dot_format_missing_argument( - summary: &FormatSummary, - args: &[Expr], - keywords: &[Keyword], - location: Range, -) -> Option { - 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 = 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 { - 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 { diff --git a/src/pyflakes/plugins/mod.rs b/src/pyflakes/plugins/mod.rs index 2e4b572d64..2a25c35e48 100644 --- a/src/pyflakes/plugins/mod.rs +++ b/src/pyflakes/plugins/mod.rs @@ -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; diff --git a/src/pyflakes/plugins/strings.rs b/src/pyflakes/plugins/strings.rs new file mode 100644 index 0000000000..8c4ac23d4a --- /dev/null +++ b/src/pyflakes/plugins/strings.rs @@ -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 = (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::>(), + ), + 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 = 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, + )); + } +}