Skip to content

Commit 11637de

Browse files
committed
fix refactor
1 parent 5133b68 commit 11637de

12 files changed

+145
-30
lines changed

src/compiler/factory.ts

+4
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,10 @@ namespace ts {
711711
return updateUnionOrIntersectionTypeNode(node, types);
712712
}
713713

714+
export function unionTypeNode(target: TypeNode, ...types: TypeNode[]) {
715+
return createUnionTypeNode(isUnionTypeNode(target) ? target.types.concat(types) : [target].concat(types));
716+
}
717+
714718
export function createIntersectionTypeNode(types: TypeNode[]): IntersectionTypeNode {
715719
return <IntersectionTypeNode>createUnionOrIntersectionTypeNode(SyntaxKind.IntersectionType, types);
716720
}

src/services/codefixes/fixStrictClassInitialization.ts

+2-3
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,8 @@ namespace ts.codefix {
8080
}
8181

8282
function addUndefinedType(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration): void {
83-
const undefinedTypeNode = createKeywordTypeNode(SyntaxKind.UndefinedKeyword);
84-
const types = isUnionTypeNode(propertyDeclaration.type) ? propertyDeclaration.type.types.concat(undefinedTypeNode) : [propertyDeclaration.type, undefinedTypeNode];
85-
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration.type, createUnionTypeNode(types));
83+
const type = unionTypeNode(propertyDeclaration.type, createKeywordTypeNode(SyntaxKind.UndefinedKeyword));
84+
changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration.type, type);
8685
}
8786

8887
function getActionForAddMissingInitializer (context: CodeFixContext, propertyDeclaration: PropertyDeclaration, newLineCharacter: string): CodeFixAction | undefined {

src/services/refactors/GenerateGetterAndSetter.ts renamed to src/services/refactors/generateGetAccessorAndSetAccessor.ts

+34-20
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,20 @@
11
/* @internal */
2-
namespace ts.refactor.GenerateGetterAndSetter {
2+
namespace ts.refactor.generateGetAccessorAndSetAccessor {
33
const actionName = "Generate 'get' and 'set' accessors";
44
const actionDescription = Diagnostics.Generate_get_and_set_accessors.message;
5-
65
registerRefactor(actionName, { getEditsForAction, getAvailableActions });
76

7+
interface Info {
8+
originalName: string;
9+
fieldName: string;
10+
accessorName: string;
11+
accessorType: TypeNode;
12+
propertyDeclaration: PropertyDeclaration;
13+
needUpdateName: boolean;
14+
hasModifiers: boolean;
15+
needUpdateModifiers: boolean;
16+
}
17+
818
function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
919
const { file, startPosition } = context;
1020

@@ -34,15 +44,17 @@ namespace ts.refactor.GenerateGetterAndSetter {
3444
const changeTracker = textChanges.ChangeTracker.fromContext(context);
3545
const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options);
3646

37-
const { fieldName, accessorName, propertyDeclaration, needUpdateName, hasModifiers, needUpdateModifiers } = fieldInfo;
38-
const accessorModifiers = hasModifiers ? createNodeArray([createToken(SyntaxKind.PublicKeyword)]) : undefined;
47+
const { fieldName, accessorName, accessorType, propertyDeclaration, needUpdateName, hasModifiers, needUpdateModifiers } = fieldInfo;
48+
const accessorModifiers = hasModifiers ? (
49+
(getModifierFlags(propertyDeclaration) & ModifierFlags.Private || !propertyDeclaration.modifiers) ? createNodeArray([createToken(SyntaxKind.PublicKeyword)]) : propertyDeclaration.modifiers
50+
) : undefined;
3951

40-
const getAccessor = generateGetAccessor(propertyDeclaration, fieldName, accessorName, accessorModifiers);
41-
const setAccessor = generateSetAccessor(propertyDeclaration, fieldName, accessorName, accessorModifiers);
52+
const getAccessor = generateGetAccessor(fieldName, accessorName, accessorType, accessorModifiers);
53+
const setAccessor = generateSetAccessor(fieldName, accessorName, accessorType, accessorModifiers);
4254

43-
const modifiers = needUpdateModifiers ? createNodeArray([createToken(SyntaxKind.PrivateKeyword)]) : propertyDeclaration.modifiers;
55+
const modifiers = hasModifiers ? createNodeArray([createToken(SyntaxKind.PrivateKeyword)]) : undefined;
4456
if (needUpdateName || needUpdateModifiers) {
45-
changeTracker.replaceNode(file, propertyDeclaration, updateOriginPropertyDeclaration(propertyDeclaration, fieldName, modifiers), {
57+
changeTracker.replaceNode(file, propertyDeclaration, updateoriginalPropertyDeclaration(propertyDeclaration, fieldName, modifiers), {
4658
suffix: newLineCharacter
4759
});
4860
}
@@ -57,13 +69,13 @@ namespace ts.refactor.GenerateGetterAndSetter {
5769
};
5870
}
5971

60-
interface Info { originName: string; fieldName: string; accessorName: string; propertyDeclaration: PropertyDeclaration; needUpdateName: boolean; hasModifiers: boolean; needUpdateModifiers: boolean; }
6172
function getConvertibleFieldAtPosition(file: SourceFile, startPosition: number): Info | undefined {
6273
const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false);
6374
const propertyDeclaration = findAncestor(node.parent, isPropertyDeclaration);
6475

65-
if (!(propertyDeclaration && propertyDeclaration.name.kind === SyntaxKind.Identifier &&
66-
(getModifierFlags(propertyDeclaration) | ModifierFlags.AccessibilityModifier) === ModifierFlags.AccessibilityModifier)) return undefined;
76+
if (!propertyDeclaration || propertyDeclaration.name.kind !== SyntaxKind.Identifier) return undefined;
77+
// make sure propertyDeclaration have only AccessibilityModifier
78+
if ((getModifierFlags(propertyDeclaration) | ModifierFlags.AccessibilityModifier) !== ModifierFlags.AccessibilityModifier) return undefined;
6779

6880
const containerClass = getContainingClass(propertyDeclaration);
6981
if (!containerClass) return undefined;
@@ -79,26 +91,28 @@ namespace ts.refactor.GenerateGetterAndSetter {
7991
if (find(members, member => needUpdateName ? member.name.getText() === fieldName : member.name.getText() === accessorName)) return undefined;
8092

8193
const hasModifiers = !!find(members, member => !!member.modifiers);
82-
const needUpdateModifiers = hasModifiers && (!propertyDeclaration.modifiers || hasModifier(propertyDeclaration, ModifierFlags.Public));
94+
const needUpdateModifiers = hasModifiers && (!propertyDeclaration.modifiers || !hasModifier(propertyDeclaration, ModifierFlags.Private));
95+
const accessorType = propertyDeclaration.questionToken ? unionTypeNode(propertyDeclaration.type, createKeywordTypeNode(SyntaxKind.UndefinedKeyword)) : propertyDeclaration.type;
8396

8497
return {
85-
originName: propertyDeclaration.name.text,
98+
originalName: propertyDeclaration.name.text,
8699
fieldName,
87100
accessorName,
101+
accessorType,
88102
propertyDeclaration,
89103
needUpdateName,
90104
hasModifiers,
91105
needUpdateModifiers
92106
};
93107
}
94108

95-
function generateGetAccessor (propertyDeclaration: PropertyDeclaration, fieldName: string, name: string, modifiers: ModifiersArray) {
109+
function generateGetAccessor (fieldName: string, name: string, type: TypeNode, modifiers: ModifiersArray) {
96110
return createGetAccessor(
97111
/*decorators*/ undefined,
98112
modifiers,
99113
name,
100114
/*parameters*/ undefined,
101-
propertyDeclaration.type,
115+
type,
102116
createBlock([
103117
createReturn(
104118
createPropertyAccess(
@@ -110,7 +124,7 @@ namespace ts.refactor.GenerateGetterAndSetter {
110124
);
111125
}
112126

113-
function generateSetAccessor (propertyDeclaration: PropertyDeclaration, fieldName: string, name: string, modifiers: ModifiersArray) {
127+
function generateSetAccessor (fieldName: string, name: string, type: TypeNode, modifiers: ModifiersArray) {
114128
return createSetAccessor(
115129
/*decorators*/ undefined,
116130
modifiers,
@@ -121,7 +135,7 @@ namespace ts.refactor.GenerateGetterAndSetter {
121135
/*dotDotDotToken*/ undefined,
122136
createIdentifier("value"),
123137
/*questionToken*/ undefined,
124-
propertyDeclaration.type
138+
type
125139
)],
126140
createBlock([
127141
createStatement(
@@ -137,13 +151,13 @@ namespace ts.refactor.GenerateGetterAndSetter {
137151
);
138152
}
139153

140-
function updateOriginPropertyDeclaration (propertyDeclaration: PropertyDeclaration, fieldName: string, modifiers: ModifiersArray) {
154+
function updateoriginalPropertyDeclaration (propertyDeclaration: PropertyDeclaration, fieldName: string, modifiers: ModifiersArray) {
141155
return updateProperty(
142156
propertyDeclaration,
143-
/*decorators*/ undefined,
157+
propertyDeclaration.decorators,
144158
modifiers,
145159
fieldName,
146-
/*questionOrExclamationToken*/ undefined,
160+
propertyDeclaration.questionToken || propertyDeclaration.exclamationToken,
147161
propertyDeclaration.type,
148162
propertyDeclaration.initializer,
149163
);

src/services/refactors/refactors.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,4 @@
44
/// <reference path="extractSymbol.ts" />
55
/// <reference path="installTypesForPackage.ts" />
66
/// <reference path="useDefaultImport.ts" />
7-
/// <reference path="GenerateGetterAndSetter.ts" />
7+
/// <reference path="generateGetAccessorAndSetAccessor.ts" />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// @foo
5+
//// /*a*/public a: string = "foo";/*b*/
6+
//// }
7+
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+
@foo
15+
private _a: string = "foo";
16+
public get a(): string {
17+
return this._a;
18+
}
19+
public set a(value: string) {
20+
this._a = value;
21+
}
22+
}`,
23+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// /*a*/public a?: string = "foo";/*b*/
5+
//// }
6+
7+
goTo.select("a", "b");
8+
edit.applyRefactor({
9+
refactorName: "Generate 'get' and 'set' accessors",
10+
actionName: "Generate 'get' and 'set' accessors",
11+
actionDescription: "Generate 'get' and 'set' accessors",
12+
newContent: `class A {
13+
private _a?: string = "foo";
14+
public get a(): string | undefined {
15+
return this._a;
16+
}
17+
public set a(value: string | undefined) {
18+
this._a = value;
19+
}
20+
}`,
21+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// /*a*/public a!: string = "foo";/*b*/
5+
//// }
6+
7+
goTo.select("a", "b");
8+
edit.applyRefactor({
9+
refactorName: "Generate 'get' and 'set' accessors",
10+
actionName: "Generate 'get' and 'set' accessors",
11+
actionDescription: "Generate 'get' and 'set' accessors",
12+
newContent: `class A {
13+
private _a!: string = "foo";
14+
public get a(): string {
15+
return this._a;
16+
}
17+
public set a(value: string) {
18+
this._a = value;
19+
}
20+
}`,
21+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// /*a*/public "a": string = "foo";/*b*/
5+
//// /*c*/public get b/*d*/ () { return 1; }
6+
//// /*e*/public set b/*f*/ (v) { }
7+
//// }
8+
9+
goTo.select("a", "b");
10+
verify.not.refactorAvailable("Generate 'get' and 'set' accessors");
11+
12+
goTo.select("c", "d");
13+
verify.not.refactorAvailable("Generate 'get' and 'set' accessors");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// /*a*/public readonly a: string = "foo";/*b*/
5+
//// /*c*/public static a: string = "foo";/*d*/
6+
//// }
7+
8+
goTo.select("a", "b");
9+
verify.not.refactorAvailable("Generate 'get' and 'set' accessors");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
//// class A {
4+
//// public _a: number = 1;
5+
//// /*a*/public a: string = "foo";/*b*/
6+
//// public b: number = 2;
7+
//// /*c*/public _b: string = "foo";/*d*/
8+
//// }
9+
10+
goTo.select("a", "b");
11+
verify.not.refactorAvailable("Generate 'get' and 'set' accessors");

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess2.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ edit.applyRefactor({
1010
actionName: "Generate 'get' and 'set' accessors",
1111
actionDescription: "Generate 'get' and 'set' accessors",
1212
newContent: `class A {
13-
protected _a: string;
14-
public get a(): string {
13+
private _a: string;
14+
protected get a(): string {
1515
return this._a;
1616
}
17-
public set a(value: string) {
17+
protected set a(value: string) {
1818
this._a = value;
1919
}
2020
}`,

tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess5.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,11 @@ edit.applyRefactor({
1010
actionName: "Generate 'get' and 'set' accessors",
1111
actionDescription: "Generate 'get' and 'set' accessors",
1212
newContent: `class A {
13-
protected _a: string;
14-
public get a(): string {
13+
private _a: string;
14+
protected get a(): string {
1515
return this._a;
1616
}
17-
public set a(value: string) {
17+
protected set a(value: string) {
1818
this._a = value;
1919
}
2020
}`,

0 commit comments

Comments
 (0)