Skip to content

Handle @typedef tag with missing type#18662

Merged
5 commits merged into
masterfrom
jsdocTypedefMissingType
Sep 25, 2017
Merged

Handle @typedef tag with missing type#18662
5 commits merged into
masterfrom
jsdocTypedefMissingType

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Sep 21, 2017

Fixes #18637

I'll have to test tomorrow if starting to check jsdoc is hurting performance. But we'll need it to fix issue #15852.

@ghost ghost requested review from rbuckton and sandersn September 21, 2017 20:29
@ghost ghost force-pushed the jsdocTypedefMissingType branch from 7867b49 to 3c7cd55 Compare September 21, 2017 20:32
Comment thread src/compiler/diagnosticMessages.json Outdated
"category": "Error",
"code": 8020
},
"JSDoc @typedef tag should either have a type annotation or be followed by @property or @member tags.": {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A JSDoc '@typedef' tag...by '@property' or '@member' tags.

@ghost ghost force-pushed the jsdocTypedefMissingType branch from 031b2be to abf0d3e Compare September 21, 2017 21:34
Comment thread src/compiler/checker.ts
case SyntaxKind.JSDocTypedefTag:
return checkJSDocTypedefTag(node as JSDocTypedefTag);
case SyntaxKind.JSDocComment:
return checkJSDocComment(node as JSDoc);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it duplicates the new jsdoc-checking loop. Or perhaps this entry is now unneeded.

Copy link
Copy Markdown
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be good once you investigate performance and whether the existing jsdoc handling in checkSourceElement is superseded.

@sandersn
Copy link
Copy Markdown
Member

To fix #15852, you'll also need binding, which means that you'll likely only be able to fix it for .js. Which is probably fine.

@ghost
Copy link
Copy Markdown
Author

ghost commented Sep 22, 2017

I ran performance tests and got little difference in compile times. (-0.16%, +0.68%, -0.99%, -1.01%, +0.97%, +1.60%)
(Since we don't have any checkJs code, I enabled the jsdoc checks even in TypeScript code for running performance tests. But the actual PR would only increase times for JS users.)

@ghost
Copy link
Copy Markdown
Author

ghost commented Sep 25, 2017

@sandersn Good catch, there was redundant code checking jsdoc specifically for functions/methods.

@ghost ghost merged commit b4018a2 into master Sep 25, 2017
@ghost ghost deleted the jsdocTypedefMissingType branch September 25, 2017 19:11
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Quick info on @typedef followed by declaration throws error

3 participants