Skip to content

Commit

Permalink
Merge pull request #125 from fatkodima/add-member-to-range_include_cop
Browse files Browse the repository at this point in the history
Support `Range#member?` method for `Performance/RangeInclude` cop
  • Loading branch information
koic authored Jun 5, 2020
2 parents f7f658e + 387ad53 commit f5de08e
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 @@ -6,6 +6,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 @@ -138,10 +138,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 @@ -815,16 +815,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 @@ -833,6 +833,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 f5de08e

Please sign in to comment.