Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove as assertions, or fix to mitigate the risk of silent failure #158

Merged
merged 8 commits into from
Oct 17, 2023
Merged

Remove as assertions, or fix to mitigate the risk of silent failure #158

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
53cf673
Replace `as T[]` assertions with `?? []`
MajorLift Oct 13, 2023
f950f3e
Replace `as` assertions with redundant type guards
MajorLift Oct 13, 2023
8da9f21
Make `as` assertion rely on inference instead of hard-coded type casting
MajorLift Oct 13, 2023
033cd4e
Fix type narrowing for `currentVersion` by consolidating conditional …
MajorLift Oct 16, 2023
44cc00d
Install `@metamask/utils` as a devDep
MajorLift Oct 16, 2023
d788964
Replace `Object.keys` with `getKnownPropertyNames` method
MajorLift Oct 16, 2023
a26b0bb
Update src/update-changelog.ts
MajorLift Oct 17, 2023
7b7eef8
Merge branch 'main' into 231013-remove-or-fix-as-assertions
MajorLift Oct 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 5 additions & 4 deletions src/changelog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getKnownPropertyNames } from '@metamask/utils';
import semver from 'semver';

import {
Expand Down Expand Up @@ -89,7 +90,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');
Expand Down Expand Up @@ -383,11 +384,11 @@ export default class Changelog {

const unreleasedChanges = this.#changes[unreleased];

for (const category of Object.keys(unreleasedChanges) as ChangeCategory[]) {
for (const category of getKnownPropertyNames(unreleasedChanges)) {
if (releaseChanges[category]) {
releaseChanges[category] = [
...(unreleasedChanges[category] as string[]),
...(releaseChanges[category] as string[]),
...(unreleasedChanges[category] ?? []),
...(releaseChanges[category] ?? []),
];
} else {
releaseChanges[category] = unreleasedChanges[category];
Expand Down
77 changes: 36 additions & 41 deletions src/update-changelog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand All @@ -247,38 +233,47 @@ export async function updateChangelog({
return undefined;
}

// Ensure release header exists, if necessary
if (
isReleaseCandidate &&
!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 });
}
if (isReleaseCandidate) {
if (!currentVersion) {
throw new Error(
`A version must be specified if 'isReleaseCandidate' is set.`,
);
}

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 (mostRecentTag === `${tagPrefixes[0]}${currentVersion ?? ''}`) {
throw new Error(
`Current version already has tag, which is unexpected for a release candidate.`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about, adding the tag value part of the error message?

Copy link
Contributor Author

@MajorLift MajorLift Oct 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean something like this?

Suggested change
`Current version already has tag, which is unexpected for a release candidate.`,
`Current version already has a tag: '${mostRecentTag}'. This is unexpected for a release candidate.`,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kanthesha Let me know if a26b0bb looks good!

);
}

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();
Expand Down
170 changes: 170 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down