[flake8-simplify] Correct behavior for str.split/rsplit with maxsplit=0 (SIM905)
#18075
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
ntBre
added
bug
ntBre
reviewed
May 14, 2025
Contributor
ntBre
left a comment
ntBre
left a comment
There was a problem hiding this comment.
Thanks! Just a couple of simplification suggestions, but the logic looks great to me. And thanks for all of the test cases!
crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs
Outdated
Show resolved
Hide resolved
crates/ruff_linter/src/rules/flake8_simplify/rules/split_static_string.rs
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Thanks for the review! This is definitely a lot better. I can also work on the logic for #18101 and include it in this PR as well. Let me know if that makes the most sense! |
Contributor
No problem, thanks for handling the suggestions! I think we can hold off on #18101 for now. I went ahead and put SIM905 in the title since that's the rule I tested, but I think it's likely more widespread, and we might be able to handle it generally rather than in each individual rule (at least I hope so!). |
ntBre
changed the title
flake8-simplify]: Correct behavior for str.split/rsplit with maxsplit=0 (SIM905)flake8-simplify] Correct behavior for str.split/rsplit with maxsplit=0 (SIM905)
ntBre
changed the title
flake8-simplify] Correct behavior for str.split/rsplit with maxsplit=0 (SIM905)flake8-simplify] Correct behavior for str.split/rsplit with maxsplit=0 (SIM905)
dcreager
added a commit
that referenced
this pull request
May 14, 2025
…rals * origin/main: [ty] Add type-expression syntax link to invalid-type-expression (#18104) [`flake8-simplify`] add fix safety section (`SIM103`) (#18086) [ty] mypy_primer: fix static-frame setup (#18103) [`flake8-simplify`] Correct behavior for `str.split`/`rsplit` with `maxsplit=0` (`SIM905`) (#18075) [ty] Fix more generics-related TODOs (#18062)
Glyphack
pushed a commit
to Glyphack/ruff
that referenced
this pull request
May 21, 2025
…axsplit=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`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #18069
Summary
This PR addresses a bug in the
flake8-simplifyruleSIM905(split-static-string) wherestr.split(maxsplit=0)andstr.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 specificmaxsplit=0scenarios.Test Plan
crates/ruff_linter/resources/test/fixtures/flake8_simplify/SIM905.pyfixture to cover the scenarios described in issue SIM905 fix breaks"".split(maxsplit=0)and similar expressions #18069.cargo test -p ruff_linter.SIM905.pyusingcargo insta review. The new snapshots confirm the corrected behavior formaxsplit=0.