Add AndNode and OrNode node extensions

This change adds node extensions for `and`- and `or` nodes. It allows
these nodes to describe themselves from any point within the AST.

lib/rubocop.rb

@@ -21,15 +21,19 @@
 require 'rubocop/string_interpreter'
 require 'rubocop/ast/sexp'
 require 'rubocop/ast/node'
+require 'rubocop/ast/node/mixin/binary_operator_node'
 require 'rubocop/ast/node/mixin/conditional_node'
 require 'rubocop/ast/node/mixin/hash_element_node'
 require 'rubocop/ast/node/mixin/modifier_node'
+require 'rubocop/ast/node/mixin/predicate_operator_node'
+require 'rubocop/ast/node/and_node'
 require 'rubocop/ast/node/array_node'
 require 'rubocop/ast/node/case_node'
 require 'rubocop/ast/node/for_node'
 require 'rubocop/ast/node/hash_node'
 require 'rubocop/ast/node/if_node'
 require 'rubocop/ast/node/keyword_splat_node'
+require 'rubocop/ast/node/or_node'
 require 'rubocop/ast/node/pair_node'
 require 'rubocop/ast/node/until_node'
 require 'rubocop/ast/node/when_node'

lib/rubocop/ast/builder.rb

@@ -14,12 +14,14 @@ module AST
     #   root_node = parser.parse(buffer)
     class Builder < Parser::Builders::Default
       NODE_MAP = {
+        AndNode          => [:and],
         ArrayNode        => [:array],
         CaseNode         => [:case],
         ForNode          => [:for],
         HashNode         => [:hash],
         IfNode           => [:if],
         KeywordSplatNode => [:kwsplat],
+        OrNode           => [:or],
         PairNode         => [:pair],
         UntilNode        => [:until, :until_post],
         WhenNode         => [:when],

lib/rubocop/ast/node.rb

@@ -110,7 +110,7 @@ def complete?
       # part of it is changed.
       def updated(type = nil, children = nil, properties = {})
         properties[:location] ||= @location
-        Node.new(type || @type, children || @children, properties)
+        self.class.new(type || @type, children || @children, properties)
       end
 
       # Returns the index of the receiver node in its siblings. (Sibling index

lib/rubocop/ast/node/and_node.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module AST
+    # A node extension for `until` nodes. This will be used in place of a plain
+    # node when the builder constructs the AST, making its methods available
+    # to all `until` nodes within RuboCop.
+    class AndNode < Node
+      include BinaryOperatorNode
+      include PredicateOperatorNode
+
+      # Returns the alternate operator of the `and` as a string.
+      # Returns `and` for `&&` and vice versa.
+      #
+      # @return [String] the alternate of the `and` operator
+      def alternate_operator
+        logical_operator? ? SEMANTIC_AND : LOGICAL_AND
+      end
+
+      # Returns the inverse keyword of the `and` node as a string.
+      # Returns `||` for `&&` and `or` for `and`.
+      #
+      # @return [String] the inverse of the `and` operator
+      def inverse_operator
+        logical_operator? ? LOGICAL_OR : SEMANTIC_OR
+      end
+
+      # Custom destructuring method. This can be used to normalize
+      # destructuring for different variations of the node.
+      #
+      # @return [Array<Node>] the different parts of the `and` predicate
+      def node_parts
+        [*self]
+      end
+    end
+  end
+end

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

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module AST
+    # Common functionality for nodes that are binary operations:
+    # `or`, `and` ...
+    module BinaryOperatorNode
+      # Returns the left hand side node of the binary operation.
+      #
+      # @return [Node] the left hand side of the binary operation
+      def lhs
+        node_parts[0]
+      end
+
+      # Returns the right hand side node of the binary operation.
+      #
+      # @return [Node] the right hand side of the binary operation
+      def rhs
+        node_parts[1]
+      end
+    end
+  end
+end

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

@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module AST
+    # Common functionality for nodes that are predicates:
+    # `or`, `and` ...
+    module PredicateOperatorNode
+      LOGICAL_AND  = '&&'.freeze
+      SEMANTIC_AND = 'and'.freeze
+      LOGICAL_OR   = '||'.freeze
+      SEMANTIC_OR  = 'or'.freeze
+
+      # Returns the operator as a string.
+      #
+      # @return [String] the operator
+      def operator
+        loc.operator.source
+      end
+
+      # Checks whether this is a logical operator.
+      #
+      # @return [Boolean] whether this is a logical operator
+      def logical_operator?
+        operator == LOGICAL_AND || operator == LOGICAL_OR
+      end
+
+      # Checks whether this is a semantic operator.
+      #
+      # @return [Boolean] whether this is a semantic operator
+      def semantic_operator?
+        operator == SEMANTIC_AND || operator == SEMANTIC_OR
+      end
+    end
+  end
+end

lib/rubocop/ast/node/or_node.rb

@@ -0,0 +1,37 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module AST
+    # A node extension for `or` nodes. This will be used in place of a plain
+    # node when the builder constructs the AST, making its methods available
+    # to all `or` nodes within RuboCop.
+    class OrNode < Node
+      include BinaryOperatorNode
+      include PredicateOperatorNode
+
+      # Returns the alternate operator of the `or` as a string.
+      # Returns `or` for `||` and vice versa.
+      #
+      # @return [String] the alternate of the `or` operator
+      def alternate_operator
+        logical_operator? ? SEMANTIC_OR : LOGICAL_OR
+      end
+
+      # Returns the inverse keyword of the `or` node as a string.
+      # Returns `and` for `or` and `&&` for `||`.
+      #
+      # @return [String] the inverse of the `or` operator
+      def inverse_operator
+        logical_operator? ? LOGICAL_AND : SEMANTIC_AND
+      end
+
+      # Custom destructuring method. This can be used to normalize
+      # destructuring for different variations of the node.
+      #
+      # @return [Array<Node>] the different parts of the `or` predicate
+      def node_parts
+        [*self]
+      end
+    end
+  end
+end

lib/rubocop/cop/style/and_or.rb

@@ -42,26 +42,21 @@ def on_until_post(node)
         private
 
         def on_conditionals(node)
-          condition_node, = *node
-
-          condition_node.each_node(*LOGICAL_OPERATOR_NODES) do |logical_node|
+          node.condition.each_node(*LOGICAL_OPERATOR_NODES) do |logical_node|
             process_logical_op(logical_node)
           end
         end
 
         def process_logical_op(node)
-          op = node.loc.operator.source
-          op_type = node.type.to_s
-          return unless op == op_type
+          return if node.logical_operator?
 
-          add_offense(node, :operator, format(MSG, OPS[op], op))
+          add_offense(node, :operator,
+                      format(MSG, node.alternate_operator, node.operator))
         end
 
         def autocorrect(node)
-          expr1, expr2 = *node
-          replacement = (node.and_type? ? '&&' : '||')
           lambda do |corrector|
-            [expr1, expr2].each do |expr|
+            [*node].each do |expr|
               if expr.send_type?
                 correct_send(expr, corrector)
               elsif expr.return_type?
@@ -70,7 +65,8 @@ def autocorrect(node)
                 correct_other(expr, corrector)
               end
             end
-            corrector.replace(node.loc.operator, replacement)
+
+            corrector.replace(node.loc.operator, node.alternate_operator)
           end
         end
 

lib/rubocop/cop/style/space_around_keyword.rb

@@ -203,11 +203,10 @@ def safe_navigation_call?(range, pos)
         def preceded_by_operator?(node, _range)
           # regular dotted method calls bind more tightly than operators
           # so we need to climb up the AST past them
-          while (ancestor = node.parent)
+          node.each_ancestor do |ancestor|
             return true if ancestor.and_type? || ancestor.or_type?
             return false unless ancestor.send_type?
             return true if operator?(ancestor.method_name)
-            node = ancestor
           end
           false
         end

spec/rubocop/ast/and_node_spec.rb

@@ -0,0 +1,151 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RuboCop::AST::AndNode do
+  let(:and_node) { parse_source(source).ast }
+
+  describe '.new' do
+    context 'with a logical and node' do
+      let(:source) do
+        ':foo && :bar'
+      end
+
+      it { expect(and_node).to be_a(described_class) }
+    end
+
+    context 'with a semantic and node' do
+      let(:source) do
+        ':foo and :bar'
+      end
+
+      it { expect(and_node).to be_a(described_class) }
+    end
+  end
+
+  describe '#logical_operator?' do
+    context 'with a logical and node' do
+      let(:source) do
+        ':foo && :bar'
+      end
+
+      it { expect(and_node).to be_logical_operator }
+    end
+
+    context 'with a semantic and node' do
+      let(:source) do
+        ':foo and :bar'
+      end
+
+      it { expect(and_node).not_to be_logical_operator }
+    end
+  end
+
+  describe '#semantic_operator?' do
+    context 'with a logical and node' do
+      let(:source) do
+        ':foo && :bar'
+      end
+
+      it { expect(and_node).not_to be_semantic_operator }
+    end
+
+    context 'with a semantic and node' do
+      let(:source) do
+        ':foo and :bar'
+      end
+
+      it { expect(and_node).to be_semantic_operator }
+    end
+  end
+
+  describe '#operator' do
+    context 'with a logical and node' do
+      let(:source) do
+        ':foo && :bar'
+      end
+
+      it { expect(and_node.operator).to eq('&&') }
+    end
+
+    context 'with a semantic and node' do
+      let(:source) do
+        ':foo and :bar'
+      end
+
+      it { expect(and_node.operator).to eq('and') }
+    end
+  end
+
+  describe '#alternate_operator' do
+    context 'with a logical and node' do
+      let(:source) do
+        ':foo && :bar'
+      end
+
+      it { expect(and_node.alternate_operator).to eq('and') }
+    end
+
+    context 'with a semantic and node' do
+      let(:source) do
+        ':foo and :bar'
+      end
+
+      it { expect(and_node.alternate_operator).to eq('&&') }
+    end
+  end
+
+  describe '#inverse_operator' do
+    context 'with a logical and node' do
+      let(:source) do
+        ':foo && :bar'
+      end
+
+      it { expect(and_node.inverse_operator).to eq('||') }
+    end
+
+    context 'with a semantic and node' do
+      let(:source) do
+        ':foo and :bar'
+      end
+
+      it { expect(and_node.inverse_operator).to eq('or') }
+    end
+  end
+
+  describe '#lhs' do
+    context 'with a logical and node' do
+      let(:source) do
+        ':foo && 42'
+      end
+
+      it { expect(and_node.lhs).to be_sym_type }
+    end
+
+    context 'with a semantic and node' do
+      let(:source) do
+        ':foo and 42'
+      end
+
+      it { expect(and_node.lhs).to be_sym_type }
+    end
+  end
+
+  describe '#rhs' do
+    context 'with a logical and node' do
+      let(:source) do
+        ':foo && 42'
+      end
+
+      it { expect(and_node.rhs).to be_int_type }
+    end
+
+    context 'with a semantic and node' do
+      let(:source) do
+        ':foo and 42'
+      end
+
+      it { expect(and_node.rhs).to be_int_type }
+    end
+  end
+end