Skip to content

Refactor jsdoc types to typescript #18747

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.

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 19 commits into from
Oct 13, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -3703,5 +3703,13 @@
"Extract to {0}": {
"category": "Message",
"code": 95004
},
"Annotate with type from JSDoc": {
"category": "Message",
"code": 95005
},
"Annotate with return type from JSDoc": {
"category": "Message",
"code": 95006
}
}
46 changes: 43 additions & 3 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -545,6 +545,7 @@ namespace ts {
case SyntaxKind.TypeReference:
return emitTypeReference(<TypeReferenceNode>node);
case SyntaxKind.FunctionType:
case SyntaxKind.JSDocFunctionType:
return emitFunctionType(<FunctionTypeNode>node);
case SyntaxKind.ConstructorType:
return emitConstructorType(<ConstructorTypeNode>node);
Expand Down Expand Up @@ -574,6 +575,18 @@ namespace ts {
return emitMappedType(<MappedTypeNode>node);
case SyntaxKind.LiteralType:
return emitLiteralType(<LiteralTypeNode>node);
case SyntaxKind.JSDocAllType:
case SyntaxKind.JSDocUnknownType:
write("any");
break;
case SyntaxKind.JSDocNullableType:
return emitJSDocNullableType(node as JSDocNullableType);
case SyntaxKind.JSDocNonNullableType:
return emitJSDocNonNullableType(node as JSDocNonNullableType);
case SyntaxKind.JSDocOptionalType:
return emitJSDocOptionalType(node as JSDocOptionalType);
case SyntaxKind.JSDocVariadicType:
return emitJSDocVariadicType(node as JSDocVariadicType);

// Binding patterns
case SyntaxKind.ObjectBindingPattern:
Expand Down Expand Up @@ -914,7 +927,15 @@ namespace ts {
emitDecorators(node, node.decorators);
emitModifiers(node, node.modifiers);
emitIfPresent(node.dotDotDotToken);
emit(node.name);
if (node.name) {
emit(node.name);
}
else if (node.parent.kind === SyntaxKind.JSDocFunctionType) {
const i = (node.parent as JSDocFunctionType).parameters.indexOf(node);
if (i > -1) {
write("arg" + i);
}
}
emitIfPresent(node.questionToken);
emitWithPrefix(": ", node.type);
emitExpressionWithPrefix(" = ", node.initializer);
Expand Down Expand Up @@ -1035,6 +1056,20 @@ namespace ts {
emit(node.type);
}

function emitJSDocNullableType(node: JSDocNullableType) {
emit(node.type);
write(" | null");
}

function emitJSDocNonNullableType(node: JSDocNonNullableType) {
emit(node.type);
}

function emitJSDocOptionalType(node: JSDocOptionalType) {
emit(node.type);
write(" | undefined");
}

function emitConstructorType(node: ConstructorTypeNode) {
write("new ");
emitTypeParameters(node, node.typeParameters);
Expand All @@ -1060,6 +1095,11 @@ namespace ts {
write("[]");
}

function emitJSDocVariadicType(node: JSDocVariadicType) {
emit(node.type);
write("[]");
}

function emitTupleType(node: TupleTypeNode) {
write("[");
emitList(node, node.elementTypes, ListFormat.TupleTypeElements);
Expand Down Expand Up @@ -2357,7 +2397,7 @@ namespace ts {
emitList(parentNode, parameters, ListFormat.Parameters);
}

function canEmitSimpleArrowHead(parentNode: FunctionTypeNode | ArrowFunction, parameters: NodeArray<ParameterDeclaration>) {
function canEmitSimpleArrowHead(parentNode: FunctionTypeNode | ArrowFunction | JSDocFunctionType, parameters: NodeArray<ParameterDeclaration>) {
const parameter = singleOrUndefined(parameters);
return parameter
&& parameter.pos === parentNode.pos // may not have parsed tokens between parent and parameter
Expand All @@ -2374,7 +2414,7 @@ namespace ts {
&& isIdentifier(parameter.name); // parameter name must be identifier
}

function emitParametersForArrow(parentNode: FunctionTypeNode | ArrowFunction, parameters: NodeArray<ParameterDeclaration>) {
function emitParametersForArrow(parentNode: FunctionTypeNode | ArrowFunction | JSDocFunctionType, parameters: NodeArray<ParameterDeclaration>) {
if (canEmitSimpleArrowHead(parentNode, parameters)) {
emitList(parentNode, parameters, ListFormat.Parameters & ~ListFormat.Parenthesis);
}
Expand Down
9 changes: 8 additions & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5034,7 +5034,14 @@ namespace ts {
|| kind === SyntaxKind.UndefinedKeyword
|| kind === SyntaxKind.NullKeyword
|| kind === SyntaxKind.NeverKeyword
|| kind === SyntaxKind.ExpressionWithTypeArguments;
|| kind === SyntaxKind.ExpressionWithTypeArguments
|| kind === SyntaxKind.JSDocAllType
|| kind === SyntaxKind.JSDocUnknownType
|| kind === SyntaxKind.JSDocNullableType
|| kind === SyntaxKind.JSDocNonNullableType
|| kind === SyntaxKind.JSDocOptionalType
|| kind === SyntaxKind.JSDocFunctionType
|| kind === SyntaxKind.JSDocVariadicType;
}

/**
Expand Down
147 changes: 147 additions & 0 deletions src/services/refactors/annotateWithTypeFromJSDoc.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/* @internal */
namespace ts.refactor.annotateWithTypeFromJSDoc {
const actionName = "annotate";

const annotateTypeFromJSDoc: Refactor = {
name: "Annotate with type from JSDoc",
description: Diagnostics.Annotate_with_type_from_JSDoc.message,
getEditsForAction,
getAvailableActions
};
const annotateReturnTypeFromJSDoc: Refactor = {
name: "Annotate with return type from JSDoc",
description: Diagnostics.Annotate_with_return_type_from_JSDoc.message,
getEditsForAction,
getAvailableActions
};

type DeclarationWithType =
| FunctionLikeDeclaration
| VariableDeclaration
| ParameterDeclaration
| PropertySignature
| PropertyDeclaration;

registerRefactor(annotateTypeFromJSDoc);
registerRefactor(annotateReturnTypeFromJSDoc);

function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
if (isInJavaScriptFile(context.file)) {
return undefined;
}

const node = getTokenAtPosition(context.file, context.startPosition, /*includeJsDocComment*/ false);
const decl = findAncestor(node, isTypedNode);
if (decl && !decl.type) {
Copy link
Member

Choose a reason for hiding this comment

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

Can you use early bailouts to reduce nesting here?

const annotate = getJSDocType(decl) ? annotateTypeFromJSDoc :
getJSDocReturnType(decl) ? annotateReturnTypeFromJSDoc :
undefined;
if (annotate) {
return [{
name: annotate.name,
description: annotate.description,
actions: [
{
description: annotate.description,
name: actionName
}
]
}];
}
}
}

function getEditsForAction(context: RefactorContext, action: string): RefactorEditInfo | undefined {
// Somehow wrong action got invoked?
if (actionName !== action) {
Debug.fail(`actionName !== action: ${actionName} !== ${action}`);
return undefined;
Copy link
Member

Choose a reason for hiding this comment

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

return Debug.fail(...)?

}

const start = context.startPosition;
const sourceFile = context.file;
const token = getTokenAtPosition(sourceFile, start, /*includeJsDocComment*/ false);
const decl = findAncestor(token, isTypedNode);
const jsdocType = getJSDocType(decl);
const jsdocReturn = getJSDocReturnType(decl);
if (!decl || !jsdocType && !jsdocReturn || decl.type) {
Debug.fail(`!decl || !jsdocType && !jsdocReturn || decl.type: !${decl} || !${jsdocType} && !{jsdocReturn} || ${decl.type}`);
return undefined;
}

const changeTracker = textChanges.ChangeTracker.fromContext(context);
if (isParameterOfSimpleArrowFunction(decl)) {
// `x => x` becomes `(x: number) => x`, but in order to make the changeTracker generate the parentheses,
// we have to replace the entire function; it doesn't check that the node it's replacing might require
// other syntax changes
const arrow = decl.parent as ArrowFunction;
const param = decl as ParameterDeclaration;
const replacementParam = createParameter(param.decorators, param.modifiers, param.dotDotDotToken, param.name, param.questionToken, jsdocType, param.initializer);
const replacement = createArrowFunction(arrow.modifiers, arrow.typeParameters, [replacementParam], arrow.type, arrow.equalsGreaterThanToken, arrow.body);
changeTracker.replaceRange(sourceFile, { pos: arrow.getStart(), end: arrow.end }, replacement);
}
else {
changeTracker.replaceRange(sourceFile, { pos: decl.getStart(), end: decl.end }, replaceType(decl, jsdocType, jsdocReturn));
}
return {
edits: changeTracker.getChanges(),
renameFilename: undefined,
renameLocation: undefined
};
}

function isTypedNode(node: Node): node is DeclarationWithType {
Copy link
Contributor

Choose a reason for hiding this comment

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

TypeNode has a rather well defined meaning in other parts of the system. so i would make it isDeclarationWithType instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, that name is much better.

return isFunctionLikeDeclaration(node) ||
node.kind === SyntaxKind.VariableDeclaration ||
node.kind === SyntaxKind.Parameter ||
node.kind === SyntaxKind.PropertySignature ||
node.kind === SyntaxKind.PropertyDeclaration;
}

function replaceType(decl: DeclarationWithType, jsdocType: TypeNode, jsdocReturn: TypeNode) {
switch (decl.kind) {
case SyntaxKind.VariableDeclaration:
return createVariableDeclaration(decl.name, jsdocType, decl.initializer);
case SyntaxKind.Parameter:
return createParameter(decl.decorators, decl.modifiers, decl.dotDotDotToken, decl.name, decl.questionToken, jsdocType, decl.initializer);
case SyntaxKind.PropertySignature:
return createPropertySignature(decl.modifiers, decl.name, decl.questionToken, jsdocType, decl.initializer);
case SyntaxKind.PropertyDeclaration:
return createProperty(decl.decorators, decl.modifiers, decl.name, decl.questionToken, jsdocType, decl.initializer);
case SyntaxKind.FunctionDeclaration:
return createFunctionDeclaration(decl.decorators, decl.modifiers, decl.asteriskToken, decl.name, decl.typeParameters, decl.parameters, jsdocReturn, decl.body);
case SyntaxKind.FunctionExpression:
return createFunctionExpression(decl.modifiers, decl.asteriskToken, decl.name, decl.typeParameters, decl.parameters, jsdocReturn, decl.body);
case SyntaxKind.ArrowFunction:
return createArrowFunction(decl.modifiers, decl.typeParameters, decl.parameters, jsdocReturn, decl.equalsGreaterThanToken, decl.body);
case SyntaxKind.MethodDeclaration:
return createMethod(decl.decorators, decl.modifiers, decl.asteriskToken, decl.name, decl.questionToken, decl.typeParameters, decl.parameters, jsdocReturn, decl.body);
case SyntaxKind.GetAccessor:
return createGetAccessor(decl.decorators, decl.modifiers, decl.name, decl.parameters, jsdocReturn, decl.body);
default:
Debug.fail(`Unexpected SyntaxKind: ${decl.kind}`);
return undefined;
}
}

function isParameterOfSimpleArrowFunction(decl: DeclarationWithType) {
return decl.kind === SyntaxKind.Parameter && decl.parent.kind === SyntaxKind.ArrowFunction && isSimpleArrowFunction(decl.parent);
}

function isSimpleArrowFunction(parentNode: FunctionTypeNode | ArrowFunction | JSDocFunctionType) {
const parameter = singleOrUndefined(parentNode.parameters);
return parameter
&& parameter.pos === parentNode.pos // may not have parsed tokens between parent and parameter
&& !(isArrowFunction(parentNode) && parentNode.type) // arrow function may not have return type annotation
&& !some(parentNode.decorators) // parent may not have decorators
&& !some(parentNode.modifiers) // parent may not have modifiers
&& !some(parentNode.typeParameters) // parent may not have type parameters
&& !some(parameter.decorators) // parameter may not have decorators
&& !some(parameter.modifiers) // parameter may not have modifiers
&& !parameter.dotDotDotToken // parameter may not be rest
&& !parameter.questionToken // parameter may not be optional
&& !parameter.type // parameter may not have a type annotation
&& !parameter.initializer // parameter may not have an initializer
&& isIdentifier(parameter.name); // parameter name must be identifier
}
}
1 change: 1 addition & 0 deletions src/services/refactors/refactors.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/// <reference path="annotateWithTypeFromJSDoc.ts" />
/// <reference path="convertFunctionToEs6Class.ts" />
/// <reference path="extractMethod.ts" />
10 changes: 10 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference path='fourslash.ts' />

// @Filename: test123.ts
/////** @type {number} */
////var /*1*/x;

verify.applicableRefactorAvailableAtMarker('1');
verify.fileAfterApplyingRefactorAtMarker('1',
`/** @type {number} */
var x: number;`, 'Annotate with type from JSDoc', 'annotate');
15 changes: 15 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc10.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/// <reference path='fourslash.ts' />

/////**
//// * @param {?} x
//// * @returns {number}
//// */
////var f = /*1*/(/*2*/x) => x

verify.applicableRefactorAvailableAtMarker('1');
verify.fileAfterApplyingRefactorAtMarker('1',
`/**
* @param {?} x
* @returns {number}
*/
var f = (x): number => x`, 'Annotate with return type from JSDoc', 'annotate');
15 changes: 15 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc11.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/// <reference path='fourslash.ts' />

/////**
//// * @param {?} x
//// * @returns {number}
//// */
////var f = /*1*/(/*2*/x) => x

verify.applicableRefactorAvailableAtMarker('2');
verify.fileAfterApplyingRefactorAtMarker('2',
`/**
* @param {?} x
* @returns {number}
*/
var f = (x: any) => x`, 'Annotate with type from JSDoc', 'annotate');
21 changes: 21 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc12.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/// <reference path='fourslash.ts' />

////class C {
//// /**
//// * @return {...*}
//// */
//// /*1*/m(x) {
//// }
////}
verify.applicableRefactorAvailableAtMarker('1');
verify.fileAfterApplyingRefactorAtMarker('1',
`class C {
/**
* @return {...*}
*/
/**
* @return {...*}
*/
m(x): any[] {
}
}`, 'Annotate with return type from JSDoc', 'annotate');
11 changes: 11 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc13.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />
////class C {
//// /** @return {number} */
//// get /*1*/c() { return 12 }
////}
verify.applicableRefactorAvailableAtMarker('1');
verify.fileAfterApplyingRefactorAtMarker('1',
`class C {
/** @return {number} */
get c(): number { return 12; }
}`, 'Annotate with return type from JSDoc', 'annotate');
11 changes: 11 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc14.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// <reference path='fourslash.ts' />
/////** @return {number} */
////function f() {
//// /*1*/return 12;
////}
verify.applicableRefactorAvailableAtMarker('1');
verify.fileAfterApplyingRefactorAtMarker('1',
`/** @return {number} */
function f(): number {
return 12;
}`, 'Annotate with return type from JSDoc', 'annotate');
6 changes: 6 additions & 0 deletions tests/cases/fourslash/annotateWithTypeFromJSDoc2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/// <reference path='fourslash.ts' />

// @Filename: test123.ts
/////** @type {number} */
////var /*1*/x: string;
verify.not.applicableRefactorAvailableAtMarker('1');
Loading