From 10949ad75deaca284300f4db0f392d5a6dba77e8 Mon Sep 17 00:00:00 2001 From: Jack Nagel Date: Thu, 27 Jun 2013 01:18:32 -0500 Subject: [PATCH] Fix some #eql? correctness issues The implementation of #eql? and #hash should ensure that if a.eql?(b), then a.hash == b.hash, but #eql? itself should not *depend* on #hash. For example, given class Thingy def eql? instance_of?(other.class) && hash == other.hash end def hash [name, *tags].hash end end if #hash produces a collision for different values of [name, *tags], two Thingy objects will appear to be eql?, even though this is not the case. Instead, #eql? should depend on the equality of name and tags directly. --- Library/Homebrew/dependency.rb | 2 +- Library/Homebrew/options.rb | 2 +- Library/Homebrew/requirement.rb | 2 +- Library/Homebrew/test/test_requirement.rb | 14 ++++++++++++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/dependency.rb b/Library/Homebrew/dependency.rb index 05318fc9ad..407623810e 100644 --- a/Library/Homebrew/dependency.rb +++ b/Library/Homebrew/dependency.rb @@ -21,7 +21,7 @@ class Dependency end def eql?(other) - instance_of?(other.class) && hash == other.hash + instance_of?(other.class) && name == other.name end def hash diff --git a/Library/Homebrew/options.rb b/Library/Homebrew/options.rb index c7d1acca22..b59cd29300 100644 --- a/Library/Homebrew/options.rb +++ b/Library/Homebrew/options.rb @@ -20,7 +20,7 @@ class Option end def eql?(other) - other.is_a?(self.class) && hash == other.hash + instance_of?(other.class) && name == other.name end def hash diff --git a/Library/Homebrew/requirement.rb b/Library/Homebrew/requirement.rb index 317675b739..f710fd0215 100644 --- a/Library/Homebrew/requirement.rb +++ b/Library/Homebrew/requirement.rb @@ -54,7 +54,7 @@ class Requirement end def eql?(other) - instance_of?(other.class) && hash == other.hash + instance_of?(other.class) && name == other.name && tags == other.tags end def hash diff --git a/Library/Homebrew/test/test_requirement.rb b/Library/Homebrew/test/test_requirement.rb index 042aa922df..a2e50126e1 100644 --- a/Library/Homebrew/test/test_requirement.rb +++ b/Library/Homebrew/test/test_requirement.rb @@ -121,4 +121,18 @@ class RequirementTests < Test::Unit::TestCase req.to_dependency.modify_build_environment end end + + def test_eql + a, b = Requirement.new, Requirement.new + assert a.eql?(b) + assert b.eql?(a) + assert_equal a.hash, b.hash + end + + def test_not_eql + a, b = Requirement.new([:optional]), Requirement.new + assert_not_equal a.hash, b.hash + assert !a.eql?(b) + assert !b.eql?(a) + end end