refactor!: parse conventional commits in manifest (#1772)

* refactor: parse conventional commits in manifest

Parse conventional commits in manifest, applying plugins earlier before
passing to strategy.

* chore: add note about plugin

Author: Benjamin E. Coe

__snapshots__/base.js

@@ -8,7 +8,7 @@ exports['Strategy buildReleasePullRequest allows overriding initial version 1']
 
 ### Miscellaneous Chores
 
-* initial commit ([abc123](https://github.com/googleapis/base-test-repo/commit/abc123))
+* initial commit ([16d3754](https://github.com/googleapis/base-test-repo/commit/16d3754a2134a6d19ee19d2e5ba4dfbc))
 
 ---
 This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@@ -24,7 +24,7 @@ exports['Strategy buildReleasePullRequest allows overriding initial version in b
 
 ### Features
 
-* initial commit ([abc123](https://github.com/googleapis/base-test-repo/commit/abc123))
+* initial commit ([a90fc00](https://github.com/googleapis/base-test-repo/commit/a90fc00ca62382346da72dd8f51078a7))
 
 ---
 This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@@ -40,7 +40,7 @@ exports['Strategy buildReleasePullRequest should pass changelogHost to default b
 
 ### Bug Fixes
 
-* a bugfix ([abc5566](https://example.com/googleapis/base-test-repo/commit/abc5566))
+* a bugfix ([6c1715c](https://example.com/googleapis/base-test-repo/commit/6c1715c07438036db68bb939cf36fe6f))
 
 ---
 This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

src/manifest.ts

@@ -15,7 +15,7 @@
 import {ChangelogSection} from './changelog-notes';
 import {GitHub, GitHubRelease, GitHubTag} from './github';
 import {Version, VersionsMap} from './version';
-import {Commit} from './commit';
+import {Commit, parseConventionalCommits} from './commit';
 import {PullRequest} from './pull-request';
 import {logger as defaultLogger, Logger} from './util/logger';
 import {CommitSplit} from './util/commit-split';
@@ -677,7 +677,10 @@ export class Manifest {
       );
       this.logger.debug(`type: ${config.releaseType}`);
       this.logger.debug(`targetBranch: ${this.targetBranch}`);
-      let pathCommits = commitsPerPath[path];
+      let pathCommits = parseConventionalCommits(
+        commitsPerPath[path],
+        this.logger
+      );
       // The processCommits hook can be implemented by plugins to
       // post-process commits. This can be used to perform cleanup, e.g,, sentence
       // casing all commit messages:

src/plugin.ts

@@ -15,7 +15,7 @@
 import {GitHub} from './github';
 import {CandidateReleasePullRequest, RepositoryConfig} from './manifest';
 import {Strategy} from './strategy';
-import {Commit} from './commit';
+import {Commit, ConventionalCommit} from './commit';
 import {Release} from './release';
 import {logger as defaultLogger, Logger} from './util/logger';
 
@@ -47,9 +47,7 @@ export abstract class ManifestPlugin {
    * @param {Commit[]} commits The set of commits that will feed into release pull request.
    * @returns {Commit[]} The modified commit objects.
    */
-  // TODO: for next major version, let's run the default conventional commit parser earlier
-  // (outside of the strategy classes) and pass in the ConventionalCommit[] objects in.
-  processCommits(commits: Commit[]): Commit[] {
+  processCommits(commits: ConventionalCommit[]): ConventionalCommit[] {
     return commits;
   }
 

src/plugins/linked-versions.ts

@@ -17,7 +17,7 @@ import {RepositoryConfig, CandidateReleasePullRequest} from '../manifest';
 import {GitHub} from '../github';
 import {Logger} from '../util/logger';
 import {Strategy} from '../strategy';
-import {Commit} from '../commit';
+import {Commit, parseConventionalCommits} from '../commit';
 import {Release} from '../release';
 import {Version} from '../version';
 import {buildStrategy} from '../factory';
@@ -87,7 +87,7 @@ export class LinkedVersions extends ManifestPlugin {
       const strategy = groupStrategies[path];
       const latestRelease = releasesByPath[path];
       const releasePullRequest = await strategy.buildReleasePullRequest(
-        commitsByPath[path],
+        parseConventionalCommits(commitsByPath[path], this.logger),
         latestRelease
       );
       if (releasePullRequest?.version) {

src/plugins/sentence-case.ts

@@ -15,7 +15,7 @@
 import {ManifestPlugin} from '../plugin';
 import {GitHub} from '../github';
 import {RepositoryConfig} from '../manifest';
-import {Commit} from '../commit';
+import {ConventionalCommit} from '../commit';
 
 // A list of words that should not be converted to uppercase:
 const SPECIAL_WORDS = ['gRPC', 'npm'];
@@ -43,9 +43,13 @@ export class SentenceCase extends ManifestPlugin {
    * @param {Commit[]} commits The set of commits that will feed into release pull request.
    * @returns {Commit[]} The modified commit objects.
    */
-  processCommits(commits: Commit[]): Commit[] {
+  processCommits(commits: ConventionalCommit[]): ConventionalCommit[] {
     this.logger.info(`SentenceCase processing ${commits.length} commits`);
     for (const commit of commits) {
+      // The parsed conventional commit message, without the type:
+      console.info(commit.bareMessage);
+      commit.bareMessage = this.toUpperCase(commit.bareMessage);
+
       // Check whether commit is in conventional commit format, if it is
       // we'll split the string by type and description:
       if (commit.message.includes(':')) {

src/strategies/base.ts

@@ -25,7 +25,7 @@ import {
 import {DefaultVersioningStrategy} from '../versioning-strategies/default';
 import {DefaultChangelogNotes} from '../changelog-notes/default';
 import {Update} from '../update';
-import {ConventionalCommit, Commit, parseConventionalCommits} from '../commit';
+import {ConventionalCommit, Commit} from '../commit';
 import {Version, VersionsMap} from '../version';
 import {TagName} from '../util/tag-name';
 import {Release} from '../release';
@@ -249,14 +249,12 @@ export abstract class BaseStrategy implements Strategy {
    *   open a pull request.
    */
   async buildReleasePullRequest(
-    commits: Commit[],
+    commits: ConventionalCommit[],
     latestRelease?: Release,
     draft?: boolean,
     labels: string[] = []
   ): Promise<ReleasePullRequest | undefined> {
-    const conventionalCommits = await this.postProcessCommits(
-      parseConventionalCommits(commits, this.logger)
-    );
+    const conventionalCommits = await this.postProcessCommits(commits);
     this.logger.info(`Considering: ${conventionalCommits.length} commits`);
     if (conventionalCommits.length === 0) {
       this.logger.info(`No commits for path: ${this.path}, skipping`);

src/strategies/java.ts

@@ -17,7 +17,7 @@ import {Version} from '../version';
 import {BaseStrategy, BaseStrategyOptions, BuildUpdatesOptions} from './base';
 import {Changelog} from '../updaters/changelog';
 import {JavaSnapshot} from '../versioning-strategies/java-snapshot';
-import {Commit} from '../commit';
+import {ConventionalCommit} from '../commit';
 import {Release} from '../release';
 import {ReleasePullRequest} from '../release-pull-request';
 import {PullRequestTitle} from '../util/pull-request-title';
@@ -74,7 +74,7 @@ export class Java extends BaseStrategy {
   }
 
   async buildReleasePullRequest(
-    commits: Commit[],
+    commits: ConventionalCommit[],
     latestRelease?: Release,
     draft?: boolean,
     labels: string[] = []
@@ -156,7 +156,7 @@ export class Java extends BaseStrategy {
   }
 
   protected async needsSnapshot(
-    commits: Commit[],
+    commits: ConventionalCommit[],
     latestRelease?: Release
   ): Promise<boolean> {
     if (this.skipSnapshot) {

test/helpers.ts

@@ -20,7 +20,11 @@ import * as snapshot from 'snap-shot-it';
 import * as suggester from 'code-suggester';
 import {CreatePullRequestUserOptions} from 'code-suggester/build/src/types';
 import {Octokit} from '@octokit/rest';
-import {Commit} from '../src/commit';
+import {
+  Commit,
+  ConventionalCommit,
+  parseConventionalCommits,
+} from '../src/commit';
 import {GitHub, GitHubTag, GitHubRelease} from '../src/github';
 import {Update} from '../src/update';
 import {expect} from 'chai';
@@ -110,6 +114,23 @@ export function readPOJO(name: string): object {
   return JSON.parse(content);
 }
 
+export function buildMockConventionalCommit(
+  message: string,
+  files: string[] = []
+): ConventionalCommit[] {
+  return parseConventionalCommits([
+    {
+      // Ensure SHA is same on Windows with replace:
+      sha: crypto
+        .createHash('md5')
+        .update(message.replace(/\r\n/g, '\n'))
+        .digest('hex'),
+      message,
+      files: files,
+    },
+  ]);
+}
+
 export function buildMockCommit(message: string, files: string[] = []): Commit {
   return {
     // Ensure SHA is same on Windows with replace:
@@ -121,6 +142,7 @@ export function buildMockCommit(message: string, files: string[] = []): Commit {
     files: files,
   };
 }
+
 export function buildGitHubFileContent(
   fixturesPath: string,
   fixture: string

test/manifest.ts

@@ -2724,7 +2724,7 @@ describe('Manifest', () => {
         mockCommits(sandbox, github, [
           {
             sha: 'aaaaaa',
-            message: 'fix: some bugfix',
+            message: 'fix: some bugfix\nfix:another fix',
             files: ['path/a/foo'],
           },
           {
@@ -2854,14 +2854,13 @@ describe('Manifest', () => {
       });
 
       it('should apply plugin hook "processCommits"', async () => {
-        const mockPlugin = sandbox.createStubInstance(SentenceCase);
-        mockPlugin.run.returnsArg(0);
-        mockPlugin.preconfigure.returnsArg(0);
-        mockPlugin.processCommits.returnsArg(0);
+        const spyPlugin = sinon.spy(new SentenceCase(github, 'main', {}));
         sandbox
           .stub(pluginFactory, 'buildPlugin')
           .withArgs(sinon.match.has('type', 'sentence-case'))
-          .returns(mockPlugin);
+          // TS compiler is having issues with sinon.spy.
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          .returns(spyPlugin as any as InstanceType<typeof SentenceCase>);
         const manifest = new Manifest(
           github,
           'main',
@@ -2881,7 +2880,34 @@ describe('Manifest', () => {
         );
         const pullRequests = await manifest.buildPullRequests();
         expect(pullRequests).not.empty;
-        sinon.assert.calledOnce(mockPlugin.processCommits);
+        // This assertion verifies that conventional commit parsing
+        // was applied before calling the processCommits plugin hook:
+        sinon.assert.calledWith(spyPlugin.processCommits, [
+          {
+            sha: 'aaaaaa',
+            message: 'fix: Another fix',
+            files: ['path/a/foo'],
+            pullRequest: undefined,
+            type: 'fix',
+            scope: null,
+            bareMessage: 'Another fix',
+            notes: [],
+            references: [],
+            breaking: false,
+          },
+          {
+            sha: 'aaaaaa',
+            message: 'fix: Some bugfix',
+            files: ['path/a/foo'],
+            pullRequest: undefined,
+            type: 'fix',
+            scope: null,
+            bareMessage: 'Some bugfix',
+            notes: [],
+            references: [],
+            breaking: false,
+          },
+        ]);
       });
     });
 

test/plugins/sentence-case.ts

@@ -16,6 +16,7 @@ import {SentenceCase} from '../../src/plugins/sentence-case';
 import {expect} from 'chai';
 
 import {GitHub} from '../../src/github';
+import {buildMockConventionalCommit} from '../helpers';
 
 describe('SentenceCase Plugin', () => {
   let github: GitHub;
@@ -30,44 +31,26 @@ describe('SentenceCase Plugin', () => {
     it('converts description to sentence case', async () => {
       const plugin = new SentenceCase(github, 'main', {});
       const commits = await plugin.processCommits([
-        {
-          sha: 'abc123',
-          message: 'fix: hello world',
-        },
-        {
-          sha: 'abc123',
-          message: 'fix: Goodnight moon',
-        },
+        ...buildMockConventionalCommit('fix: hello world'),
+        ...buildMockConventionalCommit('fix: Goodnight moon'),
       ]);
       expect(commits[0].message).to.equal('fix: Hello world');
       expect(commits[1].message).to.equal('fix: Goodnight moon');
     });
     it('leaves reserved words lowercase', async () => {
       const plugin = new SentenceCase(github, 'main', {});
       const commits = await plugin.processCommits([
-        {
-          sha: 'abc123',
-          message: 'feat: gRPC can now handle proxies',
-        },
-        {
-          sha: 'abc123',
-          message: 'fix: npm now rocks',
-        },
+        ...buildMockConventionalCommit('feat: gRPC can now handle proxies'),
+        ...buildMockConventionalCommit('fix: npm now rocks'),
       ]);
       expect(commits[0].message).to.equal('feat: gRPC can now handle proxies');
       expect(commits[1].message).to.equal('fix: npm now rocks');
     });
     it('handles sentences with now breaks', async () => {
       const plugin = new SentenceCase(github, 'main', {});
       const commits = await plugin.processCommits([
-        {
-          sha: 'abc123',
-          message: 'feat: beep-boop-hello',
-        },
-        {
-          sha: 'abc123',
-          message: 'fix:log4j.foo.bar',
-        },
+        ...buildMockConventionalCommit('feat: beep-boop-hello'),
+        ...buildMockConventionalCommit('fix:log4j.foo.bar'),
       ]);
       expect(commits[0].message).to.equal('feat: Beep-boop-hello');
       expect(commits[1].message).to.equal('fix: Log4j.foo.bar');
@@ -76,39 +59,29 @@ describe('SentenceCase Plugin', () => {
   it('allows a custom list of specialWords to be provided', async () => {
     const plugin = new SentenceCase(github, 'main', {}, ['hello']);
     const commits = await plugin.processCommits([
-      {
-        sha: 'abc123',
-        message: 'fix: hello world',
-      },
-      {
-        sha: 'abc123',
-        message: 'fix: Goodnight moon',
-      },
+      ...buildMockConventionalCommit('fix: hello world'),
+      ...buildMockConventionalCommit('fix: Goodnight moon'),
     ]);
     expect(commits[0].message).to.equal('fix: hello world');
     expect(commits[1].message).to.equal('fix: Goodnight moon');
   });
   it('handles subject with multiple : characters', async () => {
     const plugin = new SentenceCase(github, 'main', {}, []);
     const commits = await plugin.processCommits([
-      {
-        sha: 'abc123',
-        message: 'fix: hello world:goodnight moon',
-      },
+      ...buildMockConventionalCommit('abc123'),
+      ...buildMockConventionalCommit('fix: hello world:goodnight moon'),
     ]);
     expect(commits[0].message).to.equal('fix: Hello world:goodnight moon');
   });
   it('handles commit with no :', async () => {
     const plugin = new SentenceCase(github, 'main', {}, []);
     const commits = await plugin.processCommits([
-      {
-        sha: 'abc123',
-        message: 'hello world goodnight moon',
-      },
+      ...buildMockConventionalCommit('hello world goodnight moon'),
     ]);
     // Ensure there's no exception, a commit without a <type> is not
     // a conventional commit, and will not show up in CHANGELOG. We
     // Do not bother sentence-casing:
-    expect(commits[0].message).to.equal('hello world goodnight moon');
+    console.info(commits);
+    expect(commits.length).to.equal(0);
   });
 });