diff options
| author | Maxime Lapointe <hunter_spawn@hotmail.com> | 2017-07-25 09:12:11 -0400 |
|---|---|---|
| committer | Maxime Lapointe <hunter_spawn@hotmail.com> | 2017-07-25 09:50:27 -0400 |
| commit | 6d225a9870aea1fa25ab71774206d08d5b216355 (patch) | |
| tree | 40783c0a30112b4c75ae903672e8763c4930fae7 | |
| parent | 0e7ce3f4c7c17e72f905b26aff3893149f524888 (diff) | |
| download | rails-6d225a9870aea1fa25ab71774206d08d5b216355.tar.gz rails-6d225a9870aea1fa25ab71774206d08d5b216355.tar.bz2 rails-6d225a9870aea1fa25ab71774206d08d5b216355.zip | |
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
| -rw-r--r-- | lib/arel/nodes/bind_param.rb | 7 | ||||
| -rw-r--r-- | lib/arel/nodes/false.rb | 1 | ||||
| -rw-r--r-- | lib/arel/nodes/function.rb | 2 | ||||
| -rw-r--r-- | lib/arel/nodes/terminal.rb | 1 | ||||
| -rw-r--r-- | lib/arel/nodes/true.rb | 1 | ||||
| -rw-r--r-- | lib/arel/nodes/values_list.rb | 10 | ||||
| -rw-r--r-- | lib/arel/nodes/window.rb | 1 | ||||
| -rw-r--r-- | test/test_nodes.rb | 34 |
8 files changed, 56 insertions, 1 deletions
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 |