Skip to content

Commit b40e18d

Browse files
sandersnsandersn
authored
Merge pull request #19304 from Microsoft/dedupe-jsdoc-annotation-refactors
Fixes for refactor "Annotate with type from JSDoc"
2 parents f592419 + f82dd7b commit b40e18d

24 files changed

+230
-58
lines changed

‎src/compiler/checker.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7030,13 +7030,11 @@ namespace ts {
70307030
function getIntendedTypeFromJSDocTypeReference(node: TypeReferenceNode): Type {
70317031
if (isIdentifier(node.typeName)) {
70327032
if (node.typeName.escapedText === "Object") {
7033-
if (node.typeArguments && node.typeArguments.length === 2) {
7033+
if (isJSDocIndexSignature(node)) {
70347034
const indexed = getTypeFromTypeNode(node.typeArguments[0]);
70357035
const target = getTypeFromTypeNode(node.typeArguments[1]);
70367036
const index = createIndexInfo(target, /*isReadonly*/ false);
7037-
if (indexed === stringType || indexed === numberType) {
7038-
return createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, indexed === stringType && index, indexed === numberType && index);
7039-
}
7037+
return createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, indexed === stringType && index, indexed === numberType && index);
70407038
}
70417039
return anyType;
70427040
}
@@ -19179,7 +19177,10 @@ namespace ts {
1917919177
// There is no resolved symbol cached if the type resolved to a builtin
1918019178
// via JSDoc type reference resolution (eg, Boolean became boolean), none
1918119179
// of which are generic when they have no associated symbol
19182-
error(node, Diagnostics.Type_0_is_not_generic, typeToString(type));
19180+
// (additionally, JSDoc's index signature syntax, Object<string, T> actually uses generic syntax without being generic)
19181+
if (!isJSDocIndexSignature(node)) {
19182+
error(node, Diagnostics.Type_0_is_not_generic, typeToString(type));
19183+
}
1918319184
return;
1918419185
}
1918519186
let typeParameters = symbol.flags & SymbolFlags.TypeAlias && getSymbolLinks(symbol).typeParameters;

‎src/compiler/utilities.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1360,6 +1360,14 @@ namespace ts {
13601360
return node && !!(node.flags & NodeFlags.JSDoc);
13611361
}
13621362

1363+
export function isJSDocIndexSignature(node: TypeReferenceNode | ExpressionWithTypeArguments) {
1364+
return isTypeReferenceNode(node) &&
1365+
isIdentifier(node.typeName) &&
1366+
node.typeName.escapedText === "Object" &&
1367+
node.typeArguments && node.typeArguments.length === 2 &&
1368+
(node.typeArguments[0].kind === SyntaxKind.StringKeyword || node.typeArguments[0].kind === SyntaxKind.NumberKeyword);
1369+
}
1370+
13631371
/**
13641372
* Returns true if the node is a CallExpression to the identifier 'require' with
13651373
* exactly one argument (of the form 'require("name")').

‎src/services/refactors/annotateWithTypeFromJSDoc.ts

Lines changed: 57 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,9 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
55
const annotateTypeFromJSDoc: Refactor = {
66
name: "Annotate with type from JSDoc",
77
description: Diagnostics.Annotate_with_type_from_JSDoc.message,
8-
getEditsForAction: getEditsForAnnotation,
8+
getEditsForAction,
99
getAvailableActions
1010
};
11-
const annotateFunctionFromJSDoc: Refactor = {
12-
name: "Annotate with types from JSDoc",
13-
description: Diagnostics.Annotate_with_types_from_JSDoc.message,
14-
getEditsForAction: getEditsForFunctionAnnotation,
15-
getAvailableActions
16-
};
17-
1811
type DeclarationWithType =
1912
| FunctionLikeDeclaration
2013
| VariableDeclaration
@@ -23,42 +16,60 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
2316
| PropertyDeclaration;
2417

2518
registerRefactor(annotateTypeFromJSDoc);
26-
registerRefactor(annotateFunctionFromJSDoc);
2719

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

3325
const node = getTokenAtPosition(context.file, context.startPosition, /*includeJsDocComment*/ false);
34-
const decl = findAncestor(node, isDeclarationWithType);
35-
if (!decl || decl.type) {
36-
return undefined;
37-
}
38-
const jsdocType = getJSDocType(decl);
39-
const isFunctionWithJSDoc = isFunctionLikeDeclaration(decl) && (getJSDocReturnType(decl) || decl.parameters.some(p => !!getJSDocType(p)));
40-
const refactor = (isFunctionWithJSDoc || jsdocType && decl.kind === SyntaxKind.Parameter) ? annotateFunctionFromJSDoc :
41-
jsdocType ? annotateTypeFromJSDoc :
42-
undefined;
43-
if (refactor) {
26+
if (hasUsableJSDoc(findAncestor(node, isDeclarationWithType))) {
4427
return [{
45-
name: refactor.name,
46-
description: refactor.description,
28+
name: annotateTypeFromJSDoc.name,
29+
description: annotateTypeFromJSDoc.description,
4730
actions: [
4831
{
49-
description: refactor.description,
50-
name: actionName
51-
}
32+
description: annotateTypeFromJSDoc.description,
33+
name: actionName
34+
}
5235
]
5336
}];
5437
}
5538
}
5639

57-
function getEditsForAnnotation(context: RefactorContext, action: string): RefactorEditInfo | undefined {
40+
function hasUsableJSDoc(decl: DeclarationWithType): boolean {
41+
if (!decl) {
42+
return false;
43+
}
44+
if (isFunctionLikeDeclaration(decl)) {
45+
return decl.parameters.some(hasUsableJSDoc) || (!decl.type && !!getJSDocReturnType(decl));
46+
}
47+
return !decl.type && !!getJSDocType(decl);
48+
}
49+
50+
function getEditsForAction(context: RefactorContext, action: string): RefactorEditInfo | undefined {
5851
if (actionName !== action) {
5952
return Debug.fail(`actionName !== action: ${actionName} !== ${action}`);
6053
}
54+
const node = getTokenAtPosition(context.file, context.startPosition, /*includeJsDocComment*/ false);
55+
const decl = findAncestor(node, isDeclarationWithType);
56+
if (!decl || decl.type) {
57+
return undefined;
58+
}
59+
const jsdocType = getJSDocType(decl);
60+
const isFunctionWithJSDoc = isFunctionLikeDeclaration(decl) && (getJSDocReturnType(decl) || decl.parameters.some(p => !!getJSDocType(p)));
61+
if (isFunctionWithJSDoc || jsdocType && decl.kind === SyntaxKind.Parameter) {
62+
return getEditsForFunctionAnnotation(context);
63+
}
64+
else if (jsdocType) {
65+
return getEditsForAnnotation(context);
66+
}
67+
else {
68+
Debug.assert(!!refactor, "No applicable refactor found.");
69+
}
70+
}
6171

72+
function getEditsForAnnotation(context: RefactorContext): RefactorEditInfo | undefined {
6273
const sourceFile = context.file;
6374
const token = getTokenAtPosition(sourceFile, context.startPosition, /*includeJsDocComment*/ false);
6475
const decl = findAncestor(token, isDeclarationWithType);
@@ -78,11 +89,7 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
7889
};
7990
}
8091

81-
function getEditsForFunctionAnnotation(context: RefactorContext, action: string): RefactorEditInfo | undefined {
82-
if (actionName !== action) {
83-
return Debug.fail(`actionName !== action: ${actionName} !== ${action}`);
84-
}
85-
92+
function getEditsForFunctionAnnotation(context: RefactorContext): RefactorEditInfo | undefined {
8693
const sourceFile = context.file;
8794
const token = getTokenAtPosition(sourceFile, context.startPosition, /*includeJsDocComment*/ false);
8895
const decl = findAncestor(token, isFunctionLikeDeclaration);
@@ -166,7 +173,9 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
166173
case SyntaxKind.TypeReference:
167174
return transformJSDocTypeReference(node as TypeReferenceNode);
168175
default:
169-
return visitEachChild(node, transformJSDocType, /*context*/ undefined) as TypeNode;
176+
const visited = visitEachChild(node, transformJSDocType, /*context*/ undefined) as TypeNode;
177+
setEmitFlags(visited, EmitFlags.SingleLine);
178+
return visited;
170179
}
171180
}
172181

@@ -199,6 +208,9 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
199208
let name = node.typeName;
200209
let args = node.typeArguments;
201210
if (isIdentifier(node.typeName)) {
211+
if (isJSDocIndexSignature(node)) {
212+
return transformJSDocIndexSignature(node);
213+
}
202214
let text = node.typeName.text;
203215
switch (node.typeName.text) {
204216
case "String":
@@ -223,4 +235,18 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
223235
}
224236
return createTypeReferenceNode(name, args);
225237
}
238+
239+
function transformJSDocIndexSignature(node: TypeReferenceNode) {
240+
const index = createParameter(
241+
/*decorators*/ undefined,
242+
/*modifiers*/ undefined,
243+
/*dotDotDotToken*/ undefined,
244+
node.typeArguments[0].kind === SyntaxKind.NumberKeyword ? "n" : "s",
245+
/*questionToken*/ undefined,
246+
createTypeReferenceNode(node.typeArguments[0].kind === SyntaxKind.NumberKeyword ? "number" : "string", []),
247+
/*initializer*/ undefined);
248+
const indexSignature = createTypeLiteralNode([createIndexSignature(/*decorators*/ undefined, /*modifiers*/ undefined, [index], node.typeArguments[1])]);
249+
setEmitFlags(indexSignature, EmitFlags.SingleLine);
250+
return indexSignature;
251+
}
226252
}

‎tests/baselines/reference/jsdocIndexSignature.errors.txt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
tests/cases/conformance/jsdoc/indices.js(9,5): error TS2322: Type '1' is not assignable to type 'boolean'.
2+
3+
4+
==== tests/cases/conformance/jsdoc/indices.js (1 errors) ====
5+
/** @type {Object.<string, number>} */
6+
var o1;
7+
/** @type {Object.<number, boolean>} */
8+
var o2;
9+
/** @type {Object.<boolean, string>} */
10+
var o3;
11+
/** @param {Object.<string, boolean>} o */
12+
function f(o) {
13+
o.foo = 1; // error
14+
~~~~~
15+
!!! error TS2322: Type '1' is not assignable to type 'boolean'.
16+
o.bar = false; // ok
17+
}
18+

‎tests/baselines/reference/jsdocIndexSignature.symbols

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,15 @@ var o2;
1111
var o3;
1212
>o3 : Symbol(o3, Decl(indices.js, 5, 3))
1313

14+
/** @param {Object.<string, boolean>} o */
15+
function f(o) {
16+
>f : Symbol(f, Decl(indices.js, 5, 7))
17+
>o : Symbol(o, Decl(indices.js, 7, 11))
18+
19+
o.foo = 1; // error
20+
>o : Symbol(o, Decl(indices.js, 7, 11))
21+
22+
o.bar = false; // ok
23+
>o : Symbol(o, Decl(indices.js, 7, 11))
24+
}
25+

‎tests/baselines/reference/jsdocIndexSignature.types

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,23 @@ var o2;
1111
var o3;
1212
>o3 : any
1313

14+
/** @param {Object.<string, boolean>} o */
15+
function f(o) {
16+
>f : (o: { [x: string]: boolean; }) => void
17+
>o : { [x: string]: boolean; }
18+
19+
o.foo = 1; // error
20+
>o.foo = 1 : 1
21+
>o.foo : boolean
22+
>o : { [x: string]: boolean; }
23+
>foo : boolean
24+
>1 : 1
25+
26+
o.bar = false; // ok
27+
>o.bar = false : false
28+
>o.bar : boolean
29+
>o : { [x: string]: boolean; }
30+
>bar : boolean
31+
>false : false
32+
}
33+

‎tests/cases/conformance/jsdoc/jsdocIndexSignature.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,8 @@ var o1;
88
var o2;
99
/** @type {Object.<boolean, string>} */
1010
var o3;
11+
/** @param {Object.<string, boolean>} o */
12+
function f(o) {
13+
o.foo = 1; // error
14+
o.bar = false; // ok
15+
}

‎tests/cases/fourslash/annotateWithTypeFromJSDoc10.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ verify.fileAfterApplyingRefactorAtMarker('1',
1212
* @param {?} x
1313
* @returns {number}
1414
*/
15-
var f = (x: any): number => x`, 'Annotate with types from JSDoc', 'annotate');
15+
var f = (x: any): number => x`, 'Annotate with type from JSDoc', 'annotate');

‎tests/cases/fourslash/annotateWithTypeFromJSDoc11.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ verify.fileAfterApplyingRefactorAtMarker('2',
1212
* @param {?} x
1313
* @returns {number}
1414
*/
15-
var f = (x: any): number => x`, 'Annotate with types from JSDoc', 'annotate');
15+
var f = (x: any): number => x`, 'Annotate with type from JSDoc', 'annotate');

‎tests/cases/fourslash/annotateWithTypeFromJSDoc12.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,4 @@ verify.fileAfterApplyingRefactorAtMarker('1',
1515
*/
1616
m(x): any[] {
1717
}
18-
}`, 'Annotate with types from JSDoc', 'annotate');
18+
}`, 'Annotate with type from JSDoc', 'annotate');

‎tests/cases/fourslash/annotateWithTypeFromJSDoc13.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ verify.fileAfterApplyingRefactorAtMarker('1',
88
`class C {
99
/** @return {number} */
1010
get c(): number { return 12; }
11-
}`, 'Annotate with types from JSDoc', 'annotate');
11+
}`, 'Annotate with type from JSDoc', 'annotate');

‎tests/cases/fourslash/annotateWithTypeFromJSDoc14.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ verify.fileAfterApplyingRefactorAtMarker('1',
88
`/** @return {number} */
99
function f(): number {
1010
return 12;
11-
}`, 'Annotate with types from JSDoc', 'annotate');
11+
}`, 'Annotate with type from JSDoc', 'annotate');

‎tests/cases/fourslash/annotateWithTypeFromJSDoc15.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,4 @@ verify.fileAfterApplyingRefactorAtMarker('9',
2727
* @param {promise<String>} zeta
2828
*/
2929
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 types from JSDoc', 'annotate');
30+
}`, 'Annotate with type from JSDoc', 'annotate');

‎tests/cases/fourslash/annotateWithTypeFromJSDoc17.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,5 @@ verify.fileAfterApplyingRefactorAtMarker('1',
1414
*/
1515
constructor(x: number) {
1616
}
17-
}`, 'Annotate with types from JSDoc', 'annotate');
17+
}`, 'Annotate with type from JSDoc', 'annotate');
1818

‎tests/cases/fourslash/annotateWithTypeFromJSDoc18.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ verify.fileAfterApplyingRefactorAtMarker('1',
88
`class C {
99
/** @param {number} value */
1010
set c(value: number) { return 12; }
11-
}`, 'Annotate with types from JSDoc', 'annotate');
11+
}`, 'Annotate with type from JSDoc', 'annotate');

‎tests/cases/fourslash/annotateWithTypeFromJSDoc19.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,4 @@ verify.fileAfterApplyingRefactorAtMarker('1',
1616
* @param {T} b
1717
*/
1818
function f<T>(a: number, b: T) {
19-
}`, 'Annotate with types from JSDoc', 'annotate');
19+
}`, 'Annotate with type from JSDoc', 'annotate');

‎tests/cases/fourslash/annotateWithTypeFromJSDoc20.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,4 @@ verify.fileAfterApplyingRefactorAtMarker('1',
1414
* @param {T} b
1515
*/
1616
function f<T>(a: number, b: T) {
17-
}`, 'Annotate with types from JSDoc', 'annotate');
17+
}`, 'Annotate with type from JSDoc', 'annotate');

‎tests/cases/fourslash/annotateWithTypeFromJSDoc21.ts

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/// <reference path='fourslash.ts' />
2+
// @strict: true
3+
/////**
4+
//// * @return {number}
5+
//// */
6+
////function /*1*/f(x, y) {
7+
////}
8+
////
9+
/////**
10+
//// * @return {number}
11+
//// */
12+
////function /*2*/g(x, y): number {
13+
//// return 0;
14+
////}
15+
/////**
16+
//// * @param {number} x
17+
//// */
18+
////function /*3*/h(x: number, y): number {
19+
//// return 0;
20+
////}
21+
////
22+
/////**
23+
//// * @param {number} x
24+
//// * @param {string} y
25+
//// */
26+
////function /*4*/i(x: number, y: string) {
27+
////}
28+
/////**
29+
//// * @param {number} x
30+
//// * @return {boolean}
31+
//// */
32+
////function /*5*/j(x: number, y): boolean {
33+
//// return true;
34+
////}
35+
36+
verify.not.applicableRefactorAvailableAtMarker('2');
37+
verify.not.applicableRefactorAvailableAtMarker('3');
38+
verify.not.applicableRefactorAvailableAtMarker('4');
39+
verify.not.applicableRefactorAvailableAtMarker('5');
40+
verify.applicableRefactorAvailableAtMarker('1');
41+
verify.fileAfterApplyingRefactorAtMarker('1',
42+
`/**
43+
* @return {number}
44+
*/
45+
function f(x, y): number {
46+
}
47+
48+
/**
49+
* @return {number}
50+
*/
51+
function g(x, y): number {
52+
return 0;
53+
}
54+
/**
55+
* @param {number} x
56+
*/
57+
function h(x: number, y): number {
58+
return 0;
59+
}
60+
61+
/**
62+
* @param {number} x
63+
* @param {string} y
64+
*/
65+
function i(x: number, y: string) {
66+
}
67+
/**
68+
* @param {number} x
69+
* @return {boolean}
70+
*/
71+
function j(x: number, y): boolean {
72+
return true;
73+
}`, 'Annotate with type from JSDoc', 'annotate');

0 commit comments

Comments
 (0)