Add way to verify each change has associated PRs (#222)

This commit adds a new option to the `validate` command, `--pr-links`,
which will cause an error to be thrown if:

- a changelog entry does not have one or more links to pull requests
after it
- a changelog entry does have PR links present, but they do not point to
the project's repo
- a changelog entry does have PR links present, but they are not
positioned at the very end of the line

The `ensureValidPrLinksPresent` option has also been added to
`validateChangelog`.

If this option is provided, then `parseChangelog` is instructed to look
for and extract pull request numbers from changelog entries. The list of
numbers will then be checked for in the validation step. It is also used
to reconstruct pull request links when the changelog is stringified.

Note that because this commit changes what `parseChangelog` returns,
this is a breaking change.

Author: mcmire

README.md

@@ -90,6 +90,14 @@ or
 
 `npm run auto-changelog validate --tag-prefix-before-package-rename "polling-controller@" --version-before-package-name 0.2.3 --tag-prefix "@metamask/polling-controller@"`
 
+#### Validate that each changelog entry has one or more associated pull requests
+
+`yarn run auto-changelog validate --pr-links`
+
+or
+
+`npm run auto-changelog validate --pr-links`
+
 ## API Usage
 
 Each supported command is a separate named export.
@@ -116,7 +124,7 @@ await fs.writeFile('CHANGELOG.md', updatedChangelog);
 
 ### `validateChangelog`
 
-This command validates the changelog
+This command validates the changelog.
 
 ```javascript
 import { promises as fs } from 'fs';
@@ -132,6 +140,7 @@ try {
     repoUrl:
       'https://github.com/ExampleUsernameOrOrganization/ExampleRepository',
     isReleaseCandidate: false,
+    ensureValidPrLinksPresent: true,
   });
   // changelog is valid!
 } catch (error) {

src/changelog.test.ts

@@ -1,4 +1,9 @@
+import _outdent from 'outdent';
+
 import Changelog from './changelog';
+import { ChangeCategory } from './constants';
+
+const outdent = _outdent({ trimTrailingNewline: false });
 
 const emptyChangelog = `# Changelog
 All notable changes to this project will be documented in this file.
@@ -28,4 +33,43 @@ describe('Changelog', () => {
 
     expect(await changelog.toString()).toStrictEqual(emptyChangelog);
   });
+
+  it('should recreate pull request links for change entries based on the repo URL', async () => {
+    const changelog = new Changelog({
+      repoUrl: 'https://github.com/MetaMask/fake-repo',
+    });
+    changelog.addRelease({ version: '1.0.0' });
+    changelog.addChange({
+      version: '1.0.0',
+      category: ChangeCategory.Changed,
+      description: 'This is a cool change\n  - This is a sub-bullet',
+      prNumbers: ['100', '200'],
+    });
+    changelog.addChange({
+      version: '1.0.0',
+      category: ChangeCategory.Changed,
+      description: 'This is a very cool change\nAnd another line',
+      prNumbers: ['300'],
+    });
+
+    expect(await changelog.toString()).toStrictEqual(outdent`
+      # Changelog
+      All notable changes to this project will be documented in this file.
+
+      The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
+      and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+      ## [Unreleased]
+
+      ## [1.0.0]
+      ### Changed
+      - This is a very cool change ([#300](https://github.com/MetaMask/fake-repo/pull/300))
+      And another line
+      - This is a cool change ([#100](https://github.com/MetaMask/fake-repo/pull/100), [#200](https://github.com/MetaMask/fake-repo/pull/200))
+        - This is a sub-bullet
+
+      [Unreleased]: https://github.com/MetaMask/fake-repo/compare/v1.0.0...HEAD
+      [1.0.0]: https://github.com/MetaMask/fake-repo/releases/tag/v1.0.0
+    `);
+  });
 });

src/changelog.ts

@@ -72,10 +72,25 @@ type ReleaseMetadata = {
   status?: string;
 };
 
+/**
+ * A single change in the changelog.
+ */
+export type Change = {
+  /**
+   * The description of the change.
+   */
+  description: string;
+
+  /**
+   * Pull requests within the repo that are associated with this change.
+   */
+  prNumbers: string[];
+};
+
 /**
  * Release changes, organized by category.
  */
-type ReleaseChanges = Partial<Record<ChangeCategory, string[]>>;
+type ReleaseChanges = Partial<Record<ChangeCategory, Change[]>>;
 
 /**
  * Changelog changes, organized by release and by category.
@@ -91,15 +106,30 @@ type ChangelogChanges = Record<Version, ReleaseChanges> & {
  *
  * @param category - The title of the changelog category.
  * @param changes - The changes included in this category.
+ * @param repoUrl - The URL of the repository.
  * @returns The stringified category section.
  */
-function stringifyCategory(category: ChangeCategory, changes: string[]) {
+function stringifyCategory(
+  category: ChangeCategory,
+  changes: Change[],
+  repoUrl: string,
+) {
   const categoryHeader = `### ${category}`;
   if (changes.length === 0) {
     return categoryHeader;
   }
   const changeDescriptions = changes
-    .map((description) => `- ${description}`)
+    .map(({ description, prNumbers }) => {
+      const [firstLine, ...otherLines] = description.split('\n');
+      const stringifiedPrLinks = prNumbers
+        .map((prNumber) => `[#${prNumber}](${repoUrl}/pull/${prNumber})`)
+        .join(', ');
+      const parenthesizedPrLinks =
+        stringifiedPrLinks.length > 0 ? ` (${stringifiedPrLinks})` : '';
+      return [`- ${firstLine}${parenthesizedPrLinks}`, ...otherLines].join(
+        '\n',
+      );
+    })
     .join('\n');
   return `${categoryHeader}\n${changeDescriptions}`;
 }
@@ -109,6 +139,7 @@ function stringifyCategory(category: ChangeCategory, changes: string[]) {
  *
  * @param version - The release version.
  * @param categories - The categories of changes included in this release.
+ * @param repoUrl - The URL of the repository.
  * @param options - Additional release options.
  * @param options.date - The date of the release.
  * @param options.status - The status of the release (e.g., "DEPRECATED").
@@ -117,6 +148,7 @@ function stringifyCategory(category: ChangeCategory, changes: string[]) {
 function stringifyRelease(
   version: Version | typeof unreleased,
   categories: ReleaseChanges,
+  repoUrl: string,
   { date, status }: Partial<ReleaseMetadata> = {},
 ) {
   const releaseHeader = `## [${version}]${date ? ` - ${date}` : ''}${
@@ -126,7 +158,7 @@ function stringifyRelease(
     .filter((category) => categories[category])
     .map((category) => {
       const changes = categories[category] ?? [];
-      return stringifyCategory(category, changes);
+      return stringifyCategory(category, changes, repoUrl);
     })
     .join('\n\n');
   if (categorizedChanges === '') {
@@ -140,19 +172,22 @@ function stringifyRelease(
  *
  * @param releases - The releases to stringify.
  * @param changes - The set of changes to include, organized by release.
+ * @param repoUrl - The URL of the repository.
  * @returns The stringified set of release sections.
  */
 function stringifyReleases(
   releases: ReleaseMetadata[],
   changes: ChangelogChanges,
+  repoUrl: string,
 ) {
   const stringifiedUnreleased = stringifyRelease(
     unreleased,
     changes[unreleased],
+    repoUrl,
   );
   const stringifiedReleases = releases.map(({ version, date, status }) => {
     const categories = changes[version];
-    return stringifyRelease(version, categories, { date, status });
+    return stringifyRelease(version, categories, repoUrl, { date, status });
   });
 
   return [stringifiedUnreleased, ...stringifiedReleases].join('\n\n');
@@ -364,6 +399,7 @@ type AddChangeOptions = {
   category: ChangeCategory;
   description: string;
   version?: Version;
+  prNumbers?: string[];
 };
 
 /**
@@ -462,12 +498,15 @@ export default class Changelog {
    * @param options.description - The description of the change.
    * @param options.version - The version this change was released in. If this
    * is not given, the change is assumed to be unreleased.
+   * @param options.prNumbers - The pull request numbers associated with the
+   * change.
    */
   addChange({
     addToStart = true,
     category,
     description,
     version,
+    prNumbers = [],
   }: AddChangeOptions) {
     if (!category) {
       throw new Error('Category required');
@@ -482,18 +521,14 @@ export default class Changelog {
     const release = version
       ? this.#changes[version]
       : this.#changes[unreleased];
+    const releaseCategory = release[category] ?? [];
 
-    if (!release[category]) {
-      release[category] = [];
-    }
+    releaseCategory[addToStart ? 'unshift' : 'push']({
+      description,
+      prNumbers,
+    });
 
-    if (addToStart) {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      release[category]!.unshift(description);
-    } else {
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      release[category]!.push(description);
-    }
+    release[category] = releaseCategory;
   }
 
   /**
@@ -559,7 +594,7 @@ export default class Changelog {
       throw new Error(`Specified release version does not exist: '${version}'`);
     }
     const releaseChanges = this.getReleaseChanges(version);
-    return stringifyRelease(version, releaseChanges, release);
+    return stringifyRelease(version, releaseChanges, this.#repoUrl, release);
   }
 
   /**
@@ -590,7 +625,7 @@ export default class Changelog {
     const changelog = `${changelogTitle}
 ${changelogDescription}
 
-${stringifyReleases(this.#releases, this.#changes)}
+${stringifyReleases(this.#releases, this.#changes, this.#repoUrl)}
 
 ${stringifyLinkReferenceDefinitions(
   this.#repoUrl,

src/cli.ts

@@ -1,5 +1,3 @@
-#!/usr/bin/env node
-
 import { promises as fs, constants as fsConstants } from 'fs';
 import path from 'path';
 import semver from 'semver';
@@ -158,6 +156,11 @@ type ValidateOptions = {
    * The package rename properties, used in case of package is renamed
    */
   packageRename?: PackageRename;
+  /**
+   * Whether to validate that each changelog entry has one or more links to
+   * associated pull requests within the repository (true) or not (false).
+   */
+  ensureValidPrLinksPresent: boolean;
 };
 
 /**
@@ -172,7 +175,9 @@ type ValidateOptions = {
  * @param options.fix - Whether to attempt to fix the changelog or not.
  * @param options.formatter - A custom Markdown formatter to use.
  * @param options.packageRename - The package rename properties.
- * An optional, which is required only in case of package renamed.
+ * @param options.ensureValidPrLinksPresent - Whether to validate that each
+ * changelog entry has one or more links to associated pull requests within the
+ * repository (true) or not (false).
  */
 async function validate({
   changelogPath,
@@ -183,6 +188,7 @@ async function validate({
   fix,
   formatter,
   packageRename,
+  ensureValidPrLinksPresent,
 }: ValidateOptions) {
   const changelogContent = await readChangelog(changelogPath);
 
@@ -195,6 +201,7 @@ async function validate({
       tagPrefix,
       formatter,
       packageRename,
+      ensureValidPrLinksPresent,
     });
     return undefined;
   } catch (error) {
@@ -351,6 +358,12 @@ async function main() {
             description: `Expect the changelog to be formatted with Prettier.`,
             type: 'boolean',
           })
+          .option('prLinks', {
+            default: false,
+            description:
+              'Verify that each changelog entry has one or more links to associated pull requests within the repository',
+            type: 'boolean',
+          })
           .epilog(validateEpilog),
     )
     .command('init', 'Initialize a new empty changelog', (_yargs) => {
@@ -374,6 +387,7 @@ async function main() {
     versionBeforePackageRename,
     tagPrefixBeforePackageRename,
     autoCategorize,
+    prLinks,
   } = argv;
   let { currentVersion } = argv;
 
@@ -521,6 +535,7 @@ async function main() {
       fix,
       formatter,
       packageRename,
+      ensureValidPrLinksPresent: prLinks,
     });
   } else if (command === 'init') {
     await init({