Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support safe navigation operator for 15 Performance cops #363

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#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
Loading