Skip to content

Commit cc4d63f

Browse files
danparizherGlyphack
authored andcommitted
[flake8-simplify] Correct behavior for str.split/rsplit with maxsplit=0 (SIM905) (astral-sh#18075)
Fixes astral-sh#18069 <!-- Thank you for contributing to Ruff/ty! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? (Please prefix with `[ty]` for ty pull requests.) - Does this pull request include references to any relevant issues? --> ## Summary This PR addresses a bug in the `flake8-simplify` rule `SIM905` (split-static-string) where `str.split(maxsplit=0)` and `str.rsplit(maxsplit=0)` produced incorrect results for empty strings or strings starting/ending with whitespace. The fix ensures that the linting rule's suggested replacements now align with Python's native behavior for these specific `maxsplit=0` scenarios. ## Test Plan 1. Added new test cases to the existing `crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM905.py` fixture to cover the scenarios described in issue astral-sh#18069. 2. Ran `cargo test -p ruff_linter`. 3. Verified and accepted the updated snapshots for `SIM905.py` using `cargo insta review`. The new snapshots confirm the corrected behavior for `maxsplit=0`.
1 parent 94c247d commit cc4d63f

File tree

3 files changed

+401
-6
lines changed

3 files changed

+401
-6
lines changed

crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM905.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,23 @@
110110
# https://github.com/astral-sh/ruff/issues/18042
111111
print("a,b".rsplit(","))
112112
print("a,b,c".rsplit(",", 1))
113+
114+
# https://github.com/astral-sh/ruff/issues/18069
115+
116+
print("".split(maxsplit=0))
117+
print("".split(sep=None, maxsplit=0))
118+
print(" ".split(maxsplit=0))
119+
print(" ".split(sep=None, maxsplit=0))
120+
print(" x ".split(maxsplit=0))
121+
print(" x ".split(sep=None, maxsplit=0))
122+
print(" x ".split(maxsplit=0))
123+
print(" x ".split(sep=None, maxsplit=0))
124+
print("".rsplit(maxsplit=0))
125+
print("".rsplit(sep=None, maxsplit=0))
126+
print(" ".rsplit(maxsplit=0))
127+
print(" ".rsplit(sep=None, maxsplit=0))
128+
print(" x ".rsplit(maxsplit=0))
129+
print(" x ".rsplit(maxsplit=0))
130+
print(" x ".rsplit(sep=None, maxsplit=0))
131+
print(" x ".rsplit(maxsplit=0))
132+
print(" x ".rsplit(sep=None, maxsplit=0))

crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ pub(crate) fn split_static_string(
8383
let sep_arg = arguments.find_argument_value("sep", 0);
8484
let split_replacement = if let Some(sep) = sep_arg {
8585
match sep {
86-
Expr::NoneLiteral(_) => split_default(str_value, maxsplit_value),
86+
Expr::NoneLiteral(_) => split_default(str_value, maxsplit_value, direction),
8787
Expr::StringLiteral(sep_value) => {
8888
let sep_value_str = sep_value.value.to_str();
8989
Some(split_sep(
@@ -99,7 +99,7 @@ pub(crate) fn split_static_string(
9999
}
100100
}
101101
} else {
102-
split_default(str_value, maxsplit_value)
102+
split_default(str_value, maxsplit_value, direction)
103103
};
104104

105105
let mut diagnostic = Diagnostic::new(SplitStaticString, call.range());
@@ -144,14 +144,19 @@ fn construct_replacement(elts: &[&str], flags: StringLiteralFlags) -> Expr {
144144
})
145145
}
146146

147-
fn split_default(str_value: &StringLiteralValue, max_split: i32) -> Option<Expr> {
147+
fn split_default(
148+
str_value: &StringLiteralValue,
149+
max_split: i32,
150+
direction: Direction,
151+
) -> Option<Expr> {
148152
// From the Python documentation:
149153
// > If sep is not specified or is None, a different splitting algorithm is applied: runs of
150154
// > consecutive whitespace are regarded as a single separator, and the result will contain
151155
// > no empty strings at the start or end if the string has leading or trailing whitespace.
152156
// > Consequently, splitting an empty string or a string consisting of just whitespace with
153157
// > a None separator returns [].
154158
// https://docs.python.org/3/library/stdtypes.html#str.split
159+
let string_val = str_value.to_str();
155160
match max_split.cmp(&0) {
156161
Ordering::Greater => {
157162
// Autofix for `maxsplit` without separator not yet implemented, as
@@ -160,14 +165,30 @@ fn split_default(str_value: &StringLiteralValue, max_split: i32) -> Option<Expr>
160165
None
161166
}
162167
Ordering::Equal => {
163-
let list_items: Vec<&str> = vec![str_value.to_str()];
168+
// Behavior for maxsplit = 0 when sep is None:
169+
// - If the string is empty or all whitespace, result is [].
170+
// - Otherwise:
171+
// - " x ".split(maxsplit=0) -> ['x ']
172+
// - " x ".rsplit(maxsplit=0) -> [' x']
173+
// - "".split(maxsplit=0) -> []
174+
// - " ".split(maxsplit=0) -> []
175+
let processed_str = if direction == Direction::Left {
176+
string_val.trim_start()
177+
} else {
178+
string_val.trim_end()
179+
};
180+
let list_items: &[_] = if processed_str.is_empty() {
181+
&[]
182+
} else {
183+
&[processed_str]
184+
};
164185
Some(construct_replacement(
165-
&list_items,
186+
list_items,
166187
str_value.first_literal_flags(),
167188
))
168189
}
169190
Ordering::Less => {
170-
let list_items: Vec<&str> = str_value.to_str().split_whitespace().collect();
191+
let list_items: Vec<&str> = string_val.split_whitespace().collect();
171192
Some(construct_replacement(
172193
&list_items,
173194
str_value.first_literal_flags(),

0 commit comments

Comments
 (0)