diff --git a/src/services/codefixes/fixPropertyOverrideAccessor.ts b/src/services/codefixes/fixPropertyOverrideAccessor.ts index 1941b5ccd58..7879860cbad 100644 --- a/src/services/codefixes/fixPropertyOverrideAccessor.ts +++ b/src/services/codefixes/fixPropertyOverrideAccessor.ts @@ -52,6 +52,6 @@ namespace ts.codefix { else { Debug.fail("fixPropertyOverrideAccessor codefix got unexpected error code " + code); } - return generateAccessorFromProperty(file, startPosition, endPosition, context, Diagnostics.Generate_get_and_set_accessors.message); + return generateAccessorFromProperty(file, context.program, startPosition, endPosition, context, Diagnostics.Generate_get_and_set_accessors.message); } } diff --git a/src/services/codefixes/generateAccessors.ts b/src/services/codefixes/generateAccessors.ts index 518278d041c..f2b397cca3e 100644 --- a/src/services/codefixes/generateAccessors.ts +++ b/src/services/codefixes/generateAccessors.ts @@ -24,8 +24,8 @@ namespace ts.codefix { error: string }; - export function generateAccessorFromProperty(file: SourceFile, start: number, end: number, context: textChanges.TextChangesContext, _actionName: string): FileTextChanges[] | undefined { - const fieldInfo = getAccessorConvertiblePropertyAtPosition(file, start, end); + export function generateAccessorFromProperty(file: SourceFile, program: Program, start: number, end: number, context: textChanges.TextChangesContext, _actionName: string): FileTextChanges[] | undefined { + const fieldInfo = getAccessorConvertiblePropertyAtPosition(file, program, start, end); if (!fieldInfo || !fieldInfo.info) return undefined; const changeTracker = textChanges.ChangeTracker.fromContext(context); @@ -51,7 +51,7 @@ namespace ts.codefix { } } - updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers); + updateFieldDeclaration(changeTracker, file, declaration, type, fieldName, fieldModifiers); const getAccessor = generateGetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container); suppressLeadingAndTrailingTrivia(getAccessor); @@ -112,7 +112,7 @@ namespace ts.codefix { return modifierFlags; } - export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, start: number, end: number, considerEmptySpans = true): InfoOrError | undefined { + export function getAccessorConvertiblePropertyAtPosition(file: SourceFile, program: Program, start: number, end: number, considerEmptySpans = true): InfoOrError | undefined { const node = getTokenAtPosition(file, start); const cursorRequest = start === end && considerEmptySpans; const declaration = findAncestor(node.parent, isAcceptedDeclaration); @@ -145,7 +145,7 @@ namespace ts.codefix { info: { isStatic: hasStaticModifier(declaration), isReadonly: hasEffectiveReadonlyModifier(declaration), - type: getTypeAnnotationNode(declaration), + type: getDeclarationType(declaration, program), container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent, originalName: (declaration.name).text, declaration, @@ -195,14 +195,14 @@ namespace ts.codefix { ); } - function updatePropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) { + function updatePropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyDeclaration, type: TypeNode | undefined, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) { const property = factory.updatePropertyDeclaration( declaration, declaration.decorators, modifiers, fieldName, declaration.questionToken || declaration.exclamationToken, - declaration.type, + type, declaration.initializer ); changeTracker.replaceNode(file, declaration, property); @@ -213,9 +213,9 @@ namespace ts.codefix { changeTracker.replacePropertyAssignment(file, declaration, assignment); } - function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) { + function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, type: TypeNode | undefined, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) { if (isPropertyDeclaration(declaration)) { - updatePropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers); + updatePropertyDeclaration(changeTracker, file, declaration, type, fieldName, modifiers); } else if (isPropertyAssignment(declaration)) { updatePropertyAssignmentDeclaration(changeTracker, file, declaration, fieldName); @@ -251,6 +251,19 @@ namespace ts.codefix { }); } + function getDeclarationType(declaration: AcceptedDeclaration, program: Program): TypeNode | undefined { + const typeNode = getTypeAnnotationNode(declaration); + if (isPropertyDeclaration(declaration) && typeNode && declaration.questionToken) { + const typeChecker = program.getTypeChecker(); + const type = typeChecker.getTypeFromTypeNode(typeNode); + if (!typeChecker.isTypeAssignableTo(typeChecker.getUndefinedType(), type)) { + const types = isUnionTypeNode(typeNode) ? typeNode.types : [typeNode]; + return factory.createUnionTypeNode([...types, factory.createKeywordTypeNode(SyntaxKind.UndefinedKeyword)]); + } + } + return typeNode; + } + export function getAllSupers(decl: ClassOrInterface | undefined, checker: TypeChecker): readonly ClassOrInterface[] { const res: ClassLikeDeclaration[] = []; while (decl) { diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index 7ac211ed765..160f4f1b5ae 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -5,9 +5,9 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { registerRefactor(actionName, { getEditsForAction(context, actionName) { if (!context.endPosition) return undefined; - const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition); + const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.program, context.startPosition, context.endPosition); if (!info || !info.info) return undefined; - const edits = codefix.generateAccessorFromProperty(context.file, context.startPosition, context.endPosition, context, actionName); + const edits = codefix.generateAccessorFromProperty(context.file, context.program, context.startPosition, context.endPosition, context, actionName); if (!edits) return undefined; const renameFilename = context.file.fileName; @@ -19,7 +19,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { }, getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { if (!context.endPosition) return emptyArray; - const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.startPosition, context.endPosition, context.triggerReason === "invoked"); + const info = codefix.getAccessorConvertiblePropertyAtPosition(context.file, context.program, context.startPosition, context.endPosition, context.triggerReason === "invoked"); if (!info) return emptyArray; if (!info.error) { diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess37.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess37.ts new file mode 100644 index 00000000000..b741ffb1887 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess37.ts @@ -0,0 +1,24 @@ +/// + +// @strict: true + +////class A { +//// /*a*/foo?: string;/*b*/ +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Generate 'get' and 'set' accessors", + actionName: "Generate 'get' and 'set' accessors", + actionDescription: "Generate 'get' and 'set' accessors", + newContent: +`class A { + private /*RENAME*/_foo?: string | undefined; + public get foo(): string | undefined { + return this._foo; + } + public set foo(value: string | undefined) { + this._foo = value; + } +}` +}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess38.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess38.ts new file mode 100644 index 00000000000..e6bf6f78600 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess38.ts @@ -0,0 +1,24 @@ +/// + +// @strict: true + +////class A { +//// /*a*/foo?: string | undefined;/*b*/ +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Generate 'get' and 'set' accessors", + actionName: "Generate 'get' and 'set' accessors", + actionDescription: "Generate 'get' and 'set' accessors", + newContent: +`class A { + private /*RENAME*/_foo?: string | undefined; + public get foo(): string | undefined { + return this._foo; + } + public set foo(value: string | undefined) { + this._foo = value; + } +}` +}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess39.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess39.ts new file mode 100644 index 00000000000..7c7917ada79 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess39.ts @@ -0,0 +1,26 @@ +/// + +// @strict: true + +////type Foo = undefined | null; +////class A { +//// /*a*/foo?: string | Foo;/*b*/ +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Generate 'get' and 'set' accessors", + actionName: "Generate 'get' and 'set' accessors", + actionDescription: "Generate 'get' and 'set' accessors", + newContent: +`type Foo = undefined | null; +class A { + private /*RENAME*/_foo?: string | Foo; + public get foo(): string | Foo { + return this._foo; + } + public set foo(value: string | Foo) { + this._foo = value; + } +}` +}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess40.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess40.ts new file mode 100644 index 00000000000..c8373368c38 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess40.ts @@ -0,0 +1,24 @@ +/// + +// @strict: true + +////class A { +//// /*a*/foo?: any;/*b*/ +////} + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Generate 'get' and 'set' accessors", + actionName: "Generate 'get' and 'set' accessors", + actionDescription: "Generate 'get' and 'set' accessors", + newContent: +`class A { + private /*RENAME*/_foo?: any; + public get foo(): any { + return this._foo; + } + public set foo(value: any) { + this._foo = value; + } +}` +});