Skip to content

Commit

Permalink
Update Style/RedundantSort to be unsafe.
Browse files Browse the repository at this point in the history
In an array where there are multiple elements where `a <=> b` == 0, or where the transformation done by the block has the same result, there will be a different element returned between `sort.last` and `max` (or `sort_by.last` and `max_by`), because `sort.last` will take the last element but `max` will return the first element. As such, this is not a safe change, as the specific element returned will change.
  • Loading branch information
dvandersluis committed Sep 27, 2021
1 parent b0cb09f commit 71de8c2
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog/change_update_styleredundantsort_to_be_unsafe.md
@@ -0,0 +1 @@
* [#10122](https://github.com/rubocop/rubocop/pull/10122): Update `Style/RedundantSort` to be unsafe, and revert the special case for `size` from [#10061](https://github.com/rubocop/rubocop/pull/10061). ([@dvandersluis][])
3 changes: 2 additions & 1 deletion config/default.yml
Expand Up @@ -4160,7 +4160,6 @@ Style/NumericLiteralPrefix:
- zero_with_o
- zero_only


Style/NumericLiterals:
Description: >-
Add underscores to large numeric literals to improve their
Expand Down Expand Up @@ -4481,6 +4480,8 @@ Style/RedundantSort:
`max_by` instead of `sort_by...last`, etc.
Enabled: true
VersionAdded: '0.76'
VersionChanged: <<next>>
Safe: false

Style/RedundantSortBy:
Description: 'Use `sort` instead of `sort_by { |x| x }`.'
Expand Down
27 changes: 27 additions & 0 deletions lib/rubocop/cop/style/redundant_sort.rb
Expand Up @@ -12,6 +12,33 @@ module Style
# `Enumerable#max_by` can replace `Enumerable#sort_by` calls
# after which only the first or last element is used.
#
# @safety
# This cop is unsafe, because `sort...last` and `max` may not return the
# same element in all cases.
#
# In an enumerable where there are multiple elements where `a <=> b == 0`,
# or where the transformation done by the `sort_by` block has the
# same result, `sort.last` and `max` (or `sort_by.last` and `max_by`)
# will return different elements. `sort.last` will return the last
# element but `max` will return the first element.
#
# For example:
#
# [source,ruby]
# ----
# class MyString < String; end
# strings = [MyString.new('test'), 'test']
# strings.sort.last.class #=> String
# strings.max.class #=> MyString
# ----
#
# [source,ruby]
# ----
# words = %w(dog horse mouse)
# words.sort_by { |word| word.length }.last #=> 'mouse'
# words.max_by { |word| word.length } #=> 'horse'
# ----
#
# @example
# # bad
# [2, 1, 3].sort.first
Expand Down

0 comments on commit 71de8c2

Please sign in to comment.