From 53cf6735648f216ce3c8c5fe94f62cf078a63f2c Mon Sep 17 00:00:00 2001
From: Jongsun Suh <34228073+MajorLift@users.noreply.github.com>
Date: Fri, 13 Oct 2023 14:14:24 -0700
Subject: [PATCH 1/7] Replace `as T[]` assertions with `?? []`

- `as` assertions intended to exclude an empty/nullable type can be replaced by a nullish coalescing operator converting nullable values into an acceptable empty value that doesn't pollute the variable's type signature.
- TODO: document and expound in https://github.com/MetaMask/contributor-docs/issues/47
---
 src/changelog.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/changelog.ts b/src/changelog.ts
index f04addc..bdbd8a1 100644
--- a/src/changelog.ts
+++ b/src/changelog.ts
@@ -89,7 +89,7 @@ function stringifyRelease(
   const categorizedChanges = orderedChangeCategories
     .filter((category) => categories[category])
     .map((category) => {
-      const changes = categories[category] as string[];
+      const changes = categories[category] ?? [];
       return stringifyCategory(category, changes);
     })
     .join('\n\n');
@@ -386,8 +386,8 @@ export default class Changelog {
     for (const category of Object.keys(unreleasedChanges) as ChangeCategory[]) {
       if (releaseChanges[category]) {
         releaseChanges[category] = [
-          ...(unreleasedChanges[category] as string[]),
-          ...(releaseChanges[category] as string[]),
+          ...(unreleasedChanges[category] ?? []),
+          ...(releaseChanges[category] ?? []),
         ];
       } else {
         releaseChanges[category] = unreleasedChanges[category];

From f950f3e43d90403068c672eb00cbaa378bc77866 Mon Sep 17 00:00:00 2001
From: Jongsun Suh <34228073+MajorLift@users.noreply.github.com>
Date: Fri, 13 Oct 2023 14:26:44 -0700
Subject: [PATCH 2/7] Replace `as` assertions with redundant type guards

Sometimes TypeScript's type narrowing and inference doesn't fully work quite like it should. Even in these cases, using `as` assertions should be avoided.

It may seem like we know for certain what the type should be and there's no risk of it breaking, but there's always the possibility that some combination of changes elsewhere might affect the typing, in which case `as` will suppress the alerts TypeScript would otherwise provide us with. This may lead to code breaking silently.

In this case, adding type guards and null checks -- even if they're redundant -- is preferable to the above scenario.

- TODO: document and expound in https://github.com/MetaMask/contributor-docs/issues/47
---
 src/update-changelog.ts | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/update-changelog.ts b/src/update-changelog.ts
index 9e73c5d..f9d6a57 100644
--- a/src/update-changelog.ts
+++ b/src/update-changelog.ts
@@ -217,7 +217,7 @@ export async function updateChangelog({
 
   if (
     isReleaseCandidate &&
-    mostRecentTag === `${tagPrefixes[0]}${currentVersion || ''}`
+    mostRecentTag === `${tagPrefixes[0]}${currentVersion ?? ''}`
   ) {
     throw new Error(
       `Current version already has tag, which is unexpected for a release candidate.`,
@@ -250,19 +250,16 @@ export async function updateChangelog({
   // Ensure release header exists, if necessary
   if (
     isReleaseCandidate &&
+    currentVersion &&
     !changelog
       .getReleases()
       .find((release) => release.version === currentVersion)
   ) {
-    // Typecast: currentVersion will be defined here due to type guard at the
-    // top of this function.
-    changelog.addRelease({ version: currentVersion as Version });
+    changelog.addRelease({ version: currentVersion });
   }
 
-  if (isReleaseCandidate && hasUnreleasedChanges) {
-    // Typecast: currentVersion will be defined here due to type guard at the
-    // top of this function.
-    changelog.migrateUnreleasedChangesToRelease(currentVersion as Version);
+  if (isReleaseCandidate && currentVersion && hasUnreleasedChanges) {
+    changelog.migrateUnreleasedChangesToRelease(currentVersion);
   }
 
   const newChangeEntries = newCommits.map(({ prNumber, description }) => {

From 8da9f215a13db6775d178968ff343144fc3f7ffb Mon Sep 17 00:00:00 2001
From: Jongsun Suh <34228073+MajorLift@users.noreply.github.com>
Date: Fri, 13 Oct 2023 14:29:32 -0700
Subject: [PATCH 3/7] Make `as` assertion rely on inference instead of
 hard-coded type casting

If anything changes in the typing of `ChangeCategory` or `unreleasedChanges` that make them inconsistent, this code will fail silently. Relying on type inference instead of explicit type casting will make these line a little less brittle.

- TODO: document and expound in https://github.com/MetaMask/contributor-docs/issues/47
---
 src/changelog.ts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/changelog.ts b/src/changelog.ts
index bdbd8a1..8db5fef 100644
--- a/src/changelog.ts
+++ b/src/changelog.ts
@@ -383,7 +383,9 @@ export default class Changelog {
 
     const unreleasedChanges = this.#changes[unreleased];
 
-    for (const category of Object.keys(unreleasedChanges) as ChangeCategory[]) {
+    for (const category of Object.keys(
+      unreleasedChanges,
+    ) as (keyof typeof unreleasedChanges)[]) {
       if (releaseChanges[category]) {
         releaseChanges[category] = [
           ...(unreleasedChanges[category] ?? []),

From 033cd4e3f119bbeffec9f1c942e44a7549c6b7a3 Mon Sep 17 00:00:00 2001
From: Jongsun Suh <34228073+MajorLift@users.noreply.github.com>
Date: Mon, 16 Oct 2023 11:41:24 -0700
Subject: [PATCH 4/7] Fix type narrowing for `currentVersion` by consolidating
 conditional branches

- more consistent logic for `isReleaseCandidate` null check
- wasted processing overhead due to errors throwing later than strictly necessary
---
 src/update-changelog.ts | 74 ++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 38 deletions(-)

diff --git a/src/update-changelog.ts b/src/update-changelog.ts
index f9d6a57..14d0658 100644
--- a/src/update-changelog.ts
+++ b/src/update-changelog.ts
@@ -197,11 +197,6 @@ export async function updateChangelog({
   tagPrefixes = ['v'],
   formatter = undefined,
 }: UpdateChangelogOptions): Promise<string | undefined> {
-  if (isReleaseCandidate && !currentVersion) {
-    throw new Error(
-      `A version must be specified if 'isReleaseCandidate' is set.`,
-    );
-  }
   const changelog = parseChangelog({
     changelogContent,
     repoUrl,
@@ -215,15 +210,6 @@ export async function updateChangelog({
     tagPrefixes,
   });
 
-  if (
-    isReleaseCandidate &&
-    mostRecentTag === `${tagPrefixes[0]}${currentVersion ?? ''}`
-  ) {
-    throw new Error(
-      `Current version already has tag, which is unexpected for a release candidate.`,
-    );
-  }
-
   const commitRange =
     mostRecentTag === null ? 'HEAD' : `${mostRecentTag}..HEAD`;
   const commitsHashesSinceLastRelease = await getCommitHashesInRange(
@@ -247,35 +233,47 @@ export async function updateChangelog({
     return undefined;
   }
 
-  // Ensure release header exists, if necessary
-  if (
-    isReleaseCandidate &&
-    currentVersion &&
-    !changelog
-      .getReleases()
-      .find((release) => release.version === currentVersion)
-  ) {
-    changelog.addRelease({ version: currentVersion });
-  }
+  if (isReleaseCandidate) {
+    if (!currentVersion) {
+      throw new Error(
+        `A version must be specified if 'isReleaseCandidate' is set.`,
+      );
+    }
 
-  if (isReleaseCandidate && currentVersion && hasUnreleasedChanges) {
-    changelog.migrateUnreleasedChangesToRelease(currentVersion);
-  }
+    if (mostRecentTag === `${tagPrefixes[0]}${currentVersion ?? ''}`) {
+      throw new Error(
+        `Current version already has tag, which is unexpected for a release candidate.`,
+      );
+    }
 
-  const newChangeEntries = newCommits.map(({ prNumber, description }) => {
-    if (prNumber) {
-      const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`;
-      return `${description} ${suffix}`;
+    // Ensure release header exists, if necessary
+    if (
+      !changelog
+        .getReleases()
+        .find((release) => release.version === currentVersion)
+    ) {
+      changelog.addRelease({ version: currentVersion });
     }
-    return description;
-  });
 
-  for (const description of newChangeEntries.reverse()) {
-    changelog.addChange({
-      version: isReleaseCandidate ? currentVersion : undefined,
-      category: ChangeCategory.Uncategorized,
-      description,
+    if (hasUnreleasedChanges) {
+      changelog.migrateUnreleasedChangesToRelease(currentVersion);
+    }
+
+    const newChangeEntries = newCommits.map(({ prNumber, description }) => {
+      if (prNumber) {
+        const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`;
+        return `${description} ${suffix}`;
+      }
+      return description;
     });
+
+    for (const description of newChangeEntries.reverse()) {
+      changelog.addChange({
+        version: isReleaseCandidate ? currentVersion : undefined,
+        category: ChangeCategory.Uncategorized,
+        description,
+      });
+    }
   }
 
   return changelog.toString();

From 44cc00de2b8826437b38d862c96ba67ccfee7272 Mon Sep 17 00:00:00 2001
From: Jongsun Suh <34228073+MajorLift@users.noreply.github.com>
Date: Mon, 16 Oct 2023 11:42:24 -0700
Subject: [PATCH 5/7] Install `@metamask/utils` as a devDep

---
 package.json |   1 +
 yarn.lock    | 170 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 171 insertions(+)

diff --git a/package.json b/package.json
index fb3c826..4ba6d7d 100644
--- a/package.json
+++ b/package.json
@@ -43,6 +43,7 @@
     "@metamask/eslint-config-jest": "^11.1.0",
     "@metamask/eslint-config-nodejs": "^11.1.0",
     "@metamask/eslint-config-typescript": "^11.1.0",
+    "@metamask/utils": "^8.1.0",
     "@types/cross-spawn": "^6.0.2",
     "@types/diff": "^5.0.0",
     "@types/jest": "^26.0.23",
diff --git a/yarn.lock b/yarn.lock
index b3db60f..3bbca48 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -437,6 +437,48 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@ethereumjs/common@npm:^3.2.0":
+  version: 3.2.0
+  resolution: "@ethereumjs/common@npm:3.2.0"
+  dependencies:
+    "@ethereumjs/util": ^8.1.0
+    crc-32: ^1.2.0
+  checksum: cb9cc11f5c868cb577ba611cebf55046e509218bbb89b47ccce010776dafe8256d70f8f43fab238aec74cf71f62601cd5842bc03a83261200802de365732a14b
+  languageName: node
+  linkType: hard
+
+"@ethereumjs/rlp@npm:^4.0.1":
+  version: 4.0.1
+  resolution: "@ethereumjs/rlp@npm:4.0.1"
+  bin:
+    rlp: bin/rlp
+  checksum: 30db19c78faa2b6ff27275ab767646929207bb207f903f09eb3e4c273ce2738b45f3c82169ddacd67468b4f063d8d96035f2bf36f02b6b7e4d928eefe2e3ecbc
+  languageName: node
+  linkType: hard
+
+"@ethereumjs/tx@npm:^4.1.2":
+  version: 4.2.0
+  resolution: "@ethereumjs/tx@npm:4.2.0"
+  dependencies:
+    "@ethereumjs/common": ^3.2.0
+    "@ethereumjs/rlp": ^4.0.1
+    "@ethereumjs/util": ^8.1.0
+    ethereum-cryptography: ^2.0.0
+  checksum: 87a3f5f2452cfbf6712f8847525a80c213210ed453c211c793c5df801fe35ecef28bae17fadd222fcbdd94277478a47e52d2b916a90a6b30cda21f1e0cdaee42
+  languageName: node
+  linkType: hard
+
+"@ethereumjs/util@npm:^8.1.0":
+  version: 8.1.0
+  resolution: "@ethereumjs/util@npm:8.1.0"
+  dependencies:
+    "@ethereumjs/rlp": ^4.0.1
+    ethereum-cryptography: ^2.0.0
+    micro-ftch: ^0.3.1
+  checksum: 9ae5dee8f12b0faf81cd83f06a41560e79b0ba96a48262771d897a510ecae605eb6d84f687da001ab8ccffd50f612ae50f988ef76e6312c752897f462f3ac08d
+  languageName: node
+  linkType: hard
+
 "@gar/promisify@npm:^1.1.3":
   version: 1.1.3
   resolution: "@gar/promisify@npm:1.1.3"
@@ -717,6 +759,7 @@ __metadata:
     "@metamask/eslint-config-jest": ^11.1.0
     "@metamask/eslint-config-nodejs": ^11.1.0
     "@metamask/eslint-config-typescript": ^11.1.0
+    "@metamask/utils": ^8.1.0
     "@types/cross-spawn": ^6.0.2
     "@types/diff": ^5.0.0
     "@types/jest": ^26.0.23
@@ -795,6 +838,43 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@metamask/utils@npm:^8.1.0":
+  version: 8.1.0
+  resolution: "@metamask/utils@npm:8.1.0"
+  dependencies:
+    "@ethereumjs/tx": ^4.1.2
+    "@noble/hashes": ^1.3.1
+    "@types/debug": ^4.1.7
+    debug: ^4.3.4
+    semver: ^7.5.4
+    superstruct: ^1.0.3
+  checksum: 4cbee36d0c227f3e528930e83f75a0c6b71b55b332c3e162f0e87f3dd86ae017d0b20405d76ea054ab99e4d924d3d9b8b896ed12a12aae57b090350e5a625999
+  languageName: node
+  linkType: hard
+
+"@noble/curves@npm:1.1.0, @noble/curves@npm:~1.1.0":
+  version: 1.1.0
+  resolution: "@noble/curves@npm:1.1.0"
+  dependencies:
+    "@noble/hashes": 1.3.1
+  checksum: 2658cdd3f84f71079b4e3516c47559d22cf4b55c23ac8ee9d2b1f8e5b72916d9689e59820e0f9d9cb4a46a8423af5b56dc6bb7782405c88be06a015180508db5
+  languageName: node
+  linkType: hard
+
+"@noble/hashes@npm:1.3.1":
+  version: 1.3.1
+  resolution: "@noble/hashes@npm:1.3.1"
+  checksum: 7fdefc0f7a0c1ec27acc6ff88841793e3f93ec4ce6b8a6a12bfc0dd70ae6b7c4c82fe305fdfeda1735d5ad4a9eebe761e6693b3d355689c559e91242f4bc95b1
+  languageName: node
+  linkType: hard
+
+"@noble/hashes@npm:^1.3.1, @noble/hashes@npm:~1.3.0, @noble/hashes@npm:~1.3.1":
+  version: 1.3.2
+  resolution: "@noble/hashes@npm:1.3.2"
+  checksum: fe23536b436539d13f90e4b9be843cc63b1b17666a07634a2b1259dded6f490be3d050249e6af98076ea8f2ea0d56f578773c2197f2aa0eeaa5fba5bc18ba474
+  languageName: node
+  linkType: hard
+
 "@nodelib/fs.scandir@npm:2.1.5":
   version: 2.1.5
   resolution: "@nodelib/fs.scandir@npm:2.1.5"
@@ -871,6 +951,34 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@scure/base@npm:~1.1.0":
+  version: 1.1.3
+  resolution: "@scure/base@npm:1.1.3"
+  checksum: 1606ab8a4db898cb3a1ada16c15437c3bce4e25854fadc8eb03ae93cbbbac1ed90655af4b0be3da37e12056fef11c0374499f69b9e658c9e5b7b3e06353c630c
+  languageName: node
+  linkType: hard
+
+"@scure/bip32@npm:1.3.1":
+  version: 1.3.1
+  resolution: "@scure/bip32@npm:1.3.1"
+  dependencies:
+    "@noble/curves": ~1.1.0
+    "@noble/hashes": ~1.3.1
+    "@scure/base": ~1.1.0
+  checksum: 394d65f77a40651eba21a5096da0f4233c3b50d422864751d373fcf142eeedb94a1149f9ab1dbb078086dab2d0bc27e2b1afec8321bf22d4403c7df2fea5bfe2
+  languageName: node
+  linkType: hard
+
+"@scure/bip39@npm:1.2.1":
+  version: 1.2.1
+  resolution: "@scure/bip39@npm:1.2.1"
+  dependencies:
+    "@noble/hashes": ~1.3.0
+    "@scure/base": ~1.1.0
+  checksum: c5bd6f1328fdbeae2dcdd891825b1610225310e5e62a4942714db51066866e4f7bef242c7b06a1b9dcc8043a4a13412cf5c5df76d3b10aa9e36b82e9b6e3eeaa
+  languageName: node
+  linkType: hard
+
 "@sinonjs/commons@npm:^1.7.0":
   version: 1.8.1
   resolution: "@sinonjs/commons@npm:1.8.1"
@@ -953,6 +1061,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@types/debug@npm:^4.1.7":
+  version: 4.1.9
+  resolution: "@types/debug@npm:4.1.9"
+  dependencies:
+    "@types/ms": "*"
+  checksum: e88ee8b19d106f33eb0d3bc58bacff9702e98d821fd1ebd1de8942e6b97419e19a1ccf39370f1764a1dc66f79fd4619f3412e1be6eeb9f0b76412f5ffe4ead93
+  languageName: node
+  linkType: hard
+
 "@types/diff@npm:^5.0.0":
   version: 5.0.0
   resolution: "@types/diff@npm:5.0.0"
@@ -1018,6 +1135,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@types/ms@npm:*":
+  version: 0.7.32
+  resolution: "@types/ms@npm:0.7.32"
+  checksum: 610744605c5924aa2657c8a62d307052af4f0e38e2aa015f154ef03391fabb4fd903f9c9baacb41f6e5798b8697e898463c351e5faf638738603ed29137b5254
+  languageName: node
+  linkType: hard
+
 "@types/node@npm:*":
   version: 20.1.1
   resolution: "@types/node@npm:20.1.1"
@@ -2022,6 +2146,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"crc-32@npm:^1.2.0":
+  version: 1.2.2
+  resolution: "crc-32@npm:1.2.2"
+  bin:
+    crc32: bin/crc32.njs
+  checksum: ad2d0ad0cbd465b75dcaeeff0600f8195b686816ab5f3ba4c6e052a07f728c3e70df2e3ca9fd3d4484dc4ba70586e161ca5a2334ec8bf5a41bf022a6103ff243
+  languageName: node
+  linkType: hard
+
 "cross-spawn@npm:^6.0.0":
   version: 6.0.5
   resolution: "cross-spawn@npm:6.0.5"
@@ -2720,6 +2853,18 @@ __metadata:
   languageName: node
   linkType: hard
 
+"ethereum-cryptography@npm:^2.0.0":
+  version: 2.1.2
+  resolution: "ethereum-cryptography@npm:2.1.2"
+  dependencies:
+    "@noble/curves": 1.1.0
+    "@noble/hashes": 1.3.1
+    "@scure/bip32": 1.3.1
+    "@scure/bip39": 1.2.1
+  checksum: 2e8f7b8cc90232ae838ab6a8167708e8362621404d26e79b5d9e762c7b53d699f7520aff358d9254de658fcd54d2d0af168ff909943259ed27dc4cef2736410c
+  languageName: node
+  linkType: hard
+
 "exec-sh@npm:^0.3.2":
   version: 0.3.4
   resolution: "exec-sh@npm:0.3.4"
@@ -4838,6 +4983,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"micro-ftch@npm:^0.3.1":
+  version: 0.3.1
+  resolution: "micro-ftch@npm:0.3.1"
+  checksum: 0e496547253a36e98a83fb00c628c53c3fb540fa5aaeaf718438873785afd193244988c09d219bb1802984ff227d04938d9571ef90fe82b48bd282262586aaff
+  languageName: node
+  linkType: hard
+
 "micromatch@npm:^3.1.4":
   version: 3.1.10
   resolution: "micromatch@npm:3.1.10"
@@ -6069,6 +6221,17 @@ __metadata:
   languageName: node
   linkType: hard
 
+"semver@npm:^7.5.4":
+  version: 7.5.4
+  resolution: "semver@npm:7.5.4"
+  dependencies:
+    lru-cache: ^6.0.0
+  bin:
+    semver: bin/semver.js
+  checksum: 12d8ad952fa353b0995bf180cdac205a4068b759a140e5d3c608317098b3575ac2f1e09182206bf2eb26120e1c0ed8fb92c48c592f6099680de56bb071423ca3
+  languageName: node
+  linkType: hard
+
 "set-blocking@npm:^2.0.0, set-blocking@npm:~2.0.0":
   version: 2.0.0
   resolution: "set-blocking@npm:2.0.0"
@@ -6505,6 +6668,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"superstruct@npm:^1.0.3":
+  version: 1.0.3
+  resolution: "superstruct@npm:1.0.3"
+  checksum: 761790bb111e6e21ddd608299c252f3be35df543263a7ebbc004e840d01fcf8046794c274bcb351bdf3eae4600f79d317d085cdbb19ca05803a4361840cc9bb1
+  languageName: node
+  linkType: hard
+
 "supports-color@npm:^5.3.0":
   version: 5.5.0
   resolution: "supports-color@npm:5.5.0"

From d7889642faae971753df35e157a2f261c18163c8 Mon Sep 17 00:00:00 2001
From: Jongsun Suh <34228073+MajorLift@users.noreply.github.com>
Date: Mon, 16 Oct 2023 11:43:06 -0700
Subject: [PATCH 6/7] Replace `Object.keys` with `getKnownPropertyNames` method

- removes need for type casting while iterating over object keys
---
 src/changelog.ts | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/changelog.ts b/src/changelog.ts
index 8db5fef..2f61d62 100644
--- a/src/changelog.ts
+++ b/src/changelog.ts
@@ -1,3 +1,4 @@
+import { getKnownPropertyNames } from '@metamask/utils';
 import semver from 'semver';
 
 import {
@@ -383,9 +384,7 @@ export default class Changelog {
 
     const unreleasedChanges = this.#changes[unreleased];
 
-    for (const category of Object.keys(
-      unreleasedChanges,
-    ) as (keyof typeof unreleasedChanges)[]) {
+    for (const category of getKnownPropertyNames(unreleasedChanges)) {
       if (releaseChanges[category]) {
         releaseChanges[category] = [
           ...(unreleasedChanges[category] ?? []),

From a26b0bbf0a6222b4bf2e1dbb7dbd74a725679bd0 Mon Sep 17 00:00:00 2001
From: Jongsun Suh <34228073+MajorLift@users.noreply.github.com>
Date: Mon, 16 Oct 2023 17:10:42 -0700
Subject: [PATCH 7/7] Update src/update-changelog.ts

---
 src/update-changelog.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/update-changelog.ts b/src/update-changelog.ts
index 14d0658..06f1d31 100644
--- a/src/update-changelog.ts
+++ b/src/update-changelog.ts
@@ -242,7 +242,7 @@ export async function updateChangelog({
 
     if (mostRecentTag === `${tagPrefixes[0]}${currentVersion ?? ''}`) {
       throw new Error(
-        `Current version already has tag, which is unexpected for a release candidate.`,
+        `Current version already has a tag ('${mostRecentTag}'), which is unexpected for a release candidate.`,
       );
     }