Replace ComparableSet with a Requirements collection

9 files changed, 58 insertions, 72 deletions

diff --git a/Library/Homebrew/dependencies.rb b/Library/Homebrew/dependencies.rb
index 0e0637c81..96a4106b5 100644
--- a/Library/Homebrew/dependencies.rb
+++ b/Library/Homebrew/dependencies.rb
@@ -52,3 +52,28 @@ class Dependencies
   end
   alias_method :eql?, :==
 end
+
+class Requirements
+  include Enumerable
+
+  def initialize(*args)
+    @reqs = Set.new(*args)
+  end
+
+  def each(*args, &block)
+    @reqs.each(*args, &block)
+  end
+
+  def <<(other)
+    if Comparable === other
+      @reqs.grep(other.class) do |req|
+        return self if req > other
+        @reqs.delete(req)
+      end
+    end
+    @reqs << other
+    self
+  end
+
+  alias_method :to_ary, :to_a
+end
diff --git a/Library/Homebrew/dependency_collector.rb b/Library/Homebrew/dependency_collector.rb
index d5f7c977f..866f45cf0 100644
--- a/Library/Homebrew/dependency_collector.rb
+++ b/Library/Homebrew/dependency_collector.rb
@@ -31,7 +31,7 @@ class DependencyCollector
 
   def initialize
     @deps = Dependencies.new
-    @requirements = ComparableSet.new
+    @requirements = Requirements.new
   end
 
   def add(spec)
diff --git a/Library/Homebrew/extend/set.rb b/Library/Homebrew/extend/set.rb
deleted file mode 100644
index b13ece97c..000000000
--- a/Library/Homebrew/extend/set.rb
+++ /dev/null
@@ -1,24 +0,0 @@
-require 'set'
-
-class ComparableSet < Set
-  def add new
-    # smileys only
-    return super new unless new.respond_to? :>
-
-    grep(new.class) do |o|
-      return self if o > new
-      delete o
-    end
-    super new
-  end
-
-  alias_method :<<, :add
-
-  # Set#merge bypasses enumerating the set's contents,
-  # so the subclassed #add would never be called
-  def merge enum
-    enum.is_a?(Enumerable) or raise ArgumentError, "value must be enumerable"
-    enum.each { |o| add(o) }
-    self
-  end
-end
diff --git a/Library/Homebrew/requirement.rb b/Library/Homebrew/requirement.rb
index e05f98a46..d32c02b7f 100644
--- a/Library/Homebrew/requirement.rb
+++ b/Library/Homebrew/requirement.rb
@@ -1,5 +1,6 @@
 require 'dependable'
 require 'dependency'
+require 'dependencies'
 require 'build_environment'
 
 # A base class for non-formula requirements needed by formulae.
@@ -141,7 +142,7 @@ class Requirement
     # The default filter, which is applied when a block is not given, omits
     # optionals and recommendeds based on what the dependent has asked for.
     def expand(dependent, &block)
-      reqs = ComparableSet.new
+      reqs = Requirements.new
 
       formulae = dependent.recursive_dependencies.map(&:to_formula)
       formulae.unshift(dependent)
diff --git a/Library/Homebrew/requirements/x11_dependency.rb b/Library/Homebrew/requirements/x11_dependency.rb
index b71af8391..514f61f8b 100644
--- a/Library/Homebrew/requirements/x11_dependency.rb
+++ b/Library/Homebrew/requirements/x11_dependency.rb
@@ -1,5 +1,4 @@
-require 'requirement'
-require 'extend/set'
+require "requirement"
 
 class X11Dependency < Requirement
   include Comparable
diff --git a/Library/Homebrew/test/test_comparableset.rb b/Library/Homebrew/test/test_comparableset.rb
deleted file mode 100644
index 0d947bd89..000000000
--- a/Library/Homebrew/test/test_comparableset.rb
+++ /dev/null
@@ -1,41 +0,0 @@
-require 'testing_env'
-require 'extend/set'
-require 'requirements'
-
-class ComparableSetTests < Homebrew::TestCase
-  def setup
-    @set = ComparableSet.new
-  end
-
-  def test_merging_multiple_dependencies
-    @set << X11Dependency.new
-    @set << X11Dependency.new
-    assert_equal 1, @set.count
-    @set << Requirement.new
-    assert_equal 2, @set.count
-  end
-
-  def test_comparison_prefers_larger
-    @set << X11Dependency.new
-    @set << X11Dependency.new('x11', %w{2.6})
-    assert_equal 1, @set.count
-    assert_equal [X11Dependency.new('x11', %w{2.6})], @set.to_a
-  end
-
-  def test_comparison_does_not_merge_smaller
-    @set << X11Dependency.new('x11', %w{2.6})
-    @set << X11Dependency.new
-    assert_equal 1, @set.count
-    assert_equal [X11Dependency.new('x11', %w{2.6})], @set.to_a
-  end
-
-  def test_merging_sets
-    @set << X11Dependency.new
-    @set << Requirement.new
-    reqs = Set.new [X11Dependency.new('x11', %w{2.6}), Requirement.new]
-    assert_same @set, @set.merge(reqs)
-
-    assert_equal 2, @set.count
-    assert_equal X11Dependency.new('x11', %w{2.6}), @set.find {|r| r.is_a? X11Dependency}
-  end
-end
diff --git a/Library/Homebrew/test/test_dependencies.rb b/Library/Homebrew/test/test_dependencies.rb
index 88c101551..219080f03 100644
--- a/Library/Homebrew/test/test_dependencies.rb
+++ b/Library/Homebrew/test/test_dependencies.rb
@@ -1,6 +1,7 @@
 require 'testing_env'
 require 'dependencies'
 require 'dependency'
+require 'requirements'
 
 class DependenciesTests < Homebrew::TestCase
   def setup
@@ -78,3 +79,30 @@ class DependenciesTests < Homebrew::TestCase
     assert !a.eql?(b)
   end
 end
+
+class RequirementsTests < Homebrew::TestCase
+  def setup
+    @reqs = Requirements.new
+  end
+
+  def test_shovel_returns_self
+    assert_same @reqs, (@reqs << Object.new)
+  end
+
+  def test_merging_multiple_dependencies
+    @reqs << X11Dependency.new << X11Dependency.new
+    assert_equal 1, @reqs.count
+    @reqs << Requirement.new
+    assert_equal 2, @reqs.count
+  end
+
+  def test_comparison_prefers_larger
+    @reqs << X11Dependency.new << X11Dependency.new("x11", %w[2.6])
+    assert_equal [X11Dependency.new("x11", %w[2.6])], @reqs.to_a
+  end
+
+  def test_comparison_does_not_merge_smaller
+    @reqs << X11Dependency.new("x11", %w{2.6}) << X11Dependency.new
+    assert_equal [X11Dependency.new("x11", %w[2.6])], @reqs.to_a
+  end
+end
diff --git a/Library/Homebrew/test/test_dependency_collector.rb b/Library/Homebrew/test/test_dependency_collector.rb
index fcec3de33..760c3a06e 100644
--- a/Library/Homebrew/test/test_dependency_collector.rb
+++ b/Library/Homebrew/test/test_dependency_collector.rb
@@ -1,6 +1,5 @@
 require 'testing_env'
 require 'dependency_collector'
-require 'extend/set'
 
 class DependencyCollectorTests < Homebrew::TestCase
   def find_dependency(name)
@@ -52,7 +51,7 @@ class DependencyCollectorTests < Homebrew::TestCase
 
   def test_no_duplicate_requirements
     2.times { @d.add :x11 }
-    assert_equal 1, @d.requirements.length
+    assert_equal 1, @d.requirements.count
   end
 
   def test_requirement_tags
diff --git a/Library/Homebrew/test/test_x11_dependency.rb b/Library/Homebrew/test/test_x11_dependency.rb
index f174c8e23..230c5e758 100644
--- a/Library/Homebrew/test/test_x11_dependency.rb
+++ b/Library/Homebrew/test/test_x11_dependency.rb
@@ -1,5 +1,4 @@
 require 'testing_env'
-require 'extend/set'
 require 'requirements/x11_dependency'
 
 class X11DependencyTests < Homebrew::TestCase