Skip to content

Commit

Permalink
Handle ternaries in Style/SafeNavigation
Browse files Browse the repository at this point in the history
  • Loading branch information
fatkodima authored and bbatsov committed Dec 5, 2022
1 parent 7a49421 commit fe73dd9
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 9 deletions.
1 change: 1 addition & 0 deletions changelog/change_safe_navigation_to_handle_ternaries.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11116](https://github.com/rubocop/rubocop/issues/11116): Handle ternaries in `Style/SafeNavigation`. ([@fatkodima][])
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/redundant_sort.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def arg_node(node)
end

def arg_value(node)
arg_node(node).nil? ? nil : arg_node(node).node_parts.first
arg_node(node)&.node_parts&.first
end

# This gets the start of the accessor whether it has a dot
Expand Down
41 changes: 35 additions & 6 deletions lib/rubocop/cop/style/safe_navigation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ module Style
# foo && foo.bar { |e| e.something }
# foo && foo.bar(param) { |e| e.something }
#
# foo ? foo.bar : nil
# foo.nil? ? nil : foo.bar
# !foo.nil? ? foo.bar : nil
# !foo ? nil : foo.bar
#
# # good
# foo&.bar
# foo&.bar&.baz
Expand Down Expand Up @@ -105,6 +110,17 @@ class SafeNavigation < Base
}
PATTERN

# @!method ternary_safe_navigation_candidate(node)
def_node_matcher :ternary_safe_navigation_candidate, <<~PATTERN
{
(if (send $_ {:nil? :!}) nil $_)
(if (send (send $_ :nil?) :!) $_ nil)
(if $_ $_ nil)
}
PATTERN

# @!method not_nil_check?(node)
def_node_matcher :not_nil_check?, '(send (send $_ :nil?) :!)'

Expand All @@ -118,9 +134,11 @@ def on_and(node)
check_node(node)
end

private

def check_node(node)
checked_variable, receiver, method_chain, method = extract_parts(node)
return unless receiver == checked_variable
return if receiver != checked_variable || receiver.nil?
return if use_var_only_in_unless_modifier?(node, checked_variable)
return if chain_length(method_chain, method) > max_chain_length
return if unsafe_method_used?(method_chain, method)
Expand All @@ -133,10 +151,8 @@ def use_var_only_in_unless_modifier?(node, variable)
node.if_type? && node.unless? && !method_called?(variable)
end

private

def autocorrect(corrector, node)
body = node.node_parts[1]
body = extract_body(node)
method_call = method_call(node)

corrector.remove(begin_range(node, body))
Expand All @@ -147,6 +163,14 @@ def autocorrect(corrector, node)
add_safe_nav_to_all_methods_in_chain(corrector, method_call, body)
end

def extract_body(node)
if node.if_type? && node.ternary?
node.branches.find { |branch| !branch.nil_type? }
else
node.node_parts[1]
end
end

def handle_comments(corrector, node, method_call)
comments = comments(node)
return if comments.empty?
Expand Down Expand Up @@ -174,7 +198,7 @@ def relevant_comment_ranges(node)
end

def allowed_if_condition?(node)
node.else? || node.elsif? || node.ternary?
node.else? || node.elsif?
end

def method_call(node)
Expand All @@ -192,7 +216,12 @@ def extract_parts(node)
end

def extract_parts_from_if(node)
variable, receiver = modifier_if_safe_navigation_candidate(node)
variable, receiver =
if node.ternary?
ternary_safe_navigation_candidate(node)
else
modifier_if_safe_navigation_candidate(node)
end

checked_variable, matching_receiver, method = extract_common_parts(receiver, variable)

Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/variable_force/assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def operator_assignment_node
end

def multiple_assignment_node
grandparent_node = node.parent ? node.parent.parent : nil
grandparent_node = node.parent&.parent
return nil unless grandparent_node
return nil unless grandparent_node.type == MULTIPLE_ASSIGNMENT_TYPE
return nil unless node.parent.type == MULTIPLE_LEFT_HAND_SIDE_TYPE
Expand Down
28 changes: 27 additions & 1 deletion spec/rubocop/cop/style/safe_navigation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -743,11 +743,37 @@ def foobar
end

context 'ternary expression' do
it 'allows ternary expression' do
it 'allows non-convertible ternary expression' do
expect_no_offenses(<<~RUBY)
!#{variable}.nil? ? #{variable}.bar : something
RUBY
end

it 'registers an offense for convertible ternary expressions' do
expect_offense(<<~RUBY, variable: variable)
%{variable} ? %{variable}.bar : nil
^{variable}^^^^{variable}^^^^^^^^^^ Use safe navigation (`&.`) instead [...]
%{variable}.nil? ? nil : %{variable}.bar
^{variable}^^^^^^^^^^^^^^^{variable}^^^^ Use safe navigation (`&.`) instead [...]
!%{variable}.nil? ? %{variable}.bar : nil
^^{variable}^^^^^^^^^^{variable}^^^^^^^^^ Use safe navigation (`&.`) instead [...]
!%{variable} ? nil : %{variable}.bar
^^{variable}^^^^^^^^^^{variable}^^^^ Use safe navigation (`&.`) instead [...]
RUBY

expect_correction(<<~RUBY)
#{variable}&.bar
#{variable}&.bar
#{variable}&.bar
#{variable}&.bar
RUBY
end
end
end

Expand Down

0 comments on commit fe73dd9

Please sign in to comment.