From 47498de65fcf67455f36bc7447311d2bbedb26bd Mon Sep 17 00:00:00 2001
From: Marc Bernard <marc@marcbernardtools.com>
Date: Tue, 21 Jan 2025 18:29:48 +0100
Subject: [PATCH 1/2] fix: add identifier validation to `inc()`

---
 classes/semver.js           | 111 ++++++++++++++++++++----------------
 test/classes/semver.js      |  33 +++++++++++
 test/fixtures/increments.js |   3 +
 3 files changed, 99 insertions(+), 48 deletions(-)

diff --git a/classes/semver.js b/classes/semver.js
index 13e66ce4..7d4ac1c8 100644
--- a/classes/semver.js
+++ b/classes/semver.js
@@ -10,7 +10,7 @@ class SemVer {
 
     if (version instanceof SemVer) {
       if (version.loose === !!options.loose &&
-          version.includePrerelease === !!options.includePrerelease) {
+        version.includePrerelease === !!options.includePrerelease) {
         return version
       } else {
         version = version.version
@@ -176,19 +176,21 @@ 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) {
+    this.#checkIdentifiers(release, identifier, identifierBase)
+
     switch (release) {
       case 'premajor':
         this.prerelease.length = 0
         this.patch = 0
         this.minor = 0
         this.major++
-        this.inc('pre', identifier, identifierBase)
+        this.#pre(identifier, identifierBase)
         break
       case 'preminor':
         this.prerelease.length = 0
         this.patch = 0
         this.minor++
-        this.inc('pre', identifier, identifierBase)
+        this.#pre(identifier, identifierBase)
         break
       case 'prepatch':
         // If this is already a prerelease, it will bump to the next version
@@ -196,7 +198,7 @@ class SemVer {
         // relevant at this point.
         this.prerelease.length = 0
         this.inc('patch', identifier, identifierBase)
-        this.inc('pre', identifier, identifierBase)
+        this.#pre(identifier, identifierBase)
         break
       // If the input is a non-prerelease version, this acts the same as
       // prepatch.
@@ -204,7 +206,7 @@ class SemVer {
         if (this.prerelease.length === 0) {
           this.inc('patch', identifier, identifierBase)
         }
-        this.inc('pre', identifier, identifierBase)
+        this.#pre(identifier, identifierBase)
         break
 
       case 'major':
@@ -244,50 +246,7 @@ class SemVer {
         }
         this.prerelease = []
         break
-      // This probably shouldn't be used publicly.
-      // 1.0.0 'pre' would become 1.0.0-0 which is the wrong direction.
-      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 {
-          let i = this.prerelease.length
-          while (--i >= 0) {
-            if (typeof this.prerelease[i] === 'number') {
-              this.prerelease[i]++
-              i = -2
-            }
-          }
-          if (i === -1) {
-            // didn't increment anything
-            if (identifier === this.prerelease.join('.') && identifierBase === false) {
-              throw new Error('invalid increment argument: identifier already exists')
-            }
-            this.prerelease.push(base)
-          }
-        }
-        if (identifier) {
-          // 1.2.0-beta.1 bumps to 1.2.0-beta.2,
-          // 1.2.0-beta.fooblz or 1.2.0-beta bumps to 1.2.0-beta.0
-          let prerelease = [identifier, base]
-          if (identifierBase === false) {
-            prerelease = [identifier]
-          }
-          if (compareIdentifiers(this.prerelease[0], identifier) === 0) {
-            if (isNaN(this.prerelease[1])) {
-              this.prerelease = prerelease
-            }
-          } else {
-            this.prerelease = prerelease
-          }
-        }
-        break
-      }
       default:
         throw new Error(`invalid increment argument: ${release}`)
     }
@@ -297,6 +256,62 @@ class SemVer {
     }
     return this
   }
+
+  #checkIdentifiers (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}`)
+        }
+      }
+    }
+  }
+
+  // Keep 'pre' internal to avoid confusing results
+  // inc('1.0.0', 'pre') would become 1.0.0-0 which is the wrong direction.
+  #pre (identifier, identifierBase) {
+    const base = Number(identifierBase) ? 1 : 0
+
+    if (this.prerelease.length === 0) {
+      this.prerelease = [base]
+    } else {
+      let i = this.prerelease.length
+      while (--i >= 0) {
+        if (typeof this.prerelease[i] === 'number') {
+          this.prerelease[i]++
+          i = -2
+        }
+      }
+      if (i === -1) {
+        // didn't increment anything
+        if (identifier === this.prerelease.join('.') && identifierBase === false) {
+          throw new Error('invalid increment argument: identifier already exists')
+        }
+        this.prerelease.push(base)
+      }
+    }
+    if (identifier) {
+      // 1.2.0-beta.1 bumps to 1.2.0-beta.2,
+      // 1.2.0-beta.fooblz or 1.2.0-beta bumps to 1.2.0-beta.0
+      let prerelease = [identifier, base]
+      if (identifierBase === false) {
+        prerelease = [identifier]
+      }
+      if (compareIdentifiers(this.prerelease[0], identifier) === 0) {
+        if (isNaN(this.prerelease[1])) {
+          this.prerelease = prerelease
+        }
+      } else {
+        this.prerelease = prerelease
+      }
+    }
+  }
 }
 
 module.exports = SemVer
diff --git a/test/classes/semver.js b/test/classes/semver.js
index 85a0ec31..64f0dba4 100644
--- a/test/classes/semver.js
+++ b/test/classes/semver.js
@@ -106,6 +106,39 @@ test('incrementing', t => {
   }))
 })
 
+test('invalid increments', (t) => {
+  // pre was used on internally and is not valid anymore
+  t.throws(
+    () => new SemVer('1.2.3').inc('pre'),
+    Error('invalid increment argument: pre')
+  )
+  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)
diff --git a/test/fixtures/increments.js b/test/fixtures/increments.js
index 65e9530b..997d77b1 100644
--- a/test/fixtures/increments.js
+++ b/test/fixtures/increments.js
@@ -124,4 +124,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'],
 ]

From c0d679d2fa34b3c08dfd6ae2783cabd23fe3e2e1 Mon Sep 17 00:00:00 2001
From: Marc Bernard <marc@marcbernardtools.com>
Date: Tue, 21 Jan 2025 21:08:03 +0100
Subject: [PATCH 2/2] Revert pre changes

---
 classes/semver.js      | 116 +++++++++++++++++++----------------------
 test/classes/semver.js |   5 --
 2 files changed, 55 insertions(+), 66 deletions(-)

diff --git a/classes/semver.js b/classes/semver.js
index 7d4ac1c8..12832462 100644
--- a/classes/semver.js
+++ b/classes/semver.js
@@ -176,7 +176,18 @@ 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) {
-    this.#checkIdentifiers(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':
@@ -184,13 +195,13 @@ class SemVer {
         this.patch = 0
         this.minor = 0
         this.major++
-        this.#pre(identifier, identifierBase)
+        this.inc('pre', identifier, identifierBase)
         break
       case 'preminor':
         this.prerelease.length = 0
         this.patch = 0
         this.minor++
-        this.#pre(identifier, identifierBase)
+        this.inc('pre', identifier, identifierBase)
         break
       case 'prepatch':
         // If this is already a prerelease, it will bump to the next version
@@ -198,7 +209,7 @@ class SemVer {
         // relevant at this point.
         this.prerelease.length = 0
         this.inc('patch', identifier, identifierBase)
-        this.#pre(identifier, identifierBase)
+        this.inc('pre', identifier, identifierBase)
         break
       // If the input is a non-prerelease version, this acts the same as
       // prepatch.
@@ -206,7 +217,7 @@ class SemVer {
         if (this.prerelease.length === 0) {
           this.inc('patch', identifier, identifierBase)
         }
-        this.#pre(identifier, identifierBase)
+        this.inc('pre', identifier, identifierBase)
         break
 
       case 'major':
@@ -246,7 +257,46 @@ class SemVer {
         }
         this.prerelease = []
         break
+      // This probably shouldn't be used publicly.
+      // 1.0.0 'pre' would become 1.0.0-0 which is the wrong direction.
+      case 'pre': {
+        const base = Number(identifierBase) ? 1 : 0
 
+        if (this.prerelease.length === 0) {
+          this.prerelease = [base]
+        } else {
+          let i = this.prerelease.length
+          while (--i >= 0) {
+            if (typeof this.prerelease[i] === 'number') {
+              this.prerelease[i]++
+              i = -2
+            }
+          }
+          if (i === -1) {
+            // didn't increment anything
+            if (identifier === this.prerelease.join('.') && identifierBase === false) {
+              throw new Error('invalid increment argument: identifier already exists')
+            }
+            this.prerelease.push(base)
+          }
+        }
+        if (identifier) {
+          // 1.2.0-beta.1 bumps to 1.2.0-beta.2,
+          // 1.2.0-beta.fooblz or 1.2.0-beta bumps to 1.2.0-beta.0
+          let prerelease = [identifier, base]
+          if (identifierBase === false) {
+            prerelease = [identifier]
+          }
+          if (compareIdentifiers(this.prerelease[0], identifier) === 0) {
+            if (isNaN(this.prerelease[1])) {
+              this.prerelease = prerelease
+            }
+          } else {
+            this.prerelease = prerelease
+          }
+        }
+        break
+      }
       default:
         throw new Error(`invalid increment argument: ${release}`)
     }
@@ -256,62 +306,6 @@ class SemVer {
     }
     return this
   }
-
-  #checkIdentifiers (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}`)
-        }
-      }
-    }
-  }
-
-  // Keep 'pre' internal to avoid confusing results
-  // inc('1.0.0', 'pre') would become 1.0.0-0 which is the wrong direction.
-  #pre (identifier, identifierBase) {
-    const base = Number(identifierBase) ? 1 : 0
-
-    if (this.prerelease.length === 0) {
-      this.prerelease = [base]
-    } else {
-      let i = this.prerelease.length
-      while (--i >= 0) {
-        if (typeof this.prerelease[i] === 'number') {
-          this.prerelease[i]++
-          i = -2
-        }
-      }
-      if (i === -1) {
-        // didn't increment anything
-        if (identifier === this.prerelease.join('.') && identifierBase === false) {
-          throw new Error('invalid increment argument: identifier already exists')
-        }
-        this.prerelease.push(base)
-      }
-    }
-    if (identifier) {
-      // 1.2.0-beta.1 bumps to 1.2.0-beta.2,
-      // 1.2.0-beta.fooblz or 1.2.0-beta bumps to 1.2.0-beta.0
-      let prerelease = [identifier, base]
-      if (identifierBase === false) {
-        prerelease = [identifier]
-      }
-      if (compareIdentifiers(this.prerelease[0], identifier) === 0) {
-        if (isNaN(this.prerelease[1])) {
-          this.prerelease = prerelease
-        }
-      } else {
-        this.prerelease = prerelease
-      }
-    }
-  }
 }
 
 module.exports = SemVer
diff --git a/test/classes/semver.js b/test/classes/semver.js
index 64f0dba4..946fa417 100644
--- a/test/classes/semver.js
+++ b/test/classes/semver.js
@@ -107,11 +107,6 @@ test('incrementing', t => {
 })
 
 test('invalid increments', (t) => {
-  // pre was used on internally and is not valid anymore
-  t.throws(
-    () => new SemVer('1.2.3').inc('pre'),
-    Error('invalid increment argument: pre')
-  )
   t.throws(
     () => new SemVer('1.2.3').inc('prerelease', '', false),
     Error('invalid increment argument: identifier is empty')