Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix #10061] Fix a false positive for Style/RedundantSort #10062

Merged
merged 1 commit into from Sep 1, 2021

Conversation

koic
Copy link
Member

@koic koic commented Aug 31, 2021

Fixes #10061.

This PR fixes a false positive for Style/RedundantSort when using size method in the block.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 31, 2021

Hmm, why is this behaving differently for strings?

@koic koic force-pushed the mark_style_redundant_sort_as_unsafe branch from bacea01 to 2b2e16c Compare August 31, 2021 12:53
@koic koic changed the title [Fix #10061] Mark Style/RedundantSort as unsafe [Fix #10061] Fix a false positive for Style/RedundantSort Aug 31, 2021
@koic koic force-pushed the mark_style_redundant_sort_as_unsafe branch from 2b2e16c to a878752 Compare August 31, 2021 12:54
@koic
Copy link
Member Author

koic commented Aug 31, 2021

Oops! That's my mistake. In fact, it is a false positive because using size method in the block. I've update this PR.

Fixes rubocop#10061.

This PR fixes a false positive for `Style/RedundantSort`
when using `size` method in the block.
@koic koic force-pushed the mark_style_redundant_sort_as_unsafe branch from a878752 to 13e731b Compare September 1, 2021 05:27
@bbatsov bbatsov merged commit 4a67b3f into rubocop:master Sep 1, 2021
@bbatsov
Copy link
Collaborator

bbatsov commented Sep 1, 2021

Thanks!

@koic koic deleted the mark_style_redundant_sort_as_unsafe branch September 1, 2021 07:23
@Mifrill
Copy link

Mifrill commented Sep 1, 2021

@koic Thanks! It was so quick!

@koic
Copy link
Member Author

koic commented Sep 1, 2021

@Mifrill You're welcome!

@Drenmi
Copy link
Collaborator

Drenmi commented Sep 3, 2021

Is this the correct approach? If I understand it correctly, this is due to how a certain Ruby version handles ties differently between two different sorting methods. It seems a bit dangerous to rely on this tie-breaking result in our programs to begin with, as it can change between versions (or platforms, or implementations) based Ruby on internals. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RedundantSort rule not working properly in case of max_by(&:size)
4 participants