From 35cc3094b5110d3b6fea07afafe9f5206ef30025 Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Tue, 8 Nov 2022 00:10:59 +0900 Subject: [PATCH] Implement B005 (#643) --- README.md | 5 +- resources/test/fixtures/B005.py | 26 +++++++++ src/check_ast.rs | 3 ++ src/checks.rs | 8 +++ src/checks_gen.rs | 6 +++ src/flake8_bugbear/plugins/mod.rs | 2 + .../plugins/strip_with_multi_characters.rs | 28 ++++++++++ src/linter.rs | 1 + .../ruff__linter__tests__B005_B005.py.snap | 53 +++++++++++++++++++ 9 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 resources/test/fixtures/B005.py create mode 100644 src/flake8_bugbear/plugins/strip_with_multi_characters.rs create mode 100644 src/snapshots/ruff__linter__tests__B005_B005.py.snap diff --git a/README.md b/README.md index fa659237cf..0ea4322b64 100644 --- a/README.md +++ b/README.md @@ -483,6 +483,7 @@ For more, see [flake8-bugbear](https://pypi.org/project/flake8-bugbear/22.10.27/ | B002 | UnaryPrefixIncrement | Python does not support the unary prefix increment. | | | B003 | AssignmentToOsEnviron | Assigning to `os.environ` doesn't clear the environment. | | | B004 | UnreliableCallableCheck | Using `hasattr(x, '__call__')` to test if x is callable is unreliable. Use `callable(x)` for consistent results. | | +| B005 | StripWithMultiCharacters | Using `.strip()` with multi-character strings is misleading the reader. | | | B006 | MutableArgumentDefault | Do not use mutable data structures for argument defaults. | | | B007 | UnusedLoopControlVariable | Loop control variable `i` not used within the loop body. | 🛠 | | B008 | FunctionCallArgumentDefault | Do not perform function calls in argument defaults. | | @@ -626,7 +627,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/) (16/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (17/32) - [`pyupgrade`](https://pypi.org/project/pyupgrade/) (11/34) - [`autoflake`](https://pypi.org/project/autoflake/) (1/7) @@ -649,7 +650,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/) (16/32) +- [`flake8-bugbear`](https://pypi.org/project/flake8-bugbear/) (17/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/) (11/34). diff --git a/resources/test/fixtures/B005.py b/resources/test/fixtures/B005.py new file mode 100644 index 0000000000..1c582e6b80 --- /dev/null +++ b/resources/test/fixtures/B005.py @@ -0,0 +1,26 @@ +s = "qwe" +s.strip(s) # no warning +s.strip("we") # no warning +s.strip(".facebook.com") # warning +s.strip("e") # no warning +s.strip("\n\t ") # no warning +s.strip(r"\n\t ") # warning +s.lstrip(s) # no warning +s.lstrip("we") # no warning +s.lstrip(".facebook.com") # warning +s.lstrip("e") # no warning +s.lstrip("\n\t ") # no warning +s.lstrip(r"\n\t ") # warning +s.rstrip(s) # no warning +s.rstrip("we") # warning +s.rstrip(".facebook.com") # warning +s.rstrip("e") # no warning +s.rstrip("\n\t ") # no warning +s.rstrip(r"\n\t ") # warning + +from somewhere import other_type, strip + +strip("we") # no warning +other_type().lstrip() # no warning +other_type().rstrip(["a", "b", "c"]) # no warning +other_type().strip("a", "b") # no warning diff --git a/src/check_ast.rs b/src/check_ast.rs index 66cbebbc56..5358d1d396 100644 --- a/src/check_ast.rs +++ b/src/check_ast.rs @@ -1045,6 +1045,9 @@ where if self.settings.enabled.contains(&CheckCode::B004) { flake8_bugbear::plugins::unreliable_callable_check(self, expr, func, args); } + if self.settings.enabled.contains(&CheckCode::B005) { + flake8_bugbear::plugins::strip_with_multi_characters(self, expr, func, args); + } // flake8-comprehensions if self.settings.enabled.contains(&CheckCode::C400) { diff --git a/src/checks.rs b/src/checks.rs index a4078ce064..50b7857d97 100644 --- a/src/checks.rs +++ b/src/checks.rs @@ -80,6 +80,7 @@ pub enum CheckCode { B002, B003, B004, + B005, B006, B007, B008, @@ -339,6 +340,7 @@ pub enum CheckKind { UnaryPrefixIncrement, AssignmentToOsEnviron, UnreliableCallableCheck, + StripWithMultiCharacters, MutableArgumentDefault, UnusedLoopControlVariable(String), FunctionCallArgumentDefault, @@ -544,6 +546,7 @@ impl CheckCode { CheckCode::B002 => CheckKind::UnaryPrefixIncrement, CheckCode::B003 => CheckKind::AssignmentToOsEnviron, CheckCode::B004 => CheckKind::UnreliableCallableCheck, + CheckCode::B005 => CheckKind::StripWithMultiCharacters, CheckCode::B006 => CheckKind::MutableArgumentDefault, CheckCode::B007 => CheckKind::UnusedLoopControlVariable("i".to_string()), CheckCode::B008 => CheckKind::FunctionCallArgumentDefault, @@ -756,6 +759,7 @@ impl CheckCode { CheckCode::B002 => CheckCategory::Flake8Bugbear, CheckCode::B003 => CheckCategory::Flake8Bugbear, CheckCode::B004 => CheckCategory::Flake8Bugbear, + CheckCode::B005 => CheckCategory::Flake8Bugbear, CheckCode::B006 => CheckCategory::Flake8Bugbear, CheckCode::B007 => CheckCategory::Flake8Bugbear, CheckCode::B008 => CheckCategory::Flake8Bugbear, @@ -932,6 +936,7 @@ impl CheckKind { CheckKind::UnaryPrefixIncrement => &CheckCode::B002, CheckKind::AssignmentToOsEnviron => &CheckCode::B003, CheckKind::UnreliableCallableCheck => &CheckCode::B004, + CheckKind::StripWithMultiCharacters => &CheckCode::B005, CheckKind::MutableArgumentDefault => &CheckCode::B006, CheckKind::UnusedLoopControlVariable(_) => &CheckCode::B007, CheckKind::FunctionCallArgumentDefault => &CheckCode::B008, @@ -1220,6 +1225,9 @@ impl CheckKind { is callable is unreliable. Use `callable(x)` \ for consistent results." .to_string(), + CheckKind::StripWithMultiCharacters => "Using `.strip()` with multi-character strings \ + is misleading the reader." + .to_string(), CheckKind::MutableArgumentDefault => { "Do not use mutable data structures for argument defaults.".to_string() } diff --git a/src/checks_gen.rs b/src/checks_gen.rs index a0b4f99ee2..848dd8c2a3 100644 --- a/src/checks_gen.rs +++ b/src/checks_gen.rs @@ -36,6 +36,7 @@ pub enum CheckCodePrefix { B002, B003, B004, + B005, B006, B007, B008, @@ -319,6 +320,7 @@ impl CheckCodePrefix { CheckCode::B002, CheckCode::B003, CheckCode::B004, + CheckCode::B005, CheckCode::B006, CheckCode::B007, CheckCode::B008, @@ -335,6 +337,7 @@ impl CheckCodePrefix { CheckCode::B002, CheckCode::B003, CheckCode::B004, + CheckCode::B005, CheckCode::B006, CheckCode::B007, CheckCode::B008, @@ -351,6 +354,7 @@ impl CheckCodePrefix { CheckCode::B002, CheckCode::B003, CheckCode::B004, + CheckCode::B005, CheckCode::B006, CheckCode::B007, CheckCode::B008, @@ -358,6 +362,7 @@ impl CheckCodePrefix { CheckCodePrefix::B002 => vec![CheckCode::B002], CheckCodePrefix::B003 => vec![CheckCode::B003], CheckCodePrefix::B004 => vec![CheckCode::B004], + CheckCodePrefix::B005 => vec![CheckCode::B005], CheckCodePrefix::B006 => vec![CheckCode::B006], CheckCodePrefix::B007 => vec![CheckCode::B007], CheckCodePrefix::B008 => vec![CheckCode::B008], @@ -1016,6 +1021,7 @@ impl CheckCodePrefix { CheckCodePrefix::B002 => PrefixSpecificity::Explicit, CheckCodePrefix::B003 => PrefixSpecificity::Explicit, CheckCodePrefix::B004 => PrefixSpecificity::Explicit, + CheckCodePrefix::B005 => PrefixSpecificity::Explicit, CheckCodePrefix::B006 => PrefixSpecificity::Explicit, CheckCodePrefix::B007 => PrefixSpecificity::Explicit, CheckCodePrefix::B008 => PrefixSpecificity::Explicit, diff --git a/src/flake8_bugbear/plugins/mod.rs b/src/flake8_bugbear/plugins/mod.rs index 94585f756b..506b69b3d4 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 strip_with_multi_characters::strip_with_multi_characters; pub use unary_prefix_increment::unary_prefix_increment; pub use unreliable_callable_check::unreliable_callable_check; pub use unused_loop_control_variable::unused_loop_control_variable; @@ -20,6 +21,7 @@ mod duplicate_exceptions; mod function_call_argument_default; mod mutable_argument_default; mod redundant_tuple_in_exception_handler; +mod strip_with_multi_characters; mod unary_prefix_increment; mod unreliable_callable_check; mod unused_loop_control_variable; diff --git a/src/flake8_bugbear/plugins/strip_with_multi_characters.rs b/src/flake8_bugbear/plugins/strip_with_multi_characters.rs new file mode 100644 index 0000000000..7a44f8277b --- /dev/null +++ b/src/flake8_bugbear/plugins/strip_with_multi_characters.rs @@ -0,0 +1,28 @@ +use itertools::Itertools; +use rustpython_ast::{Constant, Expr, ExprKind}; + +use crate::ast::types::Range; +use crate::check_ast::Checker; +use crate::checks::{Check, CheckKind}; + +/// B005 +pub fn strip_with_multi_characters(checker: &mut Checker, expr: &Expr, func: &Expr, args: &[Expr]) { + if let ExprKind::Attribute { attr, .. } = &func.node { + if attr == "strip" || attr == "lstrip" || attr == "rstrip" { + if args.len() == 1 { + if let ExprKind::Constant { + value: Constant::Str(value), + .. + } = &args[0].node + { + if value.len() > 1 && value.chars().unique().count() != value.len() { + checker.add_check(Check::new( + CheckKind::StripWithMultiCharacters, + Range::from_located(expr), + )); + } + } + } + } + } +} diff --git a/src/linter.rs b/src/linter.rs index 3d90730488..72d1a6a1b2 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -295,6 +295,7 @@ mod tests { #[test_case(CheckCode::B002, Path::new("B002.py"); "B002")] #[test_case(CheckCode::B003, Path::new("B003.py"); "B003")] #[test_case(CheckCode::B004, Path::new("B004.py"); "B004")] + #[test_case(CheckCode::B005, Path::new("B005.py"); "B005")] #[test_case(CheckCode::B006, Path::new("B006_B008.py"); "B006")] #[test_case(CheckCode::B007, Path::new("B007.py"); "B007")] #[test_case(CheckCode::B008, Path::new("B006_B008.py"); "B008")] diff --git a/src/snapshots/ruff__linter__tests__B005_B005.py.snap b/src/snapshots/ruff__linter__tests__B005_B005.py.snap new file mode 100644 index 0000000000..dbe3b1f9a3 --- /dev/null +++ b/src/snapshots/ruff__linter__tests__B005_B005.py.snap @@ -0,0 +1,53 @@ +--- +source: src/linter.rs +expression: checks +--- +- kind: StripWithMultiCharacters + location: + row: 4 + column: 0 + end_location: + row: 4 + column: 24 + fix: ~ +- kind: StripWithMultiCharacters + location: + row: 7 + column: 0 + end_location: + row: 7 + column: 17 + fix: ~ +- kind: StripWithMultiCharacters + location: + row: 10 + column: 0 + end_location: + row: 10 + column: 25 + fix: ~ +- kind: StripWithMultiCharacters + location: + row: 13 + column: 0 + end_location: + row: 13 + column: 18 + fix: ~ +- kind: StripWithMultiCharacters + location: + row: 16 + column: 0 + end_location: + row: 16 + column: 25 + fix: ~ +- kind: StripWithMultiCharacters + location: + row: 19 + column: 0 + end_location: + row: 19 + column: 18 + fix: ~ +