Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use jsdoc aliases if visible when printing types #24153

Merged
merged 4 commits into from
May 16, 2018
Merged

Use jsdoc aliases if visible when printing types #24153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
30d2f10
Use jsdoc aliases if visible when printing types
weswigham May 16, 2018
81cda69
Modify implementation a bit, add test that aughta change in the new f…
weswigham May 16, 2018
7e0727e
Merge branch 'master' into use-jsdoc-aliases
weswigham May 16, 2018
a63f725
Accept baselines- hold off on typedef template lookup change
weswigham May 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4024,6 +4024,10 @@ namespace ts {

function determineIfDeclarationIsVisible() {
switch (node.kind) {
case SyntaxKind.JSDocTypedefTag:
Copy link
Member

Choose a reason for hiding this comment

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

Need to handle SyntaxKind.CallbackTag too

Copy link
Member Author

Choose a reason for hiding this comment

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

#23947 isn't merged yet

// Top-level jsdoc typedefs are considered exported
// First parent is comment node, second is hosting declaration or token; we only care about those tokens or declarations whose parent is a source file
return !!(node.parent && node.parent.parent && node.parent.parent.parent && isSourceFile(node.parent.parent.parent));
case SyntaxKind.BindingElement:
return isDeclarationVisible(node.parent.parent);
case SyntaxKind.VariableDeclaration:
Expand Down Expand Up @@ -5163,8 +5167,8 @@ namespace ts {
let result: TypeParameter[];
for (const node of symbol.declarations) {
if (node.kind === SyntaxKind.InterfaceDeclaration || node.kind === SyntaxKind.ClassDeclaration ||
node.kind === SyntaxKind.ClassExpression || node.kind === SyntaxKind.TypeAliasDeclaration) {
const declaration = <InterfaceDeclaration | TypeAliasDeclaration>node;
node.kind === SyntaxKind.ClassExpression || node.kind === SyntaxKind.TypeAliasDeclaration || node.kind === SyntaxKind.JSDocTypedefTag) {
const declaration = <InterfaceDeclaration | TypeAliasDeclaration | JSDocTypedefTag>node;
Copy link
Member

Choose a reason for hiding this comment

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

same. There is a predicate isJSDocTypeAlias that should maybe just be isTypeAlias and include SyntaxKind.TypeAliasDeclaration.

Copy link
Member Author

Choose a reason for hiding this comment

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

#23947 isn't merged yet

const typeParameters = getEffectiveTypeParameterDeclarations(declaration);
if (typeParameters) {
result = appendTypeParameters(result, typeParameters);
Expand Down Expand Up @@ -9046,7 +9050,7 @@ namespace ts {
}

function getAliasSymbolForTypeNode(node: TypeNode) {
return node.parent.kind === SyntaxKind.TypeAliasDeclaration ? getSymbolOfNode(node.parent) : undefined;
return (node.parent.kind === SyntaxKind.TypeAliasDeclaration || node.parent.kind === SyntaxKind.JSDocTypedefTag) ? getSymbolOfNode(node.parent) : undefined;
}

function getAliasTypeArgumentsForTypeNode(node: TypeNode) {
Expand Down
8 changes: 5 additions & 3 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3093,11 +3093,13 @@ namespace ts {
* Gets the effective type parameters. If the node was parsed in a
* JavaScript file, gets the type parameters from the `@template` tag from JSDoc.
*/
export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters) {
return node.typeParameters || (isInJavaScriptFile(node) ? getJSDocTypeParameterDeclarations(node) : undefined);
export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters | JSDocTypedefTag) {
Copy link
Member

Choose a reason for hiding this comment

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

there are a few other places that we get type parameter declarations of typedef and callback tags. They explicitly do not use getEffectiveTypeParameterDeclarations because it incorrectly (1) finds the host and (2) looks for @template in any jsdoc comment. Jsdoc type aliases only look for @template in their containing comment.

That said, it's probably the Right Thing to make getEffectiveTypeParameterDeclarations handle jsdoc type aliases, and have everybody call it.

Copy link
Member Author

Choose a reason for hiding this comment

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

That said, it's probably the Right Thing to make getEffectiveTypeParameterDeclarations handle jsdoc type aliases, and have everybody call it.

Do you not have such a fix in #23947 ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, along with the astonishing ability to forget what I did two weeks ago!

return isJSDocTypedefTag(node)
? getJSDocTypeParameterDeclarations(node)
: node.typeParameters || (isInJavaScriptFile(node) ? getJSDocTypeParameterDeclarations(node) : undefined);
}

export function getJSDocTypeParameterDeclarations(node: DeclarationWithTypeParameters) {
export function getJSDocTypeParameterDeclarations(node: DeclarationWithTypeParameters | JSDocTypedefTag) {
const templateTag = getJSDocTemplateTag(node);
return templateTag && templateTag.typeParameters;
}
Expand Down
24 changes: 12 additions & 12 deletions tests/baselines/reference/checkJsdocTypedefInParamTag1.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@
* @param {Opts} opts
*/
function foo(opts) {
>foo : (opts: { x: string; y?: string; z?: string; w?: string; }) => void
>opts : { x: string; y?: string; z?: string; w?: string; }
>foo : (opts: Opts) => void
>opts : Opts

opts.x;
>opts.x : string
>opts : { x: string; y?: string; z?: string; w?: string; }
>opts : Opts
>x : string
}

foo({x: 'abc'});
>foo({x: 'abc'}) : void
>foo : (opts: { x: string; y?: string; z?: string; w?: string; }) => void
>foo : (opts: Opts) => void
>{x: 'abc'} : { x: string; }
>x : string
>'abc' : "abc"
Expand All @@ -34,18 +34,18 @@ foo({x: 'abc'});
* @param {AnotherOpts} opts
*/
function foo1(opts) {
>foo1 : (opts: { anotherX: string; anotherY?: string; }) => void
>opts : { anotherX: string; anotherY?: string; }
>foo1 : (opts: AnotherOpts) => void
>opts : AnotherOpts

opts.anotherX;
>opts.anotherX : string
>opts : { anotherX: string; anotherY?: string; }
>opts : AnotherOpts
>anotherX : string
}

foo1({anotherX: "world"});
>foo1({anotherX: "world"}) : void
>foo1 : (opts: { anotherX: string; anotherY?: string; }) => void
>foo1 : (opts: AnotherOpts) => void
>{anotherX: "world"} : { anotherX: string; }
>anotherX : string
>"world" : "world"
Expand All @@ -60,17 +60,17 @@ foo1({anotherX: "world"});
* @param {Opts1} opts
*/
function foo2(opts) {
>foo2 : (opts: { x: string; y?: string; z?: string; w?: string; }) => void
>opts : { x: string; y?: string; z?: string; w?: string; }
>foo2 : (opts: Opts1) => void
>opts : Opts1

opts.x;
>opts.x : string
>opts : { x: string; y?: string; z?: string; w?: string; }
>opts : Opts1
>x : string
}
foo2({x: 'abc'});
>foo2({x: 'abc'}) : void
>foo2 : (opts: { x: string; y?: string; z?: string; w?: string; }) => void
>foo2 : (opts: Opts1) => void
>{x: 'abc'} : { x: string; }
>x : string
>'abc' : "abc"
Expand Down
136 changes: 2 additions & 134 deletions tests/baselines/reference/jsDocTypedef1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,140 +41,8 @@
"kind": "space"
},
{
"text": "{",
"kind": "punctuation"
},
{
"text": "\n",
"kind": "lineBreak"
},
{
"text": " ",
"kind": "space"
},
{
"text": "x",
"kind": "propertyName"
},
{
"text": ":",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "string",
"kind": "keyword"
},
{
"text": ";",
"kind": "punctuation"
},
{
"text": "\n",
"kind": "lineBreak"
},
{
"text": " ",
"kind": "space"
},
{
"text": "y",
"kind": "propertyName"
},
{
"text": "?",
"kind": "punctuation"
},
{
"text": ":",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "string",
"kind": "keyword"
},
{
"text": ";",
"kind": "punctuation"
},
{
"text": "\n",
"kind": "lineBreak"
},
{
"text": " ",
"kind": "space"
},
{
"text": "z",
"kind": "propertyName"
},
{
"text": "?",
"kind": "punctuation"
},
{
"text": ":",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "string",
"kind": "keyword"
},
{
"text": ";",
"kind": "punctuation"
},
{
"text": "\n",
"kind": "lineBreak"
},
{
"text": " ",
"kind": "space"
},
{
"text": "w",
"kind": "propertyName"
},
{
"text": "?",
"kind": "punctuation"
},
{
"text": ":",
"kind": "punctuation"
},
{
"text": " ",
"kind": "space"
},
{
"text": "string",
"kind": "keyword"
},
{
"text": ";",
"kind": "punctuation"
},
{
"text": "\n",
"kind": "lineBreak"
},
{
"text": "}",
"kind": "punctuation"
"text": "Opts",
"kind": "aliasName"
}
],
"documentation": [],
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/jsdocTemplateConstructorFunction2.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ z.u = false
*/
/** @type {A} */
const options = { value: null };
>options : { value: any; }
>options : A
>{ value: null } : { value: null; }
>value : null
>null : null
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/jsdocTypedefMissingType.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const t = 0;

/** @type Person */
const person = { name: "" };
>person : { name: string; }
>person : Person
>{ name: "" } : { name: string; }
>name : string
>"" : ""
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/jsdocTypedef_propertyWithNoType.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

/** @type {Foo} */
const x = { foo: 0 };
>x : { foo: any; }
>x : Foo
>{ foo: 0 } : { foo: number; }
>foo : number
>0 : 0
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/typedefTagNested.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ var ex;

/** @type {App} */
const app = {
>app : { name: string; icons: { image32: string; image64: string; }; }
>app : App
>{ name: 'name', icons: { image32: 'x.png', image64: 'y.png', }} : { name: string; icons: { image32: string; image64: string; }; }

name: 'name',
Expand Down Expand Up @@ -40,5 +40,5 @@ const app = {

/** @type {Opp} */
var mistake;
>mistake : { name: string; oops: { horrible: string; }; }
>mistake : Opp

Loading