Skip to content

Commit

Permalink
Support Range#member? method for Performance/RangeInclude cop
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima committed Jun 4, 2020
1 parent fe535e2 commit 387ad53
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 26 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### New features

* [#125](https://github.com/rubocop-hq/rubocop-performance/pull/125): Support `Range#member?` method for `Performance/RangeInclude` cop. ([@fatkodima][])

* [#115](https://github.com/rubocop-hq/rubocop-performance/issues/115): Support `String#sub` and `String#sub!` methods for `Performance/DeletePrefix` and `Performance/DeleteSuffix` cops. ([@fatkodima][])

### Bug fixes
Expand Down
3 changes: 2 additions & 1 deletion config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,11 @@ Performance/OpenStruct:
Safe: false

Performance/RangeInclude:
Description: 'Use `Range#cover?` instead of `Range#include?`.'
Description: 'Use `Range#cover?` instead of `Range#include?` (or `Range#member?`).'
Reference: 'https://github.com/JuanitoFatas/fast-ruby#cover-vs-include-code'
Enabled: true
VersionAdded: '0.36'
VersionChanged: '1.7'
Safe: false

Performance/RedundantBlockCall:
Expand Down
7 changes: 4 additions & 3 deletions docs/modules/ROOT/pages/cops_performance.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -727,16 +727,16 @@ end
| No
| Yes (Unsafe)
| 0.36
| -
| 1.7
|===

This cop identifies uses of `Range#include?`, which iterates over each
This cop identifies uses of `Range#include?` and `Range#member?`, which iterates over each
item in a `Range` to see if a specified item is there. In contrast,
`Range#cover?` simply compares the target item with the beginning and
end points of the `Range`. In a great majority of cases, this is what
is wanted.

This cop is `Safe: false` by default because `Range#include?` and
This cop is `Safe: false` by default because `Range#include?` (or `Range#member?`) and
`Range#cover?` are not equivalent behaviour.

=== Examples
Expand All @@ -745,6 +745,7 @@ This cop is `Safe: false` by default because `Range#include?` and
----
# bad
('a'..'z').include?('b') # => true
('a'..'z').member?('b') # => true
# good
('a'..'z').cover?('b') # => true
Expand Down
16 changes: 9 additions & 7 deletions lib/rubocop/cop/performance/range_include.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,19 @@
module RuboCop
module Cop
module Performance
# This cop identifies uses of `Range#include?`, which iterates over each
# This cop identifies uses of `Range#include?` and `Range#member?`, which iterates over each
# item in a `Range` to see if a specified item is there. In contrast,
# `Range#cover?` simply compares the target item with the beginning and
# end points of the `Range`. In a great majority of cases, this is what
# is wanted.
#
# This cop is `Safe: false` by default because `Range#include?` and
# This cop is `Safe: false` by default because `Range#include?` (or `Range#member?`) and
# `Range#cover?` are not equivalent behaviour.
#
# @example
# # bad
# ('a'..'z').include?('b') # => true
# ('a'..'z').member?('b') # => true
#
# # good
# ('a'..'z').cover?('b') # => true
Expand All @@ -24,21 +25,22 @@ module Performance
#
# ('a'..'z').cover?('yellow') # => true
class RangeInclude < Cop
MSG = 'Use `Range#cover?` instead of `Range#include?`.'
MSG = 'Use `Range#cover?` instead of `Range#%<bad_method>s`.'

# TODO: If we traced out assignments of variables to their uses, we
# might pick up on a few more instances of this issue
# Right now, we only detect direct calls on a Range literal
# (We don't even catch it if the Range is in double parens)

def_node_matcher :range_include, <<~PATTERN
(send {irange erange (begin {irange erange})} :include? ...)
(send {irange erange (begin {irange erange})} ${:include? :member?} ...)
PATTERN

def on_send(node)
return unless range_include(node)

add_offense(node, location: :selector)
range_include(node) do |bad_method|
message = format(MSG, bad_method: bad_method)
add_offense(node, location: :selector, message: message)
end
end

def autocorrect(node)
Expand Down
39 changes: 24 additions & 15 deletions spec/rubocop/cop/performance/range_include_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,26 @@
RSpec.describe RuboCop::Cop::Performance::RangeInclude do
subject(:cop) { described_class.new }

it 'autocorrects (a..b).include? without parens' do
new_source = autocorrect_source('(a..b).include? 1')
expect(new_source).to eq '(a..b).cover? 1'
end
%i[include? member?].each do |method|
it "autocorrects (a..b).#{method} without parens" do
new_source = autocorrect_source("(a..b).#{method} 1")
expect(new_source).to eq '(a..b).cover? 1'
end

it 'autocorrects (a...b).include? without parens' do
new_source = autocorrect_source('(a...b).include? 1')
expect(new_source).to eq '(a...b).cover? 1'
end
it "autocorrects (a...b).#{method} without parens" do
new_source = autocorrect_source("(a...b).#{method} 1")
expect(new_source).to eq '(a...b).cover? 1'
end

it 'autocorrects (a..b).include? with parens' do
new_source = autocorrect_source('(a..b).include?(1)')
expect(new_source).to eq '(a..b).cover?(1)'
end
it "autocorrects (a..b).#{method} with parens" do
new_source = autocorrect_source("(a..b).#{method}(1)")
expect(new_source).to eq '(a..b).cover?(1)'
end

it 'autocorrects (a...b).include? with parens' do
new_source = autocorrect_source('(a...b).include?(1)')
expect(new_source).to eq '(a...b).cover?(1)'
it "autocorrects (a...b).#{method} with parens" do
new_source = autocorrect_source("(a...b).#{method}(1)")
expect(new_source).to eq '(a...b).cover?(1)'
end
end

it 'formats the error message correctly for (a..b).include? 1' do
Expand All @@ -29,4 +31,11 @@
^^^^^^^^ Use `Range#cover?` instead of `Range#include?`.
RUBY
end

it 'formats the error message correctly for (a..b).member? 1' do
expect_offense(<<~RUBY)
(a..b).member? 1
^^^^^^^ Use `Range#cover?` instead of `Range#member?`.
RUBY
end
end

0 comments on commit 387ad53

Please sign in to comment.