Skip to content
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

Add documentation requirements for @template descriptions, similar to @param #1120

Closed
spaceribs opened this issue Jun 22, 2023 · 18 comments · Fixed by #1260 or abdulrahman305/cheerio#2 · May be fixed by abdulrahman305/cheerio#4
Closed

Comments

@spaceribs
Copy link

Motivation

To better support and require generic type descriptions via @template

Current behavior

No rules require @template to be defined, or require a description if it is defined.

Desired behavior

The following rules are created/added:

"jsdoc/require-template"
"jsdoc/require-template-description"
"jsdoc/require-template-name"

This rule should detect if a generic is added to a function, class, type alias or interface and perform
the same functionality as jsdoc/require-param

Alternatives considered

None, could not find this functionality, only similar patterns

@brettz9
Copy link
Collaborator

brettz9 commented Jun 22, 2023

You can get the require-template-name behavior with the jsdoc/match-name rule as follows:

        {
          match: [
            {
              disallowName: '/^$/',
              tags: [
                'template',
              ],
            },
          ],
        },

For require-template-description, see match-description with the tags option.

For require-template, I am having trouble seeing why you would want to add it to every function, etc.

You could use jsdoc/no-restricted-syntax though it wouldn't auto-add a missing @template (it would just report that one was missing); you'd need to add further to the context as desired:

        {
          contexts: [
            {
              comment: 'JsdocBlock:not(*:has(JsdocTag[tag=template]))',
              context: 'ArrowFunctionExpression,FunctionDeclaration,FunctionExpression,TSDeclareFunction',
              message: '@template required on each block',
            },
          ],
        },

@spaceribs
Copy link
Author

that certainly works in the meantime, but I'd assume type parameters are just as important to document as parameters? why wouldn't we want to make it a more straightforward process for requiring an explanation as to what is expected to be passed in?

@brettz9
Copy link
Collaborator

brettz9 commented Jun 22, 2023

Can you give me a precise example of code where you'd like to see @template required?

@spaceribs
Copy link
Author

@brettz9 sure, here's an example I'm working with:

/**
 * A tuple of the coordinates and the value at the coordinate.
 */
export type Pairs<D, V> = [D, V | undefined];

/**
 * For a particular axis, a tuple representing the element before
 * and after
 */
export type BeforeAfter<D, V> = [Pairs<D, V>, Pairs<D, V>];

@brettz9
Copy link
Collaborator

brettz9 commented Jun 23, 2023

Ok, I see, so you are using TypeScript syntax rather than using JavaScript with a TypeScript flavor and you want to enforce @template only when generics are used in the signature.

So I see a need for require-template as a rule, though I think require-template-name and require-template-description can be met by match-name and match-description rather than proliferating the rules here.

We might be able to add our nameRequired internal metadata to the @template tag, however, causing our valid-types rule to report any missing names for @template since I expect we can effectively require it with this tag.

@spaceribs
Copy link
Author

makes sense, I requested the others because it seems to match the established pattern, no problems nixing that.

@brettz9
Copy link
Collaborator

brettz9 commented Jul 9, 2024

We probably could use, however, a separate rule (in addition to the require-param equivalent) corresponding to check-param-names--i.e., check that if @templates are supplied, that they exist on the type. I.e., add a require-template and check-template-names.

@brettz9
Copy link
Collaborator

brettz9 commented Jul 9, 2024

And we might not just check TypeScript proper, but check any @typedef which used template values (and have a rule to check that any template values are present in the typedef if one is present on the block).

Also, in the require-template case, I think we might want an option to require that each template has a separate @template so it can be independently described (rather than using the @template T,U,V syntax).

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 10, 2024
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 10, 2024
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 10, 2024
@brettz9
Copy link
Collaborator

brettz9 commented Jul 10, 2024

I've added a PR to provide some support, but so far, only for the type alias structures (at root or exported) which you indicated. Maybe we can just release it now and gradually build support for other structures, as I'm sure there are many that could be supported.

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 10, 2024
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 10, 2024
@brettz9
Copy link
Collaborator

brettz9 commented Jul 10, 2024

Please feel free to enable the rule and try it out... jsdoc/require-template. See https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/require-template.md#readme

brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 10, 2024
brettz9 added a commit to brettz9/eslint-plugin-jsdoc that referenced this issue Jul 11, 2024
@brettz9 brettz9 reopened this Jul 13, 2024
@spaceribs
Copy link
Author

spaceribs commented Jul 17, 2024

Sorry, i've been on vacation, I'll take a look tonight and thank you for your hard work on this!

Copy link

🎉 This issue has been resolved in version 48.8.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@brettz9
Copy link
Collaborator

brettz9 commented Jul 19, 2024

Just note that there are now two rules, require-template and check-template-names. The latter was just released now.

@Arkellys
Copy link

@brettz9 It's a nice addition, thank you! However, I get errors with these two new rules:

Error: No parslet found for token: 'keyof' with value 'keyof'

For the following:

/**
 * Gets the values of `T`.
 * @template {Record<any, any>} T - Type to get the values of
 * @typedef {T[keyof T]} ValueOf
 */

Should I open an issue about it?

@brettz9
Copy link
Collaborator

brettz9 commented Jul 20, 2024

@Arkellys That is likely not due to the two new rules, but to the lack of support for the TypeScript keyof syntax. See jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#147 .

@Arkellys
Copy link

@brettz9 Ahh yes, I'm aware of this issue, that's why I had to create the ValueOf in the first place, so I don't have to disable the valid-types rule on every files. 😕 I guess I won't be able to use these new rules then.

@brettz9
Copy link
Collaborator

brettz9 commented Jul 20, 2024

We could really use someone in the know about parsers helping out with the type parser.

@brettz9
Copy link
Collaborator

brettz9 commented Aug 1, 2024

I think with the additions to date, these two rules are now mature enough to close the issue. Feel free to report desire for support of other type structures not yet covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment