Skip to content

Commit

Permalink
[Fix #10698] Enhance Style/HashExcept to support array inclusion checks
Browse files Browse the repository at this point in the history
  • Loading branch information
nobuyo committed Jun 17, 2022
1 parent f57a0e3 commit 8a5f755
Show file tree
Hide file tree
Showing 4 changed files with 450 additions and 8 deletions.
1 change: 1 addition & 0 deletions changelog/fix_encance_stylehashexcept_to_support_array.md
@@ -0,0 +1 @@
* [#10698](https://github.com/rubocop/rubocop/issues/10698): Enhance `Style/HashExcept` to support array inclusion checks. ([@nobuyo][])
1 change: 1 addition & 0 deletions config/default.yml
Expand Up @@ -3726,6 +3726,7 @@ Style/HashExcept:
that can be replaced with `Hash#except` method.
Enabled: pending
VersionAdded: '1.7'
VersionChanged: '<<next>>'

Style/HashLikeCase:
Description: >-
Expand Down
96 changes: 88 additions & 8 deletions lib/rubocop/cop/style/hash_except.rb
Expand Up @@ -19,6 +19,9 @@ module Style
# {foo: 1, bar: 2, baz: 3}.reject {|k, v| k == :bar }
# {foo: 1, bar: 2, baz: 3}.select {|k, v| k != :bar }
# {foo: 1, bar: 2, baz: 3}.filter {|k, v| k != :bar }
# {foo: 1, bar: 2, baz: 3}.reject {|k, v| %i[foo bar].include?(k) }
# {foo: 1, bar: 2, baz: 3}.select {|k, v| !%i[foo bar].include?(k) }
# {foo: 1, bar: 2, baz: 3}.filter {|k, v| !%i[foo bar].include?(k) }
#
# # good
# {foo: 1, bar: 2, baz: 3}.except(:bar)
Expand All @@ -33,15 +36,36 @@ class HashExcept < Base
MSG = 'Use `%<prefer>s` instead.'
RESTRICT_ON_SEND = %i[reject select filter].freeze

# @!method bad_method?(node)
def_node_matcher :bad_method?, <<~PATTERN
# @!method bad_method_with_poro?(node)
def_node_matcher :bad_method_with_poro?, <<~PATTERN
(block
(send _ _)
(args
(arg _)
(arg _))
(send
_ {:== :!= :eql?} _))
{
(send
_ {:== :!= :eql? :include?} _)
(send
(send
_ {:== :!= :eql? :include?} _) :!)
})
PATTERN

# @!method bad_method_with_active_support?(node)
def_node_matcher :bad_method_with_active_support?, <<~PATTERN
(block
(send _ _)
(args
(arg _)
(arg _))
{
(send
_ {:== :!= :eql? :in? :include? :exclude?} _)
(send
(send
_ {:== :!= :eql? :in? :include? :exclude?} _) :!)
})
PATTERN

def on_send(node)
Expand All @@ -52,7 +76,7 @@ def on_send(node)
return if except_key.nil? || !safe_to_register_offense?(block, except_key)

range = offense_range(node)
preferred_method = "except(#{except_key.source})"
preferred_method = "except(#{except_key_source(except_key)})"

add_offense(range, message: format(MSG, prefer: preferred_method)) do |corrector|
corrector.replace(range, preferred_method)
Expand All @@ -61,28 +85,84 @@ def on_send(node)

private

def bad_method?(block)
if active_support_extensions_enabled?
bad_method_with_active_support?(block)
else
bad_method_with_poro?(block)
end
end

def semantically_except_method?(send, block)
body = block.body

negated = body.method?('!')
body = body.receiver if negated

case send.method_name
when :reject
body.method?('==') || body.method?('eql?')
body.method?('==') || body.method?('eql?') || included?(negated, body)
when :select, :filter
body.method?('!=')
body.method?('!=') || not_included?(negated, body)
else
false
end
end

def included?(negated, body)
body.method?('include?') || body.method?('in?') || (negated && body.method?('exclude?'))
end

def not_included?(negated, body)
body.method?('exclude?') || (negated && (body.method?('include?') || body.method?('in?')))
end

def safe_to_register_offense?(block, except_key)
extracted = extract_body_if_nagated(block.body)
if extracted.method?('in?') || extracted.method?('include?') || \
extracted.method?('exclude?')
return true
end
return true if block.body.method?('eql?')

except_key.sym_type? || except_key.str_type?
end

def extract_body_if_nagated(body)
return body unless body.method?('!')

body.receiver
end

def except_key_source(key)
if key.array_type?
key = if key.percent_literal?
key.each_value.map { |v| decorate_source(v) }
else
key.each_value.map(&:source)
end
return key.join(', ')
end

key.source
end

def decorate_source(value)
return ":\"#{value.source}\"" if value.dsym_type?
return "\"#{value.source}\"" if value.dstr_type?
return ":#{value.source}" if value.sym_type?

"'#{value.source}'"
end

def except_key(node)
key_argument = node.argument_list.first.source
lhs, _method_name, rhs = *node.body
body = extract_body_if_nagated(node.body)
lhs, _method_name, rhs = *body

return lhs if body.method?('include?')
return lhs if body.method?('exclude?')
return rhs if body.method?('in?')
return if [lhs, rhs].map(&:source).none?(key_argument)

[lhs, rhs].find { |operand| operand.source != key_argument }
Expand Down

0 comments on commit 8a5f755

Please sign in to comment.