Skip to content

Commit

Permalink
Merge pull request #157 from fatkodima/add-filter-to-detect-and-count…
Browse files Browse the repository at this point in the history
…-cops

Extend `Performance/Detect` cop with check for `filter` method and `Performance/Count` cop with checks for `find_all` and `filter` methods
  • Loading branch information
koic authored Aug 31, 2020
2 parents 5792ef0 + bab73a6 commit 8bf09de
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

### Changes

* [#157](https://github.com/rubocop-hq/rubocop-performance/pull/157): Extend `Performance/Detect` cop with check for `filter` method and `Performance/Count` cop with checks for `find_all` and `filter` methods. ([@fatkodima][])
* [#154](https://github.com/rubocop-hq/rubocop-performance/pull/154): Require RuboCop 0.87 or higher. ([@koic][])

## 1.7.1 (2020-07-18)
Expand Down
12 changes: 5 additions & 7 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,14 @@ Performance/CompareWithBlock:

Performance/Count:
Description: >-
Use `count` instead of `select...size`, `reject...size`,
`select...count`, `reject...count`, `select...length`,
and `reject...length`.
Use `count` instead of `{select,find_all,filter,reject}...{size,count,length}`.
# This cop has known compatibility issues with `ActiveRecord` and other
# frameworks. ActiveRecord's `count` ignores the block that is passed to it.
# For more information, see the documentation in the cop itself.
SafeAutoCorrect: false
Enabled: true
VersionAdded: '0.31'
VersionChanged: '1.5'
VersionChanged: '1.8'

Performance/DeletePrefix:
Description: 'Use `delete_prefix` instead of `gsub`.'
Expand All @@ -88,8 +86,8 @@ Performance/DeleteSuffix:

Performance/Detect:
Description: >-
Use `detect` instead of `select.first`, `find_all.first`,
`select.last`, and `find_all.last`.
Use `detect` instead of `select.first`, `find_all.first`, `filter.first`,
`select.last`, `find_all.last`, and `filter.last`.
Reference: 'https://github.com/JuanitoFatas/fast-ruby#enumerabledetect-vs-enumerableselectfirst-code'
# This cop has known compatibility issues with `ActiveRecord` and other
# frameworks. `ActiveRecord` does not implement a `detect` method and `find`
Expand All @@ -98,7 +96,7 @@ Performance/Detect:
SafeAutoCorrect: false
Enabled: true
VersionAdded: '0.30'
VersionChanged: '1.5'
VersionChanged: '1.8'

Performance/DoubleStartEndWith:
Description: >-
Expand Down
10 changes: 6 additions & 4 deletions docs/modules/ROOT/pages/cops_performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -377,11 +377,11 @@ array.sort_by { |a| a[:foo] }
| Yes
| Yes (Unsafe)
| 0.31
| 1.5
| 1.8
|===

This cop is used to identify usages of `count` on an `Enumerable` that
follow calls to `select` or `reject`. Querying logic can instead be
follow calls to `select`, `find_all`, `filter` or `reject`. Querying logic can instead be
passed to the `count` call.

`ActiveRecord` compatibility:
Expand Down Expand Up @@ -574,11 +574,11 @@ str.sub!(/suffix$/, '')
| Yes
| Yes (Unsafe)
| 0.30
| 1.5
| 1.8
|===

This cop is used to identify usages of
`select.first`, `select.last`, `find_all.first`, and `find_all.last`
`select.first`, `select.last`, `find_all.first`, `find_all.last`, `filter.first`, and `filter.last`
and change them to use `detect` instead.

`ActiveRecord` compatibility:
Expand All @@ -595,6 +595,8 @@ considered unsafe.
[].select { |item| true }.last
[].find_all { |item| true }.first
[].find_all { |item| true }.last
[].filter { |item| true }.first
[].filter { |item| true }.last
# good
[].detect { |item| true }
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/performance/count.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module RuboCop
module Cop
module Performance
# This cop is used to identify usages of `count` on an `Enumerable` that
# follow calls to `select` or `reject`. Querying logic can instead be
# follow calls to `select`, `find_all`, `filter` or `reject`. Querying logic can instead be
# passed to the `count` call.
#
# @example
Expand Down Expand Up @@ -45,8 +45,8 @@ class Count < Base

def_node_matcher :count_candidate?, <<~PATTERN
{
(send (block $(send _ ${:select :reject}) ...) ${:count :length :size})
(send $(send _ ${:select :reject} (:block_pass _)) ${:count :length :size})
(send (block $(send _ ${:select :filter :find_all :reject}) ...) ${:count :length :size})
(send $(send _ ${:select :filter :find_all :reject} (:block_pass _)) ${:count :length :size})
}
PATTERN

Expand Down
8 changes: 5 additions & 3 deletions lib/rubocop/cop/performance/detect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module RuboCop
module Cop
module Performance
# This cop is used to identify usages of
# `select.first`, `select.last`, `find_all.first`, and `find_all.last`
# `select.first`, `select.last`, `find_all.first`, `find_all.last`, `filter.first`, and `filter.last`
# and change them to use `detect` instead.
#
# @example
Expand All @@ -13,6 +13,8 @@ module Performance
# [].select { |item| true }.last
# [].find_all { |item| true }.first
# [].find_all { |item| true }.last
# [].filter { |item| true }.first
# [].filter { |item| true }.last
#
# # good
# [].detect { |item| true }
Expand All @@ -32,8 +34,8 @@ class Detect < Base

def_node_matcher :detect_candidate?, <<~PATTERN
{
(send $(block (send _ {:select :find_all}) ...) ${:first :last} $...)
(send $(send _ {:select :find_all} ...) ${:first :last} $...)
(send $(block (send _ {:select :find_all :filter}) ...) ${:first :last} $...)
(send $(send _ {:select :find_all :filter} ...) ${:first :last} $...)
}
PATTERN

Expand Down
2 changes: 2 additions & 0 deletions spec/rubocop/cop/performance/count_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ def count(&block)
end

it_behaves_like('selectors', 'select')
it_behaves_like('selectors', 'find_all')
it_behaves_like('selectors', 'filter')
it_behaves_like('selectors', 'reject')

context 'Active Record select' do
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/cop/performance/detect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

# rspec will not let you use a variable assigned using let outside
# of `it`
select_methods = %i[select find_all].freeze
select_methods = %i[select find_all filter].freeze

select_methods.each do |method|
it "registers an offense and corrects when first is called on #{method}" do
Expand Down

0 comments on commit 8bf09de

Please sign in to comment.