JSDoc support #6024
JSDoc support #6024
Conversation
| Debug.assert(node.parent.kind === SyntaxKind.JSDocFunctionType); | ||
| let functionType = <JSDocFunctionType>node.parent; | ||
| let index = indexOf(functionType.parameters, node); | ||
| return "p" + index; |
There was a problem hiding this comment.
Will we ever manifest this name to the user?
|
The |
| @@ -1241,6 +1265,8 @@ namespace ts { | |||
|
|
|||
| case SyntaxKind.CallExpression: | |||
| if (isInJavaScriptFile(node)) { | |||
| // We're only inspecting call expressions to detect CommonJS modules, so we can skip | |||
| // this check if we've already seen the module indicator | |||
There was a problem hiding this comment.
I'm not seeing where you skip this check.
| // Keep going | ||
| } | ||
| else { | ||
| identifierStarted = true; |
There was a problem hiding this comment.
Hmm, so an identifier can start with anything but the characters checked for above, but then after that must contain proper identifier characters. That seems a bit odd.
|
|
||
| if (startPos === pos) { | ||
| function scanJsDocIdentifier(): Identifier { |
There was a problem hiding this comment.
Should be parseJSDocIdentifier. The scanning is handled by the scanner.
There was a problem hiding this comment.
Also, it should call nextJSDocToken(), similar to what parseIdentifier in the main parser does.
| return pos += 1, token = SyntaxKind.CommaToken; | ||
| } | ||
| else if (isWhiteSpace(ch)) { | ||
| // Keep going |
There was a problem hiding this comment.
This should never happen though right? As you've eaten the preceding white space, and there's nothing in here where you would consume white space inside a token.
| precedingLineBreak = savePrecedingLineBreak; | ||
| tokenValue = saveTokenValue; | ||
| hasExtendedUnicodeEscape = saveHasExtendedUnicodeEscape; | ||
| tokenIsUnterminated = saveTokenIsUnterminated; |
There was a problem hiding this comment.
It seems a little fragile to use a method (setText - which calls setTextPos) to modify the state prior to callback execution, then manually restore each field individually after. If anyone added further state changes in setText (or setTextPos), they'd need to know to come and manually undo those same changes here. Can this set/restore be made more symmetrical/colocated somehow?
There was a problem hiding this comment.
This is the same pattern we use elsewhere (e.g. in speculationHelper). I don't see any obvious improvement.
| @@ -3860,12 +3923,33 @@ namespace ts { | |||
| return result; | |||
| } | |||
|
|
|||
| function isOptionalParameter(node: ParameterDeclaration) { | |||
| function isOptionalParameter(node: ParameterDeclaration, skipSignatureCheck?: boolean) { | |||
There was a problem hiding this comment.
The skipSignatureCheck parameter never appears to be used.
| const onClose = () => { return; }; | ||
|
|
||
| // Use a socket for comms if defined | ||
| const tss_debug: string = process.env["TSS_DEBUG"]; |
There was a problem hiding this comment.
All these edits to server.ts look like changes I made in my branch to help with debugging Sublime but hadn't pushed to 'master' (and are part of bigger changes on both Sublime and tsserver to use TCP over stdin/stdout if configured to do so via TSS_DEBUG). Any idea how the changes in this file got in this code review?
|
Made it all the way through. I think we're good once my comments are addressed. |
Adds support for adding type information in JavaScript files using jsdoc tags