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 (now, jsdoc-type-pratt-parser)), 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:

• typeof C type expression not supported #133 - support for typeof, which is understood by every JS type checker, not just Typescript. Typescript types should parse correctly jsdoctypeparser/jsdoctypeparser#50 - the bug tracking the actual fix
• #96 - The bug tracking the fix for TS in general

---

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

[gajus]

PRs welcome.

[sandersn]

I have an initial PR, for typeof and import types, up at jsdoctypeparser/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 jsdoctypeparser/jsdoctypeparser#50 (comment) , 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: jsdoc/jsdoc#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 jsdoctypeparser/jsdoctypeparser#50 (comment) (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?

[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 jsdoctypeparser/jsdoctypeparser#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 jsdoctypeparser/jsdoctypeparser#50 (comment) 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 jsdoctypeparser/jsdoctypeparser#58 (comment) 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?

[brettz9]

We have made a switch (in v35.1.0) from jsdoctypeparser to jsdoc-pratt-parser (@sandersn , @simonseyock is open to adding you as well for access to the repo with which he has configured semantic-release).

He has done the highly impressive work of covering our jsdoctypeparser test cases along with catharsis'.

See jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#4 (comment) for the few remaining TypeScript apparently remaining.

[sandersn]

Thanks for the update. I think this is the right direction since I haven't had time to work with jsdoc type parsing for quite a while.

[simonseyock]

@sandersn I configured the repository so a PR can be merged with one approving review. For @brettz9 it was important to be able to publish changes. If you want I can give you write access as well, and then you two could add changes even without me.

[sandersn]

Thanks! I don't have time to work on it right now but I'll let you know if I start up again and need write access.

[thernstig]

@brettz9 should this one be closed?

[brettz9]

@thernstig : I think with a few features missing still in jsdoc-type-pratt-parser, we should keep this open until they are implemented, e.g., your issue at jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#101 ({[key: string]: string}) and jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#131 (new keyword like {new(...args: any[]): object}). I think jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#96 should probably also be reviewed as a somewhat more systematic look at whether any basic TS types are missing.

[thernstig]

@sandersn @brettz9 can you update the original post? It still links to the old typescript type parser repo.

[brettz9]

@thernstig : updated

[JounQin]

Any possible to make @param {NodeJS.Require} require work? Now it errors The type 'NodeJS' is undefined.

[brettz9]

@JounQin : I think you are looking for a way to ensure all types are loaded which ought to be covered in #99 . See also #888 re: missing TS globals.

[gian1200]

#99 is closed but the warning @JounQin mention still happens using the default mode. In my case it's with NodeJS.Signals and NodeJS.UncaughtExceptionOrigin inside comments. Is this expected, is the same thing reported or a different issue?

"eslint-plugin-jsdoc": "^46.4.2"

• .eslintrc.cjs fragment:

...
{
	"env": {
		"node": true,
		"es6": true
	},
	"extends": [
		"plugin:jsdoc/recommended"
	],
	"files": ["karma.conf.js", "**/*.{cjs,mjs}"],
	"excludedFiles": ["src/app/**/*"],
	"parserOptions": {
		"ecmaVersion": 2022
	},
	"overrides": [
		{
			"files": ["**/*.mjs"],
			"parserOptions": {
				"sourceType": "module"
			}
		}
	]
},
...

• Code fragment:

...
/**
 *
 * @param {NodeJS.Signals} signal Señal emitida
 */
const exitGracefully = (signal) => {
	logger.info(`${signal} signal received`);
	shutdownServer(0);
};

/**
 *
 * @param {Error} error Error no manejado
 * @param {NodeJS.UncaughtExceptionOrigin} signal Señal emitida
 */
const onUncaughtException = (error, signal) => {
	logger.error(`${signal} signal received`);
	logger.error(error);
	shutdownServer(1);
};

process.on("SIGTERM", exitGracefully);
process.on("SIGINT", exitGracefully);
process.on("uncaughtException", onUncaughtException);
...

[brettz9]

@gian1200 : Since TS checks this anyways, the rule no-undefined-types has now been disabled by default for typescript configs, a config you should probably be using, e.g., if you are using plain JavaScript, then use plugin:jsdoc/recommended-typescript-flavor; otherwise, use plugin:jsdoc/recommended-typescript.

If you wish to discuss further, let's do so on #888.