[Salsa] Support @typedef for jsdoc #8103

zhengbli · 2016-04-15T00:00:12Z

This PR adds support for the @typedef JSDoc tag. The current implementation includes the following formats:

Format 1:

/** @typedef { [Type Expression] } [Type Name] */

Format 2:

/** @typedef  [Type Expression] */
var [Type Name];

Format 3:

/** 
  * @typedef  {Object} [Type Name] 
  * @property {[Type Expression]} [Property Name]
  * @property {[Type Expression]} [Property Name]
  * ...
  */

Format 4:

/** 
  * @typedef  [Type Name] 
  * @type {Object}
  * @property {[Type Expression]} [Property Name]
  * @property {[Type Expression]} [Property Name]
  * ...
  */

Current the "Type name" can only be an identifier. Dotted names (qualified names) support may be added in future updates. The types defined by the @typedef tag are the same as type aliases in TypeScript, and they have scopes as well.

Basic support in language services like go to definition and rename is added as well.

Fixes #6976

…as hydrated with jsdocComment nodes.

… typedefForJsdoc # Conflicts: # src/compiler/diagnosticMessages.json # src/services/services.ts

zhengbli · 2016-04-18T22:16:51Z

@RyanCavanaugh @vladima @mhegazy comments?

yuit · 2016-04-19T20:23:45Z

src/compiler/types.ts

@@ -342,6 +342,9 @@ namespace ts {
        JSDocReturnTag,
        JSDocTypeTag,
        JSDocTemplateTag,
+        JSDocTypedefTag,
+        JSDocPropertyTag,
+        JSDocTypeLiteral,


question: what is the JSDocTypeLiteral?

A @typedef is similar to a type alias in TS. In TS you can give an alias to an object type literal:

type T = { name: string, age: number}

which is equivalent to the following jsdoc:

/** * @typedef {Object} T * @property {string} name * @property {number} age */

So I defined the @propertys as JSDocTypeLiteral, as it functions like object type literal, but with a different shape.

billti · 2016-04-19T20:32:21Z

Adding cross-reference to #7758

yuit · 2016-04-19T20:43:35Z

src/services/services.ts

@@ -269,20 +275,36 @@ namespace ts {
                scanner.setText((sourceFile || this.getSourceFile()).text);
                children = [];
                let pos = this.pos;
+                const useJSDocScanner =


Why don't u check like this : const useJSDocScanner = isJSDoc(this)? actaully question: why you don't use JSDocScanner on other JSDoc that is not listed here?

JSDocScanner limits scanning to several allowed characters, while disallowed characters are all scanned as unknown tokens. JSDoc nodes include JSDoc type nodes and JSDoc type expression nodes, and if the current position is inside a JSDoc type expression, it should allow all characters, because that part is essentially the same as code. For example, the < and > are not allowed in JSDoc tokens, they are fine when used with generics in JSDoc type expression.

RyanCavanaugh · 2016-04-19T21:47:42Z

What's the perf impact of parsing the extra comments? Any?

yuit · 2016-04-19T21:50:34Z

src/services/utilities.ts

+            if (includeJsDocComment) {
+                const jsDocChildren = ts.filter(current.getChildren(), isJSDocNode);
+                for (const jsDocChild of jsDocChildren) {
+                    let start = allowPositionInLeadingTrivia ? jsDocChild.getFullStart() : jsDocChild.getStart(sourceFile, includeJsDocComment);


Should this be "const"

… typedefForJsdoc

zhengbli · 2016-04-25T20:36:50Z

I ran the following sample for the perf bench mark:

The Setup:

In an empty folder, do

npm install ember-cli

(I chose ember-cli because it is big enough to see the bind/parsing time difference)
then create an empty jsconfig.json with the following content:

{
    "exclude": []
}

The bench mark script is:

Write-Output "TSC runs:"
0..10 | % {tsc -p ".\jsconfig.json" --noEmit --diagnostics | Select-String "Bind Time|Parse Time"}

Write-Output "PR runs:"
0..10 | % { node --max-old-space-size=2000 "C:\Users\lizhe\Documents\github\TypeScript\built\local\tsc.js" -p ".\jsconfig.json" --noEmit --diagnostics | Select-String "Bind Time|Parse Time"}

The Result:

… typedefForJsdoc # Conflicts: # src/services/services.ts

zhengbli · 2016-05-05T06:12:49Z

So it turns out that my previous benchmark wasn't very accurate at all, for several reasons:

I was running the tests on a laptop on battery mode, and there are browsers / other background programs running at the same time
The tsc.js for the PR was generated via calling jake local, which by default is in "debug mode", things like const enums are preserved for debugging purpose. I should instead use jake release local to generate the tsc.js.
I used the command line tsc for the baseline, which may have node different parameters with what I used for the PR tests.
On the PR I didn't pull from the latest master, so other changes may have impacted the results as well.

I rerun the benchmarks with these factors fixed. The new environment is:
a) a dedicated Ubuntu machine with no graphic interface (ssh)
b) a fresh installed windows 10 machine that is:

not domain-joined
no active tasks in task scheduler
desktop with high-performance power management options
as few background apps as possible (no onedrive, browser etc.)

c) use node for both cases, the new script is:

Param($count)

Write-Output "PR runs:"
0..($count - 1) | % { node --max-old-space-size=2000 "C:\Users\lan\Documents\github\TypeScript\built\local\tsc.js" -p ".\jsconfig.json" --noEmit --diagnostics | Select-String "Bind Time"}

Write-Output "Master runs:"
0..($count - 1) | % { node --max-old-space-size=2000 "C:\Program Files\nodejs\node_modules\typescript\lib\tsc.js" -p ".\jsconfig.json" --noEmit --diagnostics | Select-String "Bind Time" }

d) use the latest nightly and merge the PR with master locally. Checked the diff of two tsc.js files to make sure that the only differences are indeed caused by the PR.

And the benchmark result shows that the PR is not slower in binding time:

a) on the Ubuntu machine:

b) on the Windows machine:

RyanCavanaugh · 2016-05-05T17:04:46Z

👍 especially if it's somehow faster 😉

… typedefForJsdoc # Conflicts: # src/services/services.ts # tests/cases/unittests/jsDocParsing.ts

… typedefForJsdoc # Conflicts: # src/services/utilities.ts

mhegazy · 2016-05-26T22:22:53Z

src/compiler/types.ts

@@ -1510,6 +1516,25 @@ namespace ts {
        typeExpression: JSDocTypeExpression;
    }

+    // @kind(SyntaxKind.JSDocTypedefTag)
+    export interface JSDocTypedefTag extends JSDocTag, Declaration {
+        name: Identifier;


name, typeExpresion and type should be optional.

mhegazy · 2016-05-26T22:35:11Z

I would add a navigate to test for typedefs

… typedefForJsdoc

mhegazy · 2016-05-31T19:43:02Z

👍

mhegazy · 2016-05-31T19:44:14Z

#8849 touches the same areas, can you merge this in after that ones goes in.

…Script into typedefForJsdoc # Conflicts: # src/compiler/binder.ts

… typedefForJsdoc

zhengbli and others added 6 commits April 3, 2016 11:52

Simple case and scoping

df91ef0

Support the case with @Property and @type

23bca4e

Support the case with variable name as @typedef type name

9a8f639

Add tests for renaming and incremental parsing. Changed how the AST w…

2945f64

…as hydrated with jsdocComment nodes.

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

a3d74ad

… typedefForJsdoc # Conflicts: # src/compiler/diagnosticMessages.json # src/services/services.ts

code clean up

0dddcf4

msftclas added the cla-not-required label Apr 15, 2016

yuit reviewed Apr 19, 2016
View reviewed changes

zhengbli added 4 commits April 22, 2016 14:26

refactor

15cbb02

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

870fd84

… typedefForJsdoc

resolve build error

d568d79

Fix broken test

79bb4ba

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

9ff02b1

… typedefForJsdoc # Conflicts: # src/services/services.ts

zhengbli added 3 commits May 24, 2016 21:06

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

5f9fa69

… typedefForJsdoc # Conflicts: # src/services/services.ts # tests/cases/unittests/jsDocParsing.ts

Change how typedef tag is parsed

81ce532

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

e69976c

… typedefForJsdoc # Conflicts: # src/services/utilities.ts

zhengbli force-pushed the typedefForJsdoc branch from 95fd3a1 to e69976c Compare May 26, 2016 21:50

mhegazy reviewed May 26, 2016
View reviewed changes

zhengbli added 4 commits May 30, 2016 22:11

cr feedback

18ee4b0

Add navigationTo test for jsdoc typedef

59b188d

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

5261467

… typedefForJsdoc

Fix broken test

e93f9df

This was referenced May 31, 2016

Add jsdoc nodes to fixupParentReferences function #8702

Closed

Jsdoc @template issue in syntactic classifier #8676

Closed

zhengbli added 3 commits May 31, 2016 14:02

Merge branch 'outerControlFlows' of https://github.com/Microsoft/Type…

241920c

…Script into typedefForJsdoc # Conflicts: # src/compiler/binder.ts

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

630517b

… typedefForJsdoc

Remove unused parameter

eb0f035

zhengbli merged commit 166f399 into microsoft:master May 31, 2016

zhengbli deleted the typedefForJsdoc branch May 31, 2016 23:24

mhegazy mentioned this pull request Jun 9, 2016

[IntelliSense] [Salsa] Support @callback in JSDoc #7515

Closed

GRiMe2D mentioned this pull request Jul 10, 2016

Allow custom object types in JSDoc #9276

Closed

zhengbli mentioned this pull request Jul 11, 2016

Object member type defined by JSDoc @property tags are not recoganized #9620

Closed

microsoft locked and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Salsa] Support @typedef for jsdoc #8103

[Salsa] Support @typedef for jsdoc #8103

zhengbli commented Apr 15, 2016

zhengbli commented Apr 18, 2016

yuit Apr 19, 2016

zhengbli Apr 19, 2016

billti commented Apr 19, 2016

yuit Apr 19, 2016 •

edited

Loading

zhengbli Apr 19, 2016

RyanCavanaugh commented Apr 19, 2016

yuit Apr 19, 2016

zhengbli commented Apr 25, 2016 •

edited

Loading

zhengbli commented May 5, 2016 •

edited

Loading

RyanCavanaugh commented May 5, 2016

mhegazy May 26, 2016

mhegazy commented May 26, 2016

mhegazy commented May 31, 2016

mhegazy commented May 31, 2016

[Salsa] Support @typedef for jsdoc #8103

[Salsa] Support @typedef for jsdoc #8103

Conversation

zhengbli commented Apr 15, 2016

zhengbli commented Apr 18, 2016

yuit Apr 19, 2016

Choose a reason for hiding this comment

zhengbli Apr 19, 2016

Choose a reason for hiding this comment

billti commented Apr 19, 2016

yuit Apr 19, 2016 • edited Loading

Choose a reason for hiding this comment

zhengbli Apr 19, 2016

Choose a reason for hiding this comment

RyanCavanaugh commented Apr 19, 2016

yuit Apr 19, 2016

Choose a reason for hiding this comment

zhengbli commented Apr 25, 2016 • edited Loading

The Setup:

The Result:

zhengbli commented May 5, 2016 • edited Loading

RyanCavanaugh commented May 5, 2016

mhegazy May 26, 2016

Choose a reason for hiding this comment

mhegazy commented May 26, 2016

mhegazy commented May 31, 2016

mhegazy commented May 31, 2016

yuit Apr 19, 2016 •

edited

Loading

zhengbli commented Apr 25, 2016 •

edited

Loading

zhengbli commented May 5, 2016 •

edited

Loading