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

Add @callback jsdoc tag #9546

Closed
wants to merge 4 commits into from
Closed

Add @callback jsdoc tag #9546

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
0d28763
Add support for @callback jsdoc tag
zhengbli Jul 7, 2016
d67dea3
Add more tests for @callback tag
zhengbli Jul 7, 2016
216f364
Merge branch 'master' of https://github.com/Microsoft/TypeScript into…
zhengbli Jul 7, 2016
5cdce53
PR feedback
zhengbli Jul 7, 2016
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
32 changes: 28 additions & 4 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ namespace ts {
// other kinds of value declarations take precedence over modules
symbol.valueDeclaration = node;
}
}
}
}

// Should not be called on a declaration with a computed property name,
Expand All @@ -226,6 +226,7 @@ namespace ts {
return "__constructor";
case SyntaxKind.FunctionType:
case SyntaxKind.CallSignature:
case SyntaxKind.JSDocCallbackType:
return "__call";
case SyntaxKind.ConstructorType:
case SyntaxKind.ConstructSignature:
Expand Down Expand Up @@ -264,6 +265,17 @@ namespace ts {
let functionType = <JSDocFunctionType>node.parent;
let index = indexOf(functionType.parameters, node);
return "p" + index;
case SyntaxKind.JSDocParameterTag:
// JSDocParameterTag can be used for either function parameter type notation
// or in @callback tag declaration. We only create new symbol when the JSDocParameterTag
// is part of a @callback tag declaration.
Debug.assert(node.parent.kind === SyntaxKind.JSDocCallbackType, "Should not bind JSDocParameterTag if it is not part of a JSDocCallbackTag");
const preOrPostParameterName =(<JSDocParameterTag>node).preParameterName || (<JSDocParameterTag>node).postParameterName;
if (preOrPostParameterName) {
return preOrPostParameterName.text;
}
let jsDocCallbackType = <JSDocCallbackType>node.parent;
return "p" + indexOf(jsDocCallbackType.parameterTags, node);
Copy link
Member

@sandersn sandersn Sep 14, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

use "arg" instead of "p". That's what Roslyn does, and what my JSDoc parser overhaul does for JSDocFunctionType too.

case SyntaxKind.JSDocTypedefTag:
const parentNode = node.parent && node.parent.parent;
let nameFromParentNode: string;
Expand Down Expand Up @@ -1199,6 +1211,7 @@ namespace ts {
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
case SyntaxKind.CallSignature:
case SyntaxKind.JSDocCallbackType:
case SyntaxKind.ConstructSignature:
case SyntaxKind.IndexSignature:
case SyntaxKind.FunctionType:
Expand Down Expand Up @@ -1289,6 +1302,7 @@ namespace ts {
return declareSymbol(container.symbol.members, container.symbol, node, symbolFlags, symbolExcludes);

case SyntaxKind.FunctionType:
case SyntaxKind.JSDocCallbackType:
case SyntaxKind.ConstructorType:
case SyntaxKind.CallSignature:
case SyntaxKind.ConstructSignature:
Expand Down Expand Up @@ -1403,12 +1417,12 @@ namespace ts {
}
}

function bindFunctionOrConstructorType(node: SignatureDeclaration): void {
function bindFunctionOrConstructorType(node: SignatureDeclaration | JSDocCallbackType): void {
// For a given function symbol "<...>(...) => T" we want to generate a symbol identical
// to the one we would get for: { <...>(...): T }
//
// We do that by making an anonymous type literal symbol, and then setting the function
// symbol as its sole member. To the rest of the system, this symbol will be indistinguishable
// symbol as its sole member. To the rest of the system, this symbol will be indistinguishable
// from an actual type literal symbol you would have gotten had you used the long form.
const symbol = createSymbol(SymbolFlags.Signature, getDeclarationName(node));
addDeclarationToSymbol(symbol, node, SymbolFlags.Signature);
Expand Down Expand Up @@ -1820,7 +1834,8 @@ namespace ts {
case SyntaxKind.FunctionType:
case SyntaxKind.ConstructorType:
case SyntaxKind.JSDocFunctionType:
return bindFunctionOrConstructorType(<SignatureDeclaration>node);
case SyntaxKind.JSDocCallbackType:
return bindFunctionOrConstructorType(<SignatureDeclaration | JSDocCallbackType>node);
case SyntaxKind.TypeLiteral:
case SyntaxKind.JSDocTypeLiteral:
case SyntaxKind.JSDocRecordType:
Expand All @@ -1846,8 +1861,13 @@ namespace ts {
case SyntaxKind.InterfaceDeclaration:
return bindBlockScopedDeclaration(<Declaration>node, SymbolFlags.Interface, SymbolFlags.InterfaceExcludes);
case SyntaxKind.JSDocTypedefTag:
case SyntaxKind.JSDocCallbackTag:
case SyntaxKind.TypeAliasDeclaration:
return bindBlockScopedDeclaration(<Declaration>node, SymbolFlags.TypeAlias, SymbolFlags.TypeAliasExcludes);
case SyntaxKind.JSDocParameterTag:
return container.kind === SyntaxKind.JSDocCallbackType
Copy link
Member

Choose a reason for hiding this comment

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

I suspect that this should just be an assert(kind === JSDocCallbackType).

? bindJSDocParamTag(<JSDocParameterTag>node)
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this is needed. Doesn't the addition of JSDocCallbackTag on line 1864 take care of it since callback tags have their parameter tags as children?

: undefined;
case SyntaxKind.EnumDeclaration:
return bindEnumDeclaration(<EnumDeclaration>node);
case SyntaxKind.ModuleDeclaration:
Expand Down Expand Up @@ -2113,6 +2133,10 @@ namespace ts {
}
}

function bindJSDocParamTag(node: JSDocParameterTag) {
declareSymbolAndAddToSymbolTable(node, SymbolFlags.FunctionScopedVariable, SymbolFlags.ParameterExcludes);
}

function bindParameter(node: ParameterDeclaration) {
if (!isDeclarationFile(file) &&
!isInAmbientContext(node) &&
Expand Down
118 changes: 79 additions & 39 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2391,7 +2391,7 @@ namespace ts {
}

function buildParameterDisplay(p: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags, symbolStack?: Symbol[]) {
const parameterNode = <ParameterDeclaration>p.valueDeclaration;
const parameterNode = <ParameterDeclaration | JSDocParameterTag>p.valueDeclaration;
if (isRestParameter(parameterNode)) {
writePunctuation(writer, SyntaxKind.DotDotDotToken);
}
Expand Down Expand Up @@ -3178,6 +3178,14 @@ namespace ts {
type = checkExpressionCached((<BinaryExpression>declaration.parent).right);
}
}
else if (declaration.kind == SyntaxKind.JSDocParameterTag) {
Copy link
Member

Choose a reason for hiding this comment

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

when can this happen?

if ((<JSDocParameterTag>declaration).typeExpression) {
const jsDocType = (<JSDocParameterTag>declaration).typeExpression.type;
if (jsDocType) {
type = getTypeFromTypeNode(jsDocType);
}
}
}

if (type === undefined) {
type = getWidenedTypeForVariableLikeDeclaration(<VariableLikeDeclaration>declaration, /*reportErrors*/ true);
Expand Down Expand Up @@ -3653,18 +3661,23 @@ namespace ts {
}

let type: Type;
let declaration: JSDocTypedefTag | TypeAliasDeclaration = <JSDocTypedefTag>getDeclarationOfKind(symbol, SyntaxKind.JSDocTypedefTag);
if (declaration) {
if (declaration.jsDocTypeLiteral) {
type = getTypeFromTypeNode(declaration.jsDocTypeLiteral);
let declaration: Declaration;
if (declaration = getDeclarationOfKind(symbol, SyntaxKind.JSDocTypedefTag)) {
const jsDocTypedefTag = <JSDocTypedefTag>declaration;
if (jsDocTypedefTag.jsDocTypeLiteral) {
type = getTypeFromTypeNode(jsDocTypedefTag.jsDocTypeLiteral);
}
else {
type = getTypeFromTypeNode(declaration.typeExpression.type);
else if (jsDocTypedefTag.typeExpression) {
type = getTypeFromTypeNode(jsDocTypedefTag.typeExpression.type);
}
}
else if (declaration = getDeclarationOfKind(symbol, SyntaxKind.JSDocCallbackTag)) {
const jsDocCallbackTag = <JSDocCallbackTag>declaration;
type = getTypeFromTypeNode(jsDocCallbackTag.type);
}
else {
declaration = <TypeAliasDeclaration>getDeclarationOfKind(symbol, SyntaxKind.TypeAliasDeclaration);
type = getTypeFromTypeNode(declaration.type);
declaration = getDeclarationOfKind(symbol, SyntaxKind.TypeAliasDeclaration);
type = getTypeFromTypeNode((<TypeAliasDeclaration>declaration).type);
}

if (popTypeResolution()) {
Expand Down Expand Up @@ -3906,7 +3919,7 @@ namespace ts {
resolveObjectTypeMembers(type, source, typeParameters, typeArguments);
}

function createSignature(declaration: SignatureDeclaration, typeParameters: TypeParameter[], thisParameter: Symbol | undefined, parameters: Symbol[],
function createSignature(declaration: SignatureDeclaration | JSDocCallbackType, typeParameters: TypeParameter[], thisParameter: Symbol | undefined, parameters: Symbol[],
resolvedReturnType: Type, typePredicate: TypePredicate, minArgumentCount: number, hasRestParameter: boolean, hasStringLiterals: boolean): Signature {
const sig = new Signature(checker);
sig.declaration = declaration;
Expand Down Expand Up @@ -4421,26 +4434,35 @@ namespace ts {
return result;
}

function isJSDocOptionalParameterTag(node: JSDocParameterTag) {
if (!node) {
Copy link
Member

Choose a reason for hiding this comment

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

I would rewrite this as node && (node.isBracketed || node.typeExpression && node.typeExpression.type && node.typeExpression.type.kind === SyntaxKind.JSDocOptionalType) (with linebreaks inserted as needed).

return false;
}

if (node.isBracketed) {
return true;
}
if (node.typeExpression && node.typeExpression.type) {
return node.typeExpression.type.kind === SyntaxKind.JSDocOptionalType;
}
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Explicitly return false at the bottom

}

function isJSDocOptionalParameter(node: ParameterDeclaration) {
if (node.flags & NodeFlags.JavaScriptFile) {
if (node.type && node.type.kind === SyntaxKind.JSDocOptionalType) {
return true;
}

const paramTag = getCorrespondingJSDocParameterTag(node);
if (paramTag) {
if (paramTag.isBracketed) {
return true;
}

if (paramTag.typeExpression) {
return paramTag.typeExpression.type.kind === SyntaxKind.JSDocOptionalType;
}
}
return isJSDocOptionalParameterTag(getCorrespondingJSDocParameterTag(node));
}
}

function isOptionalParameter(node: ParameterDeclaration) {
function isOptionalParameter(node: ParameterDeclaration | JSDocParameterTag) {
if (isJSDocParameterTag(node)) {
return isJSDocOptionalParameterTag(node);
}

if (hasQuestionToken(node) || isJSDocOptionalParameter(node)) {
return true;
}
Expand Down Expand Up @@ -4474,7 +4496,7 @@ namespace ts {
}
}

function getSignatureFromDeclaration(declaration: SignatureDeclaration): Signature {
function getSignatureFromDeclaration(declaration: SignatureDeclaration | JSDocCallbackType): Signature {
Copy link
Member

Choose a reason for hiding this comment

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

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

const links = getNodeLinks(declaration);
if (!links.resolvedSignature) {
const parameters: Symbol[] = [];
Expand All @@ -4487,8 +4509,9 @@ namespace ts {
// 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
// signature.
for (let i = isJSConstructSignature ? 1 : 0, n = declaration.parameters.length; i < n; i++) {
const param = declaration.parameters[i];
const parameterNodes = isJSDocCallbackType(declaration) ? declaration.parameterTags : declaration.parameters;
for (let i = isJSConstructSignature ? 1 : 0, n = parameterNodes.length; i < n; i++) {
const param = parameterNodes[i];

let paramSymbol = param.symbol;
// Include parameter symbol instead of property symbol in the signature
Expand All @@ -4504,18 +4527,30 @@ namespace ts {
parameters.push(paramSymbol);
}

Copy link
Member

Choose a reason for hiding this comment

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

You already know whether or not you've originated from JSDoc-land or TS-land, so consider caching the value once and branching on that.

if (param.type && param.type.kind === SyntaxKind.StringLiteralType) {
hasStringLiterals = true;
}

if (param.initializer || param.questionToken || param.dotDotDotToken || isJSDocOptionalParameter(param)) {
if (minArgumentCount < 0) {
minArgumentCount = i - (hasThisParameter ? 1 : 0);
if (isJSDocParameterTag(param)) {
if (isJSDocOptionalParameterTag(param)) {
Copy link
Member

Choose a reason for hiding this comment

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

Weirdly enough, this doesn't really affect anything in the type system, since specialized signatures only rejigger the order of overloads, and I don't think these JSDoc tags really cover overloads.

You could consider removing this code, but I'd add a test if you're keeping it in.

Copy link
Member

Choose a reason for hiding this comment

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

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.

if (minArgumentCount < 0) {
minArgumentCount = i - (hasThisParameter ? 1 : 0);
}
}
else {
minArgumentCount = -1;
}
}
else {
// If we see any required parameters, it means the prior ones were not in fact optional.
minArgumentCount = -1;
if (param.type && param.type.kind === SyntaxKind.StringLiteralType) {
hasStringLiterals = true;
}

if (param.initializer || param.questionToken || param.dotDotDotToken || isJSDocOptionalParameter(param)) {
if (minArgumentCount < 0) {
minArgumentCount = i - (hasThisParameter ? 1 : 0);
}
}
else {
// If we see any required parameters, it means the prior ones were not in fact optional.
minArgumentCount = -1;
}
}
}

Expand All @@ -4531,7 +4566,7 @@ namespace ts {
}

if (minArgumentCount < 0) {
minArgumentCount = declaration.parameters.length - (hasThisParameter ? 1 : 0);
minArgumentCount = parameterNodes.length - (hasThisParameter ? 1 : 0);
}
if (isJSConstructSignature) {
minArgumentCount--;
Expand All @@ -4540,11 +4575,14 @@ namespace ts {
const classType = declaration.kind === SyntaxKind.Constructor ?
getDeclaredTypeOfClassOrInterface(getMergedSymbol((<ClassDeclaration>declaration.parent).symbol))
: undefined;
const typeParameters = classType ? classType.localTypeParameters :
declaration.typeParameters ? getTypeParametersFromDeclaration(declaration.typeParameters) :
let typeParameters: TypeParameter[];
if (!isJSDocCallbackType(declaration)) {
typeParameters = classType ? classType.localTypeParameters :
declaration.typeParameters ? getTypeParametersFromDeclaration(declaration.typeParameters) :
getTypeParametersFromJSDocTemplate(declaration);
const returnType = getSignatureReturnTypeFromDeclaration(declaration, minArgumentCount, isJSConstructSignature, classType);
const typePredicate = declaration.type && declaration.type.kind === SyntaxKind.TypePredicate ?
}
const returnType = isJSDocCallbackType(declaration) ? voidType : getSignatureReturnTypeFromDeclaration(declaration, minArgumentCount, isJSConstructSignature, classType);
const typePredicate = !isJSDocCallbackType(declaration) && declaration.type && declaration.type.kind === SyntaxKind.TypePredicate ?
createTypePredicateFromTypePredicateNode(declaration.type as TypePredicateNode) :
undefined;

Expand Down Expand Up @@ -4603,6 +4641,7 @@ namespace ts {
case SyntaxKind.FunctionExpression:
case SyntaxKind.ArrowFunction:
case SyntaxKind.JSDocFunctionType:
case SyntaxKind.JSDocCallbackType:
// Don't include signature if node is the implementation of an overloaded function. A node is considered
// an implementation node if it has a body and the previous node is of the same kind and immediately
// precedes the implementation node (i.e. has the same parent and ends where the implementation starts).
Expand All @@ -4612,7 +4651,7 @@ namespace ts {
break;
}
}
result.push(getSignatureFromDeclaration(<SignatureDeclaration>node));
result.push(getSignatureFromDeclaration(<SignatureDeclaration | JSDocCallbackType>node));
}
}
return result;
Expand Down Expand Up @@ -5357,6 +5396,7 @@ namespace ts {
case SyntaxKind.JSDocTypeLiteral:
case SyntaxKind.JSDocFunctionType:
case SyntaxKind.JSDocRecordType:
case SyntaxKind.JSDocCallbackType:
return getTypeFromTypeLiteralOrFunctionOrConstructorTypeNode(node);
// This function assumes that an identifier or qualified name is a type expression
// Callers should first ensure this by calling isTypeNode
Expand Down
Loading