Skip to content

Commit c592668

Browse files
authored
Merge pull request #512 from Earlopain/start-end-negation
Detect negated conditions with `Performance/DoubleStartEndWith`
2 parents b754de5 + b789959 commit c592668

File tree

3 files changed

+237
-89
lines changed

3 files changed

+237
-89
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#512](https://github.com/rubocop/rubocop-performance/issues/512): Detect negated conditions like `!foo.start_with('bar') && !foo.start_with('baz')` with `Performance/DoubleStartEndWith`. ([@earlopain][])

lib/rubocop/cop/performance/double_start_end_with.rb

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
module RuboCop
44
module Cop
55
module Performance
6-
# Checks for double `#start_with?` or `#end_with?` calls
7-
# separated by `||`. In some cases such calls can be replaced
8-
# with an single `#start_with?`/`#end_with?` call.
6+
# Checks for consecutive `#start_with?` or `#end_with?` calls.
7+
# These methods accept multiple arguments, so in some cases like when
8+
# they are separated by `||`, they can be combined into a single method call.
99
#
1010
# `IncludeActiveSupportAliases` configuration option is used to check for
1111
# `starts_with?` and `ends_with?`. These methods are defined by Active Support.
@@ -14,11 +14,13 @@ module Performance
1414
# # bad
1515
# str.start_with?("a") || str.start_with?(Some::CONST)
1616
# str.start_with?("a", "b") || str.start_with?("c")
17+
# !str.start_with?(foo) && !str.start_with?(bar)
1718
# str.end_with?(var1) || str.end_with?(var2)
1819
#
1920
# # good
2021
# str.start_with?("a", Some::CONST)
2122
# str.start_with?("a", "b", "c")
23+
# !str.start_with?(foo, bar)
2224
# str.end_with?(var1, var2)
2325
#
2426
# @example IncludeActiveSupportAliases: false (default)
@@ -43,20 +45,33 @@ class DoubleStartEndWith < Base
4345

4446
MSG = 'Use `%<replacement>s` instead of `%<original_code>s`.'
4547

48+
METHODS = %i[start_with? end_with?].to_set
49+
METHODS_WITH_ACTIVE_SUPPORT = %i[start_with? starts_with? end_with? ends_with?].to_set
50+
4651
def on_or(node)
47-
receiver, method, first_call_args, second_call_args = process_source(node)
52+
two_start_end_with_calls(node, methods_to_check: methods) do |*matched|
53+
check(node, *matched)
54+
end
55+
end
56+
57+
def on_and(node)
58+
two_start_end_with_calls_negated(node, methods_to_check: methods) do |*matched|
59+
check(node, *matched)
60+
end
61+
end
4862

63+
private
64+
65+
def check(node, receiver, method, first_call_args, second_call_args)
4966
return unless receiver && second_call_args.all?(&:pure?)
5067

5168
combined_args = combine_args(first_call_args, second_call_args)
5269

53-
add_offense(node, message: message(node, receiver, first_call_args, method, combined_args)) do |corrector|
70+
add_offense(node, message: message(node, receiver, method, combined_args)) do |corrector|
5471
autocorrect(corrector, first_call_args, second_call_args, combined_args)
5572
end
5673
end
5774

58-
private
59-
6075
def autocorrect(corrector, first_call_args, second_call_args, combined_args)
6176
first_argument = first_call_args.first.source_range
6277
last_argument = second_call_args.last.source_range
@@ -65,17 +80,20 @@ def autocorrect(corrector, first_call_args, second_call_args, combined_args)
6580
corrector.replace(range, combined_args)
6681
end
6782

68-
def process_source(node)
83+
def methods
6984
if check_for_active_support_aliases?
70-
check_with_active_support_aliases(node)
85+
METHODS_WITH_ACTIVE_SUPPORT
7186
else
72-
two_start_end_with_calls(node)
87+
METHODS
7388
end
7489
end
7590

76-
def message(node, receiver, first_call_args, method, combined_args)
77-
dot = first_call_args.first.parent.send_type? ? '.' : '&.'
78-
replacement = "#{receiver.source}#{dot}#{method}(#{combined_args})"
91+
def message(node, receiver, method, combined_args)
92+
parent = receiver.parent
93+
grandparent = parent.parent
94+
dot = parent.send_type? ? '.' : '&.'
95+
bang = grandparent.send_type? && grandparent.prefix_bang? ? '!' : ''
96+
replacement = "#{bang}#{receiver.source}#{dot}#{method}(#{combined_args})"
7997
format(MSG, replacement: replacement, original_code: node.source)
8098
end
8199

@@ -89,16 +107,14 @@ def check_for_active_support_aliases?
89107

90108
def_node_matcher :two_start_end_with_calls, <<~PATTERN
91109
(or
92-
(call $_recv [{:start_with? :end_with?} $_method] $...)
110+
(call $_recv [%methods_to_check $_method] $...)
93111
(call _recv _method $...))
94112
PATTERN
95113

96-
def_node_matcher :check_with_active_support_aliases, <<~PATTERN
97-
(or
98-
(call $_recv
99-
[{:start_with? :starts_with? :end_with? :ends_with?} $_method]
100-
$...)
101-
(call _recv _method $...))
114+
def_node_matcher :two_start_end_with_calls_negated, <<~PATTERN
115+
(and
116+
(send (call $_recv [%methods_to_check $_method] $...) :!)
117+
(send (call _recv _method $...) :!))
102118
PATTERN
103119
end
104120
end

0 commit comments

Comments
 (0)