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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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