Eslint-plugin-jsdoc: Typescript types should be supported in jsdoc type positions

[sandersn]

Typescript types should be supported. Most of the work needs to occur in Kuniwak/jsdoctypeparser, but there will likely be some code needed here as well. At the very least, some tests need to be added to make sure that Typescript types work.

Related:

133 -- support for typeof, which is understood by every JS type checker, not just Typescript.

Kuniwak/jsdoctypeparser#50 -- the bug tracking the actual fix

[gajus]

PRs welcome.

[sandersn]

I have an initial PR, for typeof and import types, up at Kuniwak/jsdoctypeparser#51. I'll create a matching PR here, but it'll depend on a new version of jsdoctypeparser being available.

[gajus]

I would be open to bringing JSDoc parser into this library as I can see it becoming a common bottleneck for changes (if anyone is up for the challenge).

[sandersn]

I like the jsdoctypeparser parser code itself, although it is quite old and hasn't been touched in 2.5 years or so. Maybe it just needs more maintainers.

Its licence is MIT, which I think is compatible with that of eslint-plugin-jsdoc, so copying the source would just be a technical problem, not a legal one.

[MaxMilton]

This would be amazing. I could finally get rid of all the /* eslint-disable jsdoc/valid-types */ comments in files which contain TS types (which is a lot scattered throughout many projects).

[sandersn]

I haven't heard back from my PR on jsdoctypeparser, so I think moving the code here is a good idea. I'll take a look when I have time, which may be a few weeks.

[ExE-Boss]

Also, this should support allowing object over Object, which is recommended by TypeScript and enforced by my config.

[sandersn]

@ExE-Boss I think that would be determined by rules/validTypes.js, since both types parse correctly in jsdoctypeparser.

Also, for Typescript, I don't recommend putting types in JSDoc since the types should be in the signature.

[sandersn]

@gajus I read about licencing and I am not sure whether it's OK to copy an MIT-licensed project into a BSD-licensed one. It might be, but I'm not sure how to modify the licence of eslint-plugin-jsdoc, and I don't think that's a good idea either.

Probably the right thing is to contact @Kuniwak and see whether some owners/contributors from this project could be added to the owners of jsdoctypeparser.

Is that something you are interested in? If so I'll contact @Kuniwak and see what they think about adding you, me or @brettz9 (who is the only common contributor between these two projects).

Otherwise, it would probably be safer to fork jsdoctypeparser and re-publish it as jsdoctypeparser2. Either way I think it should stay a separate project.

Another option is to use a completely different, possibly new, parser. I thought about the Typescript parser, but it's unlikely to ever support the standard jsdoc import syntax (module:foo/bar.event:MyEvent).

cc @brettz9

[ExE-Boss]

Also, for Typescript, I don't recommend putting types in JSDoc since the types should be in the signature.

I meant when I use the TypeScript compiler to type check JavaScript code.

[brettz9]

@sandersn : I support forking jsdoctypeparser as a new project. Though he was engaging me for a while, the project owner suddenly stopped and I haven't heard back from him in quite a while. I have an open PR to actually add the "event:" prefix you happen to have mentioned. If he agreed to add us as owners, I'd be happy to at least merge that though. :) But I'd advise not waiting too long for a response, as it has already been a while for my requests.

IANAL, but it seems pretty clear that the very permissive MIT is compatible with this project's BSD-3-clause (one could not, however, incorporate its BSD-3-clause content into an MIT licensed-project and keep it licensed as MIT (only), as the BSD-3-clause has an additional clause about not being used for endorsement).

[gajus]

I suggest to copy the code (and LICENSE) into this library.

Alternatively, if either of you want to take ownership of a separate project, I am happy to use that too, as long as there are multiple contributors.

[sandersn]

I'm not sure what it means to append the MIT licence to the BSD licence, but it's not standard. I'm uncomfortable doing that.

I'll contact the owner and see if @brettz9 and I can get on the owner list. In the meantime, after I've made a few fixes I'll start publishing my fork as jsdoctypeparser2 until I hear back from the owner.

[ExE-Boss]

Or publish it as @sandersn/jsdoctypeparser, in the same way @pnpm’s fork of cmd-shim is available as @zkochan/cmd-shim

[sandersn]

Update: I heard back from @Kuniwak via twitter and they suggested setting up a jsdoctypeparser organisation, then transferring ownership of the repo there.

I'll do that when I have time. I'm a bit busy with the Typescript 3.4 release today, so it may not happen until Monday.

[sandersn]

@gajus @ExE-Boss did you want to be owners of the jsdoctypeparser org as well? Let me know and I will invite you.

[gajus]

@gajus @ExE-Boss did you want to be owners of the jsdoctypeparser org as well? Let me know and I will invite you.

Yes, please.

[ExE-Boss]

@sandersn

@gajus @ExE-Boss did you want to be owners of the jsdoctypeparser org as well? Let me know and I will invite you.

Probably, but I’ll most likely not be committing much, given my busy work in other projects.

[sandersn]

OK, the jsdoctypeparser repo is now at https://github.com/jsdoctypeparser/jsdoctypeparser and I pushed a new major version, 3.0.0, whose purpose is to prevent users from upgrading automatically. It doesn't have any other changes.

[brettz9]

Thank you so much, @sandersn for moving this forward!

As per my comment at https://github.com/jsdoctypeparser/jsdoctypeparser/issues/50#issuecomment-479698344 , I'd hope this would optional, unless you feel motivated to help maintain the fairly inactive jsdoc project to support processing (and display) of these types! :)

[sandersn]

Yes, I am motivated! I would like the entire ecosystem to work with Typescript correctly.

1. Can you point me to the jsdoc project in question?
2. Do you know what parser/printer that project uses for types? Is it jsdoctypeparser? The only other [non-deprecated] project I've seen is catharsis.

[sandersn]

All right, jsdoctypeparser 3.1.0 is published, with changes from me and @brettz9. #146 updates to this version of jsdoctypeparser and adds basic tests for the newly-working typescript types. Reviews are appreciated.

[brettz9]

Awesome on both counts--the release and changing the ecosystem sounds great... I mean the original jsdoc, though you might check out that it appears there is already a PR: https://github.com/jsdoc3/jsdoc/pull/1560

[brettz9]

And if JSDoc does support Typescript already, I see no evidence of it at http://usejsdoc.org/tags-type.html (except of course to the extent Typescript is using JSDoc!)

[brettz9]

Should tracking be done here for avoiding using constructs from Typescript or JSDoc which are redundant to the other as referenced in https://github.com/jsdoctypeparser/jsdoctypeparser/issues/50#issuecomment-480340239 (if the implementers of the Typescript support can begin support for such a rule while preparing PRs to add Typescript support)?

[sandersn]

A followup question for disallowing type syntax that is redundant between TS and JSDoc:

Are these options sufficient?

• Allow all
• Allow only JSDoc/Closure type syntax
• Allow only Typescript type syntax

I don't know anything about eslint yet but it looks pretty straightforward to add rules and configs, based on a quick look at index.js. Anything I should know before starting?

[brettz9]

While I know you're not getting at this, be aware that ESLint has deprecated their JSDoc-related rules (valid-jsdoc (#107) and require-jsdoc (#162)) as well as a method for parsing JSDoc, getJSDocComment (#189).

No special advice except to use the various guides: https://eslint.org/docs/developer-guide/

• https://eslint.org/docs/developer-guide/working-with-rules
• https://eslint.org/docs/developer-guide/nodejs-api
• https://eslint.org/docs/developer-guide/working-with-plugins

It is rather well-documented, and I think well-conceived, for what I've worked with, even if they could benefit from applying jsdoc3/jsdoc to their own documentation for better interlinking of and precise definitions for types. :)

Also eslint-plugin-jsdoc's source makes usage pretty clear. My one question to @gajus on architecture though is whether there is any reason why settings seem to be preferred over individual rule options? I know settings can be shared, but it seems more convenient and intuitive to me to add an option tied to its rule unless there really is a compelling need for a global shared setting (potentially now tracking this in #216).

[Elberet]

If I may, what's the status on this? I'm getting a warning about a syntax error in /** @type {typeof import("...").default} */ that seems related to this issue and the typeof keyword - or am I doing something wrong?

(FWIW, splitting the import into a @typedef doesn't help, the error in the typeof-statement persists.)

[brettz9]

@Elberet : This looks like an issue with typeof. Per https://github.com/jsdoctypeparser/jsdoctypeparser/blob/master/peg_src/jsdoctype.pegjs#L533 , it only supports typeof currently along with a QualifiedMemberName. I suspect NamepathExpr may be more what we need here.

Note there is an issue for better supporting TypeScript types at https://github.com/jsdoctypeparser/jsdoctypeparser/issues/50 , but in this case, typeof support was already added, but it just doesn't seem to currently support going with a full-blown expression.

Btw, FWIW, for experimenting with types supported by jsdoctypeparser (and thus eslint-plugin-jsdoc), you can use https://jsdoctypeparser.github.io/ .

[Elberet]

Thanks, I guess I'll ~inline-disable the warning where it crops up~ have to disable valid-types entirely because the error occurs during the parsing stage and can't be suppressed with any inline // eslint... config comment. 😞

By the way, the full background is this:

/** @typedef {import("./BaseFoo").default} BaseFoo */
/** @type {typeof BaseFoo} */
get Foo() { return this.dynamicChildClassOfBaseFoo }

This gets linted correctly, but Typescript (and by extension VSCode's code completion) believes Foo to be typed any.

/** @type {new() => BaseFoo} */

Works in both, but incorrectly documents the Foo constructor to have no arguments.
Also, while it works in the jsdoctypeparser live playground, it throws an error when run locally:

[Error - 10:36:57] TypeError: Cannot convert undefined or null to object
Occurred while linting C:\...\src\lib\FooFactory.js:1
    at Function.keys (<anonymous>)
    at _collectChildNodeInfo (C:\...\node_modules\jsdoctypeparser\lib\traversing.js:139:10)
    at traverse (C:\...\node_modules\jsdoctypeparser\lib\traversing.js:17:27)
    at C:\...\node_modules\jsdoctypeparser\lib\traversing.js:19:7
    at Array.forEach (<anonymous>)
    at traverse (C:\...\node_modules\jsdoctypeparser\lib\traversing.js:18:19)
    at C:\...\node_modules\eslint-plugin-jsdoc\dist\rules\checkTypes.js:130:35
    at Array.forEach (<anonymous>)
    at _default.iterateAllJsdocs (C:\...\node_modules\eslint-plugin-jsdoc\dist\rules\checkTypes.js:44:29)
    at C:\...\node_modules\eslint-plugin-jsdoc\dist\iterateJsdoc.js:444:13

[brettz9]

The jsdoctypeparser demo doesn't test traversal, and it looks like that is the issue there. I encourage you to file these concerns on jsdoctypeparser.

[brettz9]

@Elberet : The new() with arrow function has been fixed in https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v18.4.2

[brettz9]

Off topic somewhat, but relevant to TS in jsdoc...

In jsdoctypeparser, SuffixNullableTypeExpr (string?) and SuffixNotNullableTypeExpr (Object!) are mentioned as deprecated (while the prefixed equivalents are not).

In support of this, in the case of the nullable version at least, a link is given to Closure source showing where it is still used: https://github.com/google/closure-library/blob/master/closure/goog/net/tmpnetwork.js#L50 . And Closure docs mention:

You may see type declarations like these in older code:

@type {Object?}

There are still postfixes for ? and ! in jsdoc's catharsis type parser.

So rather than removing them from jsdoctypeparser for all modes, I figure they should be removable in check-syntax since that rule is being used for stripping deprecated features. However, if these suffixes are not even allowed in TS, I can adjust jsdoctypeparser for "typescript" mode. I don't see mention either way on these at https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#patterns-that-are-known-not-to-be-supported (or elsewhere on the page--though it does occur as a suffix on property names).

[brettz9]

Oh, I see from https://github.com/jsdoctypeparser/jsdoctypeparser/issues/50#issuecomment-648319020 that TS supports them even into 4.0, so I think making check-syntax report them for al modes sounds reasonable if they are indeed still deprecated.

On another TS subject... While it appears from https://github.com/jsdoctypeparser/jsdoctypeparser/issues/58#issuecomment-481331955 that @external could end up supported in TS if not external: namepaths, is @module a tag we should prevent (e.g., in check-syntax)? Any other tags that are likely never to be supported or which are problematic enough now (whether recognized or not) that no one should be using them in "typescript" mode?