From 13e731b3e91c48e9c75f00cae73eb679eeaec685 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Tue, 31 Aug 2021 18:16:22 +0900 Subject: [PATCH] [Fix #10061] Fix a false positive for `Style/RedundantSort` Fixes #10061. This PR fixes a false positive for `Style/RedundantSort` when using `size` method in the block. --- ...false_positive_for_style_redundant_sort.md | 1 + lib/rubocop/cop/style/redundant_sort.rb | 19 +++++++++++++++++-- spec/rubocop/cop/style/redundant_sort_spec.rb | 12 +++++++++++- 3 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 changelog/fix_a_false_positive_for_style_redundant_sort.md diff --git a/changelog/fix_a_false_positive_for_style_redundant_sort.md b/changelog/fix_a_false_positive_for_style_redundant_sort.md new file mode 100644 index 00000000000..88956313940 --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/style/redundant_sort.rb b/lib/rubocop/cop/style/redundant_sort.rb index ff626ab31a4..bfd705e6cef 100644 --- a/lib/rubocop/cop/style/redundant_sort.rb +++ b/lib/rubocop/cop/style/redundant_sort.rb @@ -76,13 +76,30 @@ 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| @@ -90,8 +107,6 @@ def on_send(node) 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)) diff --git a/spec/rubocop/cop/style/redundant_sort_spec.rb b/spec/rubocop/cop/style/redundant_sort_spec.rb index c226e9d9727..376ae364452 100644 --- a/spec/rubocop/cop/style/redundant_sort_spec.rb +++ b/spec/rubocop/cop/style/redundant_sort_spec.rb @@ -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')