From 4badeb00145d56b40eb2e45d18f554aa59f11f34 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Fri, 13 Oct 2023 15:36:05 -0400 Subject: [PATCH] fix: sanitize newlines and pipes from markdown table cells --- lib/config-list.ts | 16 +++---- lib/rule-list.ts | 10 +++-- lib/rule-options-list.ts | 6 +-- lib/string.ts | 10 +++++ .../__snapshots__/configs-list-test.ts.snap | 19 ++++++++ .../rule-description-test.ts.snap | 12 +++++ .../rule-options-list-test.ts.snap | 14 ++++++ test/lib/generate/configs-list-test.ts | 45 +++++++++++++++++++ test/lib/generate/rule-description-test.ts | 39 ++++++++++++++++ test/lib/generate/rule-options-list-test.ts | 44 ++++++++++++++++++ 10 files changed, 199 insertions(+), 16 deletions(-) diff --git a/lib/config-list.ts b/lib/config-list.ts index 6a08119c..9eef5e4c 100644 --- a/lib/config-list.ts +++ b/lib/config-list.ts @@ -5,6 +5,7 @@ import { import { markdownTable } from 'markdown-table'; import type { ConfigsToRules, ConfigEmojis, Plugin } from './types.js'; import { ConfigFormat, configNameToDisplay } from './config-format.js'; +import { sanitizeMarkdownTable } from './string.js'; function generateConfigListMarkdown( plugin: Plugin, @@ -26,12 +27,14 @@ function generateConfigListMarkdown( } return markdownTable( - [ + sanitizeMarkdownTable([ listHeaderRow, ...Object.keys(configsToRules) .filter((configName) => !ignoreConfig.includes(configName)) .sort((a, b) => a.toLowerCase().localeCompare(b.toLowerCase())) .map((configName) => { + const description = // @ts-expect-error -- description is not an official config property. + plugin.configs?.[configName]?.description as string | undefined; return [ configEmojis.find((obj) => obj.config === configName)?.emoji || '', `\`${configNameToDisplay( @@ -39,15 +42,10 @@ function generateConfigListMarkdown( configFormat, pluginPrefix )}\``, - hasDescription - ? // @ts-expect-error -- description is not an official config property. - (plugin.configs?.[configName]?.description as - | string - | undefined) || '' - : undefined, - ].filter((col) => col !== undefined); + hasDescription ? description || '' : undefined, + ].filter((col) => col !== undefined) as string[]; }), - ], + ]), { align: 'l' } // Left-align headers. ); } diff --git a/lib/rule-list.ts b/lib/rule-list.ts index 848bca26..b768aaa8 100644 --- a/lib/rule-list.ts +++ b/lib/rule-list.ts @@ -32,7 +32,7 @@ import type { import { EMOJIS_TYPE } from './rule-type.js'; import { hasOptions } from './rule-options.js'; import { getLinkToRule } from './rule-link.js'; -import { capitalizeOnlyFirstLetter } from './string.js'; +import { capitalizeOnlyFirstLetter, sanitizeMarkdownTable } from './string.js'; import { noCase } from 'no-case'; import { getProperty } from 'dot-prop'; import { boolean, isBooleanable } from 'boolean'; @@ -178,7 +178,9 @@ function buildRuleRow( [COLUMN_TYPE.REQUIRES_TYPE_CHECKING]: rule.meta?.docs?.requiresTypeChecking ? EMOJI_REQUIRES_TYPE_CHECKING : '', - [COLUMN_TYPE.TYPE]: rule.meta?.type ? EMOJIS_TYPE[rule.meta?.type] : '', + [COLUMN_TYPE.TYPE]: rule.meta?.type + ? EMOJIS_TYPE[rule.meta?.type] || '' + : '', }; // List columns using the ordering and presence of columns specified in columnsEnabled. @@ -222,7 +224,7 @@ function generateRulesListMarkdown( }); return markdownTable( - [ + sanitizeMarkdownTable([ listHeaderRow, ...ruleNamesAndRules.map(([name, rule]) => buildRuleRow( @@ -240,7 +242,7 @@ function generateRulesListMarkdown( urlRuleDoc ) ), - ], + ]), { align: 'l' } // Left-align headers. ); } diff --git a/lib/rule-options-list.ts b/lib/rule-options-list.ts index 533fecef..85558bff 100644 --- a/lib/rule-options-list.ts +++ b/lib/rule-options-list.ts @@ -5,7 +5,7 @@ import { import { markdownTable } from 'markdown-table'; import type { RuleModule } from './types.js'; import { RuleOption, getAllNamedOptions } from './rule-options.js'; -import { capitalizeOnlyFirstLetter } from './string.js'; +import { capitalizeOnlyFirstLetter, sanitizeMarkdownTable } from './string.js'; export enum COLUMN_TYPE { // Alphabetical order. @@ -116,11 +116,11 @@ function generateRuleOptionsListMarkdown(rule: RuleModule): string { // Recreate object using correct ordering and presence of columns. return Object.keys(COLUMN_TYPE_DEFAULT_PRESENCE_AND_ORDERING) .filter((type) => columnsToDisplay[type as COLUMN_TYPE]) - .map((type) => ruleOptionColumnValues[type as COLUMN_TYPE]); + .map((type) => ruleOptionColumnValues[type as COLUMN_TYPE] || ''); }); return markdownTable( - [listHeaderRow, ...rows], + sanitizeMarkdownTable([listHeaderRow, ...rows]), { align: 'l' } // Left-align headers. ); } diff --git a/lib/string.ts b/lib/string.ts index ac3e4008..0fada49d 100644 --- a/lib/string.ts +++ b/lib/string.ts @@ -18,3 +18,13 @@ export function addTrailingPeriod(str: string) { export function capitalizeOnlyFirstLetter(str: string) { return str.charAt(0).toUpperCase() + str.slice(1).toLowerCase(); } + +function sanitizeMarkdownTableCell(text: string): string { + return text.replace(/\|/gu, '\\|').replace(/\n/gu, '
'); +} + +export function sanitizeMarkdownTable( + text: readonly (readonly string[])[] +): readonly (readonly string[])[] { + return text.map((row) => row.map((col) => sanitizeMarkdownTableCell(col))); +} diff --git a/test/lib/generate/__snapshots__/configs-list-test.ts.snap b/test/lib/generate/__snapshots__/configs-list-test.ts.snap index 5bb717c6..4d134540 100644 --- a/test/lib/generate/__snapshots__/configs-list-test.ts.snap +++ b/test/lib/generate/__snapshots__/configs-list-test.ts.snap @@ -19,6 +19,25 @@ exports[`generate (configs list) basic generates the documentation 1`] = ` " `; +exports[`generate (configs list) when a config description needs to be escaped in table generates the documentation 1`] = ` +"## Rules + + +| Name | Description | +| :----------------------------- | :------------------ | +| [no-foo](docs/rules/no-foo.md) | Description no-foo. | + + +## Configs + + +| | Name | Description | +| :- | :------------ | :---------- | +| ✅ | \`recommended\` | Foo\\|Bar | + +" +`; + exports[`generate (configs list) when a config exports a description generates the documentation 1`] = ` "## Rules diff --git a/test/lib/generate/__snapshots__/rule-description-test.ts.snap b/test/lib/generate/__snapshots__/rule-description-test.ts.snap index 5cdc88d1..c0244e4d 100644 --- a/test/lib/generate/__snapshots__/rule-description-test.ts.snap +++ b/test/lib/generate/__snapshots__/rule-description-test.ts.snap @@ -90,3 +90,15 @@ exports[`generate (rule descriptions) rule with long-enough description to requi " `; + +exports[`generate (rule descriptions) with rule description that needs to be escaped in table generates the documentation 1`] = ` +"## Rules + + +| Name | Description | +| :----------------------------- | :---------- | +| [no-foo](docs/rules/no-foo.md) | Foo\\|Bar | + + +" +`; diff --git a/test/lib/generate/__snapshots__/rule-options-list-test.ts.snap b/test/lib/generate/__snapshots__/rule-options-list-test.ts.snap index b242de0f..f1365273 100644 --- a/test/lib/generate/__snapshots__/rule-options-list-test.ts.snap +++ b/test/lib/generate/__snapshots__/rule-options-list-test.ts.snap @@ -34,5 +34,19 @@ exports[`generate (rule options list) with no options generates the documentatio +" +`; + +exports[`generate (rule options list) with string that needs to be escaped in table generates the documentation 1`] = ` +"# test/no-foo + + +## Options + + +| Name | Description | Type | +| :---- | :------------------------------ | :------------- | +| \`foo\` | test
desc | String\\|number | + " `; diff --git a/test/lib/generate/configs-list-test.ts b/test/lib/generate/configs-list-test.ts index bb9efc35..2813bd21 100644 --- a/test/lib/generate/configs-list-test.ts +++ b/test/lib/generate/configs-list-test.ts @@ -238,6 +238,51 @@ describe('generate (configs list)', function () { }); }); + describe('when a config description needs to be escaped in table', function () { + beforeEach(function () { + mockFs({ + 'package.json': JSON.stringify({ + name: 'eslint-plugin-test', + exports: 'index.js', + type: 'module', + }), + + 'index.js': ` + export default { + rules: { + 'no-foo': { + meta: { docs: { description: 'Description no-foo.' }, }, + create(context) {} + }, + }, + configs: { + recommended: { description: 'Foo|Bar' }, + } + };`, + + 'README.md': `## Rules +## Configs + +`, + + 'docs/rules/no-foo.md': '', + + // Needed for some of the test infrastructure to work. + node_modules: mockFs.load(PATH_NODE_MODULES), + }); + }); + + afterEach(function () { + mockFs.restore(); + jest.resetModules(); + }); + + it('generates the documentation', async function () { + await generate('.'); + expect(readFileSync('README.md', 'utf8')).toMatchSnapshot(); + }); + }); + describe('when there are no configs', function () { beforeEach(function () { mockFs({ diff --git a/test/lib/generate/rule-description-test.ts b/test/lib/generate/rule-description-test.ts index 8ad2c0e0..43d9fa69 100644 --- a/test/lib/generate/rule-description-test.ts +++ b/test/lib/generate/rule-description-test.ts @@ -208,4 +208,43 @@ describe('generate (rule descriptions)', function () { expect(readFileSync('docs/rules/no-bar.md', 'utf8')).toMatchSnapshot(); }); }); + + describe('with rule description that needs to be escaped in table', function () { + beforeEach(function () { + mockFs({ + 'package.json': JSON.stringify({ + name: 'eslint-plugin-test', + exports: 'index.js', + type: 'module', + }), + + 'index.js': ` + export default { + rules: { + 'no-foo': { + meta: { docs: { description: 'Foo|Bar'} }, + create(context) {}, + }, + }, + };`, + + 'README.md': '## Rules\n', + + 'docs/rules/no-foo.md': '', + + // Needed for some of the test infrastructure to work. + node_modules: mockFs.load(PATH_NODE_MODULES), + }); + }); + + afterEach(function () { + mockFs.restore(); + jest.resetModules(); + }); + + it('generates the documentation', async function () { + await generate('.'); + expect(readFileSync('README.md', 'utf8')).toMatchSnapshot(); + }); + }); }); diff --git a/test/lib/generate/rule-options-list-test.ts b/test/lib/generate/rule-options-list-test.ts index 3af6d07f..1412f097 100644 --- a/test/lib/generate/rule-options-list-test.ts +++ b/test/lib/generate/rule-options-list-test.ts @@ -163,4 +163,48 @@ describe('generate (rule options list)', function () { expect(readFileSync('docs/rules/no-foo.md', 'utf8')).toMatchSnapshot(); }); }); + + describe('with string that needs to be escaped in table', function () { + beforeEach(function () { + mockFs({ + 'package.json': JSON.stringify({ + name: 'eslint-plugin-test', + exports: 'index.js', + type: 'module', + }), + + 'index.js': ` + export default { + rules: { + 'no-foo': { + meta: { + schema: [{ type: "object", properties: { foo: { description: \`test + desc\`, type: 'string|number' } } }] + }, + create(context) {} + }, + }, + };`, + + 'README.md': '## Rules\n', + + 'docs/rules/no-foo.md': `## Options + +`, + + // Needed for some of the test infrastructure to work. + node_modules: mockFs.load(PATH_NODE_MODULES), + }); + }); + + afterEach(function () { + mockFs.restore(); + jest.resetModules(); + }); + + it('generates the documentation', async function () { + await generate('.'); + expect(readFileSync('docs/rules/no-foo.md', 'utf8')).toMatchSnapshot(); + }); + }); });