diff --git a/changelog/fix_an_autocorrect_for_style_redundant_sort.md b/changelog/fix_an_autocorrect_for_style_redundant_sort.md new file mode 100644 index 00000000000..fefff4be415 --- /dev/null +++ b/changelog/fix_an_autocorrect_for_style_redundant_sort.md @@ -0,0 +1 @@ +* [#10853](https://github.com/rubocop/rubocop/pull/10853): Fix an autocorrect for `Style/RedundantSort` with logical operator. ([@ydah][]) diff --git a/lib/rubocop/cop/style/redundant_sort.rb b/lib/rubocop/cop/style/redundant_sort.rb index f3e20b3bde6..c79cc8240d6 100644 --- a/lib/rubocop/cop/style/redundant_sort.rb +++ b/lib/rubocop/cop/style/redundant_sort.rb @@ -123,13 +123,8 @@ def find_redundant_sort(*nodes) def register_offense(node, sort_node, sorter, accessor) message = message(node, sorter, accessor) - add_offense(offense_range(sort_node, node), message: message) do |corrector| - # Remove accessor, e.g. `first` or `[-1]`. - corrector.remove(range_between(accessor_start(node), node.loc.expression.end_pos)) - - # Replace "sort" or "sort_by" with the appropriate min/max method. - corrector.replace(sort_node.loc.selector, suggestion(sorter, accessor, arg_value(node))) + autocorrect(corrector, node, sort_node, sorter, accessor) end end @@ -149,6 +144,20 @@ def message(node, sorter, accessor) accessor_source: accessor_source) end + 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)) + # Replace "sort" or "sort_by" with the appropriate min/max method. + corrector.replace(sort_node.loc.selector, suggestion(sorter, accessor, arg_value(node))) + # Replace to avoid syntax errors when followed by a logical operator. + replace_with_logical_operator(corrector, node) if with_logical_operator?(node) + end + + def replace_with_logical_operator(corrector, node) + corrector.insert_after(node.child_nodes.first, " #{node.parent.loc.operator.source}") + corrector.remove(node.parent.loc.operator) + end + def suggestion(sorter, accessor, arg) base(accessor, arg) + suffix(sorter) end @@ -187,6 +196,12 @@ def accessor_start(node) node.loc.selector.begin_pos end end + + def with_logical_operator?(node) + return unless (parent = node.parent) + + parent.or_type? || parent.and_type? + end end end end diff --git a/spec/rubocop/cop/style/redundant_sort_spec.rb b/spec/rubocop/cop/style/redundant_sort_spec.rb index f69bd6009c5..5d9716c861d 100644 --- a/spec/rubocop/cop/style/redundant_sort_spec.rb +++ b/spec/rubocop/cop/style/redundant_sort_spec.rb @@ -56,6 +56,81 @@ RUBY end + it 'registers an offense when first is called on sort_by with line breaks' do + expect_offense(<<~RUBY) + [1, 2, 3] + .sort_by { |x| x.length } + ^^^^^^^^^^^^^^^^^^^^^^^^ Use `min_by` instead of `sort_by...first`. + .first + RUBY + + expect_correction(<<~RUBY) + [1, 2, 3] + .min_by { |x| x.length } + #{' '} + RUBY + end + + it 'registers an offense when first is called on sort_by with line breaks and `||` operator' do + expect_offense(<<~RUBY) + [1, 2, 3] + .sort_by { |x| x.length } + ^^^^^^^^^^^^^^^^^^^^^^^^ Use `min_by` instead of `sort_by...first`. + .first || [] + RUBY + + expect_correction(<<~RUBY) + [1, 2, 3] + .min_by { |x| x.length } || + [] + RUBY + end + + it 'registers an offense when first is called on sort_by with line breaks and `&&` operator' do + expect_offense(<<~RUBY) + [1, 2, 3] + .sort_by { |x| x.length } + ^^^^^^^^^^^^^^^^^^^^^^^^ Use `min_by` instead of `sort_by...first`. + .first && [] + RUBY + + expect_correction(<<~RUBY) + [1, 2, 3] + .min_by { |x| x.length } && + [] + RUBY + end + + it 'registers an offense when first is called on sort_by with line breaks and `or` operator' do + expect_offense(<<~RUBY) + [1, 2, 3] + .sort_by { |x| x.length } + ^^^^^^^^^^^^^^^^^^^^^^^^ Use `min_by` instead of `sort_by...first`. + .first or [] + RUBY + + expect_correction(<<~RUBY) + [1, 2, 3] + .min_by { |x| x.length } or + [] + RUBY + end + + it 'registers an offense when first is called on sort_by with line breaks and `and` operator' do + expect_offense(<<~RUBY) + [1, 2, 3] + .sort_by { |x| x.length } + ^^^^^^^^^^^^^^^^^^^^^^^^ Use `min_by` instead of `sort_by...first`. + .first and [] + RUBY + + expect_correction(<<~RUBY) + [1, 2, 3] + .min_by { |x| x.length } and + [] + RUBY + end + it 'registers an offense when first is called on sort_by no block' do expect_offense(<<~RUBY) [1, 2].sort_by(&:something).first