Skip to content

Commit a05feed

Browse files
author
Andy
authored
generateGetAccessorAndSetAccessor: Preserve a parameter property declaration (microsoft#23318)
1 parent d6701d3 commit a05feed

6 files changed

+13
-30
lines changed

‎src/services/refactors/generateGetAccessorAndSetAccessor.ts

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
4949
: undefined;
5050
const fieldModifiers = isInClassLike ? getModifiers(isJS, isStatic, SyntaxKind.PrivateKeyword) : undefined;
5151

52-
updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers, container);
52+
updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers);
5353

5454
const getAccessor = generateGetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);
5555
const setAccessor = generateSetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);
@@ -60,7 +60,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
6060
const edits = changeTracker.getChanges();
6161
const renameFilename = file.fileName;
6262
const renameLocationOffset = isIdentifier(fieldName) ? 0 : -1;
63-
const renameLocation = renameLocationOffset + getRenameLocation(edits, renameFilename, fieldName.text, /*isDeclaredBeforeUse*/ false);
63+
const renameLocation = renameLocationOffset + getRenameLocation(edits, renameFilename, fieldName.text, /*preferLastLocation*/ isParameter(declaration));
6464
return { renameFilename, renameLocation, edits };
6565
}
6666

@@ -163,34 +163,21 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
163163
changeTracker.replaceNode(file, declaration, property);
164164
}
165165

166-
function updateParameterPropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: ParameterDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined, classLikeContainer: ClassLikeDeclaration) {
167-
const property = createProperty(
168-
declaration.decorators,
169-
modifiers,
170-
fieldName,
171-
declaration.questionToken,
172-
declaration.type,
173-
declaration.initializer
174-
);
175-
176-
changeTracker.insertNodeAtClassStart(file, classLikeContainer, property);
177-
changeTracker.deleteNodeInList(file, declaration);
178-
}
179-
180-
function updatePropertyAssignmentDeclaration (changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyAssignment, fieldName: AcceptedNameType) {
166+
function updatePropertyAssignmentDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyAssignment, fieldName: AcceptedNameType) {
181167
const assignment = updatePropertyAssignment(declaration, fieldName, declaration.initializer);
182168
changeTracker.replacePropertyAssignment(file, declaration, assignment);
183169
}
184170

185-
function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined, container: ContainerDeclaration) {
171+
function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AcceptedDeclaration, fieldName: AcceptedNameType, modifiers: ModifiersArray | undefined) {
186172
if (isPropertyDeclaration(declaration)) {
187173
updatePropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers);
188174
}
189175
else if (isPropertyAssignment(declaration)) {
190176
updatePropertyAssignmentDeclaration(changeTracker, file, declaration, fieldName);
191177
}
192178
else {
193-
updateParameterPropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers, <ClassLikeDeclaration>container);
179+
changeTracker.replaceNode(file, declaration,
180+
updateParameter(declaration, declaration.decorators, modifiers, declaration.dotDotDotToken, cast(fieldName, isIdentifier), declaration.questionToken, declaration.type, declaration.initializer));
194181
}
195182
}
196183

‎src/services/utilities.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,7 +1538,7 @@ namespace ts {
15381538
* user was before extracting it.
15391539
*/
15401540
/* @internal */
1541-
export function getRenameLocation(edits: ReadonlyArray<FileTextChanges>, renameFilename: string, name: string, isDeclaredBeforeUse: boolean): number {
1541+
export function getRenameLocation(edits: ReadonlyArray<FileTextChanges>, renameFilename: string, name: string, preferLastLocation: boolean): number {
15421542
let delta = 0;
15431543
let lastPos = -1;
15441544
for (const { fileName, textChanges } of edits) {
@@ -1550,7 +1550,7 @@ namespace ts {
15501550
lastPos = span.start + delta + index;
15511551

15521552
// If the reference comes first, return immediately.
1553-
if (!isDeclaredBeforeUse) {
1553+
if (!preferLastLocation) {
15541554
return lastPos;
15551555
}
15561556
}
@@ -1559,7 +1559,7 @@ namespace ts {
15591559
}
15601560

15611561
// If the declaration comes first, return the position of the last occurrence.
1562-
Debug.assert(isDeclaredBeforeUse);
1562+
Debug.assert(preferLastLocation);
15631563
Debug.assert(lastPos >= 0);
15641564
return lastPos;
15651565
}

‎tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@ edit.applyRefactor({
1010
actionName: "Generate 'get' and 'set' accessors",
1111
actionDescription: "Generate 'get' and 'set' accessors",
1212
newContent: `class A {
13-
private /*RENAME*/_a: string;
1413
public get a(): string {
1514
return this._a;
1615
}
1716
public set a(value: string) {
1817
this._a = value;
1918
}
2019
21-
constructor() { }
20+
constructor(private /*RENAME*/_a: string) { }
2221
}`,
2322
});

‎tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@ edit.applyRefactor({
1010
actionName: "Generate 'get' and 'set' accessors",
1111
actionDescription: "Generate 'get' and 'set' accessors",
1212
newContent: `class A {
13-
private /*RENAME*/_a: string;
1413
protected get a(): string {
1514
return this._a;
1615
}
1716
protected set a(value: string) {
1817
this._a = value;
1918
}
2019
21-
constructor() { }
20+
constructor(private /*RENAME*/_a: string) { }
2221
}`,
2322
});

‎tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess18.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,13 @@ edit.applyRefactor({
1010
actionName: "Generate 'get' and 'set' accessors",
1111
actionDescription: "Generate 'get' and 'set' accessors",
1212
newContent: `class A {
13-
private /*RENAME*/_a: string;
1413
public get a(): string {
1514
return this._a;
1615
}
1716
public set a(value: string) {
1817
this._a = value;
1918
}
2019
21-
constructor() { }
20+
constructor(private /*RENAME*/_a: string) { }
2221
}`,
2322
});

‎tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess22.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ edit.applyRefactor({
1111
actionName: "Generate 'get' and 'set' accessors",
1212
actionDescription: "Generate 'get' and 'set' accessors",
1313
newContent: `class A {
14-
private /*RENAME*/_a: string;
1514
public get a(): string {
1615
return this._a;
1716
}
@@ -20,7 +19,7 @@ edit.applyRefactor({
2019
}
2120
2221
public a_1: number;
23-
constructor() { }
22+
constructor(private /*RENAME*/_a: string) { }
2423
}`,
2524
});
2625

0 commit comments

Comments
 (0)