diff --git a/CHANGELOG.md b/CHANGELOG.md index 1288d2112f..2c0ce45d42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/config/default.yml b/config/default.yml index 549b066dab..6a33802fbf 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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`.' @@ -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` @@ -98,7 +96,7 @@ Performance/Detect: SafeAutoCorrect: false Enabled: true VersionAdded: '0.30' - VersionChanged: '1.5' + VersionChanged: '1.8' Performance/DoubleStartEndWith: Description: >- diff --git a/docs/modules/ROOT/pages/cops_performance.adoc b/docs/modules/ROOT/pages/cops_performance.adoc index 94eb1f5b03..72b58ff384 100644 --- a/docs/modules/ROOT/pages/cops_performance.adoc +++ b/docs/modules/ROOT/pages/cops_performance.adoc @@ -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: @@ -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: @@ -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 } diff --git a/lib/rubocop/cop/performance/count.rb b/lib/rubocop/cop/performance/count.rb index 826748e62f..0b20aa2141 100644 --- a/lib/rubocop/cop/performance/count.rb +++ b/lib/rubocop/cop/performance/count.rb @@ -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 @@ -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 diff --git a/lib/rubocop/cop/performance/detect.rb b/lib/rubocop/cop/performance/detect.rb index 9957c10f88..dd3995ebb5 100644 --- a/lib/rubocop/cop/performance/detect.rb +++ b/lib/rubocop/cop/performance/detect.rb @@ -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 @@ -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 } @@ -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 diff --git a/spec/rubocop/cop/performance/count_spec.rb b/spec/rubocop/cop/performance/count_spec.rb index ad483e60a2..9d6426bb43 100644 --- a/spec/rubocop/cop/performance/count_spec.rb +++ b/spec/rubocop/cop/performance/count_spec.rb @@ -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 diff --git a/spec/rubocop/cop/performance/detect_spec.rb b/spec/rubocop/cop/performance/detect_spec.rb index 529ea4d344..7e08245003 100644 --- a/spec/rubocop/cop/performance/detect_spec.rb +++ b/spec/rubocop/cop/performance/detect_spec.rb @@ -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