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 b983e3e
Show file tree
Hide file tree
Showing 7 changed files with 534 additions and 1 deletion.
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
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 b983e3e

Please sign in to comment.