Skip to content

Commit e5f91f5

Browse files
author
Andy
authored
Convert annotateWithTypeFromJSDoc refactor to a codefix (#22336)
* Convert annotateWithTypeFromJSDoc refactor to a codefix * Compute isJsFile once at top
1 parent 7826b38 commit e5f91f5

28 files changed

+232
-189
lines changed

‎src/compiler/diagnosticMessages.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3830,6 +3830,10 @@
38303830
"category": "Suggestion",
38313831
"code": 80003
38323832
},
3833+
"JSDoc types may be moved to TypeScript types.": {
3834+
"category": "Suggestion",
3835+
"code": 80004
3836+
},
38333837

38343838
"Add missing 'super()' call": {
38353839
"category": "Message",

‎src/services/refactors/annotateWithTypeFromJSDoc.ts renamed to ‎src/services/codefixes/annotateWithTypeFromJSDoc.ts

Lines changed: 43 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,108 +1,66 @@
11
/* @internal */
2-
namespace ts.refactor.annotateWithTypeFromJSDoc {
3-
const refactorName = "Annotate with type from JSDoc";
4-
const actionName = "annotate";
5-
const description = Diagnostics.Annotate_with_type_from_JSDoc.message;
6-
registerRefactor(refactorName, { getEditsForAction, getAvailableActions });
2+
namespace ts.codefix {
3+
const fixId = "annotateWithTypeFromJSDoc";
4+
const errorCodes = [Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types.code];
5+
registerCodeFix({
6+
errorCodes,
7+
getCodeActions(context) {
8+
const decl = getDeclaration(context.sourceFile, context.span.start);
9+
if (!decl) return;
10+
const description = getLocaleSpecificMessage(Diagnostics.Annotate_with_type_from_JSDoc);
11+
const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, decl));
12+
return [{ description, changes, fixId }];
13+
},
14+
fixIds: [fixId],
15+
getAllCodeActions: context => codeFixAll(context, errorCodes, (changes, diag) => {
16+
const decl = getDeclaration(diag.file!, diag.start!);
17+
if (decl) doChange(changes, diag.file!, decl);
18+
}),
19+
});
20+
21+
function getDeclaration(file: SourceFile, pos: number): DeclarationWithType | undefined {
22+
const name = getTokenAtPosition(file, pos, /*includeJsDocComment*/ false);
23+
// For an arrow function with no name, 'name' lands on the first parameter.
24+
return tryCast(isParameter(name.parent) ? name.parent.parent : name.parent, parameterShouldGetTypeFromJSDoc);
25+
}
726

827
type DeclarationWithType =
928
| FunctionLikeDeclaration
1029
| VariableDeclaration
11-
| ParameterDeclaration
1230
| PropertySignature
1331
| PropertyDeclaration;
1432

15-
function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
16-
if (isInJavaScriptFile(context.file)) {
17-
return undefined;
18-
}
19-
20-
const node = getTokenAtPosition(context.file, context.startPosition, /*includeJsDocComment*/ false);
21-
if (hasUsableJSDoc(findAncestor(node, isDeclarationWithType))) {
22-
return [{
23-
name: refactorName,
24-
description,
25-
actions: [
26-
{
27-
description,
28-
name: actionName
29-
}
30-
]
31-
}];
32-
}
33+
export function parameterShouldGetTypeFromJSDoc(node: Node): node is DeclarationWithType {
34+
return isDeclarationWithType(node) && hasUsableJSDoc(node);
3335
}
3436

35-
function hasUsableJSDoc(decl: DeclarationWithType): boolean {
36-
if (!decl) {
37-
return false;
38-
}
39-
if (isFunctionLikeDeclaration(decl)) {
40-
return decl.parameters.some(hasUsableJSDoc) || (!decl.type && !!getJSDocReturnType(decl));
41-
}
42-
return !decl.type && !!getJSDocType(decl);
37+
function hasUsableJSDoc(decl: DeclarationWithType | ParameterDeclaration): boolean {
38+
return isFunctionLikeDeclaration(decl)
39+
? decl.parameters.some(hasUsableJSDoc) || (!decl.type && !!getJSDocReturnType(decl))
40+
: !decl.type && !!getJSDocType(decl);
4341
}
4442

45-
function getEditsForAction(context: RefactorContext, action: string): RefactorEditInfo | undefined {
46-
if (actionName !== action) {
47-
return Debug.fail(`actionName !== action: ${actionName} !== ${action}`);
48-
}
49-
const node = getTokenAtPosition(context.file, context.startPosition, /*includeJsDocComment*/ false);
50-
const decl = findAncestor(node, isDeclarationWithType);
51-
if (!decl || decl.type) {
52-
return undefined;
53-
}
54-
const jsdocType = getJSDocType(decl);
55-
const isFunctionWithJSDoc = isFunctionLikeDeclaration(decl) && (getJSDocReturnType(decl) || decl.parameters.some(p => !!getJSDocType(p)));
56-
if (isFunctionWithJSDoc || jsdocType && decl.kind === SyntaxKind.Parameter) {
57-
return getEditsForFunctionAnnotation(context);
58-
}
59-
else if (jsdocType) {
60-
return getEditsForAnnotation(context);
43+
function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, decl: DeclarationWithType): void {
44+
if (isFunctionLikeDeclaration(decl) && (getJSDocReturnType(decl) || decl.parameters.some(p => !!getJSDocType(p)))) {
45+
findAncestor(decl, isFunctionLike);
46+
const fn = findAncestor(decl, isFunctionLikeDeclaration);
47+
const functionWithType = addTypesToFunctionLike(fn);
48+
suppressLeadingAndTrailingTrivia(functionWithType);
49+
changes.replaceNode(sourceFile, fn, functionWithType, textChanges.useNonAdjustedPositions);
50+
return;
6151
}
6252
else {
63-
Debug.assert(!!refactor, "No applicable refactor found.");
64-
}
65-
}
66-
67-
function getEditsForAnnotation(context: RefactorContext): RefactorEditInfo | undefined {
68-
const sourceFile = context.file;
69-
const token = getTokenAtPosition(sourceFile, context.startPosition, /*includeJsDocComment*/ false);
70-
const decl = findAncestor(token, isDeclarationWithType);
71-
const jsdocType = getJSDocType(decl);
72-
if (!decl || !jsdocType || decl.type) {
73-
return Debug.fail(`!decl || !jsdocType || decl.type: !${decl} || !${jsdocType} || ${decl.type}`);
53+
const jsdocType = Debug.assertDefined(getJSDocType(decl)); // If not defined, shouldn't have been an error to fix
54+
Debug.assert(!decl.type); // If defined, shouldn't have been an error to fix.
55+
const declarationWithType = addType(decl, transformJSDocType(jsdocType) as TypeNode);
56+
suppressLeadingAndTrailingTrivia(declarationWithType);
57+
changes.replaceNode(sourceFile, decl, declarationWithType, textChanges.useNonAdjustedPositions);
7458
}
75-
76-
const changeTracker = textChanges.ChangeTracker.fromContext(context);
77-
const declarationWithType = addType(decl, transformJSDocType(jsdocType) as TypeNode);
78-
suppressLeadingAndTrailingTrivia(declarationWithType);
79-
changeTracker.replaceNode(sourceFile, decl, declarationWithType, textChanges.useNonAdjustedPositions);
80-
return {
81-
edits: changeTracker.getChanges(),
82-
renameFilename: undefined,
83-
renameLocation: undefined
84-
};
85-
}
86-
87-
function getEditsForFunctionAnnotation(context: RefactorContext): RefactorEditInfo | undefined {
88-
const sourceFile = context.file;
89-
const token = getTokenAtPosition(sourceFile, context.startPosition, /*includeJsDocComment*/ false);
90-
const decl = findAncestor(token, isFunctionLikeDeclaration);
91-
const changeTracker = textChanges.ChangeTracker.fromContext(context);
92-
const functionWithType = addTypesToFunctionLike(decl);
93-
suppressLeadingAndTrailingTrivia(functionWithType);
94-
changeTracker.replaceNode(sourceFile, decl, functionWithType, textChanges.useNonAdjustedPositions);
95-
return {
96-
edits: changeTracker.getChanges(),
97-
renameFilename: undefined,
98-
renameLocation: undefined
99-
};
10059
}
10160

10261
function isDeclarationWithType(node: Node): node is DeclarationWithType {
10362
return isFunctionLikeDeclaration(node) ||
10463
node.kind === SyntaxKind.VariableDeclaration ||
105-
node.kind === SyntaxKind.Parameter ||
10664
node.kind === SyntaxKind.PropertySignature ||
10765
node.kind === SyntaxKind.PropertyDeclaration;
10866
}

‎src/services/codefixes/fixes.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/// <reference path="addMissingInvocationForDecorator.ts" />
2+
/// <reference path="annotateWithTypeFromJSDoc.ts" />
23
/// <reference path="convertFunctionToEs6Class.ts" />
34
/// <reference path="convertToEs6Module.ts" />
45
/// <reference path="correctQualifiedNameToIndexedAccessType.ts" />

‎src/services/refactors/refactors.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1 @@
1-
/// <reference path="annotateWithTypeFromJSDoc.ts" />
21
/// <reference path="extractSymbol.ts" />

‎src/services/suggestionDiagnostics.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,28 @@ namespace ts {
99
diags.push(createDiagnosticForNode(sourceFile.commonJsModuleIndicator, Diagnostics.File_is_a_CommonJS_module_it_may_be_converted_to_an_ES6_module));
1010
}
1111

12+
const isJsFile = isSourceFileJavaScript(sourceFile);
13+
1214
function check(node: Node) {
1315
switch (node.kind) {
1416
case SyntaxKind.FunctionDeclaration:
1517
case SyntaxKind.FunctionExpression:
16-
const symbol = node.symbol;
17-
if (symbol.members && (symbol.members.size > 0)) {
18-
diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration));
18+
if (isJsFile) {
19+
const symbol = node.symbol;
20+
if (symbol.members && (symbol.members.size > 0)) {
21+
diags.push(createDiagnosticForNode(isVariableDeclaration(node.parent) ? node.parent.name : node, Diagnostics.This_constructor_function_may_be_converted_to_a_class_declaration));
22+
}
1923
}
2024
break;
2125
}
26+
27+
if (!isJsFile && codefix.parameterShouldGetTypeFromJSDoc(node)) {
28+
diags.push(createDiagnosticForNode(node.name || node, Diagnostics.JSDoc_types_may_be_moved_to_TypeScript_types));
29+
}
30+
2231
node.forEachChild(check);
2332
}
24-
if (isInJavaScriptFile(sourceFile)) {
25-
check(sourceFile);
26-
}
33+
check(sourceFile);
2734

2835
if (getAllowSyntheticDefaultImports(program.getCompilerOptions())) {
2936
for (const importNode of sourceFile.imports) {

‎tests/cases/fourslash/annotateWithTypeFromJSDoc1.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,16 @@
22

33
// @Filename: test123.ts
44
/////** @type {number} */
5-
////var /*1*/x;
5+
////var [|x|];
66

7-
verify.applicableRefactorAvailableAtMarker('1');
8-
verify.fileAfterApplyingRefactorAtMarker('1',
7+
verify.getSuggestionDiagnostics([{
8+
message: "JSDoc types may be moved to TypeScript types.",
9+
code: 80004,
10+
}]);
11+
12+
verify.codeFix({
13+
description: "Annotate with type from JSDoc",
14+
newFileContent:
915
`/** @type {number} */
10-
var x: number;`, 'Annotate with type from JSDoc', 'annotate');
16+
var x: number;`,
17+
});

‎tests/cases/fourslash/annotateWithTypeFromJSDoc10.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
//// * @param {?} x
55
//// * @returns {number}
66
//// */
7-
////var f = /*1*/(/*2*/x) => x
7+
////var f = (x) => x
88

9-
verify.applicableRefactorAvailableAtMarker('1');
10-
verify.fileAfterApplyingRefactorAtMarker('1',
9+
verify.codeFix({
10+
description: "Annotate with type from JSDoc",
11+
newFileContent:
1112
`/**
1213
* @param {?} x
1314
* @returns {number}
1415
*/
15-
var f = (x: any): number => x`, 'Annotate with type from JSDoc', 'annotate');
16+
var f = (x: any): number => x`,
17+
});

‎tests/cases/fourslash/annotateWithTypeFromJSDoc11.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
//// * @param {?} x
55
//// * @returns {number}
66
//// */
7-
////var f = /*1*/(/*2*/x) => x
7+
////var f = (x) => x
88

9-
verify.applicableRefactorAvailableAtMarker('2');
10-
verify.fileAfterApplyingRefactorAtMarker('2',
9+
verify.codeFix({
10+
description: "Annotate with type from JSDoc",
11+
newFileContent:
1112
`/**
1213
* @param {?} x
1314
* @returns {number}
1415
*/
15-
var f = (x: any): number => x`, 'Annotate with type from JSDoc', 'annotate');
16+
var f = (x: any): number => x`,
17+
});

‎tests/cases/fourslash/annotateWithTypeFromJSDoc12.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,18 @@
44
//// /**
55
//// * @return {...*}
66
//// */
7-
//// /*1*/m(x) {
7+
//// m(x) {
88
//// }
99
////}
10-
verify.applicableRefactorAvailableAtMarker('1');
11-
verify.fileAfterApplyingRefactorAtMarker('1',
10+
11+
verify.codeFix({
12+
description: "Annotate with type from JSDoc",
13+
newFileContent:
1214
`class C {
1315
/**
1416
* @return {...*}
1517
*/
1618
m(x): any[] {
1719
}
18-
}`, 'Annotate with type from JSDoc', 'annotate');
20+
}`,
21+
});

‎tests/cases/fourslash/annotateWithTypeFromJSDoc13.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
/// <reference path='fourslash.ts' />
22
////class C {
33
//// /** @return {number} */
4-
//// get /*1*/c() { return 12 }
4+
//// get c() { return 12 }
55
////}
6-
verify.applicableRefactorAvailableAtMarker('1');
7-
verify.fileAfterApplyingRefactorAtMarker('1',
6+
7+
verify.codeFix({
8+
description: "Annotate with type from JSDoc",
9+
newFileContent:
810
`class C {
911
/** @return {number} */
1012
get c(): number { return 12; }
11-
}`, 'Annotate with type from JSDoc', 'annotate');
13+
}`,
14+
});

‎tests/cases/fourslash/annotateWithTypeFromJSDoc14.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
/// <reference path='fourslash.ts' />
22
/////** @return {number} */
33
////function f() {
4-
//// /*1*/return 12;
4+
//// return 12;
55
////}
6-
verify.applicableRefactorAvailableAtMarker('1');
7-
verify.fileAfterApplyingRefactorAtMarker('1',
6+
7+
verify.codeFix({
8+
description: "Annotate with type from JSDoc",
9+
newFileContent:
810
`/** @return {number} */
911
function f(): number {
1012
return 12;
11-
}`, 'Annotate with type from JSDoc', 'annotate');
13+
}`,
14+
});

‎tests/cases/fourslash/annotateWithTypeFromJSDoc15.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@
1111
//// * @param {Array<number>} epsilon
1212
//// * @param {promise<String>} zeta
1313
//// */
14-
////function f(/*1*/x, /*2*/y, /*3*/z, /*4*/alpha, /*5*/beta, /*6*/gamma, /*7*/delta, /*8*/epsilon, /*9*/zeta) {
14+
////function f(x, y, z, alpha, beta, gamma, delta, epsilon, zeta) {
1515
////}
16-
verify.applicableRefactorAvailableAtMarker('9');
17-
verify.fileAfterApplyingRefactorAtMarker('9',
16+
17+
verify.codeFix({
18+
description: "Annotate with type from JSDoc",
19+
newFileContent:
1820
`/**
1921
* @param {Boolean} x
2022
* @param {String} y
@@ -27,4 +29,5 @@ verify.fileAfterApplyingRefactorAtMarker('9',
2729
* @param {promise<String>} zeta
2830
*/
2931
function f(x: boolean, y: string, z: number, alpha: object, beta: Date, gamma: Promise<any>, delta: Array<any>, epsilon: Array<number>, zeta: Promise<string>) {
30-
}`, 'Annotate with type from JSDoc', 'annotate');
32+
}`,
33+
});

‎tests/cases/fourslash/annotateWithTypeFromJSDoc16.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
/// <reference path='fourslash.ts' />
22
// @strict: true
33
/////** @type {function(*, ...number, ...boolean): void} */
4-
////var /*1*/x = (x, ys, ...zs) => { };
4+
////var x = (x, ys, ...zs) => { };
55

6-
verify.applicableRefactorAvailableAtMarker('1');
7-
verify.fileAfterApplyingRefactorAtMarker('1',
6+
verify.codeFix({
7+
description: "Annotate with type from JSDoc",
8+
newFileContent:
89
`/** @type {function(*, ...number, ...boolean): void} */
9-
var x: (arg0: any, arg1: number[], ...rest: boolean[]) => void = (x, ys, ...zs) => { };`, 'Annotate with type from JSDoc', 'annotate');
10+
var x: (arg0: any, arg1: number[], ...rest: boolean[]) => void = (x, ys, ...zs) => { };`,
11+
});

‎tests/cases/fourslash/annotateWithTypeFromJSDoc17.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,18 @@
33
//// /**
44
//// * @param {number} x - the first parameter
55
//// */
6-
//// constructor(/*1*/x) {
6+
//// constructor(x) {
77
//// }
88
////}
9-
verify.applicableRefactorAvailableAtMarker('1');
10-
verify.fileAfterApplyingRefactorAtMarker('1',
9+
10+
verify.codeFix({
11+
description: "Annotate with type from JSDoc",
12+
newFileContent:
1113
`class C {
1214
/**
1315
* @param {number} x - the first parameter
1416
*/
1517
constructor(x: number) {
1618
}
17-
}`, 'Annotate with type from JSDoc', 'annotate');
18-
19+
}`,
20+
});

0 commit comments

Comments
 (0)