Skip to content

fix(47417): Code fix crash in JSDoc with strictPropertyInitialization #47449

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.

Sign up for GitHub

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 1 commit into from
Jan 16, 2022
Merged

fix(47417): Code fix crash in JSDoc with strictPropertyInitialization #47449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
80 changes: 48 additions & 32 deletions src/services/codefixes/fixStrictClassInitialization.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,33 @@ namespace ts.codefix {
registerCodeFix({
errorCodes,
getCodeActions: function getCodeActionsForStrictClassInitializationErrors(context) {
const propertyDeclaration = getPropertyDeclaration(context.sourceFile, context.span.start);
if (!propertyDeclaration) return;

const result = [
getActionForAddMissingUndefinedType(context, propertyDeclaration),
getActionForAddMissingDefiniteAssignmentAssertion(context, propertyDeclaration)
];

append(result, getActionForAddMissingInitializer(context, propertyDeclaration));
const info = getInfo(context.sourceFile, context.span.start);
if (!info) return;

const result: CodeFixAction[] = [];
append(result, getActionForAddMissingUndefinedType(context, info));
append(result, getActionForAddMissingDefiniteAssignmentAssertion(context, info));
append(result, getActionForAddMissingInitializer(context, info));
return result;
},
fixIds: [fixIdAddDefiniteAssignmentAssertions, fixIdAddUndefinedType, fixIdAddInitializer],
getAllCodeActions: context => {
return codeFixAll(context, errorCodes, (changes, diag) => {
const propertyDeclaration = getPropertyDeclaration(diag.file, diag.start);
if (!propertyDeclaration) return;
const info = getInfo(diag.file, diag.start);
if (!info) return;

switch (context.fixId) {
case fixIdAddDefiniteAssignmentAssertions:
addDefiniteAssignmentAssertion(changes, diag.file, propertyDeclaration);
addDefiniteAssignmentAssertion(changes, diag.file, info.prop);
break;
case fixIdAddUndefinedType:
addUndefinedType(changes, diag.file, propertyDeclaration);
addUndefinedType(changes, diag.file, info);
break;
case fixIdAddInitializer:
const checker = context.program.getTypeChecker();
const initializer = getInitializer(checker, propertyDeclaration);
const initializer = getInitializer(checker, info.prop);
if (!initializer) return;

addInitializer(changes, diag.file, propertyDeclaration, initializer);
addInitializer(changes, diag.file, info.prop, initializer);
break;
default:
Debug.fail(JSON.stringify(context.fixId));
Expand All @@ -47,14 +43,27 @@ namespace ts.codefix {
},
});

function getPropertyDeclaration(sourceFile: SourceFile, pos: number): PropertyDeclaration | undefined {
interface Info {
prop: PropertyDeclaration;
type: TypeNode;
isJs: boolean;
}

function getInfo(sourceFile: SourceFile, pos: number): Info | undefined {
const token = getTokenAtPosition(sourceFile, pos);
return isIdentifier(token) ? cast(token.parent, isPropertyDeclaration) : undefined;
if (isIdentifier(token) && isPropertyDeclaration(token.parent)) {
const type = getEffectiveTypeAnnotationNode(token.parent);
if (type) {
return { type, prop: token.parent, isJs: isInJSFile(token.parent) };
}
}
return undefined;
}

function getActionForAddMissingDefiniteAssignmentAssertion(context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, propertyDeclaration));
return createCodeFixAction(fixName, changes, [Diagnostics.Add_definite_assignment_assertion_to_property_0, propertyDeclaration.getText()], fixIdAddDefiniteAssignmentAssertions, Diagnostics.Add_definite_assignment_assertions_to_all_uninitialized_properties);
function getActionForAddMissingDefiniteAssignmentAssertion(context: CodeFixContext, info: Info): CodeFixAction | undefined {
if (info.isJs) return undefined;
const changes = textChanges.ChangeTracker.with(context, t => addDefiniteAssignmentAssertion(t, context.sourceFile, info.prop));
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);
}

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

function getActionForAddMissingUndefinedType(context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction {
const changes = textChanges.ChangeTracker.with(context, t => addUndefinedType(t, context.sourceFile, propertyDeclaration));
return createCodeFixAction(fixName, changes, [Diagnostics.Add_undefined_type_to_property_0, propertyDeclaration.name.getText()], fixIdAddUndefinedType, Diagnostics.Add_undefined_type_to_all_uninitialized_properties);
function getActionForAddMissingUndefinedType(context: CodeFixContext, info: Info): CodeFixAction {
const changes = textChanges.ChangeTracker.with(context, t => addUndefinedType(t, context.sourceFile, info));
return createCodeFixAction(fixName, changes, [Diagnostics.Add_undefined_type_to_property_0, info.prop.name.getText()], fixIdAddUndefinedType, Diagnostics.Add_undefined_type_to_all_uninitialized_properties);
}

function addUndefinedType(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration): void {
function addUndefinedType(changeTracker: textChanges.ChangeTracker, sourceFile: SourceFile, info: Info): void {
const undefinedTypeNode = factory.createKeywordTypeNode(SyntaxKind.UndefinedKeyword);
const type = propertyDeclaration.type!; // TODO: GH#18217
const types = isUnionTypeNode(type) ? type.types.concat(undefinedTypeNode) : [type, undefinedTypeNode];
changeTracker.replaceNode(propertyDeclarationSourceFile, type, factory.createUnionTypeNode(types));
const types = isUnionTypeNode(info.type) ? info.type.types.concat(undefinedTypeNode) : [info.type, undefinedTypeNode];
const unionTypeNode = factory.createUnionTypeNode(types);
if (info.isJs) {
changeTracker.addJSDocTags(sourceFile, info.prop, [factory.createJSDocTypeTag(/*tagName*/ undefined, factory.createJSDocTypeExpression(unionTypeNode))]);
}
else {
changeTracker.replaceNode(sourceFile, info.type, unionTypeNode);
}
}

function getActionForAddMissingInitializer(context: CodeFixContext, propertyDeclaration: PropertyDeclaration): CodeFixAction | undefined {
function getActionForAddMissingInitializer(context: CodeFixContext, info: Info): CodeFixAction | undefined {
if (info.isJs) return undefined;

const checker = context.program.getTypeChecker();
const initializer = getInitializer(checker, propertyDeclaration);
const initializer = getInitializer(checker, info.prop);
if (!initializer) return undefined;

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

function addInitializer(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration, initializer: Expression): void {
Expand Down
8 changes: 7 additions & 1 deletion src/services/textChanges.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,11 @@ namespace ts.textChanges {
if (merged) oldTags[i] = merged;
return !!merged;
}));
const tag = factory.createJSDocComment(factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n"))), factory.createNodeArray([...oldTags, ...unmergedNewTags]));
const tags = [...oldTags, ...unmergedNewTags];
const jsDoc = singleOrUndefined(parent.jsDoc);
const comment = jsDoc && positionsAreOnSameLine(jsDoc.pos, jsDoc.end, sourceFile) && !length(comments) ? undefined :
factory.createNodeArray(intersperse(comments, factory.createJSDocText("\n")));
const tag = factory.createJSDocComment(comment, factory.createNodeArray(tags));
const host = updateJSDocHost(parent);
this.insertJsdocCommentBefore(sourceFile, host, tag);
}
Expand Down Expand Up @@ -967,6 +971,8 @@ namespace ts.textChanges {
}
case SyntaxKind.JSDocReturnTag:
return factory.createJSDocReturnTag(/*tagName*/ undefined, (newTag as JSDocReturnTag).typeExpression, oldTag.comment);
case SyntaxKind.JSDocTypeTag:
return factory.createJSDocTypeTag(/*tagName*/ undefined, (newTag as JSDocTypeTag).typeExpression, oldTag.comment);
}
}

Expand Down
21 changes: 21 additions & 0 deletions tests/cases/fourslash/codeFixClassPropertyInitialization14.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/// <reference path='fourslash.ts' />

// @strict: true
// @checkJs: true
// @allowJs: true
// @filename: a.js

////class Foo {
//// /** @type {string} */
//// a;
////}

verify.codeFix({
description: `Add 'undefined' type to property 'a'`,
newFileContent:
`class Foo {
/** @type {string | undefined} */
a;
}`,
index: 2
})
26 changes: 26 additions & 0 deletions tests/cases/fourslash/codeFixClassPropertyInitialization15.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/// <reference path='fourslash.ts' />

// @strict: true
// @checkJs: true
// @allowJs: true
// @filename: a.js
////class Foo {
//// /**
//// * comment
//// * @type {string}
//// */
//// a;
////}

verify.codeFix({
description: `Add 'undefined' type to property 'a'`,
newFileContent:
`class Foo {
/**
* comment
* @type {string | undefined}
*/
a;
}`,
index: 2
})
24 changes: 24 additions & 0 deletions tests/cases/fourslash/codeFixClassPropertyInitialization16.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/// <reference path='fourslash.ts' />

// @strict: true
// @checkJs: true
// @allowJs: true
// @filename: a.js
////class Foo {
//// /**
//// * @type {string}
//// */
//// a;
////}

verify.codeFix({
description: `Add 'undefined' type to property 'a'`,
newFileContent:
`class Foo {
/**
* @type {string | undefined}
*/
a;
}`,
index: 2
})
46 changes: 46 additions & 0 deletions tests/cases/fourslash/codeFixClassPropertyInitialization_all_4.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/// <reference path='fourslash.ts' />

// @strict: true
// @checkJs: true
// @allowJs: true
// @filename: a.js
////class A {
//// /**
//// * comment
//// * @type {string}
//// */
//// a;
////}
////class B {
//// /** @type {string} */
//// a;
////}
////class C {
//// /**
//// * @type {string}
//// */
//// a;
////}

verify.codeFixAll({
fixId: 'addMissingPropertyUndefinedType',
fixAllDescription: "Add undefined type to all uninitialized properties",
newFileContent:
`class A {
/**
* comment
* @type {string | undefined}
*/
a;
}
class B {
/** @type {string | undefined} */
a;
}
class C {
/**
* @type {string | undefined}
*/
a;
}`
});