Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitize newlines and pipes from markdown table cells #482

Merged
merged 1 commit into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions lib/config-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -26,28 +27,25 @@ 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(
configName,
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.
);
}
Expand Down
10 changes: 6 additions & 4 deletions lib/rule-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -222,7 +224,7 @@ function generateRulesListMarkdown(
});

return markdownTable(
[
sanitizeMarkdownTable([
listHeaderRow,
...ruleNamesAndRules.map(([name, rule]) =>
buildRuleRow(
Expand All @@ -240,7 +242,7 @@ function generateRulesListMarkdown(
urlRuleDoc
)
),
],
]),
{ align: 'l' } // Left-align headers.
);
}
Expand Down
6 changes: 3 additions & 3 deletions lib/rule-options-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
);
}
Expand Down
10 changes: 10 additions & 0 deletions lib/string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, '<br/>');
Dismissed Show dismissed Hide dismissed
}

export function sanitizeMarkdownTable(
text: readonly (readonly string[])[]
): readonly (readonly string[])[] {
return text.map((row) => row.map((col) => sanitizeMarkdownTableCell(col)));
}
19 changes: 19 additions & 0 deletions test/lib/generate/__snapshots__/configs-list-test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,25 @@ exports[`generate (configs list) basic generates the documentation 1`] = `
<!-- end auto-generated configs list -->"
`;

exports[`generate (configs list) when a config description needs to be escaped in table generates the documentation 1`] = `
"## Rules
<!-- begin auto-generated rules list -->

| Name | Description |
| :----------------------------- | :------------------ |
| [no-foo](docs/rules/no-foo.md) | Description no-foo. |

<!-- end auto-generated rules list -->
## Configs
<!-- begin auto-generated configs list -->

| | Name | Description |
| :- | :------------ | :---------- |
| ✅ | \`recommended\` | Foo\\|Bar |

<!-- end auto-generated configs list -->"
`;

exports[`generate (configs list) when a config exports a description generates the documentation 1`] = `
"## Rules
<!-- begin auto-generated rules list -->
Expand Down
12 changes: 12 additions & 0 deletions test/lib/generate/__snapshots__/rule-description-test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,15 @@ exports[`generate (rule descriptions) rule with long-enough description to requi
<!-- end auto-generated rule header -->
"
`;

exports[`generate (rule descriptions) with rule description that needs to be escaped in table generates the documentation 1`] = `
"## Rules
<!-- begin auto-generated rules list -->

| Name | Description |
| :----------------------------- | :---------- |
| [no-foo](docs/rules/no-foo.md) | Foo\\|Bar |

<!-- end auto-generated rules list -->
"
`;
14 changes: 14 additions & 0 deletions test/lib/generate/__snapshots__/rule-options-list-test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,19 @@ exports[`generate (rule options list) with no options generates the documentatio



<!-- end auto-generated rule options list -->"
`;

exports[`generate (rule options list) with string that needs to be escaped in table generates the documentation 1`] = `
"# test/no-foo

<!-- end auto-generated rule header -->
## Options
<!-- begin auto-generated rule options list -->

| Name | Description | Type |
| :---- | :------------------------------ | :------------- |
| \`foo\` | test<br/> desc | String\\|number |

<!-- end auto-generated rule options list -->"
`;
45 changes: 45 additions & 0 deletions test/lib/generate/configs-list-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
<!-- begin auto-generated configs list -->
<!-- end auto-generated configs list -->`,

'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({
Expand Down
39 changes: 39 additions & 0 deletions test/lib/generate/rule-description-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});
44 changes: 44 additions & 0 deletions test/lib/generate/rule-options-list-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
<!-- begin auto-generated rule options list -->
<!-- end auto-generated rule options list -->`,

// 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();
});
});
});
Loading