Fix #6 Pre-1.0 upgrades should not be safe

README.md

@@ -40,7 +40,7 @@ d.build -- 'no.extensions.22'
 v'1.2.3' == v(1,2,3)         -- true
 v'1.2.3' < v(4,5,6)          -- true
 v'1.2.3-alpha' < v'1.2.3'    -- true
-v'1.2.3' < v'1.2.3-build.1'  -- true
+v'1.2.3' < v'1.2.3+build.1'  -- false, builds are ignored when comparing versions in semver
 -- (see the "notes" section for more informaion about version comparison)
 
 -- "pessimistic upgrade" operator: ^
@@ -76,12 +76,14 @@ Version comparison is done according to the semver 2.0.0 specs:
 
 Major, minor, and patch versions are always compared numerically.
 
-Pre-release and build version precedence MUST be determined by comparing each dot-separated identifier as follows:
+Pre-release precedence MUST be determined by comparing each dot-separated identifier as follows:
 
 * Identifiers consisting of only digits are compared numerically
 * Identifiers with letters or dashes are compared lexically in ASCII sort order.
 * Numeric identifiers always have lower precedence than non-numeric identifiers
 
+Builds are ignored when calculating precedence: version 1.2.3 and 1.2.3+build5 are considered equal.
+
 # Specs
 
 This project uses "busted":http://olivinelabs.com/busted/ for its specs. If you want to run the specs, you will have to install telescope first. Then just execute the following from the root inspect folder:
@@ -97,3 +99,8 @@ busted
 * Changed spec tool from telescope to busted
 * Changed README format from textile to markdown
 
+## v.1.2.0:
+* Fix error: builds were being used for comparison, but according with semver 2.0.0 they should be ignored (so v'1.0.0+build1' is equal to v'1.0.0+build2')
+* Fix several errors and inconsistencies in the way the comparisons where implemented.
+* Added a lot more tests to cover more edge cases when comparing versions
+

rockspecs/semver-1.2.0-1.rockspec

@@ -0,0 +1,25 @@
+package = "semver"
+version = "1.2.0-1"
+source = {
+   url = "git://github.com/kikito/semver.lua.git",
+}
+description = {
+   summary = "An implementation of semantic versioning (semver.org 2.0.0) in Lua",
+   detailed = [[
+      See details in http://semver.org
+   ]],
+   license = "MIT",
+   homepage = "https://github.com/kikito/semver.lua"
+}
+dependencies = {
+   "lua >= 5.1"
+}
+
+build = {
+  type = "none",
+  install = {
+    lua = {
+      "semver.lua"
+    },
+  }
+}

rockspecs/semver-1.2.1-1.rockspec

@@ -0,0 +1,25 @@
+package = "semver"
+version = "1.2.1-1"
+source = {
+   url = "git://github.com/kikito/semver.lua.git",
+}
+description = {
+   summary = "An implementation of semantic versioning (semver.org 2.0.0) in Lua",
+   detailed = [[
+      See details in http://semver.org
+   ]],
+   license = "MIT",
+   homepage = "https://github.com/kikito/semver.lua"
+}
+dependencies = {
+   "lua >= 5.1"
+}
+
+build = {
+  type = "none",
+  install = {
+    lua = {
+      "semver.lua"
+    },
+  }
+}

semver.lua

@@ -1,5 +1,5 @@
 local semver = {
-  _VERSION     = '1.1.1',
+  _VERSION     = '1.2.1',
   _DESCRIPTION = 'semver for Lua',
   _URL         = 'https://github.com/kikito/semver.lua',
   _LICENSE     = [[
@@ -99,23 +99,13 @@ local function compare(a,b)
   return a == b and 0 or a < b and -1 or 1
 end
 
-local function compareNilPrereleases(mine, other)
-  if mine == other then return  0
-  elseif not mine  then return  1
-  elseif not other then return -1
-  end -- else return nil
-end
+local function compareIds(myId, otherId)
+  if myId == otherId then return  0
+  elseif not myId    then return -1
+  elseif not otherId then return  1
+  end
 
--- notice that builds compare nils inversely than prereleases (the -1 and 1 are switched)
-local function compareNilBuilds(mine, other)
-  if mine == other then return  0
-  elseif not mine  then return -1
-  elseif not other then return  1
-  end -- else return nil
-end
-
-local function compareIds(selfId, otherId)
-  local selfNumber, otherNumber = tonumber(selfId), tonumber(otherId)
+  local selfNumber, otherNumber = tonumber(myId), tonumber(otherId)
 
   if selfNumber and otherNumber then -- numerical comparison
     return compare(selfNumber, otherNumber)
@@ -125,18 +115,16 @@ local function compareIds(selfId, otherId)
   elseif otherNumber then
     return 1
   else
-    return compare(selfId, otherId) -- alphanumerical comparison
+    return compare(myId, otherId) -- alphanumerical comparison
   end
 end
 
-local function smallerIds(mine, other, compareNil)
-  local myIds, otherIds = splitByDot(mine), splitByDot(other)
+local function smallerIdList(myIds, otherIds)
   local myLength = #myIds
-  local comparison, myId, otherId
+  local comparison
 
-  for i = 1, myLength do
-    myId, otherId = myIds[i], otherIds[i]
-    comparison = compareNil(myId, otherId) or compareIds(myId, otherId)
+  for i=1, myLength do
+    comparison = compareIds(myIds[i], otherIds[i])
     if comparison ~= 0 then
       return comparison == -1
     end
@@ -151,15 +139,7 @@ local function smallerPrerelease(mine, other)
   elseif not other then return true
   end
 
-  return smallerIds(mine, other, compareNilPrereleases)
-end
-
-local function smallerBuild(mine, other)
-  if mine == other or not other then return false
-  elseif not mine then return true
-  end
-
-  return smallerIds(mine, other, compareNilBuilds)
+  return smallerIdList(splitByDot(mine), splitByDot(other))
 end
 
 local methods = {}
@@ -179,17 +159,23 @@ function mt:__eq(other)
   return self.major == other.major and
          self.minor == other.minor and
          self.patch == other.patch and
-         self.prerelease == other.prerelease and
-         self.build == other.build
+         self.prerelease == other.prerelease
+         -- notice that build is ignored for precedence in semver 2.0.0
 end
 function mt:__lt(other)
-  return self.major < other.major or
-         self.minor < other.minor or
-         self.patch < other.patch or
-         smallerPrerelease(self.prerelease, other.prerelease) or
-         smallerBuild(self.build, other.build)
+  if self.major ~= other.major then return self.major < other.major end
+  if self.minor ~= other.minor then return self.minor < other.minor end
+  if self.patch ~= other.patch then return self.patch < other.patch end
+  return smallerPrerelease(self.prerelease, other.prerelease)
+  -- notice that build is ignored for precedence in semver 2.0.0
 end
+-- This works like the "pessimisstic operator" in Rubygems.
+-- if a and b are versions, a ^ b means "b is backwards-compatible with a"
+-- in other words, "it's safe to upgrade from a to b"
 function mt:__pow(other)
+  if self.major == 0 then
+    return self == other
+  end
   return self.major == other.major and
          self.minor <= other.minor
 end

spec/semver_spec.lua

@@ -39,59 +39,59 @@ describe('semver', function()
     end)
 
     describe("from strings", function()
-      test("1.2.3", function()
+      it("1.2.3", function()
         checkVersion( v'1.2.3', 1,2,3)
       end)
-      test("10.20.123", function()
+      it("10.20.123", function()
         checkVersion( v'10.20.123', 10,20,123)
       end)
-      test("2.0", function()
+      it("2.0", function()
         checkVersion( v'2.0', 2,0,0)
       end)
-      test("5", function()
+      it("5", function()
         checkVersion( v'5', 5,0,0)
       end)
-      test("1.2.3-alpha", function()
+      it("1.2.3-alpha", function()
         checkVersion( v'1.2.3-alpha', 1,2,3,'alpha' )
       end)
-      test("1.2.3+build.15", function()
+      it("1.2.3+build.15", function()
         checkVersion( v'1.2.3+build.15', 1,2,3,nil,'build.15' )
       end)
-      test("1.2.3-rc1+build.15", function()
+      it("1.2.3-rc1+build.15", function()
         checkVersion( v'1.2.3-rc1+build.15', 1,2,3,'rc1','build.15' )
       end)
     end)
 
     describe('errors', function()
-      test('no parameters are passed', function()
+      it('no parameters are passed', function()
         assert.error(function() v() end)
       end)
-      test('negative numbers', function()
+      it('negative numbers', function()
         assert.error(function() v(-1, 0, 0) end)
         assert.error(function() v( 0,-1, 0) end)
         assert.error(function() v( 0, 0,-1) end)
       end)
-      test('floats', function()
+      it('floats', function()
         assert.error(function() v(.1, 0, 0) end)
         assert.error(function() v( 0,.1, 0) end)
         assert.error(function() v( 0, 0,.1) end)
       end)
-      test('empty string', function()
+      it('empty string', function()
         assert.error(function() v("") end)
       end)
-      test('garbage at the beginning of the string', function()
+      it('garbage at the beginning of the string', function()
         assert.error(function() v("foobar1.2.3") end)
       end)
-      test('garbage at the end of the string', function()
+      it('garbage at the end of the string', function()
         assert.error(function() v("1.2.3foobar") end)
       end)
-      test('a non-string or number is passed', function()
+      it('a non-string or number is passed', function()
         assert.error(function() v({}) end)
       end)
-      test('an invalid prerelease', function()
+      it('an invalid prerelease', function()
         assert.error(function() v'1.2.3-%?' end)
       end)
-      test('an invalid build', function()
+      it('an invalid build', function()
         assert.error(function() v'1.2.3+%?' end)
       end)
     end)
@@ -115,115 +115,112 @@ describe('semver', function()
 
   describe("==", function()
     it("is true when major, minor and patch are the same", function()
-      assert.equal(v(1,2,3), v'1.2.3')
+      assert.equal(v'1.0.0', v'1.0.0')
     end)
-    it("is false when major, minor and patch are not the same", function()
-      assert.not_equal(v(1,2,3), v(4,5,6))
+    it("is false when major, minor, patch or prerelease are not the same", function()
+      assert.not_equal(v'1.0.0', v'1.0.1')
+      assert.not_equal(v'1.0.0', v'1.1.0')
+      assert.not_equal(v'1.0.0', v'2.0.0')
+      assert.not_equal(v'1.0.0', v'1.0.0-alpha')
     end)
-    it("false if all is the same except the prerelease", function()
-      assert.not_equal(v(1,2,3), v'1.2.3-alpha')
-    end)
-    it("false if all is the same except the build", function()
-      assert.not_equal(v(1,2,3), v'1.2.3+peter.1')
+    it("ignores builds", function()
+      assert.equal(v'1.2.3', v'1.2.3+1')
+      assert.equal(v'1.2.3+1', v'1.2.3+2')
     end)
   end)
 
   describe("<", function()
-    test("true if major < minor", function()
-      assert.less(v'1.100.10', v'2.0.0')
+    it("compares correctly when major, minor and patch are equal", function()
+      assert.not_less(v'1.0.0', v'1.0.0')
+      assert.not_greater(v'1.0.0', v'1.0.0')
+      assert.equal(v'1.0.0', v'1.0.0')
     end)
-    test("false if major > minor", function()
-      assert.greater(v'2', v'1')
+
+    it("#focus prioritizes major over minor", function()
+      --assert.less(v'1.100.10', v'2.0.0')
+      assert.not_greater(v'1.100.10', v'2.0.0')
+      --assert.greater(v'2', v'1')
+      --assert.not_less(v'2', v'1')
     end)
-    test("true if major = major but minor < minor", function()
+    it("when equal major, compares minor", function()
       assert.less(v'1.2.0', v'1.3.0')
-    end)
-    test("false if minor < minor", function()
+      assert.not_greater(v'1.2.0', v'1.3.0')
       assert.greater(v'1.1', v'1.0')
+      assert.not_less(v'1.1', v'1.0')
     end)
-    test("true if major =, minor =, but patch <", function()
+    it("when equal major and minor, compares patch", function()
       assert.less(v'0.0.1', v'0.0.10')
-    end)
-    test("false if major =, minor =, but patch >", function()
+      assert.not_greater(v'0.0.1', v'0.0.10')
       assert.greater(v'0.0.2', v'0.0.1')
+      assert.not_less(v'0.0.2', v'0.0.1')
     end)
     describe("prereleases", function()
-      test("false if exact same prerelease", function()
-        assert.not_less(v'1.0.0-beta', v'1.0.0-beta')
+      it("compares correctly when major, minor, patch and prerelease are equal", function()
+        assert.not_less(v'1.0.0-1', v'1.0.0-1')
+        assert.not_greater(v'1.0.0-1', v'1.0.0-1')
+        assert.equal(v'1.0.0-1', v'1.0.0-1')
       end)
-      test("#focus a prerelease version is less than the official version", function()
+      it("prioritizes non-prereleases over prereleases", function()
         assert.less(v'1.0.0-rc1', v'1.0.0')
         assert.not_greater(v'1.0.0-rc1', v'1.0.0')
         assert.greater(v'1.2.3', v'1.2.3-alpha')
         assert.not_less(v'1.2.3', v'1.2.3-alpha')
       end)
-      test("identifiers with only digits are compared numerically", function()
+      it("compares identifiers with only digits numerically", function()
         assert.less(v'1.0.0-1', v'1.0.0-2')
         assert.not_greater(v'1.0.0-1', v'1.0.0-2')
+        assert.greater(v'1.0.0-2', v'1.0.0-1')
+        assert.not_less(v'1.0.0-2', v'1.0.0-1')
       end)
-      test("idendifiers with letters or dashes are compared lexiconumerically", function()
+      it("compares idendifiers with letters or dashes lexiconumerically", function()
         assert.less(v'1.0.0-alpha', v'1.0.0-beta')
-        assert.less(v'1.0.0-alpha-10', v'1.0.0-alpha-2')
         assert.not_greater(v'1.0.0-alpha', v'1.0.0-beta')
+        assert.less(v'1.0.0-alpha-10', v'1.0.0-alpha-2')
         assert.not_greater(v'1.0.0-alpha-10', v'1.0.0-alpha-2')
+        assert.greater(v'1.0.0-beta', v'1.0.0-alpha')
+        assert.not_less(v'1.0.0-beta', v'1.0.0-alpha')
+        assert.greater(v'1.0.0-alpha-2', v'1.0.0-alpha-10')
+        assert.not_less(v'1.0.0-alpha-2', v'1.0.0-alpha-10')
       end)
-      test("numerical ids always have less priority than lexiconumericals", function()
+      it("prioritizes lexiconumericals over numbers", function()
         assert.less(v'1.0.0-1', v'1.0.0-alpha')
-        assert.less(v'1.0.0-2', v'1.0.0-1asdf')
         assert.not_greater(v'1.0.0-1', v'1.0.0-alpha')
+        assert.less(v'1.0.0-2', v'1.0.0-1asdf')
         assert.not_greater(v'1.0.0-2', v'1.0.0-1asdf')
+        assert.greater(v'1.0.0-alpha', v'1.0.0-1')
+        assert.not_less(v'1.0.0-alpha', v'1.0.0-1')
+        assert.greater(v'1.0.0-1asdf', v'1.0.0-2')
+        assert.not_less(v'1.0.0-1asdf', v'1.0.0-2')
       end)
-      test("identifiers can be separated by colons; they must be compared individually", function()
-        --assert.less(v'1.0.0-alpha'   , v'1.0.0-alpha.1')
-        --assert.less(v'1.0.0-alpha.1' , v'1.0.0-beta.2')
-        --assert.less(v'1.0.0-beta.2'  , v'1.0.0-beta.11')
-        --assert.less(v'1.0.0-beta.11' , v'1.0.0-rc.1')
+      it("splits identifiers by colons, comparing every pair individually", function()
+        assert.less(v'1.0.0-alpha', v'1.0.0-alpha.1')
         assert.not_greater(v'1.0.0-alpha', v'1.0.0-alpha.1')
-        --assert.not_greater(v'1.0.0-alpha.1', v'1.0.0-beta.2')
-        --assert.not_greater(v'1.0.0-beta.2' , v'1.0.0-beta.11')
-        --assert.not_greater(v'1.0.0-beta.11', v'1.0.0-rc.1')
+        assert.less(v'1.0.0-alpha.1', v'1.0.0-beta.2')
+        assert.not_greater(v'1.0.0-alpha.1', v'1.0.0-beta.2')
+        assert.less(v'1.0.0-beta.2', v'1.0.0-beta.11')
+        assert.not_greater(v'1.0.0-beta.2', v'1.0.0-beta.11')
+        assert.less(v'1.0.0-beta.11', v'1.0.0-rc.1')
+        assert.not_greater(v'1.0.0-beta.11', v'1.0.0-rc.1')
+
+        assert.greater(v'1.0.0-alpha.1', v'1.0.0-alpha')
+        assert.not_less(v'1.0.0-alpha.1', v'1.0.0-alpha')
+        assert.greater(v'1.0.0-beta.2', v'1.0.0-alpha.1')
+        assert.not_less(v'1.0.0-beta.2', v'1.0.0-alpha.1')
+        assert.greater(v'1.0.0-beta.11', v'1.0.0-beta.2')
+        assert.not_less(v'1.0.0-beta.11', v'1.0.0-beta.2')
+        assert.greater(v'1.0.0-rc.1', v'1.0.0-beta.11')
+        assert.not_less(v'1.0.0-rc.1', v'1.0.0-beta.11')
       end)
     end)
     describe("builds", function()
-      test("false if exact same build", function()
-        assert.not_less(v'1.0.0+build1', v'1.0.0+build1')
+      it("false independently of the build", function()
+        assert.not_less(v'1.0.0+build1', v'1.0.0')
+        assert.not_less(v'1.0.0+build1', v'1.0.0+build3')
+        assert.not_less(v'1.0.0-beta+build1', v'1.0.0-beta+build2')
+        assert.not_greater(v'1.0.0+build1', v'1.0.0')
+        assert.not_greater(v'1.0.0+build1', v'1.0.0+build3')
+        assert.not_greater(v'1.0.0-beta+build1', v'1.0.0-beta+build2')
       end)
-      test("a regular (not-build) version is always less than a build version", function()
-        assert.less(v'1.0.0', v'1.0.0+12')
-        assert.less(v'1.0.0', v'1.0.0+12')
-      end)
-      test("identifiers with only digits are compared numerically", function()
-        assert.less(v'1.0.0+1', v'1.0.0+2')
-        assert.less(v'1.0.0+2', v'1.0.0+10')
-        assert.not_greater(v'1.0.0+1', v'1.0.0+2')
-        assert.not_greater(v'1.0.0+2', v'1.0.0+10')
-      end)
-      test("idendifiers with letters or dashes are compared lexiconumerically", function()
-        assert.less(v'1.0.0+build1' , v'1.0.0+build2')
-        assert.less(v'1.0.0+build10', v'1.0.0+build2')
-        assert.not_greater(v'1.0.0+build1'  , v'1.0.0+build2')
-        assert.not_greater(v'1.0.0+build10' , v'1.0.0+build2')
-      end)
-      test("numerical ids always have less priority than lexiconumericals", function()
-        assert.less(v'1.0.0+1', v'1.0.0+build1')
-        assert.less(v'1.0.0+2', v'1.0.0+1build')
-        assert.not_greater(v'1.0.0+1', v'1.0.0+build1')
-        assert.not_greater(v'1.0.0+2', v'1.0.0+1build')
-      end)
-      test("identifiers can be separated by colons; they must be compared individually", function()
-        assert.less(v'1.0.0+0.3.7', v'1.3.7+build')
-        assert.less(v'1.3.7+build', v'1.3.7+build.2.b8f12d7')
-        assert.less(v'1.3.7+build.2.b8f12d7', v'1.3.7+build.11.e0f985a')
-        assert.not_greater(v'1.0.0+0.3.7', v'1.3.7+build')
-        assert.not_greater(v'1.3.7+build', v'1.3.7+build.2.b8f12d7')
-        assert.not_greater(v'1.3.7+build.2.b8f12d7', v'1.3.7+build.11.e0f985a')
-      end)
-    end)
-    test("#focus prereleases + builds", function()
-      --assert.less(v'1.0.0-rc.1', v'1.0.0-rc.1+build.1')
-      --assert.less(v'1.0.0-rc.1+build.1', v'1.0.0')
-      --assert.not_greater(v'1.0.0-rc.1', v'1.0.0-rc.1+build.1')
-      assert.not_greater(v'1.0.0-rc.1+build.1', v'1.0.0')
     end)
   end)
 
@@ -254,32 +251,39 @@ describe('semver', function()
     end)
   end)
 
-
-  -- This works like the "pessimisstic operator" in Rubygems.
-  -- if a and b are versions, a ^ b means "b is backwards-compatible with a"
-  -- in other words, "it's safe to upgrade from a to b"
   describe("^", function()
-    test("true for self", function()
+    it("true for self", function()
       assert.is_true(v(1,2,3) ^ v(1,2,3))
+      assert.is_true(v(0,1,0) ^ v(0,1,0))
+      assert.is_true(v("0.1.1+build0") ^ v(0,1,1))
     end)
-    test("different major versions mean it's always unsafe", function()
+    it("different major versions mean it's always unsafe", function()
       assert.is_false(v(2,0,0) ^ v(3,0,0))
       assert.is_false(v(2,0,0) ^ v(1,0,0))
     end)
 
-    test("patches, prereleases and builds are ignored", function()
+    it("patches, prereleases and builds are ignored", function()
       assert.is_true(v(1,2,3) ^ v(1,2,0))
       assert.is_true(v(1,2,3) ^ v(1,2,5))
       assert.is_true(v(1,2,3,'foo') ^ v(1,2,3))
       assert.is_true(v(1,2,3,nil,'bar') ^ v(1,2,3))
     end)
 
-    test("it's safe to upgrade to a newer minor version", function()
+    it("is safe to upgrade to a newer minor version", function()
       assert.is_true(v(1,2,0) ^ v(1,5,0))
     end)
-    test("it's unsafe to downgrade to an earlier minor version", function()
+    it("is unsafe to downgrade to an earlier minor version", function()
       assert.is_false(v(1,5,0) ^ v(1,2,0))
     end)
+
+    it("is unsafe to upgrade any pre-1.0 versions", function()
+      assert.is_false(v("0.0.1-alpha") ^ v(0,0,1))
+      assert.is_false(v("0.0.1") ^ v(0,0,2))
+      assert.is_false(v("0.0.1+build0") ^ v(0,0,2))
+      assert.is_false(v(0,0,1) ^ v(0,1,0))
+      assert.is_false(v(0,0,1) ^ v(1,0,0))
+      assert.is_false(v(0,0,1) ^ v("1.0.0-alpha"))
+    end)
   end)
 
   describe("_VERSION", function()