New cop Lint/SafeNavigationConsistency

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### New features
+
+* Add new `Lint/SafeNavigationConsistency` cop. ([@rrosenblum][])
+
 ### Bug fixes
 
 * [#5759](https://github.com/bbatsov/rubocop/pull/5759): Fix `Performance/RegexpMatch` cop not correcting negated match operator. ([@bdewater][])

config/enabled.yml

@@ -654,6 +654,13 @@ Lint/SafeNavigationChain:
   Description: 'Do not chain ordinary method call after safe navigation operator.'
   Enabled: true
 
+Lint/SafeNavigationConsistency:
+  Description: >-
+                 Check to make sure that if safe navigation is used for a method
+                 call in an `&&` or `||` condition that safe navigation is used
+                 for all method calls on that same object.
+  Enabled: true
+
 Lint/ScriptPermission:
   Description: 'Grant script file execute permission.'
   Enabled: true

lib/rubocop.rb

@@ -293,6 +293,7 @@
 require_relative 'rubocop/cop/lint/rescue_exception'
 require_relative 'rubocop/cop/lint/rescue_type'
 require_relative 'rubocop/cop/lint/return_in_void_context'
+require_relative 'rubocop/cop/lint/safe_navigation_consistency'
 require_relative 'rubocop/cop/lint/safe_navigation_chain'
 require_relative 'rubocop/cop/lint/script_permission'
 require_relative 'rubocop/cop/lint/shadowed_argument'

lib/rubocop/ast/node/mixin/binary_operator_node.rb

@@ -18,6 +18,26 @@ def lhs
       def rhs
         node_parts[1]
       end
+
+      # Returns all of the conditions, including nested conditions,
+      # of the binary operation.
+      #
+      # @return [Array<Node>] the left and right hand side of the binary
+      # operation and the let and right hand side of any nested binary
+      # operators
+      def conditions
+        lhs, rhs = *self
+        lhs = lhs.children.first if lhs.begin_type?
+        rhs = rhs.children.first if rhs.begin_type?
+
+        [lhs, rhs].each_with_object([]) do |side, collection|
+          if AST::Node::OPERATOR_KEYWORDS.include?(side.type)
+            collection.concat(side.conditions)
+          else
+            collection << side
+          end
+        end
+      end
     end
   end
 end

lib/rubocop/cop/lint/safe_navigation_consistency.rb

@@ -0,0 +1,80 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Lint
+      # This cop check to make sure that if safe navigation is used for a method
+      # call in an `&&` or `||` condition that safe navigation is used for all
+      # method calls on that same object.
+      #
+      # @example
+      #   # bad
+      #   foo&.bar && foo.baz
+      #
+      #   # bad
+      #   foo.bar || foo&.baz
+      #
+      #   # bad
+      #   foo&.bar && (foobar.baz || foo.baz)
+      #
+      #   # good
+      #   foo.bar && foo.baz
+      #
+      #   # good
+      #   foo&.bar || foo&.baz
+      #
+      #   # good
+      #   foo&.bar && (foobar.baz || foo&.baz)
+      #
+      class SafeNavigationConsistency < Cop
+        MSG = 'Ensure that safe navigation is used consistently ' \
+          'inside of `&&` and `||`.'.freeze
+
+        def on_csend(node)
+          return unless node.parent &&
+                        AST::Node::OPERATOR_KEYWORDS.include?(node.parent.type)
+          check(node)
+        end
+
+        def check(node)
+          ancestor = top_conditional_ancestor(node)
+          conditions = ancestor.conditions
+          safe_nav_receiver = node.receiver
+
+          method_calls = conditions.select(&:send_type?)
+          unsafe_method_calls = method_calls.select do |method_call|
+            safe_nav_receiver == method_call.receiver
+          end
+
+          unsafe_method_calls.each do |unsafe_method_call|
+            location =
+              node.loc.expression.join(unsafe_method_call.loc.expression)
+            add_offense(unsafe_method_call,
+                        location: location)
+          end
+        end
+
+        def autocorrect(node)
+          return unless node.dot?
+
+          lambda do |corrector|
+            corrector.insert_before(node.loc.dot, '&')
+          end
+        end
+
+        private
+
+        def top_conditional_ancestor(node)
+          parent = node.parent
+          unless parent &&
+                 (AST::Node::OPERATOR_KEYWORDS.include?(parent.type) ||
+                  (parent.begin_type? &&
+                   AST::Node::OPERATOR_KEYWORDS.include?(parent.parent.type)))
+            return node
+          end
+          top_conditional_ancestor(parent)
+        end
+      end
+    end
+  end
+end

manual/cops.md

@@ -235,6 +235,7 @@ In the following section you find all available cops:
 * [Lint/RescueType](cops_lint.md#lintrescuetype)
 * [Lint/ReturnInVoidContext](cops_lint.md#lintreturninvoidcontext)
 * [Lint/SafeNavigationChain](cops_lint.md#lintsafenavigationchain)
+* [Lint/SafeNavigationConsistency](cops_lint.md#lintsafenavigationconsistency)
 * [Lint/ScriptPermission](cops_lint.md#lintscriptpermission)
 * [Lint/ShadowedArgument](cops_lint.md#lintshadowedargument)
 * [Lint/ShadowedException](cops_lint.md#lintshadowedexception)

manual/cops_lint.md

@@ -1623,6 +1623,38 @@ Name | Default value | Configurable values
 --- | --- | ---
 Whitelist | `present?`, `blank?`, `presence`, `try` | Array
 
+## Lint/SafeNavigationConsistency
+
+Enabled by default | Supports autocorrection
+--- | ---
+Enabled | Yes
+
+This cop check to make sure that if safe navigation is used for a method
+call in an `&&` or `||` condition that safe navigation is used for all
+method calls on that same object.
+
+### Examples
+
+```ruby
+# bad
+foo&.bar && foo.baz
+
+# bad
+foo.bar || foo&.baz
+
+# bad
+foo&.bar && (foobar.baz || foo.baz)
+
+# good
+foo.bar && foo.baz
+
+# good
+foo&.bar || foo&.baz
+
+# good
+foo&.bar && (foobar.baz || foo&.baz)
+```
+
 ## Lint/ScriptPermission
 
 Enabled by default | Supports autocorrection