Skip to content

[flake8-simplify] Fix SIM905 autofix for rsplit creating a reversed list literal#18045

Merged
dylwil3 merged 2 commits intoastral-sh:mainfrom
LaBatata101:fix-SIM905
May 12, 2025
Merged

[flake8-simplify] Fix SIM905 autofix for rsplit creating a reversed list literal#18045
dylwil3 merged 2 commits intoastral-sh:mainfrom
LaBatata101:fix-SIM905

Conversation

@LaBatata101
Copy link
Contributor

Summary

Fixes #18042

Test Plan

Update existing snapshot tests.

@github-actions
Copy link
Contributor

github-actions bot commented May 12, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Collaborator

@dylwil3 dylwil3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! I wonder if the fix isn't quite right, and whether the change could be simpler.

Comment on lines 189 to 194
Direction::Right => value.rsplitn(split_n + 1, sep_value).collect(),
}
value.splitn(split_n + 1, sep_value).collect()
} else {
match direction {
Direction::Left => value.split(sep_value).collect(),
Direction::Right => value.rsplit(sep_value).collect(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it not work to just do .reverse after rsplit and rsplitn?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried using .rev but the compiler gave me an error, something about a trait not being implemented for the return type of these functions you mentioned.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this isn't a super performance-sensitive spot in the code, you could try collecting first?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Actually - we collect anyway and reversing a Vec doesn't re-allocate, so I think it's not even a perf hit really)

@dylwil3 dylwil3 added bug Something isn't working fixes Related to suggested fixes for violations labels May 12, 2025
@LaBatata101 LaBatata101 requested a review from dylwil3 May 12, 2025 19:42
Copy link
Collaborator

@dylwil3 dylwil3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonderful, thank you!

@dylwil3 dylwil3 merged commit 138ab91 into astral-sh:main May 12, 2025
34 checks passed
@LaBatata101 LaBatata101 deleted the fix-SIM905 branch May 12, 2025 19:53
dcreager added a commit that referenced this pull request May 13, 2025
…eepish

* origin/main:
  [ty] Induct into instances and subclasses when finding and applying generics (#18052)
  [ty] Allow classes to inherit from `type[Any]` or `type[Unknown]` (#18060)
  [ty] Allow a class to inherit from an intersection if the intersection contains a dynamic type and the intersection is not disjoint from `type` (#18055)
  [ty] Narrowing for `hasattr()` (#18053)
  Update reference documentation for `--python-version` (#18056)
  [`flake8-bugbear`] Ignore `B028` if `skip_file_prefixes` is present (#18047)
  [`airflow`] Apply try-catch guard to all AIR3 rules (`AIR3`) (#17887)
  [`pylint`] add fix safety section (`PLW3301`) (#17878)
  Update `--python` to accept paths to executables in virtual environments (#17954)
  [`pylint`] add fix safety section (`PLE4703`) (#17824)
  [`ruff`] Implement a recursive check for `RUF060` (#17976)
  [`flake8-use-pathlib`] `PTH*` suppress diagnostic for all `os.*` functions that have the `dir_fd` parameter (#17968)
  [`refurb`] Mark autofix as safe only for number literals in `FURB116` (#17692)
  [`flake8-simplify`] Fix `SIM905` autofix for `rsplit` creating a reversed list literal (#18045)
  Avoid initializing progress bars early (#18049)
dcreager added a commit that referenced this pull request May 13, 2025
…eep-dish

* origin/main:
  [ty] Infer parameter specializations of generic aliases (#18021)
  [ty] Understand homogeneous tuple annotations (#17998)
  [ty] Induct into instances and subclasses when finding and applying generics (#18052)
  [ty] Allow classes to inherit from `type[Any]` or `type[Unknown]` (#18060)
  [ty] Allow a class to inherit from an intersection if the intersection contains a dynamic type and the intersection is not disjoint from `type` (#18055)
  [ty] Narrowing for `hasattr()` (#18053)
  Update reference documentation for `--python-version` (#18056)
  [`flake8-bugbear`] Ignore `B028` if `skip_file_prefixes` is present (#18047)
  [`airflow`] Apply try-catch guard to all AIR3 rules (`AIR3`) (#17887)
  [`pylint`] add fix safety section (`PLW3301`) (#17878)
  Update `--python` to accept paths to executables in virtual environments (#17954)
  [`pylint`] add fix safety section (`PLE4703`) (#17824)
  [`ruff`] Implement a recursive check for `RUF060` (#17976)
  [`flake8-use-pathlib`] `PTH*` suppress diagnostic for all `os.*` functions that have the `dir_fd` parameter (#17968)
  [`refurb`] Mark autofix as safe only for number literals in `FURB116` (#17692)
  [`flake8-simplify`] Fix `SIM905` autofix for `rsplit` creating a reversed list literal (#18045)
  Avoid initializing progress bars early (#18049)
Glyphack pushed a commit to Glyphack/ruff that referenced this pull request May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fixes Related to suggested fixes for violations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SIM905 fix for rsplit is reversed

2 participants