fix: add identifier validation to inc() (#754)

- Adds a check of the `identifier` parameter for `inc()` when trying to
increase pre-releases. This prevents the creation of an invalid semver.
- Removes side-effects of `inc()` when it is throwing (it was changing
the version)

Closes #349

classes/semver.js

@@ -176,6 +176,19 @@ class SemVer {
   // preminor will bump the version up to the next minor release, and immediately
   // down to pre-release. premajor and prepatch work the same way.
   inc (release, identifier, identifierBase) {
+    if (release.startsWith('pre')) {
+      if (!identifier && identifierBase === false) {
+        throw new Error('invalid increment argument: identifier is empty')
+      }
+      // Avoid an invalid semver results
+      if (identifier) {
+        const match = `-${identifier}`.match(this.options.loose ? re[t.PRERELEASELOOSE] : re[t.PRERELEASE])
+        if (!match || match[1] !== identifier) {
+          throw new Error(`invalid identifier: ${identifier}`)
+        }
+      }
+    }
+
     switch (release) {
       case 'premajor':
         this.prerelease.length = 0
@@ -255,10 +268,6 @@ class SemVer {
       case 'pre': {
         const base = Number(identifierBase) ? 1 : 0
 
-        if (!identifier && identifierBase === false) {
-          throw new Error('invalid increment argument: identifier is empty')
-        }
-
         if (this.prerelease.length === 0) {
           this.prerelease = [base]
         } else {

test/classes/semver.js

@@ -106,6 +106,34 @@ test('incrementing', t => {
   }))
 })
 
+test('invalid increments', (t) => {
+  t.throws(
+    () => new SemVer('1.2.3').inc('prerelease', '', false),
+    Error('invalid increment argument: identifier is empty')
+  )
+  t.throws(
+    () => new SemVer('1.2.3-dev').inc('prerelease', 'dev', false),
+    Error('invalid increment argument: identifier already exists')
+  )
+  t.throws(
+    () => new SemVer('1.2.3').inc('prerelease', 'invalid/preid'),
+    Error('invalid identifier: invalid/preid')
+  )
+
+  t.end()
+})
+
+test('increment side-effects', (t) => {
+  const v = new SemVer('1.0.0')
+  try {
+    v.inc('prerelease', 'hot/mess')
+  } catch (er) {
+    // ignore but check that the version has not changed
+  }
+  t.equal(v.toString(), '1.0.0')
+  t.end()
+})
+
 test('compare main vs pre', (t) => {
   const s = new SemVer('1.2.3')
   t.equal(s.compareMain('2.3.4'), -1)

test/fixtures/increments.js

@@ -129,4 +129,7 @@ module.exports = [
   ['1.2.0-dev', 'prepatch', '1.2.1-dev', false, 'dev', false],
   ['1.2.0', 'prerelease', null, false, '', false],
   ['1.0.0-rc.1+build.4', 'prerelease', '1.0.0-rc.2', 'rc', false],
+  ['1.2.0', 'prerelease', null, false, 'invalid/preid'],
+  ['1.2.0', 'prerelease', null, false, 'invalid+build'],
+  ['1.2.0beta', 'prerelease', null, { loose: true }, 'invalid/preid'],
 ]