Skip to content

Commit febfd44

Browse files
a-tarasyuka-tarasyuk
authored
fix(47417): allow undefined type to be added to JSDoc types (microsoft#47449)
1 parent c71ff4d commit febfd44

File tree

6 files changed

+172
-33
lines changed

6 files changed

+172
-33
lines changed

‎src/services/codefixes/fixStrictClassInitialization.ts‎

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,37 +8,33 @@ namespace ts.codefix {
88
registerCodeFix({
99
errorCodes,
1010
getCodeActions: function getCodeActionsForStrictClassInitializationErrors(context) {
11-
const propertyDeclaration = getPropertyDeclaration(context.sourceFile, context.span.start);
12-
if (!propertyDeclaration) return;
13-
14-
const result = [
15-
getActionForAddMissingUndefinedType(context, propertyDeclaration),
16-
getActionForAddMissingDefiniteAssignmentAssertion(context, propertyDeclaration)
17-
];
18-
19-
append(result, getActionForAddMissingInitializer(context, propertyDeclaration));
11+
const info = getInfo(context.sourceFile, context.span.start);
12+
if (!info) return;
2013

14+
const result: CodeFixAction[] = [];
15+
append(result, getActionForAddMissingUndefinedType(context, info));
16+
append(result, getActionForAddMissingDefiniteAssignmentAssertion(context, info));
17+
append(result, getActionForAddMissingInitializer(context, info));
2118
return result;
2219
},
2320
fixIds: [fixIdAddDefiniteAssignmentAssertions, fixIdAddUndefinedType, fixIdAddInitializer],
2421
getAllCodeActions: context => {
2522
return codeFixAll(context, errorCodes, (changes, diag) => {
26-
const propertyDeclaration = getPropertyDeclaration(diag.file, diag.start);
27-
if (!propertyDeclaration) return;
23+
const info = getInfo(diag.file, diag.start);
24+
if (!info) return;
2825

2926
switch (context.fixId) {
3027
case fixIdAddDefiniteAssignmentAssertions:
31-
addDefiniteAssignmentAssertion(changes, diag.file, propertyDeclaration);
28+
addDefiniteAssignmentAssertion(changes, diag.file, info.prop);
3229
break;
3330
case fixIdAddUndefinedType:
34-
addUndefinedType(changes, diag.file, propertyDeclaration);
31+
addUndefinedType(changes, diag.file, info);
3532
break;
3633
case fixIdAddInitializer:
3734
const checker = context.program.getTypeChecker();
38-
const initializer = getInitializer(checker, propertyDeclaration);
35+
const initializer = getInitializer(checker, info.prop);
3936
if (!initializer) return;
40-
41-
addInitializer(changes, diag.file, propertyDeclaration, initializer);
37+
addInitializer(changes, diag.file, info.prop, initializer);
4238
break;
4339
default:
4440
Debug.fail(JSON.stringify(context.fixId));
@@ -47,14 +43,27 @@ namespace ts.codefix {
4743
},
4844
});
4945

50-
function getPropertyDeclaration(sourceFile: SourceFile, pos: number): PropertyDeclaration | undefined {
46+
interface Info {
47+
prop: PropertyDeclaration;
48+
type: TypeNode;
49+
isJs: boolean;
50+
}
51+
52+
function getInfo(sourceFile: SourceFile, pos: number): Info | undefined {
5153
const token = getTokenAtPosition(sourceFile, pos);
52-
return isIdentifier(token) ? cast(token.parent, isPropertyDeclaration) : undefined;
54+
if (isIdentifier(token) && isPropertyDeclaration(token.parent)) {
55+
const type = getEffectiveTypeAnnotationNode(token.parent);
56+
if (type) {
57+
return { type, prop: token.parent, isJs: isInJSFile(token.parent) };
58+
}
59+
}
60+
return undefined;
5361
}
5462

55-
function getActionForAddMissingDefiniteAssignmentAssertion(context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
56-
const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, propertyDeclaration));
57-
return createCodeFixAction(fixName, changes, [Diagnostics.Add_definite_assignment_assertion_to_property_0, propertyDeclaration.getText()], fixIdAddDefiniteAssignmentAssertions, Diagnostics.Add_definite_assignment_assertions_to_all_uninitialized_properties);
63+
function getActionForAddMissingDefiniteAssignmentAssertion(context: CodeFixContext, info: Info): CodeFixAction | undefined {
64+
if (info.isJs) return undefined;
65+
const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, info.prop));
66+
return createCodeFixAction(fixName, changes, [Diagnostics.Add_definite_assignment_assertion_to_property_0, info.prop.getText()], fixIdAddDefiniteAssignmentAssertions, Diagnostics.Add_definite_assignment_assertions_to_all_uninitialized_properties);
5867
}
5968

6069
function addDefiniteAssignmentAssertion(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration): void {
@@ -70,25 +79,32 @@ namespace ts.codefix {
7079
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration, property);
7180
}
7281

73-
function getActionForAddMissingUndefinedType(context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
74-
const changes = textChanges.ChangeTracker.with(context, t => addUndefinedType(t, context.sourceFile, propertyDeclaration));
75-
return createCodeFixAction(fixName, changes, [Diagnostics.Add_undefined_type_to_property_0, propertyDeclaration.name.getText()], fixIdAddUndefinedType, Diagnostics.Add_undefined_type_to_all_uninitialized_properties);
82+
function getActionForAddMissingUndefinedType(context: CodeFixContext, info: Info): CodeFixAction {
83+
const changes = textChanges.ChangeTracker.with(context, t => addUndefinedType(t, context.sourceFile, info));
84+
return createCodeFixAction(fixName, changes, [Diagnostics.Add_undefined_type_to_property_0, info.prop.name.getText()], fixIdAddUndefinedType, Diagnostics.Add_undefined_type_to_all_uninitialized_properties);
7685
}
7786

78-
function addUndefinedType(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration): void {
87+
function addUndefinedType(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, info: Info): void {
7988
const undefinedTypeNode = factory.createKeywordTypeNode(SyntaxKind.UndefinedKeyword);
80-
const type = propertyDeclaration.type!; // TODO: GH#18217
81-
const types = isUnionTypeNode(type) ? type.types.concat(undefinedTypeNode) : [type, undefinedTypeNode];
82-
changeTracker.replaceNode(propertyDeclarationSourceFile, type, factory.createUnionTypeNode(types));
89+
const types = isUnionTypeNode(info.type) ? info.type.types.concat(undefinedTypeNode) : [info.type, undefinedTypeNode];
90+
const unionTypeNode = factory.createUnionTypeNode(types);
91+
if (info.isJs) {
92+
changeTracker.addJSDocTags(sourceFile, info.prop, [factory.createJSDocTypeTag(/*tagName*/ undefined, factory.createJSDocTypeExpression(unionTypeNode))]);
93+
}
94+
else {
95+
changeTracker.replaceNode(sourceFile, info.type, unionTypeNode);
96+
}
8397
}
8498

85-
function getActionForAddMissingInitializer(context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction | undefined {
99+
function getActionForAddMissingInitializer(context: CodeFixContext, info: Info): CodeFixAction | undefined {
100+
if (info.isJs) return undefined;
101+
86102
const checker = context.program.getTypeChecker();
87-
const initializer = getInitializer(checker, propertyDeclaration);
103+
const initializer = getInitializer(checker, info.prop);
88104
if (!initializer) return undefined;
89105

90-
const changes = textChanges.ChangeTracker.with(context, t => addInitializer(t, context.sourceFile, propertyDeclaration, initializer));
91-
return createCodeFixAction(fixName, changes, [Diagnostics.Add_initializer_to_property_0, propertyDeclaration.name.getText()], fixIdAddInitializer, Diagnostics.Add_initializers_to_all_uninitialized_properties);
106+
const changes = textChanges.ChangeTracker.with(context, t => addInitializer(t, context.sourceFile, info.prop, initializer));
107+
return createCodeFixAction(fixName, changes, [Diagnostics.Add_initializer_to_property_0, info.prop.name.getText()], fixIdAddInitializer, Diagnostics.Add_initializers_to_all_uninitialized_properties);
92108
}
93109

94110
function addInitializer(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration, initializer: Expression): void {

‎src/services/textChanges.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,11 @@ namespace ts.textChanges {
502502
if (merged) oldTags[i] = merged;
503503
return !!merged;
504504
}));
505-
const tag = factory.createJSDocComment(factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n"))), factory.createNodeArray([...oldTags, ...unmergedNewTags]));
505+
const tags = [...oldTags, ...unmergedNewTags];
506+
const jsDoc = singleOrUndefined(parent.jsDoc);
507+
const comment = jsDoc && positionsAreOnSameLine(jsDoc.pos, jsDoc.end, sourceFile) && !length(comments) ? undefined :
508+
factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n")));
509+
const tag = factory.createJSDocComment(comment, factory.createNodeArray(tags));
506510
const host = updateJSDocHost(parent);
507511
this.insertJsdocCommentBefore(sourceFile, host, tag);
508512
}
@@ -967,6 +971,8 @@ namespace ts.textChanges {
967971
}
968972
case SyntaxKind.JSDocReturnTag:
969973
return factory.createJSDocReturnTag(/*tagName*/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment);
974+
case SyntaxKind.JSDocTypeTag:
975+
return factory.createJSDocTypeTag(/*tagName*/ undefined, (newTag as JSDocTypeTag).typeExpression, oldTag.comment);
970976
}
971977
}
972978

‎tests/cases/fourslash/codeFixClassPropertyInitialization14.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @strict: true
4+
// @checkJs: true
5+
// @allowJs: true
6+
// @filename: a.js
7+
8+
////class Foo {
9+
//// /** @type {string} */
10+
//// a;
11+
////}
12+
13+
verify.codeFix({
14+
description: `Add 'undefined' type to property 'a'`,
15+
newFileContent:
16+
`class Foo {
17+
/** @type {string | undefined} */
18+
a;
19+
}`,
20+
index: 2
21+
})

‎tests/cases/fourslash/codeFixClassPropertyInitialization15.ts‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @strict: true
4+
// @checkJs: true
5+
// @allowJs: true
6+
// @filename: a.js
7+
////class Foo {
8+
//// /**
9+
//// * comment
10+
//// * @type {string}
11+
//// */
12+
//// a;
13+
////}
14+
15+
verify.codeFix({
16+
description: `Add 'undefined' type to property 'a'`,
17+
newFileContent:
18+
`class Foo {
19+
/**
20+
* comment
21+
* @type {string | undefined}
22+
*/
23+
a;
24+
}`,
25+
index: 2
26+
})

‎tests/cases/fourslash/codeFixClassPropertyInitialization16.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @strict: true
4+
// @checkJs: true
5+
// @allowJs: true
6+
// @filename: a.js
7+
////class Foo {
8+
//// /**
9+
//// * @type {string}
10+
//// */
11+
//// a;
12+
////}
13+
14+
verify.codeFix({
15+
description: `Add 'undefined' type to property 'a'`,
16+
newFileContent:
17+
`class Foo {
18+
/**
19+
* @type {string | undefined}
20+
*/
21+
a;
22+
}`,
23+
index: 2
24+
})

‎tests/cases/fourslash/codeFixClassPropertyInitialization_all_4.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @strict: true
4+
// @checkJs: true
5+
// @allowJs: true
6+
// @filename: a.js
7+
////class A {
8+
//// /**
9+
//// * comment
10+
//// * @type {string}
11+
//// */
12+
//// a;
13+
////}
14+
////class B {
15+
//// /** @type {string} */
16+
//// a;
17+
////}
18+
////class C {
19+
//// /**
20+
//// * @type {string}
21+
//// */
22+
//// a;
23+
////}
24+
25+
verify.codeFixAll({
26+
fixId: 'addMissingPropertyUndefinedType',
27+
fixAllDescription: "Add undefined type to all uninitialized properties",
28+
newFileContent:
29+
`class A {
30+
/**
31+
* comment
32+
* @type {string | undefined}
33+
*/
34+
a;
35+
}
36+
class B {
37+
/** @type {string | undefined} */
38+
a;
39+
}
40+
class C {
41+
/**
42+
* @type {string | undefined}
43+
*/
44+
a;
45+
}`
46+
});

0 commit comments

Comments
 (0)