Skip to content

Commit

Permalink
Merge pull request #10119 from dvandersluis/issue/10118
Browse files Browse the repository at this point in the history
[Fix #10118] Fix crash with `Style/RedundantSort` when the block doesn't only contain a single `send` node
  • Loading branch information
koic committed Sep 27, 2021
2 parents 5015be3 + cbf3281 commit 1184830
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog/fix_fix_crash_with_styleredundantsort_when.md
@@ -0,0 +1 @@
* [#10118](https://github.com/rubocop/rubocop/issues/10118): Fix crash with `Style/RedundantSort` when the block doesn't only contain a single `send` node. ([@dvandersluis][])
9 changes: 8 additions & 1 deletion lib/rubocop/cop/style/redundant_sort.rb
Expand Up @@ -93,12 +93,19 @@ def on_send(node)
private

def use_size_method_in_block?(sort_node)
return true if sort_node.send_type? && sort_node.block_node&.body&.method?(:size)
return true if block_calls_send?(sort_node)
return false unless sort_node.block_argument?

sort_node.last_argument.children.first.value == :size
end

def block_calls_send?(node)
return false unless node.send_type? && node.block_node
return false unless node.block_node.body.send_type?

node.block_node.body.method?(:size)
end

def register_offense(ancestor, sort_node, sorter, accessor)
message = message(ancestor, sorter, accessor)

Expand Down
15 changes: 13 additions & 2 deletions spec/rubocop/cop/style/redundant_sort_spec.rb
Expand Up @@ -231,12 +231,12 @@
expect_no_offenses('[[1, 2], [3, 4]].first.sort')
end

# `[3, 1, 2].sort_by(&:size).last` is equivalent to `[3, 1, 2].max_by(&:size)`.
# `[2, 1, 3].sort_by(&:size).last` is not equivalent to `[2, 1, 3].max_by(&:size)`.
it 'does not register an offense when using `sort_by(&:size).last`' do
expect_no_offenses('[2, 1, 3].sort_by(&:size).last')
end

# `[3, 1, 2].sort_by { |i| i.size }.last` is equivalent to `[3, 1, 2].max_by { |i| i.size }`.
# `[2, 1, 3].sort_by { |i| i.size }.last` is not equivalent to `[2, 1, 3].max_by { |i| i.size }`.
it 'does not register an offense when using `sort_by { |i| i.size }.last`' do
expect_no_offenses('[2, 1, 3].sort_by { |i| i.size }.last')
end
Expand All @@ -247,6 +247,17 @@
expect_no_offenses('[2, 1, 3].sort_by.first')
end

it 'registers an offense with `sort_by { a || b }`' do
expect_offense(<<~RUBY)
x.sort_by { |y| y.foo || bar }.last
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `max_by` instead of `sort_by...last`.
RUBY

expect_correction(<<~RUBY)
x.max_by { |y| y.foo || bar }
RUBY
end

context 'when not taking first or last element' do
it 'does not register an offense when [1] is called on sort' do
expect_no_offenses('[1, 2, 3].sort[1]')
Expand Down

0 comments on commit 1184830

Please sign in to comment.