From 767f2c9b67d28b2155adf1da783293066683f2a6 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Sat, 14 Sep 2024 11:30:51 +0200 Subject: [PATCH] [Fix #407] Make `Performance/DoubleStartEndWith` aware of safe navigation In case of inconsistent safe navigation, it keeps the dot from the first receiver. This shouldn't change anything: Depending on the order, it was either not needed or it will continue raising. `Lint/SafeNavigationConsistency` takes care of this anyways --- ..._double_start_end_aware_safe_navigation.md | 1 + .../cop/performance/double_start_end_with.rb | 20 +++++------ .../performance/double_start_end_with_spec.rb | 35 +++++++++++++++++++ 3 files changed, 46 insertions(+), 10 deletions(-) create mode 100644 changelog/change_make_double_start_end_aware_safe_navigation.md diff --git a/changelog/change_make_double_start_end_aware_safe_navigation.md b/changelog/change_make_double_start_end_aware_safe_navigation.md new file mode 100644 index 0000000000..55945f4011 --- /dev/null +++ b/changelog/change_make_double_start_end_aware_safe_navigation.md @@ -0,0 +1 @@ +* [#407](https://github.com/rubocop/rubocop-performance/issues/407): Make `Performance/DoubleStartEndWith` aware of safe navigation. ([@earlopain][]) diff --git a/lib/rubocop/cop/performance/double_start_end_with.rb b/lib/rubocop/cop/performance/double_start_end_with.rb index 8ec3e10cc6..43bfa192d7 100644 --- a/lib/rubocop/cop/performance/double_start_end_with.rb +++ b/lib/rubocop/cop/performance/double_start_end_with.rb @@ -41,7 +41,7 @@ module Performance class DoubleStartEndWith < Base extend AutoCorrector - MSG = 'Use `%s.%s(%s)` instead of `%s`.' + MSG = 'Use `%s` instead of `%s`.' def on_or(node) receiver, method, first_call_args, second_call_args = process_source(node) @@ -50,7 +50,7 @@ def on_or(node) combined_args = combine_args(first_call_args, second_call_args) - add_offense(node, message: message(node, receiver, method, combined_args)) do |corrector| + add_offense(node, message: message(node, receiver, first_call_args, method, combined_args)) do |corrector| autocorrect(corrector, first_call_args, second_call_args, combined_args) end end @@ -73,10 +73,10 @@ def process_source(node) end end - def message(node, receiver, method, combined_args) - format( - MSG, receiver: receiver.source, method: method, combined_args: combined_args, original_code: node.source - ) + def message(node, receiver, first_call_args, method, combined_args) + dot = first_call_args.first.parent.send_type? ? '.' : '&.' + replacement = "#{receiver.source}#{dot}#{method}(#{combined_args})" + format(MSG, replacement: replacement, original_code: node.source) end def combine_args(first_call_args, second_call_args) @@ -89,16 +89,16 @@ def check_for_active_support_aliases? def_node_matcher :two_start_end_with_calls, <<~PATTERN (or - (send $_recv [{:start_with? :end_with?} $_method] $...) - (send _recv _method $...)) + (call $_recv [{:start_with? :end_with?} $_method] $...) + (call _recv _method $...)) PATTERN def_node_matcher :check_with_active_support_aliases, <<~PATTERN (or - (send $_recv + (call $_recv [{:start_with? :starts_with? :end_with? :ends_with?} $_method] $...) - (send _recv _method $...)) + (call _recv _method $...)) PATTERN end end diff --git a/spec/rubocop/cop/performance/double_start_end_with_spec.rb b/spec/rubocop/cop/performance/double_start_end_with_spec.rb index e2f0d9592f..e68502e3ec 100644 --- a/spec/rubocop/cop/performance/double_start_end_with_spec.rb +++ b/spec/rubocop/cop/performance/double_start_end_with_spec.rb @@ -26,6 +26,41 @@ expect_no_offenses('x.start_with?(a, "b") || x.start_with?(C, d)') end end + + context 'with safe navigation' do + it 'registers an offense' do + expect_offense(<<~RUBY) + x&.start_with?(a, b) || x&.start_with?("c", D) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `x&.start_with?(a, b, "c", D)` instead of `x&.start_with?(a, b) || x&.start_with?("c", D)`. + RUBY + + expect_correction(<<~RUBY) + x&.start_with?(a, b, "c", D) + RUBY + end + + it 'registers an offense when the first start_with uses no safe navigation' do + expect_offense(<<~RUBY) + x.start_with?(a, b) || x&.start_with?("c", D) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `x.start_with?(a, b, "c", D)` instead of `x.start_with?(a, b) || x&.start_with?("c", D)`. + RUBY + + expect_correction(<<~RUBY) + x.start_with?(a, b, "c", D) + RUBY + end + + it 'registers an offense when the second start_with uses no safe navigation' do + expect_offense(<<~RUBY) + x&.start_with?(a, b) || x.start_with?("c", D) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `x&.start_with?(a, b, "c", D)` instead of `x&.start_with?(a, b) || x.start_with?("c", D)`. + RUBY + + expect_correction(<<~RUBY) + x&.start_with?(a, b, "c", D) + RUBY + end + end end context 'with different receivers' do