From b76efbad452bfbb32026f17d4d9e4f077b893dd3 Mon Sep 17 00:00:00 2001 From: Bryan Mishkin <698306+bmish@users.noreply.github.com> Date: Fri, 25 Nov 2022 11:21:25 -0600 Subject: [PATCH] fix: correct deprecated rule replacement link with customized --path-rule-doc --- lib/generator.ts | 2 + lib/rule-doc-notices.ts | 85 +++++++---------- lib/rule-link.ts | 71 +++++++++++++- lib/rule-list.ts | 40 +++----- lib/string.ts | 24 +++++ .../rule-deprecation-test.ts.snap | 39 +++++++- test/lib/generate/rule-deprecation-test.ts | 94 +++++++++++++++---- 7 files changed, 252 insertions(+), 103 deletions(-) diff --git a/lib/generator.ts b/lib/generator.ts index 5edf8d88..9ca8ca42 100644 --- a/lib/generator.ts +++ b/lib/generator.ts @@ -200,6 +200,8 @@ export async function generate(path: string, options?: GenerateOptions) { plugin, configsToRules, pluginPrefix, + path, + pathRuleDoc, configEmojis, ignoreConfig, ruleDocNotices, diff --git a/lib/rule-doc-notices.ts b/lib/rule-doc-notices.ts index e6e6a003..86c30219 100644 --- a/lib/rule-doc-notices.ts +++ b/lib/rule-doc-notices.ts @@ -19,8 +19,8 @@ import { import { RULE_TYPE, RULE_TYPE_MESSAGES_NOTICES } from './rule-type.js'; import { RuleDocTitleFormat } from './rule-doc-title-format.js'; import { hasOptions } from './rule-options.js'; -import { replaceRulePlaceholder, goUpLevel, pathToUrl } from './rule-link.js'; -import { countOccurrencesInString } from './string.js'; +import { getLinkToRule, getUrlToRule } from './rule-link.js'; +import { toSentenceCase, removeTrailingPeriod } from './string.js'; function severityToTerminology(severity: SEVERITY_TYPE) { switch (severity) { @@ -84,6 +84,8 @@ const RULE_NOTICES: { urlConfigs?: string; replacedBy: readonly string[] | undefined; pluginPrefix: string; + pathPlugin: string; + pathRuleDoc: string; type?: RULE_TYPE; urlRuleDoc?: string; }) => string); @@ -158,21 +160,33 @@ const RULE_NOTICES: { [NOTICE_TYPE.DEPRECATED]: ({ replacedBy, pluginPrefix, + pathPlugin, + pathRuleDoc, ruleName, urlRuleDoc, }) => { - // Determine the relative path to the rule doc root so that any replacement rule links can account for this. - const nestingDepthOfCurrentRule = countOccurrencesInString(ruleName, '/'); // Nested rule names always use forward slashes. - const relativePathToRuleDocRoot = goUpLevel(nestingDepthOfCurrentRule); - + const urlCurrentPage = getUrlToRule( + ruleName, + pluginPrefix, + pathPlugin, + pathRuleDoc, + pathPlugin, + urlRuleDoc + ); + const replacementRuleList = (replacedBy ?? []).map((replacementRuleName) => + getLinkToRule( + replacementRuleName, + pluginPrefix, + pathPlugin, + pathRuleDoc, + urlCurrentPage, + true, + urlRuleDoc + ) + ); return `${EMOJI_DEPRECATED} This rule is deprecated.${ replacedBy && replacedBy.length > 0 - ? ` It was replaced by ${ruleNamesToList( - replacedBy, - pluginPrefix, - relativePathToRuleDocRoot, - urlRuleDoc - )}.` + ? ` It was replaced by ${replacementRuleList}.` : '' }`; }, @@ -208,35 +222,6 @@ const RULE_NOTICES: { [NOTICE_TYPE.REQUIRES_TYPE_CHECKING]: `${EMOJI_REQUIRES_TYPE_CHECKING} This rule requires type information.`, }; -/** - * Convert list of rule names to string list of links. - */ -function ruleNamesToList( - ruleNames: readonly string[], - pluginPrefix: string, - relativePathToRuleDocRoot: string, - urlRuleDoc?: string -) { - return ruleNames - .map((ruleName) => { - // Ignore plugin prefix if it's included in rule name. - // While we could display the prefix if we wanted, it definitely cannot be part of the link. - const ruleNameWithoutPluginPrefix = ruleName.startsWith( - `${pluginPrefix}/` - ) - ? ruleName.slice(pluginPrefix.length + 1) - : ruleName; - return `[\`${ruleNameWithoutPluginPrefix}\`](${ - urlRuleDoc - ? replaceRulePlaceholder(urlRuleDoc, ruleName) - : `${ - pathToUrl(relativePathToRuleDocRoot) + ruleNameWithoutPluginPrefix - }.md` - })`; - }) - .join(', '); -} - /** * Determine which notices should and should not be included at the top of a rule doc. */ @@ -283,6 +268,8 @@ function getRuleNoticeLines( plugin: Plugin, configsToRules: ConfigsToRules, pluginPrefix: string, + pathPlugin: string, + pathRuleDoc: string, configEmojis: ConfigEmojis, ignoreConfig: string[], ruleDocNotices: NOTICE_TYPE[], @@ -365,6 +352,8 @@ function getRuleNoticeLines( urlConfigs, replacedBy: rule.meta?.replacedBy, pluginPrefix, + pathPlugin, + pathRuleDoc, type: rule.meta?.type as RULE_TYPE, // Convert union type to enum. urlRuleDoc, }) @@ -375,16 +364,6 @@ function getRuleNoticeLines( return lines; } -function toSentenceCase(str: string) { - return str.replace(/^\w/u, function (txt) { - return txt.charAt(0).toUpperCase() + txt.slice(1).toLowerCase(); - }); -} - -function removeTrailingPeriod(str: string) { - return str.replace(/\.$/u, ''); -} - function makeRuleDocTitle( name: string, description: string | undefined, @@ -446,6 +425,8 @@ export function generateRuleHeaderLines( plugin: Plugin, configsToRules: ConfigsToRules, pluginPrefix: string, + pathPlugin: string, + pathRuleDoc: string, configEmojis: ConfigEmojis, ignoreConfig: string[], ruleDocNotices: NOTICE_TYPE[], @@ -460,6 +441,8 @@ export function generateRuleHeaderLines( plugin, configsToRules, pluginPrefix, + pathPlugin, + pathRuleDoc, configEmojis, ignoreConfig, ruleDocNotices, diff --git a/lib/rule-link.ts b/lib/rule-link.ts index 88626ab1..b8d932d0 100644 --- a/lib/rule-link.ts +++ b/lib/rule-link.ts @@ -1,4 +1,5 @@ -import { join, sep } from 'node:path'; +import { countOccurrencesInString } from './string.js'; +import { join, sep, relative } from 'node:path'; export function replaceRulePlaceholder(pathOrUrl: string, ruleName: string) { return pathOrUrl.replace(/\{name\}/gu, ruleName); @@ -9,7 +10,7 @@ export function replaceRulePlaceholder(pathOrUrl: string, ruleName: string) { * @param level * @returns the relative path to go up the given number of directories */ -export function goUpLevel(level: number): string { +function goUpLevel(level: number): string { if (level === 0) { return ''; } @@ -19,6 +20,70 @@ export function goUpLevel(level: number): string { /** * Account for how Windows paths use backslashes instead of the forward slashes that URLs require. */ -export function pathToUrl(path: string): string { +function pathToUrl(path: string): string { return path.split(sep).join('/'); } + +/** + * Get the link to a rule's documentation page. + * Will be relative to the current page. + */ +export function getUrlToRule( + ruleName: string, + pluginPrefix: string, + pathPlugin: string, + pathRuleDoc: string, + urlCurrentPage: string, + urlRuleDoc?: string +) { + const nestingDepthOfCurrentPage = countOccurrencesInString( + relative(pathPlugin, urlCurrentPage), + sep + ); + const relativePathPluginRoot = goUpLevel(nestingDepthOfCurrentPage); + + // Ignore plugin prefix if it's included in rule name. + // While we could display the prefix if we wanted, it definitely cannot be part of the link. + const ruleNameWithoutPluginPrefix = ruleName.startsWith(`${pluginPrefix}/`) + ? ruleName.slice(pluginPrefix.length + 1) + : ruleName; + + return urlRuleDoc + ? replaceRulePlaceholder(urlRuleDoc, ruleNameWithoutPluginPrefix) + : pathToUrl( + join( + relativePathPluginRoot, + replaceRulePlaceholder(pathRuleDoc, ruleNameWithoutPluginPrefix) + ) + ); +} + +/** + * Get the markdown link (title and URL) to the rule's documentation. + */ +export function getLinkToRule( + ruleName: string, + pluginPrefix: string, + pathPlugin: string, + pathRuleDoc: string, + urlCurrentPage: string, + includeBackticks: boolean, + urlRuleDoc?: string +) { + // Ignore plugin prefix if it's included in rule name. + // While we could display the prefix if we wanted, it definitely cannot be part of the link. + const ruleNameWithoutPluginPrefix = ruleName.startsWith(`${pluginPrefix}/`) + ? ruleName.slice(pluginPrefix.length + 1) + : ruleName; + const urlToRule = getUrlToRule( + ruleName, + pluginPrefix, + pathPlugin, + pathRuleDoc, + urlCurrentPage, + urlRuleDoc + ); + return `[${includeBackticks ? '`' : ''}${ruleNameWithoutPluginPrefix}${ + includeBackticks ? '`' : '' + }](${urlToRule})`; +} diff --git a/lib/rule-list.ts b/lib/rule-list.ts index 50dc55b5..3dd05891 100644 --- a/lib/rule-list.ts +++ b/lib/rule-list.ts @@ -14,10 +14,9 @@ import { getColumns, COLUMN_HEADER } from './rule-list-columns.js'; import { findSectionHeader, findFinalHeaderLevel } from './markdown.js'; import { getPluginRoot } from './package-json.js'; import { generateLegend } from './rule-list-legend.js'; -import { relative, join, sep } from 'node:path'; +import { relative } from 'node:path'; import { COLUMN_TYPE, SEVERITY_TYPE } from './types.js'; import { markdownTable } from 'markdown-table'; -import camelCase from 'camelcase'; import type { Plugin, RuleDetails, @@ -26,18 +25,8 @@ import type { } from './types.js'; import { EMOJIS_TYPE, RULE_TYPE } from './rule-type.js'; import { hasOptions } from './rule-options.js'; -import { replaceRulePlaceholder, goUpLevel, pathToUrl } from './rule-link.js'; -import { countOccurrencesInString } from './string.js'; - -// Example: theWeatherIsNice => The Weather Is Nice -function camelCaseStringToTitle(str: string) { - const text = str.replace(/([A-Z])/gu, ' $1'); - return text.charAt(0).toUpperCase() + text.slice(1); -} - -function isCamelCase(str: string) { - return camelCase(str) === str; -} +import { getLinkToRule } from './rule-link.js'; +import { camelCaseStringToTitle, isCamelCase } from './string.js'; function getPropertyFromRule( plugin: Plugin, @@ -140,22 +129,15 @@ function buildRuleRow( ? EMOJI_HAS_SUGGESTIONS : '', [COLUMN_TYPE.NAME]() { - // Determine the relative path to the plugin root from the current rule list file so that the rule link can account for this. - const nestingDepthOfCurrentRuleList = countOccurrencesInString( - relative(pathPlugin, pathRuleList), - sep - ); - const relativePathPluginRoot = goUpLevel(nestingDepthOfCurrentRuleList); - return `[${rule.name}](${ + return getLinkToRule( + rule.name, + pluginPrefix, + pathPlugin, + pathRuleDoc, + pathRuleList, + false, urlRuleDoc - ? replaceRulePlaceholder(urlRuleDoc, rule.name) - : pathToUrl( - join( - relativePathPluginRoot, - replaceRulePlaceholder(pathRuleDoc, rule.name) - ) - ) - })`; + ); }, [COLUMN_TYPE.OPTIONS]: hasOptions(rule.schema) ? EMOJI_OPTIONS : '', [COLUMN_TYPE.REQUIRES_TYPE_CHECKING]: rule.requiresTypeChecking diff --git a/lib/string.ts b/lib/string.ts index 550b0a6a..b679e04f 100644 --- a/lib/string.ts +++ b/lib/string.ts @@ -1,3 +1,27 @@ +import camelCase from 'camelcase'; + export function countOccurrencesInString(str: string, substring: string) { return str.split(substring).length - 1; } + +export function toSentenceCase(str: string) { + return str.replace(/^\w/u, function (txt) { + return txt.charAt(0).toUpperCase() + txt.slice(1).toLowerCase(); + }); +} + +export function removeTrailingPeriod(str: string) { + return str.replace(/\.$/u, ''); +} + +/** + * Example: theWeatherIsNice => The Weather Is Nice + */ +export function camelCaseStringToTitle(str: string) { + const text = str.replace(/([A-Z])/gu, ' $1'); + return text.charAt(0).toUpperCase() + text.slice(1); +} + +export function isCamelCase(str: string) { + return camelCase(str) === str; +} diff --git a/test/lib/generate/__snapshots__/rule-deprecation-test.ts.snap b/test/lib/generate/__snapshots__/rule-deprecation-test.ts.snap index 085001ef..291de183 100644 --- a/test/lib/generate/__snapshots__/rule-deprecation-test.ts.snap +++ b/test/lib/generate/__snapshots__/rule-deprecation-test.ts.snap @@ -18,7 +18,7 @@ exports[`generate (deprecated rules) several deprecated rules updates the docume exports[`generate (deprecated rules) several deprecated rules updates the documentation 2`] = ` "# Description (\`test/no-foo\`) -❌ This rule is deprecated. It was replaced by [\`no-bar\`](no-bar.md). +❌ This rule is deprecated. It was replaced by [\`no-bar\`](../../docs/rules/no-bar.md). " @@ -65,7 +65,7 @@ exports[`generate (deprecated rules) using prefix ahead of replacement rule name exports[`generate (deprecated rules) using prefix ahead of replacement rule name uses correct replacement rule link 2`] = ` "# Description (\`test/no-foo\`) -❌ This rule is deprecated. It was replaced by [\`no-bar\`](no-bar.md). +❌ This rule is deprecated. It was replaced by [\`no-bar\`](../../docs/rules/no-bar.md). " @@ -78,6 +78,37 @@ exports[`generate (deprecated rules) using prefix ahead of replacement rule name " `; +exports[`generate (deprecated rules) with --path-rule-doc has the correct links, especially replacement rule link 1`] = ` +" + +❌ Deprecated. + +| Name | Description | ❌ | +| :------------------------------------------------ | :----------- | :- | +| [category/no-bar](docs/category/no-bar/README.md) | Description. | ❌ | +| [category/no-foo](docs/category/no-foo/README.md) | Description. | ❌ | + +" +`; + +exports[`generate (deprecated rules) with --path-rule-doc has the correct links, especially replacement rule link 2`] = ` +"# Description (\`test/category/no-foo\`) + +❌ This rule is deprecated. It was replaced by [\`category/no-bar\`](../../../docs/category/no-bar/README.md). + + +" +`; + +exports[`generate (deprecated rules) with --path-rule-doc has the correct links, especially replacement rule link 3`] = ` +"# Description (\`test/category/no-bar\`) + +❌ This rule is deprecated. It was replaced by [\`category/no-foo\`](../../../docs/category/no-foo/README.md). + + +" +`; + exports[`generate (deprecated rules) with nested rule names has the correct links, especially replacement rule link 1`] = ` " @@ -94,7 +125,7 @@ exports[`generate (deprecated rules) with nested rule names has the correct link exports[`generate (deprecated rules) with nested rule names has the correct links, especially replacement rule link 2`] = ` "# Description (\`test/category/no-foo\`) -❌ This rule is deprecated. It was replaced by [\`category/no-bar\`](../category/no-bar.md). +❌ This rule is deprecated. It was replaced by [\`category/no-bar\`](../../../docs/rules/category/no-bar.md). " @@ -103,7 +134,7 @@ exports[`generate (deprecated rules) with nested rule names has the correct link exports[`generate (deprecated rules) with nested rule names has the correct links, especially replacement rule link 3`] = ` "# Description (\`test/category/no-bar\`) -❌ This rule is deprecated. It was replaced by [\`category/no-foo\`](../category/no-foo.md). +❌ This rule is deprecated. It was replaced by [\`category/no-foo\`](../../../docs/rules/category/no-foo.md). " diff --git a/test/lib/generate/rule-deprecation-test.ts b/test/lib/generate/rule-deprecation-test.ts index fe53fd5e..6a4d2ef2 100644 --- a/test/lib/generate/rule-deprecation-test.ts +++ b/test/lib/generate/rule-deprecation-test.ts @@ -148,6 +148,68 @@ describe('generate (deprecated rules)', function () { }); }); + describe('with --path-rule-doc', function () { + beforeEach(function () { + mockFs({ + 'package.json': JSON.stringify({ + name: 'eslint-plugin-test', + exports: 'index.js', + type: 'module', + }), + + 'index.js': ` + export default { + rules: { + 'category/no-foo': { + meta: { + docs: { description: 'Description.' }, + deprecated: true, + replacedBy: ['category/no-bar'], // without plugin prefix + }, + create(context) {} + }, + 'category/no-bar': { + meta: { + docs: { description: 'Description.' }, + deprecated: true, + replacedBy: ['test/category/no-foo'], // with plugin prefix + }, + create(context) {} + }, + }, + configs: {} + };`, + + 'README.md': + '', + + 'docs/category/no-foo/README.md': '', + 'docs/category/no-bar/README.md': '', + + // Needed for some of the test infrastructure to work. + node_modules: mockFs.load(PATH_NODE_MODULES), + }); + }); + + afterEach(function () { + mockFs.restore(); + jest.resetModules(); + }); + + it('has the correct links, especially replacement rule link', async function () { + await generate('.', { pathRuleDoc: 'docs/{name}/README.md' }); + + expect(readFileSync('README.md', 'utf8')).toMatchSnapshot(); + + expect( + readFileSync('docs/category/no-foo/README.md', 'utf8') + ).toMatchSnapshot(); + expect( + readFileSync('docs/category/no-bar/README.md', 'utf8') + ).toMatchSnapshot(); + }); + }); + describe('using prefix ahead of replacement rule name', function () { beforeEach(function () { mockFs({ @@ -158,23 +220,23 @@ describe('generate (deprecated rules)', function () { }), 'index.js': ` - export default { - rules: { - 'no-foo': { - meta: { - docs: { description: 'Description.' }, - deprecated: true, - replacedBy: ['test/no-bar'], - }, - create(context) {} - }, - 'no-bar': { - meta: { docs: { description: 'Description.' }, }, - create(context) {} - }, + export default { + rules: { + 'no-foo': { + meta: { + docs: { description: 'Description.' }, + deprecated: true, + replacedBy: ['test/no-bar'], }, - configs: {} - };`, + create(context) {} + }, + 'no-bar': { + meta: { docs: { description: 'Description.' }, }, + create(context) {} + }, + }, + configs: {} + };`, 'README.md': '',