From 9c4f5bd3e38480d1e2748d02f902e448e7fa06d7 Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Wed, 10 Jul 2024 09:58:29 +0800 Subject: [PATCH] feat(`require-template`): add rule; fixes part of #1120 --- .README/README.md | 2 + .README/rules/require-template.md | 54 ++++++ .ncurc.cjs | 2 +- README.md | 2 + docs/rules/require-template.md | 147 ++++++++++++++++ src/index.js | 3 + src/rules/requireTemplate.js | 119 +++++++++++++ test/rules/assertions/requireTemplate.js | 209 +++++++++++++++++++++++ test/rules/ruleNames.json | 1 + 9 files changed, 538 insertions(+), 1 deletion(-) create mode 100644 .README/rules/require-template.md create mode 100644 docs/rules/require-template.md create mode 100644 src/rules/requireTemplate.js create mode 100644 test/rules/assertions/requireTemplate.js diff --git a/.README/README.md b/.README/README.md index dd634b97d..e53bc85da 100644 --- a/.README/README.md +++ b/.README/README.md @@ -236,6 +236,7 @@ non-default-recommended fixer). |:heavy_check_mark:|:wrench:|[check-tag-names](./docs/rules/check-tag-names.md#readme)|Reports invalid jsdoc (block) tag names| |:heavy_check_mark:|:wrench:|[check-types](./docs/rules/check-types.md#readme)|Reports types deemed invalid (customizable and with defaults, for preventing and/or recommending replacements)| |:heavy_check_mark:||[check-values](./docs/rules/check-values.md#readme)|Checks for expected content within some miscellaneous tags (`@version`, `@since`, `@license`, `@author`)| +| || [convert-to-jsdoc-comments](./docs/rules/convert-to-jsdoc-comments.md#readme) | Converts line and block comments preceding or following specified nodes into JSDoc comments| |:heavy_check_mark:|:wrench:|[empty-tags](./docs/rules/empty-tags.md#readme)|Checks tags that are expected to be empty (e.g., `@abstract` or `@async`), reporting if they have content| |:heavy_check_mark:||[implements-on-classes](./docs/rules/implements-on-classes.md#readme)|Prohibits use of `@implements` on non-constructor functions (to enforce the tag only being used on classes/constructors)| |||[informative-docs](./docs/rules/informative-docs.md#readme)|Reports on JSDoc texts that serve only to restate their attached name.| @@ -270,6 +271,7 @@ non-default-recommended fixer). |:heavy_check_mark:||[require-returns-check](./docs/rules/require-returns-check.md#readme)|Requires a return statement be present in a function body if a `@returns` tag is specified in the jsdoc comment block (and reports if multiple `@returns` tags are present).| |:heavy_check_mark:||[require-returns-description](./docs/rules/require-returns-description.md#readme)|Requires that the `@returns` tag has a `description` value (not including `void`/`undefined` type returns).| |:heavy_check_mark: (off in TS)||[require-returns-type](./docs/rules/require-returns-type.md#readme)|Requires that `@returns` tag has a type value (in curly brackets).| +|:heavy_check_mark:|| [require-template](./docs/rules/require-template.md#readme) | Requires `@template` tags be present when type parameters are used.| |||[require-throws](./docs/rules/require-throws.md#readme)|Requires that throw statements are documented| |:heavy_check_mark:||[require-yields](./docs/rules/require-yields.md#readme)|Requires that yields are documented| |:heavy_check_mark:||[require-yields-check](./docs/rules/require-yields-check.md#readme)|Ensures that if a `@yields` is present that a `yield` (or `yield` with a value) is present in the function body (or that if a `@next` is present that there is a `yield` with a return value present)| diff --git a/.README/rules/require-template.md b/.README/rules/require-template.md new file mode 100644 index 000000000..73bff6b8d --- /dev/null +++ b/.README/rules/require-template.md @@ -0,0 +1,54 @@ +# `require-template` + +Checks to see that `@template` tags are present for any detected type +parameters. + +Currently checks `TSTypeAliasDeclaration` such as: + +```ts +export type Pairs = [D, V | undefined]; +``` + +or + +```js +/** + * @typedef {[D, V | undefined]} Pairs + */ +``` + +Note that in the latter TypeScript-flavor JavaScript example, there is no way +for us to firmly distinguish between `D` and `V` as type parameters or as some +other identifiers, so we use an algorithm that any single capital letters +are assumed to be templates. + +## Options + +### `requireSeparateTemplates` + +Requires that each template have its own separate line, i.e., preventing +templates of this format: + +```js +/** + * @template T, U, V + */ +``` + +Defaults to `false`. + +||| +|---|---| +|Context|everywhere| +|Tags|`template`| +|Recommended|true| +|Settings|| +|Options|`requireSeparateTemplates`| + +## Failing examples + + + +## Passing examples + + diff --git a/.ncurc.cjs b/.ncurc.cjs index 503643a43..203f17551 100644 --- a/.ncurc.cjs +++ b/.ncurc.cjs @@ -2,7 +2,7 @@ module.exports = { reject: [ - // Todo: When package converted to ESM only + // Todo: When our package converted to ESM only 'escape-string-regexp', // todo[engine:node@>=20]: Can reenable diff --git a/README.md b/README.md index 5c4192ac2..64c0e3ea5 100644 --- a/README.md +++ b/README.md @@ -263,6 +263,7 @@ non-default-recommended fixer). |:heavy_check_mark:|:wrench:|[check-tag-names](./docs/rules/check-tag-names.md#readme)|Reports invalid jsdoc (block) tag names| |:heavy_check_mark:|:wrench:|[check-types](./docs/rules/check-types.md#readme)|Reports types deemed invalid (customizable and with defaults, for preventing and/or recommending replacements)| |:heavy_check_mark:||[check-values](./docs/rules/check-values.md#readme)|Checks for expected content within some miscellaneous tags (`@version`, `@since`, `@license`, `@author`)| +| || [convert-to-jsdoc-comments](./docs/rules/convert-to-jsdoc-comments.md#readme) | Converts line and block comments preceding or following specified nodes into JSDoc comments| |:heavy_check_mark:|:wrench:|[empty-tags](./docs/rules/empty-tags.md#readme)|Checks tags that are expected to be empty (e.g., `@abstract` or `@async`), reporting if they have content| |:heavy_check_mark:||[implements-on-classes](./docs/rules/implements-on-classes.md#readme)|Prohibits use of `@implements` on non-constructor functions (to enforce the tag only being used on classes/constructors)| |||[informative-docs](./docs/rules/informative-docs.md#readme)|Reports on JSDoc texts that serve only to restate their attached name.| @@ -297,6 +298,7 @@ non-default-recommended fixer). |:heavy_check_mark:||[require-returns-check](./docs/rules/require-returns-check.md#readme)|Requires a return statement be present in a function body if a `@returns` tag is specified in the jsdoc comment block (and reports if multiple `@returns` tags are present).| |:heavy_check_mark:||[require-returns-description](./docs/rules/require-returns-description.md#readme)|Requires that the `@returns` tag has a `description` value (not including `void`/`undefined` type returns).| |:heavy_check_mark: (off in TS)||[require-returns-type](./docs/rules/require-returns-type.md#readme)|Requires that `@returns` tag has a type value (in curly brackets).| +|:heavy_check_mark:|| [require-template](./docs/rules/require-template.md#readme) | Requires `@template` tags be present when type parameters are used.| |||[require-throws](./docs/rules/require-throws.md#readme)|Requires that throw statements are documented| |:heavy_check_mark:||[require-yields](./docs/rules/require-yields.md#readme)|Requires that yields are documented| |:heavy_check_mark:||[require-yields-check](./docs/rules/require-yields-check.md#readme)|Ensures that if a `@yields` is present that a `yield` (or `yield` with a value) is present in the function body (or that if a `@next` is present that there is a `yield` with a return value present)| diff --git a/docs/rules/require-template.md b/docs/rules/require-template.md new file mode 100644 index 000000000..8bef1adfb --- /dev/null +++ b/docs/rules/require-template.md @@ -0,0 +1,147 @@ + + +# require-template + +Checks to see that `@template` tags are present for any detected type +parameters. + +Currently checks `TSTypeAliasDeclaration` such as: + +```ts +export type Pairs = [D, V | undefined]; +``` + +or + +```js +/** + * @typedef {[D, V | undefined]} Pairs + */ +``` + +Note that in the latter TypeScript-flavor JavaScript example, there is no way +for us to firmly distinguish between `D` and `V` as type parameters or as some +other identifiers, so we use an algorithm that any single capital letters +are assumed to be templates. + + + +## Options + + + +### requireSeparateTemplates + +Requires that each template have its own separate line, i.e., preventing +templates of this format: + +```js +/** + * @template T, U, V + */ +``` + +Defaults to `false`. + +||| +|---|---| +|Context|everywhere| +|Tags|`template`| +|Recommended|true| +|Settings|| +|Options|`requireSeparateTemplates`| + + + +## Failing examples + +The following patterns are considered problems: + +````js +/** + * + */ +type Pairs = [D, V | undefined]; +// Message: Missing @template D + +/** + * + */ +export type Pairs = [D, V | undefined]; +// Message: Missing @template D + +/** + * @typedef {[D, V | undefined]} Pairs + */ +// Message: Missing @template D + +/** + * @typedef {[D, V | undefined]} Pairs + */ +// Settings: {"jsdoc":{"mode":"permissive"}} +// Message: Missing @template D + +/** + * @template D, U + */ +export type Extras = [D, U, V | undefined]; +// Message: Missing @template V + +/** + * @template D, U + * @typedef {[D, U, V | undefined]} Extras + */ +// Message: Missing @template V + +/** + * @template D, V + */ +export type Pairs = [D, V | undefined]; +// "jsdoc/require-template": ["error"|"warn", {"requireSeparateTemplates":true}] +// Message: Missing separate @template for V + +/** + * @template D, V + * @typedef {[D, V | undefined]} Pairs + */ +// "jsdoc/require-template": ["error"|"warn", {"requireSeparateTemplates":true}] +// Message: Missing separate @template for V +```` + + + + + +## Passing examples + +The following patterns are not considered problems: + +````js +/** + * @template D + * @template V + */ +export type Pairs = [D, V | undefined]; + +/** + * @template D + * @template V + * @typedef {[D, V | undefined]} Pairs + */ + +/** + * @template D, U, V + */ +export type Extras = [D, U, V | undefined]; + +/** + * @template D, U, V + * @typedef {[D, U, V | undefined]} Extras + */ + +/** + * @typedef {[D, U, V | undefined]} Extras + * @typedef {[D, U, V | undefined]} Extras + */ +```` + diff --git a/src/index.js b/src/index.js index 535ed0b62..8f2159a0f 100644 --- a/src/index.js +++ b/src/index.js @@ -45,6 +45,7 @@ import requireReturns from './rules/requireReturns.js'; import requireReturnsCheck from './rules/requireReturnsCheck.js'; import requireReturnsDescription from './rules/requireReturnsDescription.js'; import requireReturnsType from './rules/requireReturnsType.js'; +import requireTemplate from './rules/requireTemplate.js'; import requireThrows from './rules/requireThrows.js'; import requireYields from './rules/requireYields.js'; import requireYieldsCheck from './rules/requireYieldsCheck.js'; @@ -118,6 +119,7 @@ const index = { 'require-returns-check': requireReturnsCheck, 'require-returns-description': requireReturnsDescription, 'require-returns-type': requireReturnsType, + 'require-template': requireTemplate, 'require-throws': requireThrows, 'require-yields': requireYields, 'require-yields-check': requireYieldsCheck, @@ -191,6 +193,7 @@ const createRecommendedRuleset = (warnOrError, flatName) => { 'jsdoc/require-returns-check': warnOrError, 'jsdoc/require-returns-description': warnOrError, 'jsdoc/require-returns-type': warnOrError, + 'jsdoc/require-template': warnOrError, 'jsdoc/require-throws': 'off', 'jsdoc/require-yields': warnOrError, 'jsdoc/require-yields-check': warnOrError, diff --git a/src/rules/requireTemplate.js b/src/rules/requireTemplate.js new file mode 100644 index 000000000..015902eb0 --- /dev/null +++ b/src/rules/requireTemplate.js @@ -0,0 +1,119 @@ +import { + parse as parseType, + traverse, + tryParse as tryParseType, +} from '@es-joy/jsdoccomment'; +import iterateJsdoc from '../iterateJsdoc.js'; + +export default iterateJsdoc(({ + context, + utils, + node, + settings, + report, +}) => { + const { + requireSeparateTemplates = false, + } = context.options[0] || {}; + + const { + mode + } = settings; + + const usedNames = new Set(); + const templateTags = utils.getTags('template'); + const templateNames = templateTags.flatMap(({name}) => { + return name.split(/,\s*/); + }); + + for (const tag of templateTags) { + const {name} = tag; + const names = name.split(/,\s*/); + if (requireSeparateTemplates && names.length > 1) { + report(`Missing separate @template for ${names[1]}`, null, tag); + } + } + + /** + * @param {import('@typescript-eslint/types').TSESTree.TSTypeAliasDeclaration} aliasDeclaration + */ + const checkParameters = (aliasDeclaration) => { + /* c8 ignore next -- Guard */ + const {params} = aliasDeclaration.typeParameters ?? {params: []}; + for (const {name: {name}} of params) { + usedNames.add(name); + } + for (const usedName of usedNames) { + if (!templateNames.includes(usedName)) { + report(`Missing @template ${usedName}`); + } + } + }; + + const handleTypeAliases = () => { + const nde = /** @type {import('@typescript-eslint/types').TSESTree.Node} */ ( + node + ); + if (!nde) { + return; + } + switch (nde.type) { + case 'ExportNamedDeclaration': + if (nde.declaration?.type === 'TSTypeAliasDeclaration') { + checkParameters(nde.declaration); + } + break; + case 'TSTypeAliasDeclaration': + checkParameters(nde); + break; + } + }; + + const typedefTags = utils.getTags('typedef'); + if (!typedefTags.length || typedefTags.length >= 2) { + handleTypeAliases(); + return; + } + + const potentialType = typedefTags[0].type; + const parsedType = mode === 'permissive' ? + tryParseType(/** @type {string} */ (potentialType)) : + parseType(/** @type {string} */ (potentialType), mode) + + traverse(parsedType, (nde) => { + const { + type, + value, + } = /** @type {import('jsdoc-type-pratt-parser').NameResult} */ (nde); + if (type === 'JsdocTypeName' && (/^[A-Z]$/).test(value)) { + usedNames.add(value); + } + }); + + // Could check against whitelist/blacklist + for (const usedName of usedNames) { + if (!templateNames.includes(usedName)) { + report(`Missing @template ${usedName}`, null, typedefTags[0]); + } + } +}, { + iterateAllJsdocs: true, + meta: { + docs: { + description: 'Requires template tags for each generic type parameter', + url: 'https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/require-template.md#repos-sticky-header', + }, + schema: [ + { + additionalProperties: false, + properties: { + requireSeparateTemplates: { + type: 'boolean' + } + }, + type: 'object', + }, + ], + type: 'suggestion', + }, +}); diff --git a/test/rules/assertions/requireTemplate.js b/test/rules/assertions/requireTemplate.js new file mode 100644 index 000000000..6bb092b45 --- /dev/null +++ b/test/rules/assertions/requireTemplate.js @@ -0,0 +1,209 @@ +import {parser as typescriptEslintParser} from 'typescript-eslint'; + +export default { + invalid: [ + { + code: ` + /** + * + */ + type Pairs = [D, V | undefined]; + `, + errors: [ + { + line: 2, + message: 'Missing @template D', + }, + { + line: 2, + message: 'Missing @template V', + }, + ], + languageOptions: { + parser: typescriptEslintParser + }, + }, + { + code: ` + /** + * + */ + export type Pairs = [D, V | undefined]; + `, + errors: [ + { + line: 2, + message: 'Missing @template D', + }, + { + line: 2, + message: 'Missing @template V', + }, + ], + languageOptions: { + parser: typescriptEslintParser + }, + }, + { + code: ` + /** + * @typedef {[D, V | undefined]} Pairs + */ + `, + errors: [ + { + line: 3, + message: 'Missing @template D', + }, + { + line: 3, + message: 'Missing @template V', + }, + ], + }, + { + code: ` + /** + * @typedef {[D, V | undefined]} Pairs + */ + `, + errors: [ + { + line: 3, + message: 'Missing @template D', + }, + { + line: 3, + message: 'Missing @template V', + }, + ], + settings: { + jsdoc: { + mode: 'permissive', + }, + }, + }, + { + code: ` + /** + * @template D, U + */ + export type Extras = [D, U, V | undefined]; + `, + errors: [ + { + line: 2, + message: 'Missing @template V', + }, + ], + languageOptions: { + parser: typescriptEslintParser + }, + }, + { + code: ` + /** + * @template D, U + * @typedef {[D, U, V | undefined]} Extras + */ + `, + errors: [ + { + line: 4, + message: 'Missing @template V', + }, + ], + }, + { + code: ` + /** + * @template D, V + */ + export type Pairs = [D, V | undefined]; + `, + errors: [ + { + line: 3, + message: 'Missing separate @template for V', + }, + ], + languageOptions: { + parser: typescriptEslintParser + }, + options: [ + { + requireSeparateTemplates: true, + } + ], + }, + { + code: ` + /** + * @template D, V + * @typedef {[D, V | undefined]} Pairs + */ + `, + errors: [ + { + line: 3, + message: 'Missing separate @template for V', + }, + ], + options: [ + { + requireSeparateTemplates: true, + } + ], + }, + ], + valid: [ + { + code: ` + /** + * @template D + * @template V + */ + export type Pairs = [D, V | undefined]; + `, + languageOptions: { + parser: typescriptEslintParser + }, + }, + { + code: ` + /** + * @template D + * @template V + * @typedef {[D, V | undefined]} Pairs + */ + `, + }, + { + code: ` + /** + * @template D, U, V + */ + export type Extras = [D, U, V | undefined]; + `, + languageOptions: { + parser: typescriptEslintParser + }, + }, + { + code: ` + /** + * @template D, U, V + * @typedef {[D, U, V | undefined]} Extras + */ + `, + }, + { + code: ` + /** + * @typedef {[D, U, V | undefined]} Extras + * @typedef {[D, U, V | undefined]} Extras + */ + `, + }, + ], +}; diff --git a/test/rules/ruleNames.json b/test/rules/ruleNames.json index 15418c0d8..c158f48e4 100644 --- a/test/rules/ruleNames.json +++ b/test/rules/ruleNames.json @@ -46,6 +46,7 @@ "require-returns-check", "require-returns-description", "require-returns-type", + "require-template", "require-throws", "require-yields", "require-yields-check",