Skip to content

Fix type when annotated with a JSDoc function type #22692

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

Merged
merged 7 commits into from
Mar 19, 2018
Merged

Fix type when annotated with a JSDoc function type #22692

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aeaf83c
Fix type when annotated with a JSDoc function type
sandersn Mar 19, 2018
d79b4df
Update baselines and add constructor test
sandersn Mar 19, 2018
34c977e
Handle ConstructorType too
sandersn Mar 19, 2018
dfbae12
Add test:method sig inside literal type
sandersn Mar 19, 2018
1f6106b
Contextually type parameters by parent sig's JSDoc
sandersn Mar 19, 2018
e09bd63
Remove redundant check:isUntypedSignatureInJSFile
sandersn Mar 19, 2018
deba8e2
Positive check for value signatures
sandersn Mar 19, 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
8 changes: 7 additions & 1 deletion src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6699,7 +6699,13 @@ namespace ts {
let hasThisParameter: boolean;
const iife = getImmediatelyInvokedFunctionExpression(declaration);
const isJSConstructSignature = isJSDocConstructSignature(declaration);
const isUntypedSignatureInJSFile = !iife && !isJSConstructSignature && isInJavaScriptFile(declaration) && !hasJSDocParameterTags(declaration);
const isUntypedSignatureInJSFile = !iife &&
Copy link
Member

@weswigham weswigham Mar 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

What case is handling when the declaration is an object literal with call/construct signatures, (ie, {(): void})?

Copy link
Member Author

Choose a reason for hiding this comment

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

!getJSDocType(declaration). I'll add a test case. Note that the new code in getJSDocType doesn't look deep enough to get type of the parameter out of the method declaration inside the object literal type.

!isJSConstructSignature &&
Copy link

Choose a reason for hiding this comment

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

!isJSConstructSignature is redundant if we test declaration.kind !== SyntaxKind.FunctionType anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch. Removed.

isInJavaScriptFile(declaration) &&
Copy link
Member

Choose a reason for hiding this comment

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

Rather than skipping (some of the) the type-space signature declaration kinds, can we just check if it is a concrete declaration kind? That is the point here, right; to exclude type-space declaration kinds?

export type ConcreteSignatureDeclaration =
        | FunctionDeclaration
        | MethodDeclaration
        | ConstructorDeclaration
        | AccessorDeclaration
        | FunctionExpression
        | ArrowFunction;

export function isConcreteSignatureDeclaration(declaration: Node): node is ConcreteSignatureDeclaration {
    return isFunctionExpression(declaration) || isArrowFunction(declaration) || isMethodOrAccessor(declaration) || isFunctionDeclaration(declaration) || isConstructorDeclaration(declaration);
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, a type signature is by definition not untyped, even if it's in a JS file. I used your code in the new commit.

declaration.kind !== SyntaxKind.FunctionType &&
Copy link
Member

Choose a reason for hiding this comment

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

ConstructorType?

declaration.kind !== SyntaxKind.JSDocFunctionType &&
!hasJSDocParameterTags(declaration) &&
!getJSDocType(declaration);

// If this is a JSDoc construct signature, then skip the first parameter in the
// parameter list. The first parameter represents the return type of the construct
Expand Down
9 changes: 9 additions & 0 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4510,6 +4510,15 @@ namespace ts {
let tag: JSDocTypeTag | JSDocParameterTag | undefined = getFirstJSDocTag(node, isJSDocTypeTag);
if (!tag && isParameter(node)) {
tag = find(getJSDocParameterTags(node), tag => !!tag.typeExpression);
if (!tag && isFunctionLike(node.parent)) {
const typeTag = getJSDocTypeTag(node.parent);
if (typeTag && typeTag.typeExpression && isFunctionLike(typeTag.typeExpression.type)) {
const i = node.parent.parameters.indexOf(node);
if (i > -1) {
return typeTag.typeExpression.type.parameters[i].type;
Copy link

Choose a reason for hiding this comment

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

Needs a length check -- we don't know that typeTag.typeExpression.type has the same # parameters as the actual function.

}
}
}
}

return tag && tag.typeExpression && tag.typeExpression.type;
Expand Down
7 changes: 7 additions & 0 deletions tests/baselines/reference/jsdocTypeTag.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ var obj;

/** @type {Function} */
var Func;

/** @type {(s: string) => number} */
var f;

//// [b.ts]
var S: string;
Expand All @@ -82,6 +85,7 @@ var nullable: number | null;
var Obj: any;
var obj: any;
var Func: Function;
var f: (s: string) => number;


//// [a.js]
Expand Down Expand Up @@ -125,6 +129,8 @@ var Obj;
var obj;
/** @type {Function} */
var Func;
/** @type {(s: string) => number} */
var f;
//// [b.js]
var S;
var s;
Expand All @@ -146,3 +152,4 @@ var nullable;
var Obj;
var obj;
var Func;
var f;
8 changes: 8 additions & 0 deletions tests/baselines/reference/jsdocTypeTag.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ var obj;
var Func;
>Func : Symbol(Func, Decl(a.js, 58, 3), Decl(b.ts, 19, 3))

/** @type {(s: string) => number} */
var f;
>f : Symbol(f, Decl(a.js, 61, 3), Decl(b.ts, 20, 3))

=== tests/cases/conformance/jsdoc/b.ts ===
var S: string;
>S : Symbol(S, Decl(a.js, 1, 3), Decl(b.ts, 0, 3))
Expand Down Expand Up @@ -143,3 +147,7 @@ var Func: Function;
>Func : Symbol(Func, Decl(a.js, 58, 3), Decl(b.ts, 19, 3))
>Function : Symbol(Function, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))

var f: (s: string) => number;
>f : Symbol(f, Decl(a.js, 61, 3), Decl(b.ts, 20, 3))
>s : Symbol(s, Decl(b.ts, 20, 8))

8 changes: 8 additions & 0 deletions tests/baselines/reference/jsdocTypeTag.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ var obj;
var Func;
>Func : Function

/** @type {(s: string) => number} */
var f;
>f : (s: string) => number

=== tests/cases/conformance/jsdoc/b.ts ===
var S: string;
>S : string
Expand Down Expand Up @@ -146,3 +150,7 @@ var Func: Function;
>Func : Function
>Function : Function

var f: (s: string) => number;
>f : (s: string) => number
>s : string

18 changes: 18 additions & 0 deletions tests/baselines/reference/jsdocTypeTagParameterType.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
tests/cases/conformance/jsdoc/a.js(3,5): error TS2322: Type '1' is not assignable to type 'string'.
tests/cases/conformance/jsdoc/a.js(7,5): error TS2322: Type '1' is not assignable to type 'string'.


==== tests/cases/conformance/jsdoc/a.js (2 errors) ====
/** @type {function(string): void} */
const f = (value) => {
value = 1 // should error
~~~~~
!!! error TS2322: Type '1' is not assignable to type 'string'.
};
/** @type {(s: string) => void} */
function g(s) {
s = 1 // Should error
~
!!! error TS2322: Type '1' is not assignable to type 'string'.
}

19 changes: 19 additions & 0 deletions tests/baselines/reference/jsdocTypeTagParameterType.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
=== tests/cases/conformance/jsdoc/a.js ===
/** @type {function(string): void} */
const f = (value) => {
>f : Symbol(f, Decl(a.js, 1, 5))
>value : Symbol(value, Decl(a.js, 1, 11))

value = 1 // should error
>value : Symbol(value, Decl(a.js, 1, 11))

};
/** @type {(s: string) => void} */
function g(s) {
>g : Symbol(g, Decl(a.js, 3, 2))
>s : Symbol(s, Decl(a.js, 5, 11))

s = 1 // Should error
>s : Symbol(s, Decl(a.js, 5, 11))
}

24 changes: 24 additions & 0 deletions tests/baselines/reference/jsdocTypeTagParameterType.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
=== tests/cases/conformance/jsdoc/a.js ===
/** @type {function(string): void} */
const f = (value) => {
>f : (arg0: string) => void
>(value) => { value = 1 // should error} : (value: string) => void
>value : string

value = 1 // should error
>value = 1 : 1
>value : string
>1 : 1

};
/** @type {(s: string) => void} */
function g(s) {
>g : (s: string) => void
>s : string

s = 1 // Should error
>s = 1 : 1
>s : string
>1 : 1
}

19 changes: 19 additions & 0 deletions tests/baselines/reference/jsdocTypeTagRequiredParameters.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
tests/cases/conformance/jsdoc/a.js(8,1): error TS2554: Expected 1 arguments, but got 0.
tests/cases/conformance/jsdoc/a.js(9,1): error TS2554: Expected 1 arguments, but got 0.


==== tests/cases/conformance/jsdoc/a.js (2 errors) ====
/** @type {function(string): void} */
const f = (value) => {
};
/** @type {(s: string) => void} */
function g(s) {
}

f() // should error
~~~
!!! error TS2554: Expected 1 arguments, but got 0.
g() // should error
~~~
!!! error TS2554: Expected 1 arguments, but got 0.

19 changes: 19 additions & 0 deletions tests/baselines/reference/jsdocTypeTagRequiredParameters.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
=== tests/cases/conformance/jsdoc/a.js ===
/** @type {function(string): void} */
const f = (value) => {
>f : Symbol(f, Decl(a.js, 1, 5))
>value : Symbol(value, Decl(a.js, 1, 11))

};
/** @type {(s: string) => void} */
function g(s) {
>g : Symbol(g, Decl(a.js, 2, 2))
>s : Symbol(s, Decl(a.js, 4, 11))
}

f() // should error
>f : Symbol(f, Decl(a.js, 1, 5))

g() // should error
>g : Symbol(g, Decl(a.js, 2, 2))

22 changes: 22 additions & 0 deletions tests/baselines/reference/jsdocTypeTagRequiredParameters.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
=== tests/cases/conformance/jsdoc/a.js ===
/** @type {function(string): void} */
const f = (value) => {
>f : (arg0: string) => void
>(value) => {} : (value: string) => void
>value : string

};
/** @type {(s: string) => void} */
function g(s) {
>g : (s: string) => void
>s : string
}

f() // should error
>f() : void
>f : (arg0: string) => void

g() // should error
>g() : void
>g : (s: string) => void

4 changes: 4 additions & 0 deletions tests/cases/conformance/jsdoc/jsdocTypeTag.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ var obj;
/** @type {Function} */
var Func;

/** @type {(s: string) => number} */
var f;

// @filename: b.ts
var S: string;
var s: string;
Expand All @@ -84,3 +87,4 @@ var nullable: number | null;
var Obj: any;
var obj: any;
var Func: Function;
var f: (s: string) => number;
15 changes: 15 additions & 0 deletions tests/cases/conformance/jsdoc/jsdocTypeTagParameterType.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// @allowJs: true
// @checkJs: true
// @noEmit: true
// @strict: true
// @noImplicitAny: true
// @Filename: a.js

/** @type {function(string): void} */
const f = (value) => {
value = 1 // should error
};
/** @type {(s: string) => void} */
function g(s) {
s = 1 // Should error
}
16 changes: 16 additions & 0 deletions tests/cases/conformance/jsdoc/jsdocTypeTagRequiredParameters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// @allowJs: true
// @checkJs: true
// @noEmit: true
// @strict: true
// @noImplicitAny: true
// @Filename: a.js

/** @type {function(string): void} */
const f = (value) => {
};
/** @type {(s: string) => void} */
function g(s) {
}

f() // should error
g() // should error