chore(prlint): stricter validation around breaking changes (#14278)

Increase the scrutiny on how breaking changes are formatted.

Motivation
A previous PR - #14227 - incorrectly specified breaking changes that
standard-release failed to record in the changelog.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

tools/prlint/index.js

@@ -83,13 +83,20 @@ function hasLabel(issue, labelName) {
  * to be said note, but got misspelled as "BREAKING CHANGES:" or
  * "BREAKING CHANGES(module):"
  */
-function validateBreakingChangeFormat(body) {
+function validateBreakingChangeFormat(title, body) {
     const re = /^BREAKING.*$/m;
     const m = re.exec(body);
     if (m) {
         if (!m[0].startsWith('BREAKING CHANGE: ')) {
             throw new LinterError(`Breaking changes should be indicated by starting a line with 'BREAKING CHANGE: ', variations are not allowed. (found: '${m[0]}')`);
         }
+        if (m[0].substr('BREAKING CHANGE:'.length).trim().length === 0) {
+            throw new LinterError("The description of the first breaking change should immediately follow the 'BREAKING CHANGE: ' clause")
+        }
+        const titleRe = /^[a-z]+\([0-9a-z-_]+\)/;
+        if (!titleRe.exec(title)) {
+            throw new LinterError("The title of this PR must specify the module name that the first breaking change should be associated to");
+        }
     }
 }
 
@@ -125,7 +132,7 @@ async function validatePr(number) {
         fixContainsTest(issue, files);
     }
 
-    validateBreakingChangeFormat(issue.body);
+    validateBreakingChangeFormat(issue.title, issue.body);
 
     console.log("✅  Success")
 

tools/prlint/package.json

@@ -13,9 +13,12 @@
   "dependencies": {
     "github-api": "^3.4.0"
   },
+  "devDependencies": {
+    "jest": "^26.6.3"
+  },
   "scripts": {
     "build": "echo success",
-    "test": "echo success",
+    "test": "jest",
     "build+test": "npm run build test && npm run test"
   }
 }

tools/prlint/test/index.test.js

@@ -0,0 +1,69 @@
+const GitHub = require('github-api');
+jest.mock('github-api');
+const linter = require('../index');
+
+beforeEach(() => {
+  GitHub.mockClear();
+});
+
+describe('breaking changes format', () => {
+  test('disallow variations to "BREAKING CHANGE:"', async () => {
+    const issue = {
+      body: 'BREAKING CHANGES:',
+      labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-readme' }]
+    };
+    configureMock(issue, undefined);
+    await expect(linter.validatePr(1000)).rejects.toThrow(/'BREAKING CHANGE: ', variations are not allowed/);
+  });
+
+  test('the first breaking change should immediately follow "BREAKING CHANGE:"', async () => {
+    const issue = {
+      body: `BREAKING CHANGE: 
+             * **module:** another change`,
+      labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-readme' }]
+    };
+    configureMock(issue, undefined);
+    await expect(linter.validatePr(1000)).rejects.toThrow(/description of the first breaking change should immediately follow/);
+  });
+
+  test('invalid title', async () => {
+    const issue = {
+      title: 'chore(foo/bar): some title',
+      body: 'BREAKING CHANGE: this breaking change',
+      labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-readme' }]
+    };
+    configureMock(issue, undefined);
+    await expect(linter.validatePr(1000)).rejects.toThrow(/must specify the module name that the first breaking change/);
+  });
+
+  test('valid title', async () => {
+    const issue = {
+      title: 'chore(foo): some title',
+      body: 'BREAKING CHANGE: this breaking change',
+      labels: [{ name: 'pr-linter/exempt-test' }, { name: 'pr-linter/exempt-readme' }]
+    };
+    configureMock(issue, undefined);
+    await linter.validatePr(1000); // not throw
+  });
+});
+
+function configureMock(issue, prFiles) {
+  GitHub.mockImplementation(() => {
+    return {
+      getIssues: () => {
+        return {
+          getIssue: () => {
+            return { data: issue };
+          },
+        };
+      },
+      getRepo: () => {
+        return {
+          listPullRequestFiles: () => {
+            return { data: prFiles };
+          }
+        }
+      }
+    };
+  });
+}