Skip to content

Commit

Permalink
Add new Performance/SortReverse cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Jun 8, 2020
1 parent bb3cf35 commit c4eca8e
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### New features

* [#130](https://github.com/rubocop-hq/rubocop-performance/pull/130): Add new `Performance/SortReverse` cop. ([@fatkodima][])
* [#81](https://github.com/rubocop-hq/rubocop-performance/issues/81): Add new `Performance/StringInclude` cop. ([@fatkodima][])
* [#123](https://github.com/rubocop-hq/rubocop-performance/pull/123): Add new `Performance/AncestorsInclude` cop. ([@fatkodima][])
* [#125](https://github.com/rubocop-hq/rubocop-performance/pull/125): Support `Range#member?` method for `Performance/RangeInclude` cop. ([@fatkodima][])
Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ Performance/Size:
Enabled: true
VersionAdded: '0.30'

Performance/SortReverse:
Description: 'Use `sort.reverse` instead of `sort { |a, b| b <=> a }`.'
Enabled: true
VersionAdded: '1.7'

Performance/StartWith:
Description: 'Use `start_with?` instead of a regex match anchored to the beginning of a string.'
Reference: 'https://github.com/JuanitoFatas/fast-ruby#stringmatch-vs-stringstart_withstringend_with-code-start-code-end'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
* xref:cops_performance.adoc#performanceregexpmatch[Performance/RegexpMatch]
* xref:cops_performance.adoc#performancereverseeach[Performance/ReverseEach]
* xref:cops_performance.adoc#performancesize[Performance/Size]
* xref:cops_performance.adoc#performancesortreverse[Performance/SortReverse]
* xref:cops_performance.adoc#performancestartwith[Performance/StartWith]
* xref:cops_performance.adoc#performancestringinclude[Performance/StringInclude]
* xref:cops_performance.adoc#performancestringreplacement[Performance/StringReplacement]
Expand Down
26 changes: 26 additions & 0 deletions docs/modules/ROOT/pages/cops_performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -1167,6 +1167,32 @@ have been assigned to an array or a hash.

* https://github.com/JuanitoFatas/fast-ruby#arraylength-vs-arraysize-vs-arraycount-code

== Performance/SortReverse

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Enabled
| Yes
| Yes
| 1.7
| -
|===

This cop identifies places where `sort { |a, b| b <=> a }`
can be replaced by a faster `sort.reverse`.

=== Examples

[source,ruby]
----
# bad
array.sort { |a, b| b <=> a }
# good
array.sort.reverse
----

== Performance/StartWith

|===
Expand Down
69 changes: 69 additions & 0 deletions lib/rubocop/cop/performance/sort_reverse.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Performance
# This cop identifies places where `sort { |a, b| b <=> a }`
# can be replaced by a faster `sort.reverse`.
#
# @example
# # bad
# array.sort { |a, b| b <=> a }
#
# # good
# array.sort.reverse
#
class SortReverse < Cop
include RangeHelp

MSG = 'Use `sort.reverse` instead of `%<bad_method>s`.'

def_node_matcher :sort_with_block?, <<~PATTERN
(block
$(send _ :sort)
(args (arg $_a) (arg $_b))
$send)
PATTERN

def_node_matcher :replaceable_body?, <<~PATTERN
(send (lvar %1) :<=> (lvar %2))
PATTERN

def on_block(node)
sort_with_block?(node) do |send, var_a, var_b, body|
replaceable_body?(body, var_b, var_a) do
range = sort_range(send, node)

add_offense(
node,
location: range,
message: message(var_a, var_b)
)
end
end
end

def autocorrect(node)
sort_with_block?(node) do |send, _var_a, _var_b, _body|
lambda do |corrector|
range = sort_range(send, node)
replacement = 'sort.reverse'
corrector.replace(range, replacement)
end
end
end

private

def sort_range(send, node)
range_between(send.loc.selector.begin_pos, node.loc.end.end_pos)
end

def message(var_a, var_b)
bad_method = "sort { |#{var_a}, #{var_b}| #{var_b} <=> #{var_a} }"
format(MSG, bad_method: bad_method)
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/performance_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
require_relative 'performance/regexp_match'
require_relative 'performance/reverse_each'
require_relative 'performance/size'
require_relative 'performance/sort_reverse'
require_relative 'performance/start_with'
require_relative 'performance/string_include'
require_relative 'performance/string_replacement'
Expand Down
39 changes: 39 additions & 0 deletions spec/rubocop/cop/performance/sort_reverse_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Performance::SortReverse do
subject(:cop) { described_class.new(config) }

let(:config) do
# Suppress ChainArrayAllocation offences
RuboCop::Config.new('Performance/ChainArrayAllocation' => { 'Enabled' => false })
end

it 'registers an offense and corrects when sorting in reverse order' do
expect_offense(<<~RUBY)
array.sort { |a, b| b <=> a }
^^^^^^^^^^^^^^^^^^^^^^^ Use `sort.reverse` instead of `sort { |a, b| b <=> a }`.
RUBY

expect_correction(<<~RUBY)
array.sort.reverse
RUBY
end

it 'does not register an offense when sorting in direct order' do
expect_no_offenses(<<~RUBY)
array.sort { |a, b| a <=> b }
RUBY
end

it 'does not register an offense when sorting in reverse order by some property' do
expect_no_offenses(<<~RUBY)
array.sort { |a, b| b.x <=> a.x }
RUBY
end

it 'does not register an offense when using `sort.reverse`' do
expect_no_offenses(<<~RUBY)
array.sort.reverse
RUBY
end
end

0 comments on commit c4eca8e

Please sign in to comment.