Skip to content

Commit 88bf927

Browse files
committed
add support for readonly modifier
1 parent 123ae53 commit 88bf927

7 files changed

+198
-11
lines changed

src/services/refactors/generateGetAccessorAndSetAccessor.ts

+62-10
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
1111
interface Info {
1212
container: ContainerDeclaration;
1313
isStatic: boolean;
14+
isReadonly: boolean;
1415
type: TypeNode | undefined;
1516
declaration: AcceptedDeclaration;
1617
fieldName: AcceptedNameType;
@@ -41,21 +42,40 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
4142

4243
const isJS = isSourceFileJavaScript(file);
4344
const changeTracker = textChanges.ChangeTracker.fromContext(context);
44-
const { isStatic, fieldName, accessorName, type, container, declaration } = fieldInfo;
45+
const { isStatic, isReadonly, fieldName, accessorName, type, container, declaration } = fieldInfo;
46+
47+
suppressLeadingAndTrailingTrivia(fieldName);
48+
suppressLeadingAndTrailingTrivia(declaration);
49+
suppressLeadingAndTrailingTrivia(container);
4550

4651
const isInClassLike = isClassLike(container);
52+
// avoid Readonly modifier because it will convert to get accessor
53+
const modifierFlags = getModifierFlags(declaration) & ~ModifierFlags.Readonly;
4754
const accessorModifiers = isInClassLike
48-
? !declaration.modifiers || getModifierFlags(declaration) & ModifierFlags.Private ? getModifiers(isJS, isStatic, SyntaxKind.PublicKeyword) : declaration.modifiers
55+
? !modifierFlags || modifierFlags & ModifierFlags.Private
56+
? getModifiers(isJS, isStatic, SyntaxKind.PublicKeyword)
57+
: createNodeArray(createModifiersFromModifierFlags(modifierFlags))
4958
: undefined;
5059
const fieldModifiers = isInClassLike ? getModifiers(isJS, isStatic, SyntaxKind.PrivateKeyword) : undefined;
5160

5261
updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers);
5362

5463
const getAccessor = generateGetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);
55-
const setAccessor = generateSetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);
56-
64+
suppressLeadingAndTrailingTrivia(getAccessor);
5765
insertAccessor(changeTracker, file, getAccessor, declaration, container);
58-
insertAccessor(changeTracker, file, setAccessor, declaration, container);
66+
67+
if (isReadonly) {
68+
// readonly modifier only existed in classLikeDeclaration
69+
const constructor = getFirstConstructorWithBody(<ClassLikeDeclaration>container);
70+
if (constructor) {
71+
updateReadonlyPropertyInitializerStatementConstructor(changeTracker, file, constructor, accessorName, fieldName);
72+
}
73+
}
74+
else {
75+
const setAccessor = generateSetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);
76+
suppressLeadingAndTrailingTrivia(setAccessor);
77+
insertAccessor(changeTracker, file, setAccessor, declaration, container);
78+
}
5979

6080
const edits = changeTracker.getChanges();
6181
const renameFilename = file.fileName;
@@ -92,16 +112,15 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
92112
function getConvertibleFieldAtPosition(file: SourceFile, startPosition: number): Info | undefined {
93113
const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false);
94114
const declaration = findAncestor(node.parent, isAcceptedDeclaration);
95-
// make sure propertyDeclaration have AccessibilityModifier or Static Modifier
96-
const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static;
115+
// make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier
116+
const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly;
97117
if (!declaration || !isConvertableName(declaration.name) || (getModifierFlags(declaration) | meaning) !== meaning) return undefined;
98118

99119
const fieldName = createPropertyName(getUniqueName(`_${declaration.name.text}`, file.text), declaration.name);
100120
const accessorName = createPropertyName(declaration.name.text, declaration.name);
101-
suppressLeadingAndTrailingTrivia(fieldName);
102-
suppressLeadingAndTrailingTrivia(declaration);
103121
return {
104122
isStatic: hasStaticModifier(declaration),
123+
isReadonly: hasReadonlyModifier(declaration),
105124
type: getTypeAnnotationNode(declaration),
106125
container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent,
107126
declaration,
@@ -159,7 +178,6 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
159178
declaration.type,
160179
declaration.initializer
161180
);
162-
163181
changeTracker.replaceNode(file, declaration, property);
164182
}
165183

@@ -186,4 +204,38 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor {
186204
? changeTracker.insertNodeAtClassStart(file, <ClassLikeDeclaration>container, accessor)
187205
: changeTracker.insertNodeAfter(file, declaration, accessor);
188206
}
207+
208+
function updateReadonlyPropertyInitializerStatementConstructor(changeTracker: textChanges.ChangeTracker, file: SourceFile, constructor: ConstructorDeclaration, accessorName: AcceptedNameType, fieldName: AcceptedNameType) {
209+
if (constructor.body) {
210+
const initializerStatement = find(constructor.body.statements, (stmt =>
211+
isExpressionStatement(stmt) &&
212+
isAssignmentExpression(stmt.expression) &&
213+
stmt.expression.operatorToken.kind === SyntaxKind.EqualsToken &&
214+
(isPropertyAccessExpression(stmt.expression.left) || isElementAccessExpression(stmt.expression.left)) &&
215+
isThis(stmt.expression.left.expression) &&
216+
(isPropertyAccessExpression(stmt.expression.left)
217+
? (getNameFromPropertyName(stmt.expression.left.name) === accessorName.text)
218+
: (isPropertyName(stmt.expression.left.argumentExpression) && isConvertableName(stmt.expression.left.argumentExpression) && getNameFromPropertyName(stmt.expression.left.argumentExpression) === accessorName.text
219+
))
220+
));
221+
if (initializerStatement) {
222+
const initializerLeftHead = (<PropertyAccessExpression | ElementAccessExpression>(<AssignmentExpression<Token<SyntaxKind.EqualsToken>>>(<ExpressionStatement>initializerStatement).expression).left);
223+
224+
if (isPropertyAccessExpression(initializerLeftHead)) {
225+
changeTracker.replaceNode(file, initializerLeftHead, updatePropertyAccess(
226+
initializerLeftHead,
227+
initializerLeftHead.expression,
228+
createIdentifier(fieldName.text)
229+
));
230+
}
231+
else {
232+
changeTracker.replaceNode(file, initializerLeftHead, updateElementAccess(
233+
initializerLeftHead,
234+
initializerLeftHead.expression,
235+
createPropertyName(fieldName.text, <AcceptedNameType>initializerLeftHead.argumentExpression)
236+
));
237+
}
238+
}
239+
}
240+
}
189241
}

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts

+12-1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,15 @@
55
//// }
66

77
goTo.select("a", "b");
8-
verify.not.refactorAvailable("Generate 'get' and 'set' accessors");
8+
goTo.select("a", "b");
9+
edit.applyRefactor({
10+
refactorName: "Generate 'get' and 'set' accessors",
11+
actionName: "Generate 'get' and 'set' accessors",
12+
actionDescription: "Generate 'get' and 'set' accessors",
13+
newContent: `class A {
14+
private /*RENAME*/_a: string = "foo";
15+
public get a(): string {
16+
return this._a;
17+
}
18+
}`,
19+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// public readonly /*a*/a/*b*/: number;
5+
//// constructor () {
6+
//// this.a = 1;
7+
//// }
8+
//// }
9+
10+
goTo.select("a", "b");
11+
edit.applyRefactor({
12+
refactorName: "Generate 'get' and 'set' accessors",
13+
actionName: "Generate 'get' and 'set' accessors",
14+
actionDescription: "Generate 'get' and 'set' accessors",
15+
newContent: `class A {
16+
private /*RENAME*/_a: number;
17+
public get a(): number {
18+
return this._a;
19+
}
20+
constructor () {
21+
this._a = 1;
22+
}
23+
}`,
24+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// public readonly /*a*/a/*b*/: number;
5+
//// constructor () {
6+
//// this["a"] = 1;
7+
//// }
8+
//// }
9+
10+
goTo.select("a", "b");
11+
edit.applyRefactor({
12+
refactorName: "Generate 'get' and 'set' accessors",
13+
actionName: "Generate 'get' and 'set' accessors",
14+
actionDescription: "Generate 'get' and 'set' accessors",
15+
newContent: `class A {
16+
private /*RENAME*/_a: number;
17+
public get a(): number {
18+
return this._a;
19+
}
20+
constructor () {
21+
this["_a"] = 1;
22+
}
23+
}`,
24+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// public readonly /*a*/"a"/*b*/: number;
5+
//// constructor () {
6+
//// this["a"] = 1;
7+
//// }
8+
//// }
9+
10+
goTo.select("a", "b");
11+
edit.applyRefactor({
12+
refactorName: "Generate 'get' and 'set' accessors",
13+
actionName: "Generate 'get' and 'set' accessors",
14+
actionDescription: "Generate 'get' and 'set' accessors",
15+
newContent: `class A {
16+
private /*RENAME*/"_a": number;
17+
public get "a"(): number {
18+
return this["_a"];
19+
}
20+
constructor () {
21+
this["_a"] = 1;
22+
}
23+
}`,
24+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// public readonly /*a*/"a-a"/*b*/: number;
5+
//// constructor () {
6+
//// this["a-a"] = 1;
7+
//// }
8+
//// }
9+
10+
goTo.select("a", "b");
11+
edit.applyRefactor({
12+
refactorName: "Generate 'get' and 'set' accessors",
13+
actionName: "Generate 'get' and 'set' accessors",
14+
actionDescription: "Generate 'get' and 'set' accessors",
15+
newContent: `class A {
16+
private /*RENAME*/"_a-a": number;
17+
public get "a-a"(): number {
18+
return this["_a-a"];
19+
}
20+
constructor () {
21+
this["_a-a"] = 1;
22+
}
23+
}`,
24+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// public readonly /*a*/a/*b*/: number;
5+
//// constructor () {
6+
//// if (Math.random()) {
7+
//// this.a = 1; // only top level assignment
8+
//// }
9+
//// }
10+
//// }
11+
12+
goTo.select("a", "b");
13+
edit.applyRefactor({
14+
refactorName: "Generate 'get' and 'set' accessors",
15+
actionName: "Generate 'get' and 'set' accessors",
16+
actionDescription: "Generate 'get' and 'set' accessors",
17+
newContent: `class A {
18+
private /*RENAME*/_a: number;
19+
public get a(): number {
20+
return this._a;
21+
}
22+
constructor () {
23+
if (Math.random()) {
24+
this.a = 1; // only top level assignment
25+
}
26+
}
27+
}`,
28+
});

0 commit comments

Comments
 (0)