From ce7bbc3685d6eaabb2b10b269d957ac2baaa7eea Mon Sep 17 00:00:00 2001
From: eileencodes <eileencodes@gmail.com>
Date: Tue, 10 Mar 2020 13:20:32 -0400
Subject: [PATCH] Refactor fetch_attribute

Similar to the work done in #38636, instead of using case statements we
can make these classes respond to `fetch_attribute`.

New classes can implement `fetch_attribute` instead of adding to the
case statement, it's more object oriented, and nicer looking.

Co-authored-by: Aaron Patterson <aaron.patterson@gmail.com>
---
 activerecord/lib/arel.rb                   | 12 ++-----
 activerecord/lib/arel/nodes/binary.rb      | 40 +++++++++++++---------
 activerecord/lib/arel/nodes/equality.rb    |  8 +++++
 activerecord/lib/arel/nodes/grouping.rb    |  3 ++
 activerecord/lib/arel/nodes/node.rb        |  3 ++
 activerecord/lib/arel/nodes/sql_literal.rb |  3 ++
 6 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/activerecord/lib/arel.rb b/activerecord/lib/arel.rb
index b7a53fd8ea..2a7b2d75df 100644
--- a/activerecord/lib/arel.rb
+++ b/activerecord/lib/arel.rb
@@ -47,16 +47,8 @@ module Arel
   end
 
   def self.fetch_attribute(value, &block) # :nodoc:
-    case value
-    when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality,
-         Arel::Nodes::NotEqual, Arel::Nodes::LessThan, Arel::Nodes::LessThanOrEqual,
-         Arel::Nodes::GreaterThan, Arel::Nodes::GreaterThanOrEqual
-      attribute_value = value.detect_attribute
-      yield attribute_value if attribute_value
-    when Arel::Nodes::Or
-      fetch_attribute(value.left, &block) && fetch_attribute(value.right, &block)
-    when Arel::Nodes::Grouping
-      fetch_attribute(value.expr, &block)
+    unless String === value
+      value.fetch_attribute(&block)
     end
   end
 
diff --git a/activerecord/lib/arel/nodes/binary.rb b/activerecord/lib/arel/nodes/binary.rb
index 258000025f..cbe56384c2 100644
--- a/activerecord/lib/arel/nodes/binary.rb
+++ b/activerecord/lib/arel/nodes/binary.rb
@@ -11,14 +11,6 @@ module Arel # :nodoc: all
         @right = right
       end
 
-      def detect_attribute
-        if self.left.is_a?(Arel::Attributes::Attribute)
-          self.left
-        elsif self.right.is_a?(Arel::Attributes::Attribute)
-          self.right
-        end
-      end
-
       def initialize_copy(other)
         super
         @left  = @left.clone if @left
@@ -37,18 +29,34 @@ module Arel # :nodoc: all
       alias :== :eql?
     end
 
+    module FetchAttribute
+      def fetch_attribute
+        if left.is_a?(Arel::Attributes::Attribute)
+          yield left
+        elsif right.is_a?(Arel::Attributes::Attribute)
+          yield right
+        end
+      end
+    end
+
+    class Between < Binary; include FetchAttribute; end
+    class NotIn < Binary; include FetchAttribute; end
+    class GreaterThan < Binary; include FetchAttribute; end
+    class GreaterThanOrEqual < Binary; include FetchAttribute; end
+    class NotEqual < Binary; include FetchAttribute; end
+    class LessThan < Binary; include FetchAttribute; end
+    class LessThanOrEqual < Binary; include FetchAttribute; end
+
+    class Or < Binary
+      def fetch_attribute(&block)
+        left.fetch_attribute(&block) && right.fetch_attribute(&block)
+      end
+    end
+
     %w{
       As
       Assignment
-      Between
-      GreaterThan
-      GreaterThanOrEqual
       Join
-      LessThan
-      LessThanOrEqual
-      NotEqual
-      NotIn
-      Or
       Union
       UnionAll
       Intersect
diff --git a/activerecord/lib/arel/nodes/equality.rb b/activerecord/lib/arel/nodes/equality.rb
index 7dd93a46d6..8503096e44 100644
--- a/activerecord/lib/arel/nodes/equality.rb
+++ b/activerecord/lib/arel/nodes/equality.rb
@@ -10,6 +10,14 @@ module Arel # :nodoc: all
       def invert
         Arel::Nodes::NotEqual.new(left, right)
       end
+
+      def fetch_attribute
+        if left.is_a?(Arel::Attributes::Attribute)
+          yield left
+        elsif right.is_a?(Arel::Attributes::Attribute)
+          yield right
+        end
+      end
     end
 
     class IsDistinctFrom < Equality
diff --git a/activerecord/lib/arel/nodes/grouping.rb b/activerecord/lib/arel/nodes/grouping.rb
index 4d0bd69d4d..e01d97ffcb 100644
--- a/activerecord/lib/arel/nodes/grouping.rb
+++ b/activerecord/lib/arel/nodes/grouping.rb
@@ -3,6 +3,9 @@
 module Arel # :nodoc: all
   module Nodes
     class Grouping < Unary
+      def fetch_attribute(&block)
+        expr.fetch_attribute(&block)
+      end
     end
   end
 end
diff --git a/activerecord/lib/arel/nodes/node.rb b/activerecord/lib/arel/nodes/node.rb
index 10f61f1f5d..81ea7e170a 100644
--- a/activerecord/lib/arel/nodes/node.rb
+++ b/activerecord/lib/arel/nodes/node.rb
@@ -41,6 +41,9 @@ module Arel # :nodoc: all
         collector = engine.connection.visitor.accept self, collector
         collector.value
       end
+
+      def fetch_attribute
+      end
     end
   end
 end
diff --git a/activerecord/lib/arel/nodes/sql_literal.rb b/activerecord/lib/arel/nodes/sql_literal.rb
index d25a8521b7..f260c1e27a 100644
--- a/activerecord/lib/arel/nodes/sql_literal.rb
+++ b/activerecord/lib/arel/nodes/sql_literal.rb
@@ -11,6 +11,9 @@ module Arel # :nodoc: all
       def encode_with(coder)
         coder.scalar = self.to_s
       end
+
+      def fetch_attribute
+      end
     end
   end
 end