From ba3a6a3bd26bf1f2d8a8df1e60df3f6b8e746b88 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 10 Apr 2018 14:26:14 -0700 Subject: [PATCH] generateGetAccessorAndSetAccessor: Preserve a parameter property declaration --- .../generateGetAccessorAndSetAccessor.ts | 23 ++++--------------- src/services/utilities.ts | 6 ++--- ...efactorConvertToGetAccessAndSetAccess16.ts | 3 +-- ...efactorConvertToGetAccessAndSetAccess17.ts | 3 +-- ...efactorConvertToGetAccessAndSetAccess18.ts | 3 +-- ...efactorConvertToGetAccessAndSetAccess22.ts | 3 +-- 6 files changed, 12 insertions(+), 29 deletions(-) diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index f1b1499a83876..1144ae77a002a 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -50,7 +50,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { const accessorModifiers = getAccessorModifiers(isJS, declaration, isStatic, isInClassLike); const fieldModifiers = getFieldModifiers(isJS, isStatic, isInClassLike); - 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); @@ -61,7 +61,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 }; } @@ -213,26 +213,12 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { changeTracker.replaceNode(file, declaration, property); } - function updateParameterPropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: ParameterDeclaration, fieldName: AccepedNameType, 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: AccepedNameType) { const assignment = updatePropertyAssignment(declaration, fieldName, declaration.initializer); changeTracker.replacePropertyAssignment(file, declaration, assignment); } - function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AccepedDeclaration, fieldName: AccepedNameType, modifiers: ModifiersArray | undefined, container: ContainerDeclation) { + function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AccepedDeclaration, fieldName: AccepedNameType, modifiers: ModifiersArray | undefined) { if (isPropertyDeclaration(declaration)) { updatePropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers); } @@ -240,7 +226,8 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { updatePropertyAssignmentDeclaration(changeTracker, file, declaration, fieldName); } else { - updateParameterPropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers, container); + changeTracker.replaceNode(file, declaration, + updateParameter(declaration, declaration.decorators, modifiers, declaration.dotDotDotToken, cast(fieldName, isIdentifier), declaration.questionToken, declaration.type, declaration.initializer)); } } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index abb487003214b..8b58d48c6c09b 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1542,7 +1542,7 @@ namespace ts { * user was before extracting it. */ /* @internal */ - export function getRenameLocation(edits: ReadonlyArray, renameFilename: string, name: string, isDeclaredBeforeUse: boolean): number { + export function getRenameLocation(edits: ReadonlyArray, renameFilename: string, name: string, preferLastLocation: boolean): number { let delta = 0; let lastPos = -1; for (const { fileName, textChanges } of edits) { @@ -1554,7 +1554,7 @@ namespace ts { lastPos = span.start + delta + index; // If the reference comes first, return immediately. - if (!isDeclaredBeforeUse) { + if (!preferLastLocation) { return lastPos; } } @@ -1563,7 +1563,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; } diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts index dd2cc4cf848dd..5206eada35d95 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts @@ -10,13 +10,12 @@ 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; } public set a(value: string) { this._a = value; } - constructor() { } + constructor(private /*RENAME*/_a: string) { } }`, }); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts index ee212b63502a2..71e058b7c8346 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts @@ -10,13 +10,12 @@ 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; } protected set a(value: string) { this._a = value; } - constructor() { } + constructor(private /*RENAME*/_a: string) { } }`, }); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess18.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess18.ts index 37676a8a047d0..f1203730b3c7c 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess18.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess18.ts @@ -10,13 +10,12 @@ 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; } public set a(value: string) { this._a = value; } - constructor() { } + constructor(private /*RENAME*/_a: string) { } }`, }); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess22.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess22.ts index 0c6cd0f984492..ceec272170973 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess22.ts +++ b/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; } @@ -19,7 +18,7 @@ edit.applyRefactor({ this._a = value; } public a_1: number; - constructor() { } + constructor(private /*RENAME*/_a: string) { } }`, });