Skip to content

Adding a used-types rule #1058

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

Closed
wants to merge 12 commits into from
2 changes: 2 additions & 0 deletions .README/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ Finally, enable all of the rules that you would like to use.
"jsdoc/empty-tags": 1, // Recommended
"jsdoc/implements-on-classes": 1, // Recommended
"jsdoc/informative-docs": 1,
"jsdoc/mark-used": 1,
"jsdoc/match-description": 1,
"jsdoc/multiline-blocks": 1, // Recommended
"jsdoc/no-bad-blocks": 1,
Expand Down Expand Up @@ -203,6 +204,7 @@ Problems reported by rules which have a wrench :wrench: below can be fixed autom
|: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 restart their attached name.|
|||[mark-used](./docs/rules/mark-used.md#readme)|Marks all types referenced in `{@link}`d tags as used.|
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't all of these document comments be expressed more broadly that this is for more than just {@link} tags?

Copy link
Contributor Author

@kraenhansen kraenhansen May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally - good catch. I had the link as my primary objective when writing that.

How about:

Suggested change
|||[mark-used](./docs/rules/mark-used.md#readme)|Marks all types referenced in `{@link}`d tags as used.|
|||[mark-used](./docs/rules/mark-used.md#readme)|Marks all types referenced from JSDoc tags as used.|

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. If you could also do so where the other docs mention it too.

|||[match-description](./docs/rules/match-description.md#readme)|Defines customizable regular expression rules for your tag descriptions|
||:wrench:|[match-name](./docs/rules/match-name.md#readme)|Reports the name portion of a JSDoc tag if matching or not matching a given regular expression|
|:heavy_check_mark:|:wrench:|[multiline-blocks](./docs/rules/multiline-blocks.md#readme)|Controls how and whether jsdoc blocks can be expressed as single or multiple line blocks|
Expand Down
27 changes: 27 additions & 0 deletions .README/rules/mark-used.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# `mark-used`
Copy link
Contributor Author

@kraenhansen kraenhansen May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like some feedback on the name. It doesn't convey that this has to do with "types" being marked as used. An alternative would be used-types as a tribute to the valid-types rule, which already exists in the package.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

used-types sounds good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a rename to "used-types".


{"gitdown": "contents", "rootId": "mark-used"}

Marks all types referenced in `{@link}` tags as used.
Copy link
Contributor Author

@kraenhansen kraenhansen May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Marks all types referenced in `{@link}` tags as used.
Marks all types referenced from JSDoc tags as used.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit: change "jsdoc" to "JSDoc"


## Fixer

Not applicable.

#### Options

|||
|---|---|
|Context|everywhere|
|Tags|N/A|
|Recommended|false|
|Settings||
|Options||

## Failing examples

<!-- assertions-failing markUsed -->

## Passing examples

<!-- assertions-passing markUsed -->
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Finally, enable all of the rules that you would like to use.
"jsdoc/empty-tags": 1, // Recommended
"jsdoc/implements-on-classes": 1, // Recommended
"jsdoc/informative-docs": 1,
"jsdoc/mark-used": 1,
"jsdoc/match-description": 1,
"jsdoc/multiline-blocks": 1, // Recommended
"jsdoc/no-bad-blocks": 1,
Expand Down Expand Up @@ -224,6 +225,7 @@ Problems reported by rules which have a wrench :wrench: below can be fixed autom
|: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 restart their attached name.|
|||[mark-used](./docs/rules/mark-used.md#readme)|Marks all types referenced in `{@link}`d tags as used.|
|||[match-description](./docs/rules/match-description.md#readme)|Defines customizable regular expression rules for your tag descriptions|
||:wrench:|[match-name](./docs/rules/match-name.md#readme)|Reports the name portion of a JSDoc tag if matching or not matching a given regular expression|
|:heavy_check_mark:|:wrench:|[multiline-blocks](./docs/rules/multiline-blocks.md#readme)|Controls how and whether jsdoc blocks can be expressed as single or multiple line blocks|
Expand Down
66 changes: 66 additions & 0 deletions docs/rules/mark-used.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
<a name="user-content-mark-used"></a>
<a name="mark-used"></a>
# <code>mark-used</code>

* [Fixer](#user-content-mark-used-fixer)
* [Failing examples](#user-content-mark-used-failing-examples)
* [Passing examples](#user-content-mark-used-passing-examples)


Marks all types referenced in `{@link}` tags as used.

<a name="user-content-mark-used-fixer"></a>
<a name="mark-used-fixer"></a>
## Fixer

Not applicable.

<a name="user-content-mark-used-fixer-options"></a>
<a name="mark-used-fixer-options"></a>
#### Options

|||
|---|---|
|Context|everywhere|
|Tags|N/A|
|Recommended|false|
|Settings||
|Options||

<a name="user-content-mark-used-failing-examples"></a>
<a name="mark-used-failing-examples"></a>
## Failing examples

The following patterns are considered problems:

````js

````



<a name="user-content-mark-used-passing-examples"></a>
<a name="mark-used-passing-examples"></a>
## Passing examples

The following patterns are not considered problems:

````js
class Foo {}
/** @param {Foo} */
function foo() {}
foo();

class Foo {}
/** @returns {Foo} */
function foo() {}
foo();

class Foo {}
class Bar {}
class Baz {}
class Qux {}
/** @type {(!Foo|?Bar|...Baz|Qux[]|foo=)} */
let foo = null;
````

2 changes: 2 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import checkValues from './rules/checkValues';
import emptyTags from './rules/emptyTags';
import implementsOnClasses from './rules/implementsOnClasses';
import informativeDocs from './rules/informativeDocs';
import markUsed from './rules/markUsed';
import matchDescription from './rules/matchDescription';
import matchName from './rules/matchName';
import multilineBlocks from './rules/multilineBlocks';
Expand Down Expand Up @@ -68,6 +69,7 @@ const index = {
'empty-tags': emptyTags,
'implements-on-classes': implementsOnClasses,
'informative-docs': informativeDocs,
'mark-used': markUsed,
'match-description': matchDescription,
'match-name': matchName,
'multiline-blocks': multilineBlocks,
Expand Down
59 changes: 59 additions & 0 deletions src/rules/markUsed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import iterateJsdoc from '../iterateJsdoc';

/**
* Extracts the type names from a type declaration string.
*
* @see https://jsdoc.app/tags-type.html
*/
const extractTypeNames = (type) => {
if (type.startsWith('(') && type.endsWith(')')) {
// Type union
return type.slice(1, type.length - 1).split('|').flatMap(extractTypeNames);
} else if (type.endsWith('[]')) {
// Arrays
return extractTypeNames(type.slice(0, Math.max(0, type.length - 2)));
} else if (type.startsWith('!') || type.startsWith('?')) {
// Nullable type or non-nullable type
return extractTypeNames(type.slice(1));
} else if (type.startsWith('...')) {
// Variable number of that type
return extractTypeNames(type.slice(3));
} else if (type.endsWith('=')) {
// Optional parameter
return extractTypeNames(type.slice(0, Math.max(0, type.length - 1)));
} else {
return [
type,
];
}
};

export default iterateJsdoc(({
jsdoc,
jsdocNode,
context,
}) => {
const sourceCode = context.getSourceCode();
for (const tag of jsdoc.tags) {
const typeNames = extractTypeNames(tag.type);
for (const typeName of typeNames) {
sourceCode.markVariableAsUsed(typeName, jsdocNode);
}
}
}, {
iterateAllJsdocs: true,
meta: {
docs: {
description: 'Marks all types referenced in {@link} tags as used',
url: 'https://github.com/gajus/eslint-plugin-jsdoc#eslint-plugin-jsdoc-rules-mark-used',
},
fixable: 'code',
schema: [
{
additionalProperties: false,
properties: {},
},
],
type: 'suggestion',
},
});
51 changes: 51 additions & 0 deletions test/rules/assertions/markUsed.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
export default {
invalid: [],
valid: [
// {
// code: `
// const foo = "bar";
// /** This thing uses {@link foo} for something */
// `,
// /*
// rules: {
// 'no-unused-vars': 'error',
// },
// */
// },
{
code: `
class Foo {}
/** @param {Foo} */
function foo() {}
foo();
`,
rules: {
'no-unused-vars': 'error',
},
},
{
code: `
class Foo {}
/** @returns {Foo} */
function foo() {}
foo();
`,
rules: {
'no-unused-vars': 'error',
},
},
{
code: `
class Foo {}
class Bar {}
class Baz {}
class Qux {}
/** @type {(!Foo|?Bar|...Baz|Qux[]|foo=)} */
let foo = null;
`,
rules: {
'no-unused-vars': 'error',
},
},
],
};
1 change: 1 addition & 0 deletions test/rules/ruleNames.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"empty-tags",
"implements-on-classes",
"informative-docs",
"mark-used",
"match-description",
"match-name",
"multiline-blocks",
Expand Down