Skip to content

Commit

Permalink
Remove SafeMode from Count and Detect
Browse files Browse the repository at this point in the history
  • Loading branch information
Ryan Rosenblum committed Jul 18, 2019
1 parent 69ba7f7 commit ddfe1e0
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 67 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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][])
Expand Down Expand Up @@ -47,3 +49,4 @@
[@bquorning]: https://github.com/bquorning
[@dduugg]: https://github.com/dduugg
[@tejasbubane]: https://github.com/tejasbubane
[@rrosenblum]: https://github.com/rrosenblum
5 changes: 2 additions & 3 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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'
Expand Down
6 changes: 5 additions & 1 deletion lib/rubocop/cop/performance/count.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion lib/rubocop/cop/performance/detect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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?
Expand Down
24 changes: 10 additions & 14 deletions manual/cops_performance.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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)
Expand Down
24 changes: 0 additions & 24 deletions spec/rubocop/cop/performance/count_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
24 changes: 0 additions & 24 deletions spec/rubocop/cop/performance/detect_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit ddfe1e0

Please sign in to comment.