From cbf3281121ad3ece66a1bfea54a8f82a8dd8b844 Mon Sep 17 00:00:00 2001 From: Daniel Vandersluis Date: Thu, 23 Sep 2021 13:32:14 -0400 Subject: [PATCH] [Fix #10118] Fix crash with `Style/RedundantSort` when the block doesn't only contain a single `send` node. --- .../fix_fix_crash_with_styleredundantsort_when.md | 1 + lib/rubocop/cop/style/redundant_sort.rb | 9 ++++++++- spec/rubocop/cop/style/redundant_sort_spec.rb | 15 +++++++++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 changelog/fix_fix_crash_with_styleredundantsort_when.md diff --git a/changelog/fix_fix_crash_with_styleredundantsort_when.md b/changelog/fix_fix_crash_with_styleredundantsort_when.md new file mode 100644 index 00000000000..8fe28195335 --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/style/redundant_sort.rb b/lib/rubocop/cop/style/redundant_sort.rb index bfd705e6cef..e4af9691c17 100644 --- a/lib/rubocop/cop/style/redundant_sort.rb +++ b/lib/rubocop/cop/style/redundant_sort.rb @@ -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) diff --git a/spec/rubocop/cop/style/redundant_sort_spec.rb b/spec/rubocop/cop/style/redundant_sort_spec.rb index 376ae364452..da7eb3bdce6 100644 --- a/spec/rubocop/cop/style/redundant_sort_spec.rb +++ b/spec/rubocop/cop/style/redundant_sort_spec.rb @@ -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 @@ -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]')