Add @callback jsdoc tag #9546

zhengbli · 2016-07-07T04:38:04Z

Example:

/**
  * @callback FooHandler
  * @param {string} [eventName]
  * @param eventName2 {number | string}
  * @param eventName3
  */

is the equivalent of TypeScript code:

type FooHandler = (eventName?: string, eventName2: number | string, eventName3: any) => void;

Types defined by @callback tag are block scoped, and will show up in the navigate to list.

… addCallbackJsdocTag # Conflicts: # src/compiler/checker.ts

DanielRosenwasser · 2016-07-07T05:15:33Z

src/compiler/binder.ts

@@ -264,6 +265,14 @@ namespace ts {
                    let functionType = <JSDocFunctionType>node.parent;
                    let index = indexOf(functionType.parameters, node);
                    return "p" + index;
+                case SyntaxKind.JSDocParameterTag:
+                    Debug.assert(node.parent.kind === SyntaxKind.JSDocCallbackType);


Can you add a message to the assert here and above? There's not much distinguishing a stack trace from a user otherwise.

mhegazy · 2016-09-13T21:40:43Z

@zhengbli please check with @sandersn on this PR and #10671

sandersn · 2016-09-14T20:26:42Z

src/compiler/types.ts

+    }
+
+    export interface JSDocCallbackType extends JSDocType, Declaration {
+        parameterTags?: NodeArray<JSDocParameterTag>;


You can use @return here in @callback too, right? I think JSDocCallbackType should have a field for that too.

sandersn · 2016-09-14T20:48:49Z

src/services/navigationBar.ts

@@ -221,7 +221,7 @@ namespace ts.NavigationBar {
                if (node.jsDocComments) {
                    for (const jsDocComment of node.jsDocComments) {
                        for (const tag of jsDocComment.tags) {
-                            if (tag.kind === SyntaxKind.JSDocTypedefTag) {
+                            if (tag.kind === SyntaxKind.JSDocTypedefTag || tag.kind === SyntaxKind.JSDocCallbackTag) {


does the fourslash rename test cover this? I didn't think that it does

sandersn · 2016-09-14T21:01:01Z

src/compiler/utilities.ts

+                // This is different kind of jsDoc parameter tag. Here is the address the case when
+                // the parameter declaration is a normal one (in the code), while the type of the
+                // parameter may be found in the jsdoc comment.
+                const paramTag = getCorrespondingJSDocParameterTag(node);


I think it would be better to simplify this code and have getCorrespondingJSDocParameterTag return a paramTag that works whether or not node is aParameterDeclaration or a JSDocParameterTag.

sandersn · 2016-09-14T21:24:28Z

src/compiler/types.ts

@@ -1528,6 +1530,15 @@ namespace ts {
        jsDocTypeLiteral?: JSDocTypeLiteral;
    }

+    export interface JSDocCallbackTag extends JSDocTag, Declaration {
+        name: Identifier;
+        type: JSDocCallbackType;


This is actually quite a bit like an @typedef in that they both support multiple lines and both produce a single type (declaration?). Could the interfaces be shared?

Maybe even some of the machinery could be shared. I haven't read that part yet.

sandersn · 2016-09-14T21:27:58Z

src/compiler/types.ts

@@ -2397,7 +2408,7 @@ namespace ts {
    }

    export interface Signature {
-        declaration: SignatureDeclaration;  // Originating declaration
+        declaration: SignatureDeclaration | JSDocCallbackType;  // Originating declaration


why doesn't @typedef need to be special-cased onto a declaration like this? At least I don't find an example of it happening.

sandersn · 2016-09-14T22:00:48Z

src/compiler/checker.ts

@@ -3178,6 +3178,14 @@ namespace ts {
                        type = checkExpressionCached((<BinaryExpression>declaration.parent).right);
                    }
                }
+                else if (declaration.kind == SyntaxKind.JSDocParameterTag) {


when can this happen?

sandersn · 2016-09-14T22:09:56Z

src/compiler/checker.ts

-                        if (minArgumentCount < 0) {
-                            minArgumentCount = i - (hasThisParameter ? 1 : 0);
+                    if (isJSDocParameterTag(param)) {
+                        if (isJSDocOptionalParameterTag(param)) {


this is really almost the same as the code in the !isJSDocParameterTag branch. I think you should get rid of the true branch and find a way to make the already-existing isJSDocOptionalParameterTag (currently, line 4545) check work.

sandersn · 2016-09-14T22:10:39Z

src/compiler/checker.ts

@@ -4474,7 +4496,7 @@ namespace ts {
            }
        }

-        function getSignatureFromDeclaration(declaration: SignatureDeclaration): Signature {
+        function getSignatureFromDeclaration(declaration: SignatureDeclaration | JSDocCallbackType): Signature {


why do we have to getSignatureFromDeclaration for JSDocCallbackType and not for JSDocTypedefTag?

sandersn · 2016-09-14T22:28:38Z

src/compiler/parser.ts

+                            case SyntaxKind.AtToken:
+                                if (canParseTag) {
+                                    parentTagTerminated = !tryParseChildTag(parentTag);
+                                    if (!parentTagTerminated) {


can you invert parentTagTerminated and rename it? I'd much rather read parentTagContinues = tryParseChild(parentTag); if (parentTagContinues) resumePos = scanner.getStartPos();

sandersn · 2016-09-14T22:43:32Z

tests/cases/fourslash/server/jsdocCallbackTag.ts

+
+//// /**
+////  * @callback FooHandler
+////  * @param {string} eventName


can you add some documentation? I don't think it'll work now, but once my jsdoc change is also merged, then parseParamTag should also parse documentation. In the meantime, it should be skipped correctly.

Note that the parser in services almost certainly won't skip it or retrieve it correctly. I don't think that's a problem.

sandersn

The parser changes look good but the binder looks like it could be simplified and I don't understand why some parts of the checker need to change, like getDeclarationFromSignature.

sandersn · 2016-09-14T22:47:10Z

We should discuss which order to merge this vs #10671. As far as I can tell, they shouldn't overlap very much except

a few additions of skipWhitespace() in the parser (since Remove service's jsdoc parser and enhance parser's jsdoc parser #10671 makes jsdoc now track whitespace)
doc comments on callback parameters need to work, which means more tests and possibly some code changes too.

mhegazy · 2016-12-30T22:51:26Z

@zhengbli can you update his change?

mhegazy · 2017-04-20T00:57:21Z

closing this for now.

sandersn · 2018-04-27T21:24:53Z

Note that once @typedef supports generics via @template, @callback should too.

zhengbli added 3 commits July 6, 2016 19:27

Add support for @callback jsdoc tag

0d28763

Add more tests for @callback tag

d67dea3

Merge branch 'master' of https://github.com/Microsoft/TypeScript into…

216f364

… addCallbackJsdocTag # Conflicts: # src/compiler/checker.ts

msftclas added the cla-not-required label Jul 7, 2016

zhengbli changed the title ~~Add callback jsdoc tag~~ Add @callback jsdoc tag Jul 7, 2016

DanielRosenwasser reviewed Jul 7, 2016
View reviewed changes

PR feedback

5cdce53

zhengbli mentioned this pull request Jul 8, 2016

Allow custom object types in JSDoc #9276

Closed

sandersn reviewed Sep 14, 2016

View reviewed changes

sandersn requested changes Sep 14, 2016

View reviewed changes

mhegazy closed this Apr 20, 2017

sandersn mentioned this pull request Apr 27, 2018

Create and add a guideline for typings webpack/webpack#7122

Closed

1 task

microsoft locked and limited conversation to collaborators Jul 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add @callback jsdoc tag #9546

Add @callback jsdoc tag #9546

zhengbli commented Jul 7, 2016

DanielRosenwasser Jul 7, 2016

mhegazy commented Sep 13, 2016

sandersn Sep 14, 2016 •

edited

Loading

sandersn Sep 14, 2016 •

edited

Loading

sandersn Sep 14, 2016

sandersn Sep 14, 2016

sandersn Sep 14, 2016

sandersn Sep 14, 2016

sandersn Sep 14, 2016

sandersn Sep 14, 2016

sandersn Sep 14, 2016

sandersn Sep 14, 2016

sandersn Sep 14, 2016

sandersn left a comment

sandersn commented Sep 14, 2016

mhegazy commented Dec 30, 2016

mhegazy commented Apr 20, 2017

sandersn commented Apr 27, 2018

Add @callback jsdoc tag #9546

Add @callback jsdoc tag #9546

Conversation

zhengbli commented Jul 7, 2016

Choose a reason for hiding this comment

mhegazy commented Sep 13, 2016

sandersn Sep 14, 2016 • edited Loading

Choose a reason for hiding this comment

sandersn Sep 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn left a comment

Choose a reason for hiding this comment

sandersn commented Sep 14, 2016

mhegazy commented Dec 30, 2016

mhegazy commented Apr 20, 2017

sandersn commented Apr 27, 2018

sandersn Sep 14, 2016 •

edited

Loading

sandersn Sep 14, 2016 •

edited

Loading