Switch PkgVersion to use composition

Comparing PkgVersion and Version objects can produce nonsensical
results. For example, equality is not symmetric:

irb(main):002:0> PkgVersion.new("1.0", 0) == Version.new("1.0")
=> false
irb(main):003:0> Version.new("1.0") == PkgVersion.new("1.0", 0)
=> true

Rather than attempt to deal with subclass-superclass equality, let's use
composition and punt on the problem altogether.

Library/Homebrew/pkg_version.rb

@@ -1,36 +1,45 @@
-require 'version'
+require "version"
 
-class PkgVersion < Version
+class PkgVersion
-  attr_reader :version, :revision
+  include Comparable
 
   RX = /\A(.+?)(?:_(\d+))?\z/
 
   def self.parse(path)
     _, version, revision = *path.match(RX)
-    new(version, revision)
+    version = Version.new(version)
+    new(version, revision.to_i)
   end
 
   def initialize(version, revision)
-    super(version)
+    @version = version
+    @revision = version.head? ? 0 : revision
+  end
 
-    if head?
+  def head?
-      @revision = 0
+    version.head?
-    else
-      @revision = revision.to_i
-    end
   end
 
   def to_s
     if revision > 0
       "#{version}_#{revision}"
     else
-      version
+      version.to_s
     end
   end
   alias_method :to_str, :to_s
 
   def <=>(other)
-    return unless Version === other
+    return unless PkgVersion === other
-    super.nonzero? || revision <=> other.revision
+    (version <=> other.version).nonzero? || revision <=> other.revision
   end
+  alias_method :eql?, :==
+
+  def hash
+    version.hash ^ revision.hash
+  end
+
+  protected
+
+  attr_reader :version, :revision
 end

Library/Homebrew/test/test_pkg_version.rb

@@ -7,13 +7,13 @@ class PkgVersionTests < Homebrew::TestCase
   end
 
   def test_parse
-    assert_equal PkgVersion.new("1.0", 1), PkgVersion.parse("1.0_1")
+    assert_equal PkgVersion.new(Version.new("1.0"), 1), PkgVersion.parse("1.0_1")
-    assert_equal PkgVersion.new("1.0", 1), PkgVersion.parse("1.0_1")
+    assert_equal PkgVersion.new(Version.new("1.0"), 1), PkgVersion.parse("1.0_1")
-    assert_equal PkgVersion.new("1.0", 0), PkgVersion.parse("1.0")
+    assert_equal PkgVersion.new(Version.new("1.0"), 0), PkgVersion.parse("1.0")
-    assert_equal PkgVersion.new("1.0", 0), PkgVersion.parse("1.0_0")
+    assert_equal PkgVersion.new(Version.new("1.0"), 0), PkgVersion.parse("1.0_0")
-    assert_equal PkgVersion.new("2.1.4", 0), PkgVersion.parse("2.1.4_0")
+    assert_equal PkgVersion.new(Version.new("2.1.4"), 0), PkgVersion.parse("2.1.4_0")
-    assert_equal PkgVersion.new("2.1.4_1", 0), PkgVersion.parse("2.1.4_1_0")
+    assert_equal PkgVersion.new(Version.new("2.1.4_1"), 0), PkgVersion.parse("2.1.4_1_0")
-    assert_equal PkgVersion.new("1.0.1e", 1), PkgVersion.parse("1.0.1e_1")
+    assert_equal PkgVersion.new(Version.new("1.0.1e"), 1), PkgVersion.parse("1.0.1e_1")
   end
 
   def test_comparison
@@ -25,16 +25,17 @@ class PkgVersionTests < Homebrew::TestCase
     assert_operator v("HEAD"), :>, v("1.0")
     assert_operator v("1.0"), :<, v("HEAD")
 
-    v = PkgVersion.new("1.0", 0)
+    v = PkgVersion.new(Version.new("1.0"), 0)
     assert_nil v <=> Object.new
     assert_raises(ArgumentError) { v > Object.new }
+    assert_raises(ArgumentError) { v > Version.new("1.0") }
   end
 
   def test_to_s
-    assert_equal "1.0", PkgVersion.new("1.0", 0).to_s
+    assert_equal "1.0", PkgVersion.new(Version.new("1.0"), 0).to_s
-    assert_equal "1.0_1", PkgVersion.new("1.0", 1).to_s
+    assert_equal "1.0_1", PkgVersion.new(Version.new("1.0"), 1).to_s
-    assert_equal "1.0", PkgVersion.new("1.0", 0).to_s
+    assert_equal "1.0", PkgVersion.new(Version.new("1.0"), 0).to_s
-    assert_equal "1.0", PkgVersion.new("1.0", 0).to_s
+    assert_equal "1.0", PkgVersion.new(Version.new("1.0"), 0).to_s
-    assert_equal "HEAD", PkgVersion.new("HEAD", 1).to_s
+    assert_equal "HEAD", PkgVersion.new(Version.new("HEAD"), 1).to_s
   end
 end