Skip to content

Commit

Permalink
[Fix rubocop#10061] Fix a false positive for Style/RedundantSort
Browse files Browse the repository at this point in the history
Fixes rubocop#10061.

This PR fixes a false positive for `Style/RedundantSort`
when using `size` method in the block.
  • Loading branch information
koic committed Sep 1, 2021
1 parent 2a49404 commit 13e731b
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog/fix_a_false_positive_for_style_redundant_sort.md
@@ -0,0 +1 @@
* [#10061](https://github.com/rubocop/rubocop/issues/10061): Fix a false positive for `Style/RedundantSort` when using `size` method in the block. ([@koic][])
19 changes: 17 additions & 2 deletions lib/rubocop/cop/style/redundant_sort.rb
Expand Up @@ -76,22 +76,37 @@ class RedundantSort < Base

def on_send(node)
if (sort_node, sorter, accessor = redundant_sort?(node.parent))
return if use_size_method_in_block?(sort_node)

ancestor = node.parent
elsif (sort_node, sorter, accessor = redundant_sort?(node.parent&.parent))
return if use_size_method_in_block?(sort_node)

ancestor = node.parent.parent
else
return
end

register_offense(ancestor, sort_node, sorter, accessor)
end

private

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

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

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

add_offense(offense_range(sort_node, ancestor), message: message) do |corrector|
autocorrect(corrector, ancestor, sort_node, sorter, accessor)
end
end

private

def autocorrect(corrector, node, sort_node, sorter, accessor)
# Remove accessor, e.g. `first` or `[-1]`.
corrector.remove(range_between(accessor_start(node), node.loc.expression.end_pos))
Expand Down
12 changes: 11 additions & 1 deletion spec/rubocop/cop/style/redundant_sort_spec.rb
Expand Up @@ -231,7 +231,17 @@
expect_no_offenses('[[1, 2], [3, 4]].first.sort')
end

# `[2, 1, 3].sort_by.first` is equivalent to `[2, 1, 3].first`, but this
# `[3, 1, 2].sort_by(&:size).last` is equivalent to `[3, 1, 2].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 }`.
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

# `[2, 1, 3].sort_by(&:size).first` is not equivalent to `[2, 1, 3].first`, but this
# cop would "correct" it to `[2, 1, 3].min_by`.
it 'does not register an offense when sort_by is not given a block' do
expect_no_offenses('[2, 1, 3].sort_by.first')
Expand Down

0 comments on commit 13e731b

Please sign in to comment.