Skip to content

Commit

Permalink
Improve B005 documentation to reflect duplicate-character behavior (#…
Browse files Browse the repository at this point in the history
…7601)

## Summary

B005 only flags `.strip()` calls for which the argument includes
duplicate characters. This is consistent with bugbear, but isn't
explained in the documentation.
  • Loading branch information
charliermarsh committed Sep 22, 2023
1 parent 9d16e46 commit f137819
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,25 @@ use crate::checkers::ast::Checker;
/// `str.removesuffix` to remove an exact prefix or suffix from a string,
/// respectively, which should be preferred when possible.
///
/// ## Known problems
/// As a heuristic, this rule only flags multi-character strings that contain
/// duplicate characters. This allows usages like `.strip("xyz")`, which
/// removes all occurrences of the characters `x`, `y`, and `z` from the
/// leading and trailing ends of the string, but not `.strip("foo")`.
///
/// The use of unique, multi-character strings may be intentional and
/// consistent with the intent of `.strip()`, `.lstrip()`, or `.rstrip()`,
/// while the use of duplicate-character strings is very likely to be a
/// mistake.
///
/// ## Example
/// ```python
/// "abcba".strip("ab") # "c"
/// "text.txt".strip(".txt") # "ex"
/// ```
///
/// Use instead:
/// ```python
/// "abcba".removeprefix("ab").removesuffix("ba") # "c"
/// "text.txt".removesuffix(".txt") # "text"
/// ```
///
/// ## References
Expand All @@ -39,7 +50,7 @@ pub struct StripWithMultiCharacters;
impl Violation for StripWithMultiCharacters {
#[derive_message_formats]
fn message(&self) -> String {
format!("Using `.strip()` with multi-character strings is misleading the reader")
format!("Using `.strip()` with multi-character strings is misleading")
}
}

Expand All @@ -65,8 +76,7 @@ pub(crate) fn strip_with_multi_characters(
return;
};

let num_chars = value.chars().count();
if num_chars > 1 && num_chars != value.chars().unique().count() {
if value.chars().count() > 1 && !value.chars().all_unique() {
checker
.diagnostics
.push(Diagnostic::new(StripWithMultiCharacters, expr.range()));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
---
B005.py:4:1: B005 Using `.strip()` with multi-character strings is misleading the reader
B005.py:4:1: B005 Using `.strip()` with multi-character strings is misleading
|
2 | s.strip(s) # no warning
3 | s.strip("we") # no warning
Expand All @@ -11,7 +11,7 @@ B005.py:4:1: B005 Using `.strip()` with multi-character strings is misleading th
6 | s.strip("\n\t ") # no warning
|

B005.py:7:1: B005 Using `.strip()` with multi-character strings is misleading the reader
B005.py:7:1: B005 Using `.strip()` with multi-character strings is misleading
|
5 | s.strip("e") # no warning
6 | s.strip("\n\t ") # no warning
Expand All @@ -21,7 +21,7 @@ B005.py:7:1: B005 Using `.strip()` with multi-character strings is misleading th
9 | s.lstrip("we") # no warning
|

B005.py:10:1: B005 Using `.strip()` with multi-character strings is misleading the reader
B005.py:10:1: B005 Using `.strip()` with multi-character strings is misleading
|
8 | s.lstrip(s) # no warning
9 | s.lstrip("we") # no warning
Expand All @@ -31,7 +31,7 @@ B005.py:10:1: B005 Using `.strip()` with multi-character strings is misleading t
12 | s.lstrip("\n\t ") # no warning
|

B005.py:13:1: B005 Using `.strip()` with multi-character strings is misleading the reader
B005.py:13:1: B005 Using `.strip()` with multi-character strings is misleading
|
11 | s.lstrip("e") # no warning
12 | s.lstrip("\n\t ") # no warning
Expand All @@ -41,7 +41,7 @@ B005.py:13:1: B005 Using `.strip()` with multi-character strings is misleading t
15 | s.rstrip("we") # warning
|

B005.py:16:1: B005 Using `.strip()` with multi-character strings is misleading the reader
B005.py:16:1: B005 Using `.strip()` with multi-character strings is misleading
|
14 | s.rstrip(s) # no warning
15 | s.rstrip("we") # warning
Expand All @@ -51,7 +51,7 @@ B005.py:16:1: B005 Using `.strip()` with multi-character strings is misleading t
18 | s.rstrip("\n\t ") # no warning
|

B005.py:19:1: B005 Using `.strip()` with multi-character strings is misleading the reader
B005.py:19:1: B005 Using `.strip()` with multi-character strings is misleading
|
17 | s.rstrip("e") # no warning
18 | s.rstrip("\n\t ") # no warning
Expand All @@ -61,7 +61,7 @@ B005.py:19:1: B005 Using `.strip()` with multi-character strings is misleading t
21 | s.strip("") # no warning
|

B005.py:22:1: B005 Using `.strip()` with multi-character strings is misleading the reader
B005.py:22:1: B005 Using `.strip()` with multi-character strings is misleading
|
20 | s.strip("a") # no warning
21 | s.strip("") # no warning
Expand All @@ -71,7 +71,7 @@ B005.py:22:1: B005 Using `.strip()` with multi-character strings is misleading t
24 | s.strip("\u0074\u0065\u0073\u0074") # warning
|

B005.py:24:1: B005 Using `.strip()` with multi-character strings is misleading the reader
B005.py:24:1: B005 Using `.strip()` with multi-character strings is misleading
|
22 | s.strip("ああ") # warning
23 | s.strip("\ufeff") # no warning
Expand Down

0 comments on commit f137819

Please sign in to comment.