feat(api-history): --check-descriptions

bin/lint-markdown-api-history.ts

@@ -19,6 +19,7 @@ import { DocsWorkspace } from '../lib/markdown';
 // "<any char>: <match group>"
 const possibleStringRegex = /^[ \S]+?: *?(\S[ \S]+?)$/gm;
 const nonAlphaNumericDotRegex = /[^a-zA-Z0-9.]/g;
+const possibleDescriptionRegex = /^[ \S]+?description: *?(\S[ \S]+?)$/gm;
 
 interface ChangeSchema {
   'pr-url': string;
@@ -39,6 +40,8 @@ interface Options {
   breakingChangesFile: string;
   // Check if the API history block contains strings that might cause issues when parsing the YAML
   checkStrings: boolean;
+  // Check if the API history block contains descriptions that aren't surrounded by double quotation marks
+  checkDescriptions: boolean;
   // Array of glob patterns to ignore when processing files
   ignoreGlobs: string[];
   // Check if the API history block's YAML adheres to the JSON schema at this filepath
@@ -98,7 +101,14 @@ type LintingResults = {
 async function main(
   workspaceRoot: string,
   globs: string[],
-  { checkPlacement, breakingChangesFile, checkStrings, schema, ignoreGlobs = [] }: Options,
+  {
+    checkPlacement,
+    breakingChangesFile,
+    checkStrings,
+    checkDescriptions,
+    schema,
+    ignoreGlobs = [],
+  }: Options,
 ): Promise<LintingResults> {
   let documentCounter = 0;
   let historyBlockCounter = 0;
@@ -169,7 +179,7 @@ async function main(
 
       const possibleHistoryBlocks = await findPossibleApiHistoryBlocks(documentText);
 
-      for (const possibleHistoryBlock of possibleHistoryBlocks) {
+      historyBlockForLoop: for (const possibleHistoryBlock of possibleHistoryBlocks) {
         historyBlockCounter++;
 
         const {
@@ -237,6 +247,35 @@ async function main(
           }
         }
 
+        // Throw an error if a description isn't surrounded by double quotation marks
+        if (checkDescriptions) {
+          const possibleDescription = codeBlock.value.matchAll(possibleDescriptionRegex);
+
+          for (const [matchedLine, matchedGroup] of possibleDescription) {
+            const trimmedMatchedGroup = matchedGroup.trim();
+            const isMatchedGroupInsideQuotes =
+              trimmedMatchedGroup.startsWith('"') && trimmedMatchedGroup.endsWith('"');
+
+            if (!isMatchedGroupInsideQuotes) {
+              console.error(
+                'Error occurred while parsing Markdown document:\n\n' +
+                  `'${filepath}'\n\n` +
+                  'Possible description field is not surrounded by double quotes.\n\n' +
+                  'This might cause issues when parsing the YAML (might not throw an error)\n\n' +
+                  'Matched group:\n\n' +
+                  `${matchedGroup}\n\n` +
+                  'Matched line:\n\n' +
+                  `${matchedLine}\n\n` +
+                  'API history block:\n\n' +
+                  `${possibleHistoryBlock.value}\n`,
+              );
+              errorCounter++;
+              // Behold, one of the rare occasions when a labeled statement is useful.
+              continue historyBlockForLoop;
+            }
+          }
+        }
+
         if (checkPlacement) {
           if (possibleHistoryBlock.previousNode?.type !== 'heading') {
             console.error(
@@ -341,7 +380,7 @@ function parseCommandLine() {
       console.log(
         'Usage: lint-roller-markdown-api-history [--root <dir>] <globs>' +
           ' [-h|--help]' +
-          ' [--check-placement] [--breaking-changes-file <path>] [--check-strings]' +
+          ' [--check-placement] [--breaking-changes-file <path>] [--check-strings] [--check-descriptions]' +
           ' [--schema <path>]' +
           ' [--ignore <globs>] [--ignore-path <path>]',
       );
@@ -352,12 +391,13 @@ function parseCommandLine() {
   };
 
   const opts = minimist(process.argv.slice(2), {
-    boolean: ['help', 'check-placement', 'check-strings'],
+    boolean: ['help', 'check-placement', 'check-strings', 'check-descriptions'],
     string: ['root', 'ignore', 'ignore-path', 'schema', 'breaking-changes-file'],
     unknown: showUsage,
     default: {
       'check-placement': true,
       'check-strings': true,
+      'check-descriptions': true,
     },
   });
 
@@ -403,6 +443,7 @@ async function init() {
         checkPlacement: opts['check-placement'],
         breakingChangesFile: opts['breaking-changes-file'],
         checkStrings: opts['check-strings'],
+        checkDescriptions: opts['check-descriptions'],
         ignoreGlobs: opts.ignore,
         schema: opts.schema,
       },

tests/fixtures/api-history-description-invalid.md

@@ -0,0 +1,17 @@
+#### `win.setTrafficLightPosition(position)` _macOS_ _Deprecated_
+
+<!--
+```YAML history
+added:
+  - pr-url: https://github.com/electron/electron/pull/22533
+changes:
+  - pr-url: https://github.com/electron/electron/pull/26789
+    description: Made `trafficLightPosition` option work for `customButtonOnHover`.
+deprecated:
+  - pr-url: https://github.com/electron/electron/pull/37094
+    breaking-changes-header: deprecated-browserwindowsettrafficlightpositionposition
+```
+-->
+
+Set a custom position for the traffic light buttons in frameless window.
+Passing `{ x: 0, y: 0 }` will reset the position to default.

tests/fixtures/api-history-format-invalid.md

@@ -7,7 +7,7 @@ added:
   - pr-url: https://github.com/electron/electron/pull/22533
 changes:
   - pr-url: https://github.com/electron/electron/pull/26789
-    description: Made `trafficLightPosition` option work for `customButtonOnHover`.
+    description: "Made `trafficLightPosition` option work for `customButtonOnHover`."
 deprecated:
   - pr-url: https://github.com/electron/electron/pull/37094
     breaking-changes-header: deprecated-browserwindowsettrafficlightpositionposition

tests/fixtures/api-history-heading-missing.md

@@ -6,7 +6,7 @@ added:
   - pr-url: https://github.com/electron/electron/pull/22533
 changes:
   - pr-url: https://github.com/electron/electron/pull/26789
-    description: Made `trafficLightPosition` option work for `customButtonOnHover`.
+    description: "Made `trafficLightPosition` option work for `customButtonOnHover`."
 deprecated:
   - pr-url: https://github.com/electron/electron/pull/37094
     breaking-changes-header: deprecated-missing-header

tests/fixtures/api-history-placement-invalid.md

@@ -9,7 +9,7 @@ added:
   - pr-url: https://github.com/electron/electron/pull/22533
 changes:
   - pr-url: https://github.com/electron/electron/pull/26789
-    description: Made `trafficLightPosition` option work for `customButtonOnHover`.
+    description: "Made `trafficLightPosition` option work for `customButtonOnHover`."
 deprecated:
   - pr-url: https://github.com/electron/electron/pull/37094
     breaking-changes-header: deprecated-browserwindowsettrafficlightpositionposition

tests/fixtures/api-history-schema-invalid.md

@@ -6,7 +6,7 @@ added:
   - pr-url: https://github.com/electron/electron/pull/22533
 changes:
   - pr-url: https://github.com/electron/electron/pull/26789
-    description: Ma
+    description: "Ma"
 deprecated:
   - pr-url: https://github.com/electron/electron/pull/37094
     breaking-changes-header: deprecated-browserwindowsettrafficlightpositionposition

tests/fixtures/api-history-string-invalid.md

@@ -5,10 +5,10 @@
 added:
   - pr-url: https://github.com/electron/electron/pull/22533
 changes:
-  - pr-url: https://github.com/electron/electron/pull/26789
-    description: Made `trafficLightPosition` option work for `customButtonOnHover`:
+  - pr-url: https://github.com/electron/electron/pull/26789:
+    description: "Made `trafficLightPosition` option work for `customButtonOnHover`"
   - pr-url: https://github.com/electron/electron/pull/1337
-    description: "Made `trafficLightPosition` option work for `customButtonOnHover`:"
+    description: "Made `trafficLightPosition` option work for `customButtonOnHover`"
 ```
 -->
 

tests/fixtures/api-history-valid.md

@@ -6,7 +6,7 @@ added:
   - pr-url: https://github.com/electron/electron/pull/22533
 changes:
   - pr-url: https://github.com/electron/electron/pull/26789
-    description: Made `trafficLightPosition` option work for `customButtonOnHover`.
+    description: "Made `trafficLightPosition` option work for `customButtonOnHover`."
 deprecated:
   - pr-url: https://github.com/electron/electron/pull/37094
     breaking-changes-header: deprecated-browserwindowsettrafficlightpositionposition

tests/fixtures/api-history-yaml-invalid.md

@@ -5,7 +5,7 @@
 added: pr-url: https://github.com/electron/electron/pull/22533
 changes:
   - pr-url: https://github.com/electron/electron/pull/26789
-    description: Made `trafficLightPosition` option work for `customButtonOnHover`.
+    description: "Made `trafficLightPosition` option work for `customButtonOnHover`."
 deprecated:
   - pr-url: https://github.com/electron/electron/pull/37094
     breaking-changes-header: deprecated-browserwindowsettrafficlightpositionposition

tests/lint-roller-markdown-api-history.spec.ts

@@ -72,7 +72,7 @@ async function generateRandomApiDocuments(): Promise<GenerateRandomApiDocumentsR
 
     if (isError) {
       // Will cause a warning but not a yaml parse error
-      return { isError, stringWarnChar: '$' };
+      return { isError, stringWarnChar: '#' };
     } else {
       return { isError, stringWarnChar: '' };
     }
@@ -117,10 +117,10 @@ async function generateRandomApiDocuments(): Promise<GenerateRandomApiDocumentsR
         `  - pr-url: ${generateRandomPrUrl()}\n` +
         'changes:\n' +
         `  - pr-url: ${generateRandomPrUrl()}\n` +
-        `    description: ${stringWarnChar}Made \`trafficLightPosition\` work for \`customButtonOnHover\`.\n` +
+        `    description: "Made \`trafficLightPosition\` work for \`customButtonOnHover\`."\n` +
         'deprecated:\n' +
         `  - pr-url: ${generateRandomPrUrl()}\n` +
-        `    breaking-changes-header: ${generateRandomBreakingHeadingId()}\n` +
+        `    breaking-changes-header: ${generateRandomBreakingHeadingId()} ${stringWarnChar}\n` +
         '```\n' +
         '-->\n\n' +
         'Set a custom position for the traffic light buttons in frameless window.\n' +
@@ -161,6 +161,7 @@ describe('lint-roller-markdown-api-history', () => {
       BREAKING_CHANGES_FILE,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       'api-history-valid.md',
     );
 
@@ -183,6 +184,7 @@ describe('lint-roller-markdown-api-history', () => {
       API_HISTORY_SCHEMA,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       'api-history-yaml-invalid.md',
     );
 
@@ -205,6 +207,7 @@ describe('lint-roller-markdown-api-history', () => {
       API_HISTORY_SCHEMA,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       'api-history-schema-invalid.md',
     );
 
@@ -227,6 +230,7 @@ describe('lint-roller-markdown-api-history', () => {
       API_HISTORY_SCHEMA,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       'api-history-format-invalid.md',
     );
 
@@ -251,6 +255,7 @@ describe('lint-roller-markdown-api-history', () => {
       BREAKING_CHANGES_FILE,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       'api-history-heading-missing.md',
     );
 
@@ -275,6 +280,7 @@ describe('lint-roller-markdown-api-history', () => {
       BREAKING_CHANGES_FILE,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       'api-history-placement-invalid.md',
     );
 
@@ -299,6 +305,7 @@ describe('lint-roller-markdown-api-history', () => {
       BREAKING_CHANGES_FILE,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       'api-history-string-invalid.md',
     );
 
@@ -314,6 +321,31 @@ describe('lint-roller-markdown-api-history', () => {
     expect(status).toEqual(1);
   });
 
+  it('should not run clean when there are description errors', () => {
+    const { status, stdout, stderr } = runLintMarkdownApiHistory(
+      '--root',
+      FIXTURES_DIR,
+      '--schema',
+      API_HISTORY_SCHEMA,
+      '--breaking-changes-file',
+      BREAKING_CHANGES_FILE,
+      '--check-placement',
+      '--check-strings',
+      '--check-descriptions',
+      'api-history-description-invalid.md',
+    );
+
+    expect(stderr).toMatch(/Possible description field is not surrounded by double quotes./);
+
+    const [blocks, documents, errors, warnings] = stdoutRegex.exec(stdout)?.slice(1, 5) ?? [];
+
+    expect(Number(blocks)).toEqual(1);
+    expect(Number(documents)).toEqual(1);
+    expect(Number(errors)).toEqual(1);
+    expect(Number(warnings)).toEqual(0);
+    expect(status).toEqual(1);
+  });
+
   it('can ignore a glob', () => {
     const { status, stdout } = runLintMarkdownApiHistory(
       '--root',
@@ -322,6 +354,7 @@ describe('lint-roller-markdown-api-history', () => {
       API_HISTORY_SCHEMA,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       '--ignore',
       '**/api-history-yaml-invalid.md',
       '{api-history-valid,api-history-yaml-invalid}.md',
@@ -344,6 +377,7 @@ describe('lint-roller-markdown-api-history', () => {
       API_HISTORY_SCHEMA,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       '--ignore',
       '**/api-history-valid.md',
       '--ignore',
@@ -370,6 +404,7 @@ describe('lint-roller-markdown-api-history', () => {
       resolve(FIXTURES_DIR, 'ignorepaths'),
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       '{api-history-valid,api-history-yaml-invalid}.md',
     );
 
@@ -392,6 +427,7 @@ describe('lint-roller-markdown-api-history', () => {
       BREAKING_CHANGES_FILE,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       '{api-history-valid,api-history-yaml-invalid,api-history-heading-missing}.md',
     );
 
@@ -426,6 +462,7 @@ describe('lint-roller-markdown-api-history', () => {
       BREAKING_CHANGES_FILE,
       '--check-placement',
       '--check-strings',
+      '--check-descriptions',
       '*.md',
     );