generateGetAccessorAndSetAccessor: Preserve a parameter property declaration (#23318)

src/services/refactors/generateGetAccessorAndSetAccessor.ts

@@ -49,7 +49,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
             : undefined;
         const fieldModifiers = isInClassLike ? getModifiers(isJS, isStatic, SyntaxKind.PrivateKeyword) : undefined;
 
-        updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers, container);
+        updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers);
 
         const getAccessor = generateGetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);
         const setAccessor = generateSetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);
@@ -60,7 +60,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
         const edits = changeTracker.getChanges();
         const renameFilename = file.fileName;
         const renameLocationOffset = isIdentifier(fieldName) ? 0 : -1;
-        const renameLocation = renameLocationOffset + getRenameLocation(edits, renameFilename, fieldName.text, /*isDeclaredBeforeUse*/ false);
+        const renameLocation = renameLocationOffset + getRenameLocation(edits, renameFilename, fieldName.text, /*preferLastLocation*/ isParameter(declaration));
         return { renameFilename, renameLocation, edits };
     }
 
@@ -163,26 +163,12 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
         changeTracker.replaceNode(file, declaration, property);
     }
 
-    function updateParameterPropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: ParameterDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined, classLikeContainer: ClassLikeDeclaration) {
-        const property = createProperty(
-            declaration.decorators,
-            modifiers,
-            fieldName,
-            declaration.questionToken,
-            declaration.type,
-            declaration.initializer
-        );
-
-        changeTracker.insertNodeAtClassStart(file, classLikeContainer, property);
-        changeTracker.deleteNodeInList(file, declaration);
-    }
-
-    function updatePropertyAssignmentDeclaration (changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyAssignment, fieldName: AcceptedNameType) {
+    function updatePropertyAssignmentDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyAssignment, fieldName: AcceptedNameType) {
         const assignment = updatePropertyAssignment(declaration, fieldName, declaration.initializer);
         changeTracker.replacePropertyAssignment(file, declaration, assignment);
     }
 
-    function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined, container: ContainerDeclaration) {
+    function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) {
         if (isPropertyDeclaration(declaration)) {
             updatePropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers);
         }
@@ -190,7 +176,8 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
             updatePropertyAssignmentDeclaration(changeTracker, file, declaration, fieldName);
         }
         else {
-            updateParameterPropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers, <ClassLikeDeclaration>container);
+            changeTracker.replaceNode(file, declaration,
+                updateParameter(declaration, declaration.decorators, modifiers, declaration.dotDotDotToken, cast(fieldName, isIdentifier), declaration.questionToken, declaration.type, declaration.initializer));
         }
     }
 

src/services/utilities.ts

@@ -1538,7 +1538,7 @@ namespace ts {
      * user was before extracting it.
      */
     /* @internal */
-    export function getRenameLocation(edits: ReadonlyArray<FileTextChanges>, renameFilename: string, name: string, isDeclaredBeforeUse: boolean): number {
+    export function getRenameLocation(edits: ReadonlyArray<FileTextChanges>, renameFilename: string, name: string, preferLastLocation: boolean): number {
         let delta = 0;
         let lastPos = -1;
         for (const { fileName, textChanges } of edits) {
@@ -1550,7 +1550,7 @@ namespace ts {
                     lastPos = span.start + delta + index;
 
                     // If the reference comes first, return immediately.
-                    if (!isDeclaredBeforeUse) {
+                    if (!preferLastLocation) {
                         return lastPos;
                     }
                 }
@@ -1559,7 +1559,7 @@ namespace ts {
         }
 
         // If the declaration comes first, return the position of the last occurrence.
-        Debug.assert(isDeclaredBeforeUse);
+        Debug.assert(preferLastLocation);
         Debug.assert(lastPos >= 0);
         return lastPos;
     }

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts

@@ -10,7 +10,6 @@ edit.applyRefactor({
     actionName: "Generate 'get' and 'set' accessors",
     actionDescription: "Generate 'get' and 'set' accessors",
     newContent: `class A {
-    private /*RENAME*/_a: string;
     public get a(): string {
         return this._a;
     }
@@ -18,6 +17,6 @@ edit.applyRefactor({
         this._a = value;
     }
 
-    constructor() { }
+    constructor(private /*RENAME*/_a: string) { }
 }`,
 });

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts

@@ -10,7 +10,6 @@ edit.applyRefactor({
     actionName: "Generate 'get' and 'set' accessors",
     actionDescription: "Generate 'get' and 'set' accessors",
     newContent: `class A {
-    private /*RENAME*/_a: string;
     protected get a(): string {
         return this._a;
     }
@@ -18,6 +17,6 @@ edit.applyRefactor({
         this._a = value;
     }
 
-    constructor() { }
+    constructor(private /*RENAME*/_a: string) { }
 }`,
 });

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess18.ts

@@ -10,7 +10,6 @@ edit.applyRefactor({
     actionName: "Generate 'get' and 'set' accessors",
     actionDescription: "Generate 'get' and 'set' accessors",
     newContent: `class A {
-    private /*RENAME*/_a: string;
     public get a(): string {
         return this._a;
     }
@@ -18,6 +17,6 @@ edit.applyRefactor({
         this._a = value;
     }
 
-    constructor() { }
+    constructor(private /*RENAME*/_a: string) { }
 }`,
 });

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess22.ts

@@ -11,7 +11,6 @@ edit.applyRefactor({
     actionName: "Generate 'get' and 'set' accessors",
     actionDescription: "Generate 'get' and 'set' accessors",
     newContent: `class A {
-    private /*RENAME*/_a: string;
     public get a(): string {
         return this._a;
     }
@@ -20,7 +19,7 @@ edit.applyRefactor({
     }
 
     public a_1: number;
-    constructor() { }
+    constructor(private /*RENAME*/_a: string) { }
 }`,
 });