diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ccd6897bf..a4e81e1fe5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## master (unreleased) +* [#69](https://github.com/rubocop-hq/rubocop-performance/issues/69): Remove `SafeMode` from `Performance/Count` and `Performance/Detect`. Set `SafeAutoCorrect` to `false` for these cops by default. ([@rrosenblum][]) + ### Bug fixes * [#67](https://github.com/rubocop-hq/rubocop-performance/issues/67): Fix an error for `Performance/RedundantMerge` when `MaxKeyValuePairs` option is set to `null`. ([@koic][]) @@ -47,3 +49,4 @@ [@bquorning]: https://github.com/bquorning [@dduugg]: https://github.com/dduugg [@tejasbubane]: https://github.com/tejasbubane +[@rrosenblum]: https://github.com/rrosenblum diff --git a/config/default.yml b/config/default.yml index cc896dd05d..69ef750f1c 100644 --- a/config/default.yml +++ b/config/default.yml @@ -44,8 +44,7 @@ Performance/Count: # 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. - # If you understand the known risk, you can disable `SafeMode`. - SafeMode: true + SafeAutoCorrect: false Enabled: true VersionAdded: '0.31' VersionChanged: '0.39' @@ -59,7 +58,7 @@ Performance/Detect: # frameworks. `ActiveRecord` does not implement a `detect` method and `find` # has its own meaning. Correcting `ActiveRecord` methods with this cop # should be considered unsafe. - SafeMode: true + SafeAutoCorrect: false Enabled: true VersionAdded: '0.30' VersionChanged: '0.39' diff --git a/lib/rubocop/cop/performance/count.rb b/lib/rubocop/cop/performance/count.rb index 7dc54ec9ae..2d11d55766 100644 --- a/lib/rubocop/cop/performance/count.rb +++ b/lib/rubocop/cop/performance/count.rb @@ -26,6 +26,10 @@ module Performance # Model.select(:value).count # # `ActiveRecord` compatibility: + # + # Due to the compatibility issue, this cop is automatically disabled if + # the `Rails` cops are enabled. + # # `ActiveRecord` will ignore the block that is passed to `count`. # Other methods, such as `select`, will convert the association to an # array and then run the block on the array. A simple work around to @@ -51,7 +55,7 @@ class Count < Cop PATTERN def on_send(node) - return if rails_safe_mode? + return if rails? count_candidate?(node) do |selector_node, selector, counter| return unless eligible_node?(node) diff --git a/lib/rubocop/cop/performance/detect.rb b/lib/rubocop/cop/performance/detect.rb index b22ea5d44f..4486fb7582 100644 --- a/lib/rubocop/cop/performance/detect.rb +++ b/lib/rubocop/cop/performance/detect.rb @@ -19,6 +19,10 @@ module Performance # [].reverse.detect { |item| true } # # `ActiveRecord` compatibility: + # + # Due to the compatibility issue, this cop is automatically disabled if + # the `Rails` cops are enabled. + # # `ActiveRecord` does not implement a `detect` method and `find` has its # own meaning. Correcting ActiveRecord methods with this cop should be # considered unsafe. @@ -38,7 +42,7 @@ class Detect < Cop PATTERN def on_send(node) - return if rails_safe_mode? + return if rails? detect_candidate?(node) do |receiver, second_method, args| return unless args.empty? diff --git a/manual/cops_performance.md b/manual/cops_performance.md index f8d156f3f2..6b7a30cd14 100644 --- a/manual/cops_performance.md +++ b/manual/cops_performance.md @@ -184,13 +184,17 @@ array.sort_by { |a| a[:foo] } Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged --- | --- | --- | --- | --- -Enabled | Yes | Yes | 0.31 | 0.39 +Enabled | Yes | Yes (Unsafe) | 0.31 | 0.39 This cop is used to identify usages of `count` on an `Enumerable` that follow calls to `select` or `reject`. Querying logic can instead be passed to the `count` call. `ActiveRecord` compatibility: + +Due to the compatibility issue, this cop is automatically disabled if +the `Rails` cops are enabled. + `ActiveRecord` will ignore the block that is passed to `count`. Other methods, such as `select`, will convert the association to an array and then run the block on the array. A simple work around to @@ -224,23 +228,21 @@ Model.select('field AS field_one').count Model.select(:value).count ``` -### Configurable attributes - -Name | Default value | Configurable values ---- | --- | --- -SafeMode | `true` | Boolean - ## Performance/Detect Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged --- | --- | --- | --- | --- -Enabled | Yes | Yes | 0.30 | 0.39 +Enabled | Yes | Yes (Unsafe) | 0.30 | 0.39 This cop is used to identify usages of `select.first`, `select.last`, `find_all.first`, and `find_all.last` and change them to use `detect` instead. `ActiveRecord` compatibility: + +Due to the compatibility issue, this cop is automatically disabled if +the `Rails` cops are enabled. + `ActiveRecord` does not implement a `detect` method and `find` has its own meaning. Correcting ActiveRecord methods with this cop should be considered unsafe. @@ -259,12 +261,6 @@ considered unsafe. [].reverse.detect { |item| true } ``` -### Configurable attributes - -Name | Default value | Configurable values ---- | --- | --- -SafeMode | `true` | Boolean - ### References * [https://github.com/JuanitoFatas/fast-ruby#enumerabledetect-vs-enumerableselectfirst-code](https://github.com/JuanitoFatas/fast-ruby#enumerabledetect-vs-enumerableselectfirst-code) diff --git a/spec/rubocop/cop/performance/count_spec.rb b/spec/rubocop/cop/performance/count_spec.rb index f2b098f4c5..aaebb0ad55 100644 --- a/spec/rubocop/cop/performance/count_spec.rb +++ b/spec/rubocop/cop/performance/count_spec.rb @@ -271,28 +271,4 @@ def count(&block) end end end - - context 'SafeMode true' do - subject(:cop) { described_class.new(config) } - - let(:config) do - RuboCop::Config.new( - 'Rails' => { - 'Enabled' => true - }, - 'Performance/Count' => { - 'SafeMode' => true - } - ) - end - - shared_examples 'selectors' do |selector| - it "allows using array.#{selector}...size" do - expect_no_offenses("[1, 2, 3].#{selector} { |e| e.even? }.size") - end - end - - it_behaves_like('selectors', 'select') - it_behaves_like('selectors', 'reject') - end end diff --git a/spec/rubocop/cop/performance/detect_spec.rb b/spec/rubocop/cop/performance/detect_spec.rb index 46408315d3..d2a630b0f3 100644 --- a/spec/rubocop/cop/performance/detect_spec.rb +++ b/spec/rubocop/cop/performance/detect_spec.rb @@ -212,28 +212,4 @@ it_behaves_like 'detect_autocorrect', 'detect' it_behaves_like 'detect_autocorrect', 'find' end - - context 'SafeMode true' do - let(:config) do - RuboCop::Config.new( - 'Rails' => { - 'Enabled' => true - }, - 'Style/CollectionMethods' => { - 'PreferredMethods' => { - 'detect' => collection_method - } - }, - 'Performance/Detect' => { - 'SafeMode' => true - } - ) - end - - select_methods.each do |method| - it "doesn't register an offense when first is called on #{method}" do - expect_no_offenses("[1, 2, 3].#{method} { |i| i % 2 == 0 }.first") - end - end - end end