diff --git a/README.md b/README.md index b1ce339155..d6096dadb2 100644 --- a/README.md +++ b/README.md @@ -500,6 +500,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/22.10.27/ | B017 | NoAssertRaisesException | `assertRaises(Exception):` should be considered evil. | | | B018 | UselessExpression | Found useless expression. Either assign it to a variable or remove it. | | | B025 | DuplicateTryBlockException | try-except block with duplicate exception `Exception` | | +| B026 | StarArgUnpackingAfterKeywordArg | Star-arg unpacking after a keyword argument is strongly discouraged. | | ### flake8-builtins @@ -654,7 +655,7 @@ including: - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/) - [`flake8-annotations`](https://pypi.org/project/flake8-annotations/) - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) -- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (17/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (18/32) - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34) - [`autoflake`](https://pypi.org/project/autoflake/) (1/7) @@ -677,7 +678,7 @@ Today, Ruff can be used to replace Flake8 when used with any of the following pl - [`flake8-quotes`](https://pypi.org/project/flake8-quotes/) - [`flake8-annotations`](https://pypi.org/project/flake8-annotations/) - [`flake8-comprehensions`](https://pypi.org/project/flake8-comprehensions/) -- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (17/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (18/32) Ruff also implements the functionality that you get from [`yesqa`](https://github.com/asottile/yesqa), and a subset of the rules implemented in [`pyupgrade`](https://pypi.org/project/pyupgrade/) (14/34). diff --git a/resources/test/fixtures/B026.py b/resources/test/fixtures/B026.py new file mode 100644 index 0000000000..2116d7c59a --- /dev/null +++ b/resources/test/fixtures/B026.py @@ -0,0 +1,21 @@ +""" +Should emit: +B026 - on lines 16, 17, 18, 19, 20, 21 +""" + + +def foo(bar, baz, bam): + print(bar, baz, bam) + + +bar_baz = ["bar", "baz"] + +foo("bar", "baz", bam="bam") +foo("bar", baz="baz", bam="bam") +foo(bar="bar", baz="baz", bam="bam") +foo(bam="bam", *["bar", "baz"]) +foo(bam="bam", *bar_baz) +foo(baz="baz", bam="bam", *["bar"]) +foo(bar="bar", baz="baz", bam="bam", *[]) +foo(bam="bam", *["bar"], *["baz"]) +foo(*["bar"], bam="bam", *["baz"]) diff --git a/src/check_ast.rs b/src/check_ast.rs index 7627b07de0..853d988679 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1064,6 +1064,11 @@ where if self.settings.enabled.contains(&CheckCode::B005) { flake8_bugbear::plugins::strip_with_multi_characters(self, expr, func, args); } + if self.settings.enabled.contains(&CheckCode::B026) { + flake8_bugbear::plugins::star_arg_unpacking_after_keyword_arg( + self, args, keywords, + ); + } // flake8-comprehensions if self.settings.enabled.contains(&CheckCode::C400) { diff --git a/src/checks.rs b/src/checks.rs index dc798f8e27..078bd925fb 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -92,6 +92,7 @@ pub enum CheckCode { B017, B018, B025, + B026, // flake8-comprehensions C400, C401, @@ -356,6 +357,7 @@ pub enum CheckKind { NoAssertRaisesException, UselessExpression, DuplicateTryBlockException(String), + StarArgUnpackingAfterKeywordArg, // flake8-comprehensions UnnecessaryGeneratorList, UnnecessaryGeneratorSet, @@ -569,6 +571,7 @@ impl CheckCode { CheckCode::B017 => CheckKind::NoAssertRaisesException, CheckCode::B018 => CheckKind::UselessExpression, CheckCode::B025 => CheckKind::DuplicateTryBlockException("Exception".to_string()), + CheckCode::B026 => CheckKind::StarArgUnpackingAfterKeywordArg, // flake8-comprehensions CheckCode::C400 => CheckKind::UnnecessaryGeneratorList, CheckCode::C401 => CheckKind::UnnecessaryGeneratorSet, @@ -784,6 +787,7 @@ impl CheckCode { CheckCode::B017 => CheckCategory::Flake8Bugbear, CheckCode::B018 => CheckCategory::Flake8Bugbear, CheckCode::B025 => CheckCategory::Flake8Bugbear, + CheckCode::B026 => CheckCategory::Flake8Bugbear, CheckCode::C400 => CheckCategory::Flake8Comprehensions, CheckCode::C401 => CheckCategory::Flake8Comprehensions, CheckCode::C402 => CheckCategory::Flake8Comprehensions, @@ -965,6 +969,7 @@ impl CheckKind { CheckKind::NoAssertRaisesException => &CheckCode::B017, CheckKind::UselessExpression => &CheckCode::B018, CheckKind::DuplicateTryBlockException(_) => &CheckCode::B025, + CheckKind::StarArgUnpackingAfterKeywordArg => &CheckCode::B026, // flake8-comprehensions CheckKind::UnnecessaryGeneratorList => &CheckCode::C400, CheckKind::UnnecessaryGeneratorSet => &CheckCode::C401, @@ -1297,6 +1302,13 @@ impl CheckKind { CheckKind::DuplicateTryBlockException(name) => { format!("try-except block with duplicate exception `{name}`") } + CheckKind::StarArgUnpackingAfterKeywordArg => { + "Star-arg unpacking after a keyword argument is strongly discouraged, because it \ + only works when the keyword parameter is declared after all parameters supplied \ + by the unpacked sequence, and this change of ordering can surprise and mislead \ + readers." + .to_string() + } // flake8-comprehensions CheckKind::UnnecessaryGeneratorList => { "Unnecessary generator (rewrite as a `list` comprehension)".to_string() @@ -1675,6 +1687,9 @@ impl CheckKind { CheckKind::NoAssertRaisesException => { "`assertRaises(Exception):` should be considered evil.".to_string() } + CheckKind::StarArgUnpackingAfterKeywordArg => { + "Star-arg unpacking after a keyword argument is strongly discouraged.".to_string() + } _ => self.body(), } } diff --git a/src/checks_gen.rs b/src/checks_gen.rs index ba09d4fb2e..1c929ee18e 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -53,6 +53,7 @@ pub enum CheckCodePrefix { B018, B02, B025, + B026, C, C4, C40, @@ -343,6 +344,7 @@ impl CheckCodePrefix { CheckCode::B017, CheckCode::B018, CheckCode::B025, + CheckCode::B026, ], CheckCodePrefix::B0 => vec![ CheckCode::B002, @@ -360,6 +362,7 @@ impl CheckCodePrefix { CheckCode::B017, CheckCode::B018, CheckCode::B025, + CheckCode::B026, ], CheckCodePrefix::B00 => vec![ CheckCode::B002, @@ -393,8 +396,9 @@ impl CheckCodePrefix { CheckCodePrefix::B016 => vec![CheckCode::B016], CheckCodePrefix::B017 => vec![CheckCode::B017], CheckCodePrefix::B018 => vec![CheckCode::B018], - CheckCodePrefix::B02 => vec![CheckCode::B025], + CheckCodePrefix::B02 => vec![CheckCode::B025, CheckCode::B026], CheckCodePrefix::B025 => vec![CheckCode::B025], + CheckCodePrefix::B026 => vec![CheckCode::B026], CheckCodePrefix::C => vec![ CheckCode::C400, CheckCode::C401, @@ -1057,6 +1061,7 @@ impl CheckCodePrefix { CheckCodePrefix::B018 => PrefixSpecificity::Explicit, CheckCodePrefix::B02 => PrefixSpecificity::Tens, CheckCodePrefix::B025 => PrefixSpecificity::Explicit, + CheckCodePrefix::B026 => PrefixSpecificity::Explicit, CheckCodePrefix::C => PrefixSpecificity::Category, CheckCodePrefix::C4 => PrefixSpecificity::Hundreds, CheckCodePrefix::C40 => PrefixSpecificity::Tens, diff --git a/src/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs index 506b69b3d4..02cf75a14e 100644 --- a/src/flake8_bugbear/plugins/mod.rs +++ b/src/flake8_bugbear/plugins/mod.rs @@ -6,6 +6,7 @@ pub use duplicate_exceptions::{duplicate_exceptions, duplicate_handler_exception pub use function_call_argument_default::function_call_argument_default; pub use mutable_argument_default::mutable_argument_default; pub use redundant_tuple_in_exception_handler::redundant_tuple_in_exception_handler; +pub use star_arg_unpacking_after_keyword_arg::star_arg_unpacking_after_keyword_arg; pub use strip_with_multi_characters::strip_with_multi_characters; pub use unary_prefix_increment::unary_prefix_increment; pub use unreliable_callable_check::unreliable_callable_check; @@ -21,6 +22,7 @@ mod duplicate_exceptions; mod function_call_argument_default; mod mutable_argument_default; mod redundant_tuple_in_exception_handler; +mod star_arg_unpacking_after_keyword_arg; mod strip_with_multi_characters; mod unary_prefix_increment; mod unreliable_callable_check; diff --git a/src/flake8_bugbear/plugins/star_arg_unpacking_after_keyword_arg.rs b/src/flake8_bugbear/plugins/star_arg_unpacking_after_keyword_arg.rs new file mode 100644 index 0000000000..f684ed3611 --- /dev/null +++ b/src/flake8_bugbear/plugins/star_arg_unpacking_after_keyword_arg.rs @@ -0,0 +1,25 @@ +use rustpython_ast::{Expr, ExprKind, Keyword}; + +use crate::ast::types::Range; +use crate::check_ast::Checker; +use crate::checks::{Check, CheckKind}; + +/// B026 +pub fn star_arg_unpacking_after_keyword_arg( + checker: &mut Checker, + args: &[Expr], + keywords: &[Keyword], +) { + if let Some(keyword) = keywords.first() { + for arg in args { + if let ExprKind::Starred { .. } = arg.node { + if arg.location > keyword.location { + checker.add_check(Check::new( + CheckKind::StarArgUnpackingAfterKeywordArg, + Range::from_located(arg), + )); + } + } + } + } +} diff --git a/src/linter.rs b/src/linter.rs index 36fcdf7161..81dfdce620 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -307,6 +307,7 @@ mod tests { #[test_case(CheckCode::B017, Path::new("B017.py"); "B017")] #[test_case(CheckCode::B018, Path::new("B018.py"); "B018")] #[test_case(CheckCode::B025, Path::new("B025.py"); "B025")] + #[test_case(CheckCode::B026, Path::new("B026.py"); "B026")] #[test_case(CheckCode::C400, Path::new("C400.py"); "C400")] #[test_case(CheckCode::C401, Path::new("C401.py"); "C401")] #[test_case(CheckCode::C402, Path::new("C402.py"); "C402")] diff --git a/src/snapshots/ruff__linter__tests__B026_B026.py.snap b/src/snapshots/ruff__linter__tests__B026_B026.py.snap new file mode 100644 index 0000000000..6b7c8dc5de --- /dev/null +++ b/src/snapshots/ruff__linter__tests__B026_B026.py.snap @@ -0,0 +1,61 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: StarArgUnpackingAfterKeywordArg + location: + row: 16 + column: 15 + end_location: + row: 16 + column: 30 + fix: ~ +- kind: StarArgUnpackingAfterKeywordArg + location: + row: 17 + column: 15 + end_location: + row: 17 + column: 23 + fix: ~ +- kind: StarArgUnpackingAfterKeywordArg + location: + row: 18 + column: 26 + end_location: + row: 18 + column: 34 + fix: ~ +- kind: StarArgUnpackingAfterKeywordArg + location: + row: 19 + column: 37 + end_location: + row: 19 + column: 40 + fix: ~ +- kind: StarArgUnpackingAfterKeywordArg + location: + row: 20 + column: 15 + end_location: + row: 20 + column: 23 + fix: ~ +- kind: StarArgUnpackingAfterKeywordArg + location: + row: 20 + column: 25 + end_location: + row: 20 + column: 33 + fix: ~ +- kind: StarArgUnpackingAfterKeywordArg + location: + row: 21 + column: 25 + end_location: + row: 21 + column: 33 + fix: ~ +