Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SafeMode from Count and Detect #72

Merged
merged 1 commit into from
Aug 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Bug fixes

* [#74](https://github.com/rubocop-hq/rubocop-performance/pull/74): Fix an error for `Performance/RedundantMerge` when `MaxKeyValuePairs` option is set to `null`. ([@koic][])
* [#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][])

## 1.4.1 (2019-07-29)

Expand Down Expand Up @@ -54,3 +55,4 @@
[@bquorning]: https://github.com/bquorning
[@dduugg]: https://github.com/dduugg
[@tejasbubane]: https://github.com/tejasbubane
[@rrosenblum]: https://github.com/rrosenblum
9 changes: 4 additions & 5 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,10 @@ 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'
VersionChanged: '1.5'

Performance/Detect:
Description: >-
Expand All @@ -59,10 +58,10 @@ 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'
VersionChanged: '1.5'

Performance/DoubleStartEndWith:
Description: >-
Expand Down
3 changes: 0 additions & 3 deletions lib/rubocop/cop/performance/count.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ module Performance
#
# Model.where(id: [1, 2, 3]).to_a.count { |m| m.method == true }
class Count < Cop
include SafeMode
include RangeHelp

MSG = 'Use `count` instead of `%<selector>s...%<counter>s`.'
Expand All @@ -51,8 +50,6 @@ class Count < Cop
PATTERN

def on_send(node)
return if rails_safe_mode?

count_candidate?(node) do |selector_node, selector, counter|
return unless eligible_node?(node)

Expand Down
4 changes: 0 additions & 4 deletions lib/rubocop/cop/performance/detect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ module Performance
# own meaning. Correcting ActiveRecord methods with this cop should be
# considered unsafe.
class Detect < Cop
include SafeMode

MSG = 'Use `%<prefer>s` instead of ' \
'`%<first_method>s.%<second_method>s`.'
REVERSE_MSG = 'Use `reverse.%<prefer>s` instead of ' \
Expand All @@ -38,8 +36,6 @@ class Detect < Cop
PATTERN

def on_send(node)
return if rails_safe_mode?

detect_candidate?(node) do |receiver, second_method, args|
return unless args.empty?
return unless receiver
Expand Down
16 changes: 2 additions & 14 deletions manual/cops_performance.md
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ 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 | 1.5

This cop is used to identify usages of `count` on an `Enumerable` that
follow calls to `select` or `reject`. Querying logic can instead be
Expand Down Expand Up @@ -224,17 +224,11 @@ 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 | 1.5

This cop is used to identify usages of
`select.first`, `select.last`, `find_all.first`, and `find_all.last`
Expand All @@ -259,12 +253,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