Skip to content

Commit 0f47c8a

Browse files
author
Andy
authored
annotateWithTypeFromJSDoc: Do less special-casing for arrow functions (#22407)
* annotateWithTypeFromJSDoc: Do less special-casing for arrow functions * Code review
1 parent 2170f6e commit 0f47c8a

File tree

3 files changed

+44
-23
lines changed

3 files changed

+44
-23
lines changed

src/services/codefixes/annotateWithTypeFromJSDoc.ts

+14-20
Original file line numberDiff line numberDiff line change
@@ -42,28 +42,22 @@ namespace ts.codefix {
4242

4343
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, decl: DeclarationWithType): void {
4444
if (isFunctionLikeDeclaration(decl) && (getJSDocReturnType(decl) || decl.parameters.some(p => !!getJSDocType(p)))) {
45-
const typeParameters = getJSDocTypeParameterDeclarations(decl);
46-
const returnType = getJSDocReturnType(decl);
47-
const returnTypeNode = returnType && transformJSDocType(returnType);
48-
49-
if (isArrowFunction(decl) && !findChildOfKind(decl, SyntaxKind.OpenParenToken, sourceFile)) {
50-
const params = decl.parameters.map(p => {
51-
const paramType = getJSDocType(p);
52-
return paramType && !p.type ? updateParameter(p, p.decorators, p.modifiers, p.dotDotDotToken, p.name, p.questionToken, transformJSDocType(paramType), p.initializer) : p;
53-
});
54-
changes.replaceNode(sourceFile, decl, updateArrowFunction(decl, decl.modifiers, decl.typeParameters || typeParameters, params, decl.type || returnTypeNode, decl.equalsGreaterThanToken, decl.body));
45+
if (!decl.typeParameters) {
46+
const typeParameters = getJSDocTypeParameterDeclarations(decl);
47+
if (typeParameters) changes.insertTypeParameters(sourceFile, decl, typeParameters);
5548
}
56-
else {
57-
if (typeParameters && !decl.typeParameters) {
58-
changes.insertTypeParameters(sourceFile, decl, typeParameters);
49+
const needParens = isArrowFunction(decl) && !findChildOfKind(decl, SyntaxKind.OpenParenToken, sourceFile);
50+
if (needParens) changes.insertNodeBefore(sourceFile, first(decl.parameters), createToken(SyntaxKind.OpenParenToken));
51+
for (const param of decl.parameters) {
52+
if (!param.type) {
53+
const paramType = getJSDocType(param);
54+
if (paramType) changes.insertTypeAnnotation(sourceFile, param, transformJSDocType(paramType));
5955
}
60-
for (const param of decl.parameters) {
61-
if (!param.type) {
62-
const paramType = getJSDocType(param);
63-
if (paramType) changes.insertTypeAnnotation(sourceFile, param, transformJSDocType(paramType));
64-
}
65-
}
66-
if (returnTypeNode && !decl.type) changes.insertTypeAnnotation(sourceFile, decl, returnTypeNode);
56+
}
57+
if (needParens) changes.insertNodeAfter(sourceFile, last(decl.parameters), createToken(SyntaxKind.CloseParenToken));
58+
if (!decl.type) {
59+
const returnType = getJSDocReturnType(decl);
60+
if (returnType) changes.insertTypeAnnotation(sourceFile, decl, transformJSDocType(returnType));
6761
}
6862
}
6963
else {

src/services/textChanges.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -352,14 +352,16 @@ namespace ts.textChanges {
352352
/** Prefer this over replacing a node with another that has a type annotation, as it avoids reformatting the other parts of the node. */
353353
public insertTypeAnnotation(sourceFile: SourceFile, node: TypeAnnotatable, type: TypeNode): void {
354354
const end = (isFunctionLike(node)
355-
? findChildOfKind(node, SyntaxKind.CloseParenToken, sourceFile)!
355+
// If no `)`, is an arrow function `x => x`, so use the end of the first parameter
356+
? findChildOfKind(node, SyntaxKind.CloseParenToken, sourceFile) || first(node.parameters)
356357
: node.kind !== SyntaxKind.VariableDeclaration && node.questionToken ? node.questionToken : node.name).end;
357358
this.insertNodeAt(sourceFile, end, type, { prefix: ": " });
358359
}
359360

360361
public insertTypeParameters(sourceFile: SourceFile, node: SignatureDeclaration, typeParameters: ReadonlyArray<TypeParameterDeclaration>): void {
361-
const lparen = findChildOfKind(node, SyntaxKind.OpenParenToken, sourceFile)!.pos;
362-
this.insertNodesAt(sourceFile, lparen, typeParameters, { prefix: "<", suffix: ">" });
362+
// If no `(`, is an arrow function `x => x`, so use the pos of the first parameter
363+
const start = (findChildOfKind(node, SyntaxKind.OpenParenToken, sourceFile) || first(node.parameters)).getStart(sourceFile);
364+
this.insertNodesAt(sourceFile, start, typeParameters, { prefix: "<", suffix: ">" });
363365
}
364366

365367
private getOptionsForInsertNodeBefore(before: Node, doubleNewlines: boolean): ChangeNodeOptions {
@@ -369,6 +371,9 @@ namespace ts.textChanges {
369371
else if (isVariableDeclaration(before)) { // insert `x = 1, ` into `const x = 1, y = 2;
370372
return { suffix: ", " };
371373
}
374+
else if (isParameter(before)) {
375+
return {};
376+
}
372377
return Debug.failBadSyntaxKind(before); // We haven't handled this kind of node yet -- add it
373378
}
374379

@@ -453,6 +458,9 @@ namespace ts.textChanges {
453458
else if (isVariableDeclaration(node)) {
454459
return { prefix: ", " };
455460
}
461+
else if (isParameter(node)) {
462+
return {};
463+
}
456464
return Debug.failBadSyntaxKind(node); // We haven't handled this kind of node yet -- add it
457465
}
458466

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
/////**
4+
//// * @template {T}
5+
//// * @param {T} x
6+
//// * @returns {T}
7+
//// */
8+
////var f = /*a*/x/*b*/ => x
9+
10+
verify.codeFix({
11+
description: "Annotate with type from JSDoc",
12+
newFileContent:
13+
`/**
14+
* @template {T}
15+
* @param {T} x
16+
* @returns {T}
17+
*/
18+
var f = <T>(x: T): T => x`,
19+
});

0 commit comments

Comments
 (0)