Adding a `used-types` rule #1058

kraenhansen · 2023-05-02T18:48:53Z

This adds a mark-used rule which will mark types referenced by jsdoc as used.

TODO

Squash commits to a single commit before merging, as per contribution guidelines.
Implement support for inline tags, most notably {@link ...} via a PR to @es-joy/jsdoccomment adding support for parsing of inline tags, as per @brettz9's suggestiion.

src/rules/markUsed.js

docs/rules/mark-used.md

src/rules/markUsed.js

brettz9 · 2023-05-02T22:01:09Z

.README/README.md

@@ -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.|


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

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

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

kraenhansen · 2023-05-02T22:18:29Z

.README/rules/mark-used.md

+
+{"gitdown": "contents", "rootId": "mark-used"}
+
+Marks all types referenced in `{@link}` tags as used.


Suggested change

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

Marks all types referenced from JSDoc tags as used.

One nit: change "jsdoc" to "JSDoc"

kraenhansen · 2023-05-02T22:21:13Z

.README/rules/mark-used.md

@@ -0,0 +1,27 @@
+# `mark-used`


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.

used-types sounds good to me

Pushed a rename to "used-types".

kraenhansen · 2023-05-03T08:12:18Z

@brettz9 oh my. I just realised that the no-undefined-types are already marking the types from regular (non inline) tags as used:

eslint-plugin-jsdoc/src/rules/noUndefinedTypes.js

Line 214 in ec278a2

context.markVariableAsUsed(value);

I'm contemplating closing this PR and extending that rule to cover inline tags instead. What do you think?

brettz9 · 2023-05-03T13:03:08Z

@brettz9 oh my. I just realised that the no-undefined-types are already marking the types from regular (non inline) tags as used:

eslint-plugin-jsdoc/src/rules/noUndefinedTypes.js

Line 214 in ec278a2

context.markVariableAsUsed(value);

I'm contemplating closing this PR and extending that rule to cover inline tags instead. What do you think?

It became a part of issue #99 (see comment to try to avoid the markVariableAsUsed syntax there as it was buggy. Although a solution which fixed the bug would be ideal, I was thinking to separate the logic into its own rule.

Re: the source of the bugginess, see the discussion up to this comment including its reference to this ESLint issue which, to my knowledge, has not yet been fixed.

Btw, in #721 , there was some similar confusion, perhaps by myself as well, about @type not being supported, though I'm not sure why—perhaps due to the bugginess (that particular issue doesn't appear to report any unused errors currently except due to the last variable val not being used). Anyways, I thought the issue could be used to track moving the functionality to its own rule. WDYT?

kraenhansen · 2023-05-03T14:39:34Z

Although a solution which fixed the bug would be ideal, I was thinking to separate the logic into its own rule.

Okay. This makes sense. I'll continue my work here then 👍

I just read the issues you referenced and I'm starting to get the big picture.
To be perfectly honest, I'm mostly interested in fixing this for inline link tags ({@link ...}) and leave the typedef tags not yet implemented, since I'll personally be using this from TypeScript. Would you be okay with merging a PR for a new used-types rule which mark types referenced by everything except the typedef tags? I'd expect that to be a possible future enhancement, but is that a hard requirement for adding the rule?

brettz9 · 2023-05-03T15:01:33Z

I just read the issues you referenced and I'm starting to get the big picture. To be perfectly honest, I'm mostly interested in fixing this for inline link tags ({@link ...}) and leave the typedef tags not yet implemented, since I'll personally be using this from TypeScript. Would you be okay with merging a PR for a new used-types rule which mark types referenced by everything except the typedef tags? I'd expect that to be a possible future enhancement, but is that a hard requirement for adding the rule?

I think we need to either add support for @link to no-undefined-types or, if it makes sense (i.e., it doesn't introduce too big of performance problems by having to iterate everything all over again), migrate entirely to a separate rule, including @typedef support and dropping the markVariableAsUsed within the no-undefined-types rule. A separate rule (or config) should have the benefit of avoiding these unused variable errors without requiring users to get reporting of undefined types. Maybe just an option to no-undefined-types to disable the reporting (while allowing the marking of unused variables) would be ideal for performance and features.

We wouldn't want to drop support for existing users, nor duplicate functionality across rules. I'm sorry for the ambivalence about a separate rule (and any extra work it's led to for you), but I'm just not fully sure about it either way at this point. I'm open to suggestions.

kraenhansen · 2023-05-03T21:13:28Z

I was thinking to separate the logic into its own rule.

Okay - I'll continue in this PR then 👍

migrate entirely to a separate rule, including @typedef support

But, could that be a follow-up PR? It seems like a completely different approach is needed for that to work, right?
And since it doesn't scratch my itch, I don't think I'd be able to justify spending the time fixing it right now.

brettz9 · 2023-05-03T21:16:25Z

At this point, and as a result of our discussions, I'm leaning to changing the no-undefined-types rule. The typedef support is already there, so this would just be adding the link support there. I can add the options myself to disable the variable marking and/or the undefined types.

kraenhansen · 2023-05-03T21:37:26Z

I'm leaning to changing the no-undefined-types rule.

Okay - I can open another PR trying to adopt no-undefined-types to use the inline tags from es-joy/jsdoccomment#12?

Somewhat related I'm seeing test failures in the @es-joy/jsdoc-eslint-parser tests, when I pnpm link it with @es-joy/jsdoccomment from my branch. I think it has to do with the latter having changes that the parser has not been adjusted to handle - is that correct?

brettz9 · 2023-05-03T21:41:51Z

I'm leaning to changing the no-undefined-types rule.

Okay - I can open another PR trying to adopt no-undefined-types to use the inline tags from es-joy/jsdoccomment#12?

Ok, and that is looking great, by the way. Quite a speed!

Somewhat related I'm seeing test failures in the @es-joy/jsdoc-eslint-parser tests, when I pnpm link it with @es-joy/jsdoccomment from my branch. I think it has to do with the latter having changes that the parser has not been adjusted to handle - is that correct?

Yes, that may well be.

brettz9 · 2023-05-10T11:33:01Z

I'll go ahead and close as I've decided to keep things in the no-undefined-types rule now that we have config which can selectively disable the desired portions. Thanks for all your work on these PRs!

kraenhansen added 5 commits May 2, 2023 19:08

Adding scaffolding for a new 'mark-used' rule

0fa5ad9

Adding the start of a failing test

e992e3e

Updated docs

a5a5fdb

Ran "create-readme"

0206aa1

Implemented rule for non-inline tags

aa17ecd

kraenhansen mentioned this pull request May 2, 2023

Rule to mark types in JSDoc as used #858

Closed

brettz9 reviewed May 2, 2023

View reviewed changes

src/rules/markUsed.js Outdated Show resolved Hide resolved

kraenhansen added 2 commits May 2, 2023 22:42

Moved "jsdoc-type-pratt-parser" up from a dev dependency

41260d3

Using "jsdoc-type-pratt-parser"

7802a7a

kraenhansen commented May 2, 2023

View reviewed changes

docs/rules/mark-used.md Outdated Show resolved Hide resolved

kraenhansen commented May 2, 2023

View reviewed changes

src/rules/markUsed.js Outdated Show resolved Hide resolved

brettz9 reviewed May 2, 2023

View reviewed changes

src/rules/markUsed.js Outdated Show resolved Hide resolved

kraenhansen added 3 commits May 2, 2023 23:22

Added explicit case for "JsdocTypeSpecialNamePath"

45be8f4

Switched to a switch-case

8a9f2b6

Using ignoreReadme on the bloated tests

4db539e

brettz9 reviewed May 2, 2023

View reviewed changes

kraenhansen commented May 2, 2023

View reviewed changes

kraenhansen added 2 commits May 3, 2023 09:40

Renamed rule to "used-types"

aaa9ce5

Updated rule description

8eda774

kraenhansen changed the title ~~Adding a mark-used rule~~ Adding a used-types rule May 3, 2023

brettz9 mentioned this pull request May 3, 2023

no-undefined-types should check @type #721

Closed

kraenhansen mentioned this pull request May 5, 2023

feat(no-undefined-types): checking inline tags #1062

Merged

brettz9 closed this May 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding a `used-types` rule #1058

Adding a `used-types` rule #1058

kraenhansen commented May 2, 2023

brettz9 May 2, 2023

kraenhansen May 2, 2023 •

edited

Loading

brettz9 May 2, 2023

kraenhansen May 2, 2023 •

edited

Loading

brettz9 May 2, 2023

kraenhansen May 2, 2023 •

edited

Loading

brettz9 May 2, 2023

kraenhansen May 3, 2023

kraenhansen commented May 3, 2023

brettz9 commented May 3, 2023

kraenhansen commented May 3, 2023

brettz9 commented May 3, 2023

kraenhansen commented May 3, 2023

brettz9 commented May 3, 2023

kraenhansen commented May 3, 2023

brettz9 commented May 3, 2023

brettz9 commented May 10, 2023

	\|\|\|[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.\|


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

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

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

Adding a used-types rule #1058

Adding a used-types rule #1058

Conversation

kraenhansen commented May 2, 2023

TODO

brettz9 May 2, 2023

Choose a reason for hiding this comment

kraenhansen May 2, 2023 • edited Loading

Choose a reason for hiding this comment

brettz9 May 2, 2023

Choose a reason for hiding this comment

kraenhansen May 2, 2023 • edited Loading

Choose a reason for hiding this comment

brettz9 May 2, 2023

Choose a reason for hiding this comment

kraenhansen May 2, 2023 • edited Loading

Choose a reason for hiding this comment

brettz9 May 2, 2023

Choose a reason for hiding this comment

kraenhansen May 3, 2023

Choose a reason for hiding this comment

kraenhansen commented May 3, 2023

brettz9 commented May 3, 2023

kraenhansen commented May 3, 2023

brettz9 commented May 3, 2023

kraenhansen commented May 3, 2023

brettz9 commented May 3, 2023

kraenhansen commented May 3, 2023

brettz9 commented May 3, 2023

brettz9 commented May 10, 2023

Adding a `used-types` rule #1058

Adding a `used-types` rule #1058

kraenhansen May 2, 2023 •

edited

Loading

kraenhansen May 2, 2023 •

edited

Loading

kraenhansen May 2, 2023 •

edited

Loading