Support typescript contexts for recommended rules

[obvioussean]

Motivation

I really want to use the jsdoc/recommended ruleset for my typescript project, but that means quite a few rules that support the contexts param won't run correctly. An example is interfaces, which require me to pass TSInterfaceDeclaration to contexts.

Current behavior

For each rule that in the recommended list I need to also include in the rules section and pass a long list of contexts.

Desired behavior

Either just always include the impacted typescript contexts if typescript parser is being used, or support a settings which holds the default contexts for any rule that supports a context parameter.

Alternatives considered

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[aoberoi]

i'd be willing to help implement some of this if the maintainers wanted to provide some guidance on which direction to take.

also @obvioussean, have you found any workarounds in the meantime?

[brettz9]

We currently use settings.jsdoc.mode to determine whether TypeScript parsing and such should be applied. That mode is auto-set by default if the @typescript-eslint/parser is found as the parser.

The problem with adding all contexts is that, besides the performance issues it may cause, it probably won't be so easy to come to an agreement about which additional contexts are even desirable. For example, does one want require-jsdoc to require a jsdoc block above every single TypeScript feature? Almost certainly not.

But I would agree that we could have more reasonable defaults, e.g., that publicly exported interfaces or whatever should be allowed. But as one who is not a regular TS user, I don't have as strong of a sense of this, and I think people supporting this can present their arguments in this issue or in the PR and negotiate with the rest of the community (or Gajus of course) as to what is considered reasonable.

[aoberoi]

here's one idea: we can build new rules for typescript contexts that "extend" from the existing rules. these new rules would have additional options regarding which contexts they would apply to, but the defaults would express what the community thinks make sense for each particular rule. they would support all existing options from the parent rule, and delegate to that rule when possible (so we don't end up duplicating logic unless its necessary). last, we aggregate all these new rules in a config called recommended-typescript.

i think this solves the issues where there isn't a uniform set of contexts that makes sense for all rules, allows users not to have to select contexts manually, and provides extension points for those who want to configure their rules a bit differently.

[brettz9]

We could take such an approach, sure, but I don't see a need for it at this point.

Auto-detection and "typescript" mode allow us to add TypeScript-specific features, and we can also add options which are specific to and/or detect this config.

My issue is more that contexts will differ even between TypeScript projects.

For example, one project might want all TSTypeAliasDeclaration to be annotated:

type Constructor = new (...args: any[]) => {};

Another project might only want those types which were exported to be annotated.

And another project might not want these annotated at all, focusing only on say the public API.

[aoberoi]

@brettz9 got it! how would you recommend we move forward then?

i recognize the issue around varied contexts across projects that you brought up. my idea tries to address that very issue which is what i referred to with "provides extension points for those who want to configure their rules a bit differently".

my hope is that someone can simply extend from a built-in config (whether that be "recommended" or "recommended-typescript" or something else), and get some sane defaults for the context. but also someone who has a different opinion regarding a rule (like the example you made) can be a little more specific. but would they want to be specific by updating one setting across all the rules, or be specific by updating the config of a single rule? my understanding of the automatic detection based solution limits this user to updating one setting across all rules. but i'm not too familiar with the code today.

[brettz9]

None of the "recommended" rules currently set any options (we only have this one config currently and each rule can internally default to the recommended behavior). However, configs can enable rules with their own options, e.g., if we were to add additional configs. So there could be a typescript config or configs which prepopulated with its own reasonable options, e.g., one config adding a contexts option for one rule or multiple rules to include TSTypeAliasDeclaration and whatever other TS AST.

But if we only come up with one reasonable TypeScript behavior, we might just want to allow the user to avoid even needing to choose a "recommended-typescript" config and just continue to use the "typescript" mode for this purpose, as it acts independently of the configs. For example, check-tag-names is set to "warn" in the "recommended" config with no options, but behind the scenes, if we detect the mode is "typescript" instead of "jsdoc", then using @template (a tag used by TS and Closure but not documented on the official jsdoc site, https://jsdoc.app/ ) will not give an error.

One could similarly detect "typescript" mode behind the scenes and auto-insert some contexts by default for those in "typescript" mode (if it seems reasonable for existing TS users to automatically get some contexts options pre-set for certain rules as they continue using the "recommended" config).

However, if it seems like we desire say a "typescript-heavy-documentation" config whereby rules would require or check many types of TS AST, then it might make sense to add this as a separate config which sets everything by explicit options.

But we probably wouldn't want to start a plain "typescript" config now, not only because settings.jsdoc.mode has been doing this (and there'd also need to be a "closure" config added if we changed things), but we auto-set mode based on the presence of @typescript-eslint/parser, a dynamic behavior which seems less confusing to me than if we were to dynamically set the "config", e.g., from "recommended" to a "recommended-typescript".

So my suggestion is:

1. If you think most TS users will want a certain change to the defaults, just continue to use the approach of detecting "typescript" mode within the rules and act accordingly, e.g., const contexts = settings.mode === 'typescript' ? ['TSTypeAliasDeclaration', 'TS...', etc.] : [].
2. If you think it is compelling for a subset of TS users to have a config that is alternative to the "typescript" mode default, then add a separate config for this, e.g., "typescript-special", with rule definitions like jsdoc/check-types: ['warn', {contexts: ['TSTypeAliasDeclaration', 'TS...', etc.]}].

[JoshuaKGoldberg]

I have the same desire here: I'd like to use this plugin's recommended ruleset, except for the rules that are made redundant by TypeScript. How about exposing an equivalent to recommended that does that?

"extends": "plugin:jsdoc/recommended-typescript"

[JoshuaKGoldberg]

@brettz9 @gajus bumping this issue since it's been a few months - if I sent a PR adding a recommended-typescript ruleset, would you be up for taking it in? And/or, do you think I should file a separate issue for that?

[RebeccaStevens]

I think the only rule changes needed for recommended-typescript would be:

{
  "jsdoc/no-types": "warn",
  "jsdoc/require-param-type": "off",
  "jsdoc/require-property-type": "off",
  "jsdoc/require-returns-type": "off",
}

[brettz9]

An update for this issue is that those not wanting to set individual contexts for each rule can now set them through:

{
  settings: {
    jsdoc: {
      contexts: [/*...*/]
    }
  }
}

@JoshuaKGoldberg : Yes, I am open to a PR, though I hope a regular TS user gives good thought to each rule so we hopefully won't have to tweak the ruleset much later.

Note that if someone has changes related to the JSDoc TS flavor (e.g., which tags, types, or tag name/description structure are allowed or anything which every TS user will absolutely want done a particular way including those using TS within JavaScript), this can instead be handled with our internal use of the "typescript" mode.

Btw, we are currently using mode in just one place which should I think instead be changed in the proposed PR; the default setting of exemptGenerators to true in the require-returns-check rule (since I think the avoidance is relevant to TypeScript proper only) should be set by the recommended-typescript config rather than by mode.

[JoshuaKGoldberg]

👍 awesome thanks. I won't be able to spend the time for mode changes or other infrastructural things in this package. So I hope someone else in this thread can step up 😄. Filed #997 for specifically the ruleset request.

[brettz9]

Ok, thank you @JoshuaKGoldberg for the PR, and @RebeccaStevens for the rule suggestions.

Re: the mode change I mentioned, I wish I could remember why we thought generators should be exempted from return checks in TypeScript only, but in any case, I'm not so sure the change is really needed (we might instead, however, avoid checks if void were present in the @returns type as in @returns {Generator<number, void, boolean>} and not otherwise for TS mode or a detected TS parser, but that would be a separate issue).

Since as mentioned there is now an opportunity for global contexts, and I'm not convinced a single combination of contexts would appeal across project, I think we can close this issue. If someone wants an issue for providing aliases whereby one alias could target a related group of contexts, I'd be open to a separate issue/PR (or we could wrap it into #384).