Skip to content

Commit 3754bb4

Browse files
a-tarasyuka-tarasyuk
authored
fix(40994): change type for optional properties (microsoft#41011)
1 parent 292b778 commit 3754bb4

7 files changed

+124
-13
lines changed

‎src/services/codefixes/fixPropertyOverrideAccessor.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,6 @@ namespace ts.codefix {
5252
else {
5353
Debug.fail("fixPropertyOverrideAccessor codefix got unexpected error code " + code);
5454
}
55-
return generateAccessorFromProperty(file, startPosition, endPosition, context, Diagnostics.Generate_get_and_set_accessors.message);
55+
return generateAccessorFromProperty(file, context.program, startPosition, endPosition, context, Diagnostics.Generate_get_and_set_accessors.message);
5656
}
5757
}

‎src/services/codefixes/generateAccessors.ts‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ namespace ts.codefix {
2424
error: string
2525
};
2626

27-
export function generateAccessorFromProperty(file: SourceFile, start: number, end: number, context: textChanges.TextChangesContext, _actionName: string): FileTextChanges[] | undefined {
28-
const fieldInfo = getAccessorConvertiblePropertyAtPosition(file, start, end);
27+
export function generateAccessorFromProperty(file: SourceFile, program: Program, start: number, end: number, context: textChanges.TextChangesContext, _actionName: string): FileTextChanges[] | undefined {
28+
const fieldInfo = getAccessorConvertiblePropertyAtPosition(file, program, start, end);
2929
if (!fieldInfo || !fieldInfo.info) return undefined;
3030

3131
const changeTracker = textChanges.ChangeTracker.fromContext(context);
@@ -51,7 +51,7 @@ namespace ts.codefix {
5151
}
5252
}
5353

54-
updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers);
54+
updateFieldDeclaration(changeTracker, file, declaration, type, fieldName, fieldModifiers);
5555

5656
const getAccessor = generateGetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);
5757
suppressLeadingAndTrailingTrivia(getAccessor);
@@ -112,7 +112,7 @@ namespace ts.codefix {
112112
return modifierFlags;
113113
}
114114

115-
export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number, considerEmptySpans = true): InfoOrError | undefined {
115+
export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, program: Program, start: number, end: number, considerEmptySpans = true): InfoOrError | undefined {
116116
const node = getTokenAtPosition(file, start);
117117
const cursorRequest = start === end && considerEmptySpans;
118118
const declaration = findAncestor(node.parent, isAcceptedDeclaration);
@@ -145,7 +145,7 @@ namespace ts.codefix {
145145
info: {
146146
isStatic: hasStaticModifier(declaration),
147147
isReadonly: hasEffectiveReadonlyModifier(declaration),
148-
type: getTypeAnnotationNode(declaration),
148+
type: getDeclarationType(declaration, program),
149149
container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent,
150150
originalName: (<AcceptedNameType>declaration.name).text,
151151
declaration,
@@ -195,14 +195,14 @@ namespace ts.codefix {
195195
);
196196
}
197197

198-
function updatePropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) {
198+
function updatePropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyDeclaration, type: TypeNode | undefined, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) {
199199
const property = factory.updatePropertyDeclaration(
200200
declaration,
201201
declaration.decorators,
202202
modifiers,
203203
fieldName,
204204
declaration.questionToken || declaration.exclamationToken,
205-
declaration.type,
205+
type,
206206
declaration.initializer
207207
);
208208
changeTracker.replaceNode(file, declaration, property);
@@ -213,9 +213,9 @@ namespace ts.codefix {
213213
changeTracker.replacePropertyAssignment(file, declaration, assignment);
214214
}
215215

216-
function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) {
216+
function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, type: TypeNode | undefined, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) {
217217
if (isPropertyDeclaration(declaration)) {
218-
updatePropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers);
218+
updatePropertyDeclaration(changeTracker, file, declaration, type, fieldName, modifiers);
219219
}
220220
else if (isPropertyAssignment(declaration)) {
221221
updatePropertyAssignmentDeclaration(changeTracker, file, declaration, fieldName);
@@ -251,6 +251,19 @@ namespace ts.codefix {
251251
});
252252
}
253253

254+
function getDeclarationType(declaration: AcceptedDeclaration, program: Program): TypeNode | undefined {
255+
const typeNode = getTypeAnnotationNode(declaration);
256+
if (isPropertyDeclaration(declaration) && typeNode && declaration.questionToken) {
257+
const typeChecker = program.getTypeChecker();
258+
const type = typeChecker.getTypeFromTypeNode(typeNode);
259+
if (!typeChecker.isTypeAssignableTo(typeChecker.getUndefinedType(), type)) {
260+
const types = isUnionTypeNode(typeNode) ? typeNode.types : [typeNode];
261+
return factory.createUnionTypeNode([...types, factory.createKeywordTypeNode(SyntaxKind.UndefinedKeyword)]);
262+
}
263+
}
264+
return typeNode;
265+
}
266+
254267
export function getAllSupers(decl: ClassOrInterface | undefined, checker: TypeChecker): readonly ClassOrInterface[] {
255268
const res: ClassLikeDeclaration[] = [];
256269
while (decl) {

‎src/services/refactors/generateGetAccessorAndSetAccessor.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
55
registerRefactor(actionName, {
66
getEditsForAction(context, actionName) {
77
if (!context.endPosition) return undefined;
8-
const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition);
8+
const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.program, context.startPosition, context.endPosition);
99
if (!info || !info.info) return undefined;
10-
const edits = codefix.generateAccessorFromProperty(context.file, context.startPosition, context.endPosition, context, actionName);
10+
const edits = codefix.generateAccessorFromProperty(context.file, context.program, context.startPosition, context.endPosition, context, actionName);
1111
if (!edits) return undefined;
1212

1313
const renameFilename = context.file.fileName;
@@ -19,7 +19,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
1919
},
2020
getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] {
2121
if (!context.endPosition) return emptyArray;
22-
const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition, context.triggerReason === "invoked");
22+
const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.program, context.startPosition, context.endPosition, context.triggerReason === "invoked");
2323
if (!info) return emptyArray;
2424

2525
if (!info.error) {

‎tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess37.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+
5+
////class A {
6+
//// /*a*/foo?: string;/*b*/
7+
////}
8+
9+
goTo.select("a", "b");
10+
edit.applyRefactor({
11+
refactorName: "Generate 'get' and 'set' accessors",
12+
actionName: "Generate 'get' and 'set' accessors",
13+
actionDescription: "Generate 'get' and 'set' accessors",
14+
newContent:
15+
`class A {
16+
private /*RENAME*/_foo?: string | undefined;
17+
public get foo(): string | undefined {
18+
return this._foo;
19+
}
20+
public set foo(value: string | undefined) {
21+
this._foo = value;
22+
}
23+
}`
24+
});

‎tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess38.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+
5+
////class A {
6+
//// /*a*/foo?: string | undefined;/*b*/
7+
////}
8+
9+
goTo.select("a", "b");
10+
edit.applyRefactor({
11+
refactorName: "Generate 'get' and 'set' accessors",
12+
actionName: "Generate 'get' and 'set' accessors",
13+
actionDescription: "Generate 'get' and 'set' accessors",
14+
newContent:
15+
`class A {
16+
private /*RENAME*/_foo?: string | undefined;
17+
public get foo(): string | undefined {
18+
return this._foo;
19+
}
20+
public set foo(value: string | undefined) {
21+
this._foo = value;
22+
}
23+
}`
24+
});

‎tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess39.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+
5+
////type Foo = undefined | null;
6+
////class A {
7+
//// /*a*/foo?: string | Foo;/*b*/
8+
////}
9+
10+
goTo.select("a", "b");
11+
edit.applyRefactor({
12+
refactorName: "Generate 'get' and 'set' accessors",
13+
actionName: "Generate 'get' and 'set' accessors",
14+
actionDescription: "Generate 'get' and 'set' accessors",
15+
newContent:
16+
`type Foo = undefined | null;
17+
class A {
18+
private /*RENAME*/_foo?: string | Foo;
19+
public get foo(): string | Foo {
20+
return this._foo;
21+
}
22+
public set foo(value: string | Foo) {
23+
this._foo = value;
24+
}
25+
}`
26+
});

‎tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess40.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+
5+
////class A {
6+
//// /*a*/foo?: any;/*b*/
7+
////}
8+
9+
goTo.select("a", "b");
10+
edit.applyRefactor({
11+
refactorName: "Generate 'get' and 'set' accessors",
12+
actionName: "Generate 'get' and 'set' accessors",
13+
actionDescription: "Generate 'get' and 'set' accessors",
14+
newContent:
15+
`class A {
16+
private /*RENAME*/_foo?: any;
17+
public get foo(): any {
18+
return this._foo;
19+
}
20+
public set foo(value: any) {
21+
this._foo = value;
22+
}
23+
}`
24+
});

0 commit comments

Comments
 (0)