diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index df1799e336adb..024a27f5a926e 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -41,7 +41,6 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { const fieldInfo = getConvertibleFieldAtPosition(context); if (!fieldInfo) return undefined; - const isJS = isSourceFileJS(file); const changeTracker = textChanges.ChangeTracker.fromContext(context); const { isStatic, isReadonly, fieldName, accessorName, originalName, type, container, declaration, renameAccessor } = fieldInfo; @@ -50,15 +49,20 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { suppressLeadingAndTrailingTrivia(declaration); suppressLeadingAndTrailingTrivia(container); - const isInClassLike = isClassLike(container); - // avoid Readonly modifier because it will convert to get accessor - const modifierFlags = getEffectiveModifierFlags(declaration) & ~ModifierFlags.Readonly; - const accessorModifiers = isInClassLike - ? !modifierFlags || modifierFlags & ModifierFlags.Private - ? getModifiers(isJS, isStatic, SyntaxKind.PublicKeyword) - : createNodeArray(createModifiersFromModifierFlags(modifierFlags)) - : undefined; - const fieldModifiers = isInClassLike ? getModifiers(isJS, isStatic, SyntaxKind.PrivateKeyword) : undefined; + let accessorModifiers: ModifiersArray | undefined; + let fieldModifiers: ModifiersArray | undefined; + if (isClassLike(container)) { + const modifierFlags = getEffectiveModifierFlags(declaration); + if (isSourceFileJS(file)) { + const modifiers = createModifiers(modifierFlags); + accessorModifiers = modifiers; + fieldModifiers = modifiers; + } + else { + accessorModifiers = createModifiers(prepareModifierFlagsForAccessor(modifierFlags)); + fieldModifiers = createModifiers(prepareModifierFlagsForField(modifierFlags)); + } + } updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers); @@ -105,12 +109,26 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { return isIdentifier(fieldName) ? createPropertyAccess(leftHead, fieldName) : createElementAccess(leftHead, createLiteral(fieldName)); } - function getModifiers(isJS: boolean, isStatic: boolean, accessModifier: SyntaxKind.PublicKeyword | SyntaxKind.PrivateKeyword): NodeArray | undefined { - const modifiers = append( - !isJS ? [createToken(accessModifier) as Token | Token] : undefined, - isStatic ? createToken(SyntaxKind.StaticKeyword) : undefined - ); - return modifiers && createNodeArray(modifiers); + function createModifiers(modifierFlags: ModifierFlags): ModifiersArray | undefined { + return modifierFlags ? createNodeArray(createModifiersFromModifierFlags(modifierFlags)) : undefined; + } + + function prepareModifierFlagsForAccessor(modifierFlags: ModifierFlags): ModifierFlags { + modifierFlags &= ~ModifierFlags.Readonly; // avoid Readonly modifier because it will convert to get accessor + modifierFlags &= ~ModifierFlags.Private; + + if (!(modifierFlags & ModifierFlags.Protected)) { + modifierFlags |= ModifierFlags.Public; + } + + return modifierFlags; + } + + function prepareModifierFlagsForField(modifierFlags: ModifierFlags): ModifierFlags { + modifierFlags &= ~ModifierFlags.Public; + modifierFlags &= ~ModifierFlags.Protected; + modifierFlags |= ModifierFlags.Private; + return modifierFlags; } function getConvertibleFieldAtPosition(context: RefactorContext): Info | undefined { diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts index 095cc4fdd79e5..c3c6c824fbec3 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts @@ -4,14 +4,13 @@ //// /*a*/public readonly a: string = "foo";/*b*/ //// } -goTo.select("a", "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*/_a: string = "foo"; + private readonly /*RENAME*/_a: string = "foo"; public get a(): string { return this._a; } diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts index aa224b8b31be2..f490c27f7149b 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts @@ -22,7 +22,7 @@ edit.applyRefactor({ actionName: "Generate 'get' and 'set' accessors", actionDescription: "Generate 'get' and 'set' accessors", newContent: `class A { - private /*RENAME*/_a: number; + private readonly /*RENAME*/_a: number; public get a(): number { return this._a; }