eslint-plugin-jsdoc
eslint-plugin-jsdoc copied to clipboard
gajus
Way to feed `no-undefined-types` additional files or have it auto-detect them (and finding unused types within the same set of files); avoid defects of tampering with `no-unused-vars`
For no-undefined-types, I'd like a way to feed one's project files, whether:
- Detected by static or dynamic
import/require, with the entry file(s) determined by:- a new
eslint-plugin-jsdocoption - use of
mainorexportsinpackage.json - use of jsdoc config
- a new
- By a file being present in one's jsdoc config
- If eslint-plugin-jsdoc were supplied an array of files for finding typedefs
I need more context because some of my types are in:
- A purely-comment-based typedefs.js file which includes external and generic types
- Other files and modules
I'd like it to report for:
/**
* @param {MyType}
*/
...where MyType is not defined anywhere within JSDoc typedefs/interfaces across all files. The corollary is thus not to report errors for:
// One file, such as `typedefs.js`
/**
* @typedef {object} MyType
*/
/**
* @interface AnotherType
*/
// Another file:
/**
* @param {MyType}
*/
/**
* @param {AnotherType}
*/
Thanks very much for the very useful plugin!
Update: While traversing files of a project, also consider supporting @inheritdoc per #765
Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.
Given that JSDoc has its own means of expressing user custom types such as through @typedef and @interface, wouldn't JSDoc types make sense as the default way to feed in definitions (and without extra effort to redefine such types)?
Btw, re: those PRs, I am quite impressed how intuitive your code is, that it looks very easy to make one... Will see if I can set aside some time for it...
Given that JSDoc has its own means of expressing user custom types such as through @typedef and @interface, wouldn't JSDoc types make sense as the default way to feed in definitions (and without extra effort to redefine such types)?
Misunderstood the original issue.
Generally, The ESLint-friendly way of doing this is exporting the types using /* exported MyType */ and importing them as /* global MyType */, this is also how ESLint suggests to deal with variables defined in different files in browser context (without modules) when using no-unused-vars and no-undef. I don't think there's a general need in the community for this feature, and exported/global can be satisfactory.
@bary12 : Maybe you have misunderstood the request also? This is not talking about JavaScript variables representing types, but JSDoc types... I've updated my original post with some examples...
If you did actually mean leveraging exported/imported for JSDoc types, besides the fact that the no-unused-vars rule would be reported when the type was not used in code, it would be awfully cumbersome to import and export JSDoc types manually when the information was easily accessible elsewhere within JSDoc comments.
The rule treats JSDoc types defined in typedefs as normal variables. It scans the file for typedefs and adds them to the scope exactly like normal variables.
Yes, but it only scans the current file... Note my reference in the updated comment to these being in different files.. If one has defined generic types like "PlainObject" in another file, one would have to copy the definitions back in to each file, which is very cumbersome...
Or use /* global PlainObject */, the same way as ESLint recommends doing for variables when you have no form of import/export, or even add PlainObject to the list of globals in eslintrc.
Gotcha... Yes, that is better than pasting in the whole typedef and the list of globals in .eslintrc would at least work across files, but besides adding redundant work--adding comments to lint one's comments seems particularly wasteful--I don't think type information should be polluting variable information.
For one, it may mislead developers into thinking that such a variable is in use within the file. Secondly, this may distract from what the real globals are, with globals often being a temporary evil necessity that should be easier to quickly review and occasionally re-review for possible removal/conversion to modules; if types are mixed in there, it becomes harder to notice the real global variables in use. Thirdly, it may improperly allow variables as types where the variables of that name don't actually exist.
If https://github.com/jsdoc3/jsdoc/issues/1632 gets approved (for jsdoc to support Typescript documentation's import syntax), this issue may be easier to resolve, as files would point to the modules from which they were "importing" documentation.
@brettz9 Not if the @typedef was made with @global. The reason for @global in my case is that only then does jsdoc create links to the type (in the HTML it generates). I prefer sending all my readers to some "global" file with all types even if it gets big and has them all mixed over expecting them to scroll to the bottom of whatever module file HTML doc they are in to find the type — most people will just think there is no documentation for the type if it's not a link.
AFAICT, we are introspecting on all typedefs with a single file. If we allow input from additional files, the files with the @global @typedef's can be fed in too. As far as making the links work, one has to do some serious finagling (and possibly tag abusing) to get them to work in some instances. You might take a look at our https://github.com/SVG-Edit/svgedit code (with docs at https://svg-edit.github.io/svgedit/releases/latest/docs/jsdoc/module-SVGEditor.html ) which managed to get modules linking together. I think in a few cases, I had to just make a link rather than inlining other content, but I don't think there are any pointers to dead links. But even in this project, we have a global typedefs file for defining types like "Float", so I'd rather not that each file had to redefine that type.
so I'd rather not that each file had to redefine that type.
Okay, so I'm a bit confused, I was talking about jsdoc the HTML creating tool so no complaint about this plugin. It's their bug or missing feature, but given the lack of development there I didn't file an issue but just used whatever workaround(s) I need (which are a lot). Linking within the same generated HTML file should be just as easy as to the global file for jsdoc. In the end you seem to agree that types defined with @global not in the same file are a quite normal use case, so they should not cause an "is undefined" warning, even if that takes quite a bit more effort (and since local information such as the import statements are of no help since they won't ... ahem... okay forget what I wrote, I will still have my TypeScript of Flow import statements so yes, you could use those, as you said. I just got up and the brain isn't fully working yet....
Actually, now I remember what I was thinking: Using import statements only works for types used directly for type annotations. Types mentioned/linked in text might not be imported by the type checker and would be a JSDoc-only issue.
Good point; but we could still add that as part of the scan if we were following those links anyways.
One approach we can use here for regular jsdoc is to parse (and cache) a jsdoc config file: https://jsdoc.app/about-configuring-jsdoc.html (or a string containing the equivalent of jsdoc command line args).
(We could also use this config file to feed info on allowed tags in place of the definedTags option for check-tag-names.)
Besides for @typedef I've come across a few other cases where it would be useful to intelligently parse jsdoc tags across multiple files, i.e., for @todo and @license (and it may be useful just for getting at other tags, e.g., finding an API which accepts or returns a certain type). I'm thinking that as a precursor to this issue, a separate repo just for iterating files and collecting desired jsdoc metadata (whether specifying contexts, e.g., functions, or not) would be in order (including the option to follow the files via static or dynamic imports/require).
My comment here is therefore just a suggestion that implementing this be done as a generic tool so it can be reused with other projects (e.g., perhaps by license-checker or the todo tool concept outlined in https://github.com/gajus/eslint-plugin-jsdoc/issues/299 ) as well as for @typedef reading.
And actually, I think we can just extract much of what we have already (that is not ESLint-specific, like settings retrieval) out of the iterateJsdoc.js file.
Though this is really an enhancement, I'm also adding the bug label because as per #507 , there are limitations in the current approach which lead to bugs, and we really should avoid interacting with no-unused-vars in the first place (see the discussion in that issue).
I've created https://github.com/brettz9/es-file-traverse which should hopefully help with this issue. It allows iterating through JavaScript files (including cyclic dependencies), but only the files found to be in use (through import, require, or for AMD, define) given some entry file(s) (HTML or JavaScript), and supports supplying a callback which is passed the site source and AST which could be used to find the typedefs in use in a project. I imagine we could accept user options in eslint-plugin-jsdoc for the user's project's entry file(s) (or just use package.json main) and use es-file-traverse with these files so as to find all the typedefs in use in the project. (It'd still be good though to be able to look at the jsdoc config source property, especially since es-file-traverse can also handle globs, albeit not in the same exact manner.)
Too busy with other plans to implement myself these days, but I could see about helping if someone else wanted to give it a shot.
Unfortunately, ESLint does not currently support async rules. I'm not sure whether we might be able to avoid reporting on the first run, waiting for file retrieval results (which we would then cache), and then somehow retrigger reporting or report on any subsequent runs. This may at least make the rule useful within IDEs.
es-file-traverse could be refactored to allow work synchronously, but I'm not really inclined to go that route.
seeing how this ticket exists... I was just trying to clean up my project when I came across this working tutorial: https://stackoverflow.com/a/55767692/818732
basically it says one should extend jsconfig.json to also read whatever js files one uses to store @typedefs in to enable intellisense to do its magic in vscode.
did that, got really happy that Intellisense did in fact work this way. Then noticed how eslint started to throw errors /warnings about it.
Couldnt we follow a similar approach here? a simple "look here to find my type defs" config variable that is parsed on top of currently open files?
With #1098 now dropping jsdoc/no-undefined-types from the recommended rules for typescript configs (including a new config recommended-typescript-flavor for plain JavaScript), and with TypeScript handling this properly, I think we can drop this. We can accept PRs if anyone can fix this for non-TypeScript modes, but I don't think this should be a priority with TypeScript now being the default mode.
With #1098 now dropping
jsdoc/no-undefined-typesfrom the recommended rules for typescript configs (including a new configrecommended-typescript-flavorfor plain JavaScript), and with TypeScript handling this properly, I think we can drop this. We can accept PRs if anyone can fix this for non-TypeScript modes, but I don't think this should be a priority with TypeScript now being the default mode.
Hey @brettz9 !
I believe there is still a problem here. With more complex types, such as when generics are present in the function, we will still encounter an error stating that the variable is not being used:
❌ Linter error: @typescript-eslint/no-unused-vars / no-unused-vars: HTMLFormOperationalControlElement in unused
/**
* @param {<T extends unknown>(element: HTMLFormOperationalControlElement) => T} getFormControlElementPayloadCallback
* @returns {void}
*/
const getFieldsetControlElementValue = (
✅ There is no any linter errors:
/**
* @param {(element: HTMLFormOperationalControlElement) => unknown} getFormControlElementPayloadCallback
* @returns {void}
*/
const getFieldsetControlElementValue = (
I'll happily accept PRs, but digging into scope analysis, assuming it can be done properly given the bugs we've seen with this rule previously, is not something I have the time or energy to tackle myself.