Skip to content

Commit

Permalink
Support safe navigationo operator for 15 Performance cops
Browse files Browse the repository at this point in the history
This PR supports safe navigation operator for `Performance/ArraySemiInfiniteRangeSlice`,
`Performance/DeletePrefix`, `Performance/DeleteSuffix`, `Performance/Detect`, `Performance/EndWith`,
`Performance/InefficientHashSearch`, `Performance/MapCompact`, `Performance/RedundantSplitRegexpArgument`,
`Performance/ReverseEach`, `Performance/ReverseFirst`, `Performance/SelectMap`, `Performance/Squeeze`,
`Performance/StartWith`, `Performance/StringInclude`, and `Performance/StringReplacement` cops.
  • Loading branch information
koic committed Jul 7, 2023
1 parent 563eb44 commit c65d8dd
Show file tree
Hide file tree
Showing 31 changed files with 291 additions and 30 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
* [#363](https://github.com/rubocop/rubocop-performance/pull/363):
Support safe navigation operator for `Performance/ArraySemiInfiniteRangeSlice`, `Performance/DeletePrefix`, `Performance/DeleteSuffix`, `Performance/Detect`, `Performance/EndWith`, `Performance/InefficientHashSearch`, `Performance/MapCompact`, `Performance/RedundantSplitRegexpArgument`, `Performance/ReverseEach`, `Performance/ReverseFirst`, `Performance/SelectMap`, `Performance/Squeeze`, `Performance/StartWith`, `Performance/StringInclude`, and `Performance/StringReplacement` cops. ([@koic][])
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class ArraySemiInfiniteRangeSlice < Base
RESTRICT_ON_SEND = SLICE_METHODS

def_node_matcher :endless_range_slice?, <<~PATTERN
(send $_ $%SLICE_METHODS $#endless_range?)
(call $_ $%SLICE_METHODS $#endless_range?)
PATTERN

def_node_matcher :endless_range?, <<~PATTERN
Expand All @@ -59,6 +59,7 @@ def on_send(node)
end
end
end
alias on_csend on_send

private

Expand Down
7 changes: 5 additions & 2 deletions lib/rubocop/cop/performance/delete_prefix.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ class DeletePrefix < Base
}.freeze

def_node_matcher :delete_prefix_candidate?, <<~PATTERN
(send $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_start?) (regopt)) (str $_))
(call $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_start?) (regopt)) (str $_))
PATTERN

# rubocop:disable Metrics/AbcSize
def on_send(node)
return unless (receiver, bad_method, regexp_str, replace_string = delete_prefix_candidate?(node))
return unless replace_string.empty?
Expand All @@ -80,11 +81,13 @@ def on_send(node)
regexp_str = interpret_string_escapes(regexp_str)
string_literal = to_string_literal(regexp_str)

new_code = "#{receiver.source}.#{good_method}(#{string_literal})"
new_code = "#{receiver.source}#{node.loc.dot.source}#{good_method}(#{string_literal})"

corrector.replace(node, new_code)
end
end
# rubocop:enable Metrics/AbcSize
alias on_csend on_send
end
end
end
Expand Down
7 changes: 5 additions & 2 deletions lib/rubocop/cop/performance/delete_suffix.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@ class DeleteSuffix < Base
}.freeze

def_node_matcher :delete_suffix_candidate?, <<~PATTERN
(send $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_end?) (regopt)) (str $_))
(call $!nil? ${:gsub :gsub! :sub :sub!} (regexp (str $#literal_at_end?) (regopt)) (str $_))
PATTERN

# rubocop:disable Metrics/AbcSize
def on_send(node)
return unless (receiver, bad_method, regexp_str, replace_string = delete_suffix_candidate?(node))
return unless replace_string.empty?
Expand All @@ -80,11 +81,13 @@ def on_send(node)
regexp_str = interpret_string_escapes(regexp_str)
string_literal = to_string_literal(regexp_str)

new_code = "#{receiver.source}.#{good_method}(#{string_literal})"
new_code = "#{receiver.source}#{node.loc.dot.source}#{good_method}(#{string_literal})"

corrector.replace(node, new_code)
end
end
# rubocop:enable Metrics/AbcSize
alias on_csend on_send
end
end
end
Expand Down
5 changes: 3 additions & 2 deletions lib/rubocop/cop/performance/detect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ class Detect < Base

def_node_matcher :detect_candidate?, <<~PATTERN
{
(send $(block (send _ %CANDIDATE_METHODS) ...) ${:first :last} $...)
(send $(block (call _ %CANDIDATE_METHODS) ...) ${:first :last} $...)
(send $(block (send _ %CANDIDATE_METHODS) ...) $:[] (int ${0 -1}))
(send $(send _ %CANDIDATE_METHODS ...) ${:first :last} $...)
(send $(call _ %CANDIDATE_METHODS ...) ${:first :last} $...)
(send $(send _ %CANDIDATE_METHODS ...) $:[] (int ${0 -1}))
}
PATTERN
Expand All @@ -63,6 +63,7 @@ def on_send(node)
register_offense(node, receiver, second_method, index)
end
end
alias on_csend on_send

private

Expand Down
6 changes: 4 additions & 2 deletions lib/rubocop/cop/performance/end_with.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class EndWith < Base
RESTRICT_ON_SEND = %i[match =~ match?].freeze

def_node_matcher :redundant_regex?, <<~PATTERN
{(send $!nil? {:match :=~ :match?} (regexp (str $#literal_at_end?) (regopt)))
{(call $!nil? {:match :=~ :match?} (regexp (str $#literal_at_end?) (regopt)))
(send (regexp (str $#literal_at_end?) (regopt)) {:match :match?} $_)
(match-with-lvasgn (regexp (str $#literal_at_end?) (regopt)) $_)}
PATTERN
Expand All @@ -66,12 +66,14 @@ def on_send(node)
receiver, regex_str = regex_str, receiver if receiver.is_a?(String)
regex_str = drop_end_metacharacter(regex_str)
regex_str = interpret_string_escapes(regex_str)
dot = node.loc.dot ? node.loc.dot.source : '.'

new_source = "#{receiver.source}.end_with?(#{to_string_literal(regex_str)})"
new_source = "#{receiver.source}#{dot}end_with?(#{to_string_literal(regex_str)})"

corrector.replace(node, new_source)
end
end
alias on_csend on_send
alias on_match_with_lvasgn on_send
end
end
Expand Down
24 changes: 15 additions & 9 deletions lib/rubocop/cop/performance/inefficient_hash_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class InefficientHashSearch < Base
RESTRICT_ON_SEND = %i[include?].freeze

def_node_matcher :inefficient_include?, <<~PATTERN
(send (send $_ {:keys :values}) :include? _)
(send (call $_ {:keys :values}) :include? _)
PATTERN

def on_send(node)
Expand All @@ -56,21 +56,23 @@ def on_send(node)
add_offense(node, message: message) do |corrector|
# Replace `keys.include?` or `values.include?` with the appropriate
# `key?`/`value?` method.
corrector.replace(
node,
"#{autocorrect_hash_expression(node)}.#{autocorrect_method(node)}(#{autocorrect_argument(node)})"
)
corrector.replace(node, replacement(node))
end
end
end
alias on_csend on_send

private

def message(node)
"Use `##{autocorrect_method(node)}` instead of `##{current_method(node)}.include?`."
"Use `##{correct_method(node)}` instead of `##{current_method(node)}.include?`."
end

def autocorrect_method(node)
def replacement(node)
"#{correct_hash_expression(node)}#{correct_dot(node)}#{correct_method(node)}(#{correct_argument(node)})"
end

def correct_method(node)
case current_method(node)
when :keys then use_long_method ? 'has_key?' : 'key?'
when :values then use_long_method ? 'has_value?' : 'value?'
Expand All @@ -86,13 +88,17 @@ def use_long_method
preferred_config && preferred_config['EnforcedStyle'] == 'long' && preferred_config['Enabled']
end

def autocorrect_argument(node)
def correct_argument(node)
node.arguments.first.source
end

def autocorrect_hash_expression(node)
def correct_hash_expression(node)
node.receiver.receiver.source
end

def correct_dot(node)
node.receiver.loc.dot.source
end
end
end
end
Expand Down
5 changes: 3 additions & 2 deletions lib/rubocop/cop/performance/map_compact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ class MapCompact < Base
def_node_matcher :map_compact, <<~PATTERN
{
(send
$(send _ {:map :collect}
$(call _ {:map :collect}
(block_pass
(sym _))) _)
(send
(block
$(send _ {:map :collect})
$(call _ {:map :collect})
(args ...) _) _)
}
PATTERN
Expand All @@ -61,6 +61,7 @@ def on_send(node)
remove_compact_method(corrector, map_node, node, node.parent)
end
end
alias on_csend on_send

private

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class RedundantSplitRegexpArgument < Base
STR_SPECIAL_CHARS = %w[\n \" \' \\\\ \t \b \f \r].freeze

def_node_matcher :split_call_with_regexp?, <<~PATTERN
{(send !nil? :split $regexp)}
{(call !nil? :split $regexp)}
PATTERN

def on_send(node)
Expand All @@ -35,6 +35,7 @@ def on_send(node)
corrector.replace(regexp_node, "\"#{new_argument}\"")
end
end
alias on_csend on_send

private

Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/performance/reverse_each.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ReverseEach < Base
RESTRICT_ON_SEND = %i[each].freeze

def_node_matcher :reverse_each?, <<~MATCHER
(send (send _ :reverse) :each)
(send (call _ :reverse) :each)
MATCHER

def on_send(node)
Expand All @@ -41,6 +41,7 @@ def on_send(node)
end
end
end
alias on_csend on_send

private

Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/performance/reverse_first.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class ReverseFirst < Base
RESTRICT_ON_SEND = %i[first].freeze

def_node_matcher :reverse_first_candidate?, <<~PATTERN
(send $(send _ :reverse) :first (int _)?)
(send $(call _ :reverse) :first (int _)?)
PATTERN

def on_send(node)
Expand All @@ -39,6 +39,7 @@ def on_send(node)
end
end
end
alias on_csend on_send

private

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/performance/select_map.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def on_send(node)
range = offense_range(node, map_method)
add_offense(range, message: format(MSG, method_name: node.method_name))
end
alias on_csend on_send

private

Expand Down
7 changes: 5 additions & 2 deletions lib/rubocop/cop/performance/squeeze.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ class Squeeze < Base
PREFERRED_METHODS = { gsub: :squeeze, gsub!: :squeeze! }.freeze

def_node_matcher :squeeze_candidate?, <<~PATTERN
(send
(call
$!nil? ${:gsub :gsub!}
(regexp
(str $#repeating_literal?)
(regopt))
(str $_))
PATTERN

# rubocop:disable Metrics/AbcSize
def on_send(node)
squeeze_candidate?(node) do |receiver, bad_method, regexp_str, replace_str|
regexp_str = regexp_str[0..-2] # delete '+' from the end
Expand All @@ -46,12 +47,14 @@ def on_send(node)

add_offense(node.loc.selector, message: message) do |corrector|
string_literal = to_string_literal(replace_str)
new_code = "#{receiver.source}.#{good_method}(#{string_literal})"
new_code = "#{receiver.source}#{node.loc.dot.source}#{good_method}(#{string_literal})"

corrector.replace(node, new_code)
end
end
end
# rubocop:enable Metrics/AbcSize
alias on_csend on_send

private

Expand Down
6 changes: 4 additions & 2 deletions lib/rubocop/cop/performance/start_with.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class StartWith < Base
RESTRICT_ON_SEND = %i[match =~ match?].freeze

def_node_matcher :redundant_regex?, <<~PATTERN
{(send $!nil? {:match :=~ :match?} (regexp (str $#literal_at_start?) (regopt)))
{(call $!nil? {:match :=~ :match?} (regexp (str $#literal_at_start?) (regopt)))
(send (regexp (str $#literal_at_start?) (regopt)) {:match :match?} $_)
(match-with-lvasgn (regexp (str $#literal_at_start?) (regopt)) $_)}
PATTERN
Expand All @@ -66,12 +66,14 @@ def on_send(node)
receiver, regex_str = regex_str, receiver if receiver.is_a?(String)
regex_str = drop_start_metacharacter(regex_str)
regex_str = interpret_string_escapes(regex_str)
dot = node.loc.dot ? node.loc.dot.source : '.'

new_source = "#{receiver.source}.start_with?(#{to_string_literal(regex_str)})"
new_source = "#{receiver.source}#{dot}start_with?(#{to_string_literal(regex_str)})"

corrector.replace(node, new_source)
end
end
alias on_csend on_send
alias on_match_with_lvasgn on_send
end
end
Expand Down
8 changes: 6 additions & 2 deletions lib/rubocop/cop/performance/string_include.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,12 @@ class StringInclude < Base
RESTRICT_ON_SEND = %i[match =~ !~ match?].freeze

def_node_matcher :redundant_regex?, <<~PATTERN
{(send $!nil? {:match :=~ :!~ :match?} (regexp (str $#literal?) (regopt)))
{(call $!nil? {:match :=~ :!~ :match?} (regexp (str $#literal?) (regopt)))
(send (regexp (str $#literal?) (regopt)) {:match :match?} $_)
(match-with-lvasgn (regexp (str $#literal?) (regopt)) $_)}
PATTERN

# rubocop:disable Metrics/AbcSize
def on_send(node)
return unless (receiver, regex_str = redundant_regex?(node))

Expand All @@ -40,12 +41,15 @@ def on_send(node)
add_offense(node, message: message) do |corrector|
receiver, regex_str = regex_str, receiver if receiver.is_a?(String)
regex_str = interpret_string_escapes(regex_str)
dot = node.loc.dot ? node.loc.dot.source : '.'

new_source = "#{'!' if negation}#{receiver.source}.include?(#{to_string_literal(regex_str)})"
new_source = "#{'!' if negation}#{receiver.source}#{dot}include?(#{to_string_literal(regex_str)})"

corrector.replace(node, new_source)
end
end
# rubocop:enable Metrics/AbcSize
alias on_csend on_send
alias on_match_with_lvasgn on_send

private
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/performance/string_replacement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class StringReplacement < Base
BANG = '!'

def_node_matcher :string_replacement?, <<~PATTERN
(send _ {:gsub :gsub!}
(call _ {:gsub :gsub!}
${regexp str (send (const nil? :Regexp) {:new :compile} _)}
$str)
PATTERN
Expand All @@ -42,6 +42,7 @@ def on_send(node)
offense(node, first_param, second_param)
end
end
alias on_csend on_send

private

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@
RUBY
end

it 'registers an offense and corrects when using `slice` with semi-infinite ranges and safe navigation operator' do
expect_offense(<<~RUBY)
array&.slice(2..)
^^^^^^^^^^^^^^^^^ Use `drop` instead of `slice` with semi-infinite range.
array&.slice(..2)
^^^^^^^^^^^^^^^^^ Use `take` instead of `slice` with semi-infinite range.
RUBY

expect_correction(<<~RUBY)
array.drop(2)
array.take(3)
RUBY
end

it 'does not register an offense when using `[]` with full range' do
expect_no_offenses(<<~RUBY)
array[0..2]
Expand Down

0 comments on commit c65d8dd

Please sign in to comment.