From 6d225a9870aea1fa25ab71774206d08d5b216355 Mon Sep 17 00:00:00 2001
From: Maxime Lapointe <hunter_spawn@hotmail.com>
Date: Tue, 25 Jul 2017 09:12:11 -0400
Subject: Add missing hash, eql?, == to various node classes Some of the nodes
 classes are missing either one or many of the common comparison methods
 #hash, #eql? and #==.

This makes comparision and working with the ast sometimes painful, as equality or operations likes array differences (which uses a hash behind the scene) produces unexpected results.

A test has been added that ensures that every descendants of Node:
* have all 3 methods
* that all 3 methods were defined from the same class
* that the class defining all 3 is also a descendant of Node, to avoid the default ones that rely on identity only
---
 lib/arel/nodes/bind_param.rb  |  7 ++++++-
 lib/arel/nodes/false.rb       |  1 +
 lib/arel/nodes/function.rb    |  2 ++
 lib/arel/nodes/terminal.rb    |  1 +
 lib/arel/nodes/true.rb        |  1 +
 lib/arel/nodes/values_list.rb | 10 ++++++++++
 lib/arel/nodes/window.rb      |  1 +
 test/test_nodes.rb            | 34 ++++++++++++++++++++++++++++++++++
 8 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 test/test_nodes.rb

diff --git a/lib/arel/nodes/bind_param.rb b/lib/arel/nodes/bind_param.rb
index 225fcc4798..efa4f452d4 100644
--- a/lib/arel/nodes/bind_param.rb
+++ b/lib/arel/nodes/bind_param.rb
@@ -9,10 +9,15 @@ module Arel
         super()
       end
 
-      def ==(other)
+      def hash
+        [self.class, self.value].hash
+      end
+
+      def eql?(other)
         other.is_a?(BindParam) &&
           value == other.value
       end
+      alias :== :eql?
 
       def nil?
         value.nil?
diff --git a/lib/arel/nodes/false.rb b/lib/arel/nodes/false.rb
index 26b4e5db97..fb821dd522 100644
--- a/lib/arel/nodes/false.rb
+++ b/lib/arel/nodes/false.rb
@@ -9,6 +9,7 @@ module Arel
       def eql? other
         self.class == other.class
       end
+      alias :== :eql?
     end
   end
 end
diff --git a/lib/arel/nodes/function.rb b/lib/arel/nodes/function.rb
index 28a394e9f3..b2b89ee9ff 100644
--- a/lib/arel/nodes/function.rb
+++ b/lib/arel/nodes/function.rb
@@ -29,6 +29,8 @@ module Arel
           self.alias == other.alias &&
           self.distinct == other.distinct
       end
+      alias :== :eql?
+
     end
 
     %w{
diff --git a/lib/arel/nodes/terminal.rb b/lib/arel/nodes/terminal.rb
index 6f60fe006f..421f039904 100644
--- a/lib/arel/nodes/terminal.rb
+++ b/lib/arel/nodes/terminal.rb
@@ -9,6 +9,7 @@ module Arel
       def eql? other
         self.class == other.class
       end
+      alias :== :eql?
     end
   end
 end
diff --git a/lib/arel/nodes/true.rb b/lib/arel/nodes/true.rb
index 796b5b9348..bb9d8c1414 100644
--- a/lib/arel/nodes/true.rb
+++ b/lib/arel/nodes/true.rb
@@ -9,6 +9,7 @@ module Arel
       def eql? other
         self.class == other.class
       end
+      alias :== :eql?
     end
   end
 end
diff --git a/lib/arel/nodes/values_list.rb b/lib/arel/nodes/values_list.rb
index b39aaa1465..89cea1790d 100644
--- a/lib/arel/nodes/values_list.rb
+++ b/lib/arel/nodes/values_list.rb
@@ -8,6 +8,16 @@ module Arel
         @rows = rows
         super()
       end
+
+      def hash
+        @rows.hash
+      end
+
+      def eql? other
+        self.class == other.class &&
+          self.rows == other.rows
+      end
+      alias :== :eql?
     end
   end
 end
diff --git a/lib/arel/nodes/window.rb b/lib/arel/nodes/window.rb
index 535c0c6238..23a005daba 100644
--- a/lib/arel/nodes/window.rb
+++ b/lib/arel/nodes/window.rb
@@ -107,6 +107,7 @@ module Arel
       def eql? other
         self.class == other.class
       end
+      alias :== :eql?
     end
 
     class Preceding < Unary
diff --git a/test/test_nodes.rb b/test/test_nodes.rb
new file mode 100644
index 0000000000..bf0082396d
--- /dev/null
+++ b/test/test_nodes.rb
@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+require 'helper'
+
+module Arel
+  module Nodes
+    class TestNodes < Minitest::Test
+      def test_every_arel_nodes_have_hash_eql_eqeq_from_same_class
+        # #descendants code from activesupport
+        node_descendants = []
+        ObjectSpace.each_object(Arel::Nodes::Node.singleton_class) do |k|
+          next if k.respond_to?(:singleton_class?) && k.singleton_class?
+          node_descendants.unshift k unless k == self
+        end
+        node_descendants.delete(Arel::Nodes::Node)
+
+        bad_node_descendants = node_descendants.reject do |subnode|
+          eqeq_owner = subnode.instance_method(:==).owner
+          eql_owner = subnode.instance_method(:eql?).owner
+          hash_owner = subnode.instance_method(:hash).owner
+
+          eqeq_owner < Arel::Nodes::Node &&
+              eqeq_owner == eql_owner &&
+              eqeq_owner == hash_owner
+        end
+
+        problem_msg = 'Some subclasses of Arel::Nodes::Node do not have a' \
+            ' #== or #eql? or #hash defined from the same class as the others'
+        assert_empty bad_node_descendants, problem_msg
+      end
+
+
+    end
+  end
+end
-- 
cgit v1.2.3