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 an autocorrect for Style/RedundantSort with logical operator #10853

Merged
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_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][])
27 changes: 21 additions & 6 deletions lib/rubocop/cop/style/redundant_sort.rb
Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
75 changes: 75 additions & 0 deletions spec/rubocop/cop/style/redundant_sort_spec.rb
Expand Up @@ -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
Expand Down