Skip to content

Commit

Permalink
feat(require-template): add rule; fixes part of gajus#1120
Browse files Browse the repository at this point in the history
  • Loading branch information
brettz9 committed Jul 10, 2024
1 parent 1bb8aa5 commit 9c4f5bd
Show file tree
Hide file tree
Showing 9 changed files with 538 additions and 1 deletion.
2 changes: 2 additions & 0 deletions .README/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.|
Expand Down Expand Up @@ -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)|
Expand Down
54 changes: 54 additions & 0 deletions .README/rules/require-template.md
Original file line number Diff line number Diff line change
@@ -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> = [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

<!-- assertions-failing requireTemplate -->

## Passing examples

<!-- assertions-passing requireTemplate -->
2 changes: 1 addition & 1 deletion .ncurc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.|
Expand Down Expand Up @@ -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)|
Expand Down
147 changes: 147 additions & 0 deletions docs/rules/require-template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
<a name="user-content-require-template"></a>
<a name="require-template"></a>
# <code>require-template</code>

Checks to see that `@template` tags are present for any detected type
parameters.

Currently checks `TSTypeAliasDeclaration` such as:

```ts
export type Pairs<D, V> = [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.

<a name="user-content-require-template-options"></a>
<a name="require-template-options"></a>
## Options

<a name="user-content-require-template-options-requireseparatetemplates"></a>
<a name="require-template-options-requireseparatetemplates"></a>
### <code>requireSeparateTemplates</code>

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`|

<a name="user-content-require-template-failing-examples"></a>
<a name="require-template-failing-examples"></a>
## Failing examples

The following patterns are considered problems:

````js
/**
*
*/
type Pairs<D, V> = [D, V | undefined];
// Message: Missing @template D

/**
*
*/
export type Pairs<D, V> = [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> = [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> = [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
````



<a name="user-content-require-template-passing-examples"></a>
<a name="require-template-passing-examples"></a>
## Passing examples

The following patterns are not considered problems:

````js
/**
* @template D
* @template V
*/
export type Pairs<D, V> = [D, V | undefined];

/**
* @template D
* @template V
* @typedef {[D, V | undefined]} Pairs
*/

/**
* @template D, U, V
*/
export type Extras<D, U, V> = [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
*/
````

3 changes: 3 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
119 changes: 119 additions & 0 deletions src/rules/requireTemplate.js
Original file line number Diff line number Diff line change
@@ -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',
},
});
Loading

0 comments on commit 9c4f5bd

Please sign in to comment.