diff --git a/bin/lint-markdown-api-history.ts b/bin/lint-markdown-api-history.ts index 5f6be88..019a609 100644 --- a/bin/lint-markdown-api-history.ts +++ b/bin/lint-markdown-api-history.ts @@ -19,6 +19,7 @@ import { DocsWorkspace } from '../lib/markdown'; // ": " 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 { 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 ] ' + ' [-h|--help]' + - ' [--check-placement] [--breaking-changes-file ] [--check-strings]' + + ' [--check-placement] [--breaking-changes-file ] [--check-strings] [--check-descriptions]' + ' [--schema ]' + ' [--ignore ] [--ignore-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, }, diff --git a/tests/fixtures/api-history-description-invalid.md b/tests/fixtures/api-history-description-invalid.md new file mode 100644 index 0000000..7b5a246 --- /dev/null +++ b/tests/fixtures/api-history-description-invalid.md @@ -0,0 +1,17 @@ +#### `win.setTrafficLightPosition(position)` _macOS_ _Deprecated_ + + + +Set a custom position for the traffic light buttons in frameless window. +Passing `{ x: 0, y: 0 }` will reset the position to default. diff --git a/tests/fixtures/api-history-format-invalid.md b/tests/fixtures/api-history-format-invalid.md index e602747..61b48de 100644 --- a/tests/fixtures/api-history-format-invalid.md +++ b/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 diff --git a/tests/fixtures/api-history-heading-missing.md b/tests/fixtures/api-history-heading-missing.md index 30fa4d1..b40ccdb 100644 --- a/tests/fixtures/api-history-heading-missing.md +++ b/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 diff --git a/tests/fixtures/api-history-placement-invalid.md b/tests/fixtures/api-history-placement-invalid.md index 8ff360b..9140d53 100644 --- a/tests/fixtures/api-history-placement-invalid.md +++ b/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 diff --git a/tests/fixtures/api-history-schema-invalid.md b/tests/fixtures/api-history-schema-invalid.md index 7ad1385..bbbd9dc 100644 --- a/tests/fixtures/api-history-schema-invalid.md +++ b/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 diff --git a/tests/fixtures/api-history-string-invalid.md b/tests/fixtures/api-history-string-invalid.md index ae8e247..6d975a7 100644 --- a/tests/fixtures/api-history-string-invalid.md +++ b/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`" ``` --> diff --git a/tests/fixtures/api-history-valid.md b/tests/fixtures/api-history-valid.md index 7b5a246..a409567 100644 --- a/tests/fixtures/api-history-valid.md +++ b/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 diff --git a/tests/fixtures/api-history-yaml-invalid.md b/tests/fixtures/api-history-yaml-invalid.md index da40ac1..39f0da1 100644 --- a/tests/fixtures/api-history-yaml-invalid.md +++ b/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 diff --git a/tests/lint-roller-markdown-api-history.spec.ts b/tests/lint-roller-markdown-api-history.spec.ts index 170e60b..8b4a6c7 100644 --- a/tests/lint-roller-markdown-api-history.spec.ts +++ b/tests/lint-roller-markdown-api-history.spec.ts @@ -72,7 +72,7 @@ async function generateRandomApiDocuments(): Promise\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', );