diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index 799fa58d6d7..55e0780b9e8 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -16,6 +16,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { declaration: AcceptedDeclaration; fieldName: AcceptedNameType; accessorName: AcceptedNameType; + originalName: AcceptedNameType; } function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined { @@ -42,7 +43,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { const isJS = isSourceFileJavaScript(file); const changeTracker = textChanges.ChangeTracker.fromContext(context); - const { isStatic, isReadonly, fieldName, accessorName, type, container, declaration } = fieldInfo; + const { isStatic, isReadonly, fieldName, accessorName, originalName, type, container, declaration } = fieldInfo; suppressLeadingAndTrailingTrivia(fieldName); suppressLeadingAndTrailingTrivia(declaration); @@ -68,7 +69,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { // readonly modifier only existed in classLikeDeclaration const constructor = getFirstConstructorWithBody(container); if (constructor) { - updateReadonlyPropertyInitializerStatementConstructor(changeTracker, file, constructor, accessorName, fieldName); + updateReadonlyPropertyInitializerStatementConstructor(changeTracker, context, constructor, fieldName, originalName); } } else { @@ -123,6 +124,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { isReadonly: hasReadonlyModifier(declaration), type: getTypeAnnotationNode(declaration), container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent, + originalName: declaration.name, declaration, fieldName, accessorName, @@ -205,37 +207,23 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { : changeTracker.insertNodeAfter(file, declaration, accessor); } - function updateReadonlyPropertyInitializerStatementConstructor(changeTracker: textChanges.ChangeTracker, file: SourceFile, constructor: ConstructorDeclaration, accessorName: AcceptedNameType, fieldName: AcceptedNameType) { - if (constructor.body) { - const initializerStatement = find(constructor.body.statements, (stmt => - isExpressionStatement(stmt) && - isAssignmentExpression(stmt.expression) && - stmt.expression.operatorToken.kind === SyntaxKind.EqualsToken && - (isPropertyAccessExpression(stmt.expression.left) || isElementAccessExpression(stmt.expression.left)) && - isThis(stmt.expression.left.expression) && - (isPropertyAccessExpression(stmt.expression.left) - ? (getNameFromPropertyName(stmt.expression.left.name) === accessorName.text) - : (isPropertyName(stmt.expression.left.argumentExpression) && isConvertableName(stmt.expression.left.argumentExpression) && getNameFromPropertyName(stmt.expression.left.argumentExpression) === accessorName.text - )) - )); - if (initializerStatement) { - const initializerLeftHead = ((>>(initializerStatement).expression).left); + function updateReadonlyPropertyInitializerStatementConstructor(changeTracker: textChanges.ChangeTracker, context: RefactorContext, constructor: ConstructorDeclaration, fieldName: AcceptedNameType, originalName: AcceptedNameType) { + if (!constructor.body) return; + const { file, program, cancellationToken } = context; - if (isPropertyAccessExpression(initializerLeftHead)) { - changeTracker.replaceNode(file, initializerLeftHead, updatePropertyAccess( - initializerLeftHead, - initializerLeftHead.expression, - createIdentifier(fieldName.text) - )); - } - else { - changeTracker.replaceNode(file, initializerLeftHead, updateElementAccess( - initializerLeftHead, - initializerLeftHead.expression, - createPropertyName(fieldName.text, initializerLeftHead.argumentExpression) - )); - } - } - } + const referenceEntries = mapDefined(FindAllReferences.getReferenceEntriesForNode(-1, originalName, program, [file], cancellationToken), entry => ( + (entry.type === "node" && rangeContainsRange(constructor, entry.node) && isIdentifier(entry.node) && isWriteAccess(entry.node)) ? entry.node : undefined + )); + + forEach(referenceEntries, entry => { + const parent = entry.parent; + const accessorName = createIdentifier(fieldName.text); + const node = isBinaryExpression(parent) + ? updateBinary(parent, accessorName, parent.right, parent.operatorToken) + : isPropertyAccessExpression(parent) + ? updatePropertyAccess(parent, parent.expression, accessorName) + : Debug.fail("Unexpected write access token"); + changeTracker.replaceNode(file, parent, node); + }); } } diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts index 0a2897212df..d74f246fdfe 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts @@ -2,9 +2,18 @@ //// class A { //// public readonly /*a*/a/*b*/: number; +//// public b: number; //// constructor () { -//// this.a = 1; +//// this.a = 1; // convert +//// this.a++; // convert +//// ++this.a; // convert +//// if (Math.random()) { +//// this.a = 2; // convert +//// } +//// console.log(this.a); // preserve +//// this.b = this.a; // preserve //// } +//// foo () { this.a = 2; } //// } goTo.select("a", "b"); @@ -17,8 +26,17 @@ edit.applyRefactor({ public get a(): number { return this._a; } + public b: number; constructor () { - this._a = 1; + this._a = 1; // convert + this._a++; // convert + ++this._a; // convert + if (Math.random()) { + this._a = 2; // convert + } + console.log(this.a); // preserve + this.b = this.a; // preserve } + foo () { this.a = 2; } }`, }); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess34.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess34.ts deleted file mode 100644 index 170db65d21c..00000000000 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess34.ts +++ /dev/null @@ -1,24 +0,0 @@ -/// - -//// class A { -//// public readonly /*a*/a/*b*/: number; -//// constructor () { -//// this["a"] = 1; -//// } -//// } - -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*/_a: number; - public get a(): number { - return this._a; - } - constructor () { - this["_a"] = 1; - } -}`, -}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess35.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess35.ts deleted file mode 100644 index 36cc668ef69..00000000000 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess35.ts +++ /dev/null @@ -1,24 +0,0 @@ -/// - -//// class A { -//// public readonly /*a*/"a"/*b*/: number; -//// constructor () { -//// this["a"] = 1; -//// } -//// } - -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*/"_a": number; - public get "a"(): number { - return this["_a"]; - } - constructor () { - this["_a"] = 1; - } -}`, -}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess36.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess36.ts deleted file mode 100644 index f75caa271db..00000000000 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess36.ts +++ /dev/null @@ -1,24 +0,0 @@ -/// - -//// class A { -//// public readonly /*a*/"a-a"/*b*/: number; -//// constructor () { -//// this["a-a"] = 1; -//// } -//// } - -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*/"_a-a": number; - public get "a-a"(): number { - return this["_a-a"]; - } - constructor () { - this["_a-a"] = 1; - } -}`, -}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess37.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess37.ts deleted file mode 100644 index 60ba795d448..00000000000 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess37.ts +++ /dev/null @@ -1,28 +0,0 @@ -/// - -//// class A { -//// public readonly /*a*/a/*b*/: number; -//// constructor () { -//// if (Math.random()) { -//// this.a = 1; // only top level assignment -//// } -//// } -//// } - -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*/_a: number; - public get a(): number { - return this._a; - } - constructor () { - if (Math.random()) { - this.a = 1; // only top level assignment - } - } -}`, -});