Skip to content

Commit

Permalink
Fix an autocorrect for Style/RedundantSort with logical operator
Browse files Browse the repository at this point in the history
This PR is fix an autocorrect for `Style/RedundantSort` with predicate operator

If you have a code like the following:
```ruby
[1, 2, 3]
  .sort_by { |x| x.length }
  .first || []
```

After autocorrection it will look like this:
```ruby
[1, 2, 3]
  .sort_by { |x| x.length }
  || []
```

This is a syntax error.
```
Error: : eval:3: syntax error, unexpected '|', expecting end-of-input
  || []
  ^
 (SyntaxError)
 ```
  • Loading branch information
ydah committed Aug 2, 2022
1 parent 20990ed commit 4dd187a
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 6 deletions.
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

0 comments on commit 4dd187a

Please sign in to comment.