From e39a889726b3eef4490c9b8cd816baac8a9a66da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Mon, 26 Feb 2018 15:45:00 +0800 Subject: [PATCH] fix refactor --- .../codefixes/fixStrictClassInitialization.ts | 5 +- ...s => generateGetAccessorAndSetAccessor.ts} | 56 ++++++++++++------- src/services/refactors/refactors.ts | 2 +- src/services/utilities.ts | 4 ++ ...efactorConvertToGetAccessAndSetAccess12.ts | 23 ++++++++ ...efactorConvertToGetAccessAndSetAccess13.ts | 21 +++++++ ...efactorConvertToGetAccessAndSetAccess14.ts | 21 +++++++ ...efactorConvertToGetAccessAndSetAccess15.ts | 16 ++++++ ...efactorConvertToGetAccessAndSetAccess16.ts | 9 +++ ...efactorConvertToGetAccessAndSetAccess17.ts | 11 ++++ ...refactorConvertToGetAccessAndSetAccess2.ts | 6 +- ...refactorConvertToGetAccessAndSetAccess5.ts | 6 +- 12 files changed, 149 insertions(+), 31 deletions(-) rename src/services/refactors/{GenerateGetterAndSetter.ts => generateGetAccessorAndSetAccessor.ts} (63%) create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess12.ts create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess13.ts create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess15.ts create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts diff --git a/src/services/codefixes/fixStrictClassInitialization.ts b/src/services/codefixes/fixStrictClassInitialization.ts index 1199e6a059dcb..41fe2c8cc6103 100644 --- a/src/services/codefixes/fixStrictClassInitialization.ts +++ b/src/services/codefixes/fixStrictClassInitialization.ts @@ -80,9 +80,8 @@ namespace ts.codefix { } function addUndefinedType(changeTracker: textChanges.ChangeTracker, propertyDeclarationSourceFile: SourceFile, propertyDeclaration: PropertyDeclaration): void { - const undefinedTypeNode = createKeywordTypeNode(SyntaxKind.UndefinedKeyword); - const types = isUnionTypeNode(propertyDeclaration.type) ? propertyDeclaration.type.types.concat(undefinedTypeNode) : [propertyDeclaration.type, undefinedTypeNode]; - changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration.type, createUnionTypeNode(types)); + const type = mergeTypeNodeToUnion(propertyDeclaration.type, createKeywordTypeNode(SyntaxKind.UndefinedKeyword)); + changeTracker.replaceNode(propertyDeclarationSourceFile, propertyDeclaration.type, type); } function getActionForAddMissingInitializer (context: CodeFixContext, propertyDeclaration: PropertyDeclaration, newLineCharacter: string): CodeFixAction | undefined { diff --git a/src/services/refactors/GenerateGetterAndSetter.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts similarity index 63% rename from src/services/refactors/GenerateGetterAndSetter.ts rename to src/services/refactors/generateGetAccessorAndSetAccessor.ts index f3aeaeeaaffb9..96ce19809e90b 100644 --- a/src/services/refactors/GenerateGetterAndSetter.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -1,10 +1,20 @@ /* @internal */ -namespace ts.refactor.GenerateGetterAndSetter { +namespace ts.refactor.generateGetAccessorAndSetAccessor { const actionName = "Generate 'get' and 'set' accessors"; const actionDescription = Diagnostics.Generate_get_and_set_accessors.message; - registerRefactor(actionName, { getEditsForAction, getAvailableActions }); + interface Info { + originalName: string; + fieldName: string; + accessorName: string; + accessorType: TypeNode; + propertyDeclaration: PropertyDeclaration; + needUpdateName: boolean; + hasModifiers: boolean; + needUpdateModifiers: boolean; + } + function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined { const { file, startPosition } = context; @@ -34,15 +44,17 @@ namespace ts.refactor.GenerateGetterAndSetter { const changeTracker = textChanges.ChangeTracker.fromContext(context); const newLineCharacter = getNewLineOrDefaultFromHost(context.host, context.formatContext.options); - const { fieldName, accessorName, propertyDeclaration, needUpdateName, hasModifiers, needUpdateModifiers } = fieldInfo; - const accessorModifiers = hasModifiers ? createNodeArray([createToken(SyntaxKind.PublicKeyword)]) : undefined; + const { fieldName, accessorName, accessorType, propertyDeclaration, needUpdateName, hasModifiers, needUpdateModifiers } = fieldInfo; + const accessorModifiers = hasModifiers ? ( + (getModifierFlags(propertyDeclaration) & ModifierFlags.Private || !propertyDeclaration.modifiers) ? createNodeArray([createToken(SyntaxKind.PublicKeyword)]) : propertyDeclaration.modifiers + ) : undefined; - const getAccessor = generateGetAccessor(propertyDeclaration, fieldName, accessorName, accessorModifiers); - const setAccessor = generateSetAccessor(propertyDeclaration, fieldName, accessorName, accessorModifiers); + const getAccessor = generateGetAccessor(fieldName, accessorName, accessorType, accessorModifiers); + const setAccessor = generateSetAccessor(fieldName, accessorName, accessorType, accessorModifiers); - const modifiers = needUpdateModifiers ? createNodeArray([createToken(SyntaxKind.PrivateKeyword)]) : propertyDeclaration.modifiers; + const modifiers = hasModifiers ? createNodeArray([createToken(SyntaxKind.PrivateKeyword)]) : undefined; if (needUpdateName || needUpdateModifiers) { - changeTracker.replaceNode(file, propertyDeclaration, updateOriginPropertyDeclaration(propertyDeclaration, fieldName, modifiers), { + changeTracker.replaceNode(file, propertyDeclaration, updateoriginalPropertyDeclaration(propertyDeclaration, fieldName, modifiers), { suffix: newLineCharacter }); } @@ -57,13 +69,13 @@ namespace ts.refactor.GenerateGetterAndSetter { }; } - interface Info { originName: string; fieldName: string; accessorName: string; propertyDeclaration: PropertyDeclaration; needUpdateName: boolean; hasModifiers: boolean; needUpdateModifiers: boolean; } function getConvertibleFieldAtPosition(file: SourceFile, startPosition: number): Info | undefined { const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false); const propertyDeclaration = findAncestor(node.parent, isPropertyDeclaration); - if (!(propertyDeclaration && propertyDeclaration.name.kind === SyntaxKind.Identifier && - (getModifierFlags(propertyDeclaration) | ModifierFlags.AccessibilityModifier) === ModifierFlags.AccessibilityModifier)) return undefined; + if (!propertyDeclaration || propertyDeclaration.name.kind !== SyntaxKind.Identifier) return undefined; + // make sure propertyDeclaration have only AccessibilityModifier + if ((getModifierFlags(propertyDeclaration) | ModifierFlags.AccessibilityModifier) !== ModifierFlags.AccessibilityModifier) return undefined; const containerClass = getContainingClass(propertyDeclaration); if (!containerClass) return undefined; @@ -79,12 +91,14 @@ namespace ts.refactor.GenerateGetterAndSetter { if (find(members, member => needUpdateName ? member.name.getText() === fieldName : member.name.getText() === accessorName)) return undefined; const hasModifiers = !!find(members, member => !!member.modifiers); - const needUpdateModifiers = hasModifiers && (!propertyDeclaration.modifiers || hasModifier(propertyDeclaration, ModifierFlags.Public)); + const needUpdateModifiers = hasModifiers && (!propertyDeclaration.modifiers || !hasModifier(propertyDeclaration, ModifierFlags.Private)); + const accessorType = propertyDeclaration.questionToken ? mergeTypeNodeToUnion(propertyDeclaration.type, createKeywordTypeNode(SyntaxKind.UndefinedKeyword)) : propertyDeclaration.type; return { - originName: propertyDeclaration.name.text, + originalName: propertyDeclaration.name.text, fieldName, accessorName, + accessorType, propertyDeclaration, needUpdateName, hasModifiers, @@ -92,13 +106,13 @@ namespace ts.refactor.GenerateGetterAndSetter { }; } - function generateGetAccessor (propertyDeclaration: PropertyDeclaration, fieldName: string, name: string, modifiers: ModifiersArray) { + function generateGetAccessor (fieldName: string, name: string, type: TypeNode, modifiers: ModifiersArray) { return createGetAccessor( /*decorators*/ undefined, modifiers, name, /*parameters*/ undefined, - propertyDeclaration.type, + type, createBlock([ createReturn( createPropertyAccess( @@ -110,7 +124,7 @@ namespace ts.refactor.GenerateGetterAndSetter { ); } - function generateSetAccessor (propertyDeclaration: PropertyDeclaration, fieldName: string, name: string, modifiers: ModifiersArray) { + function generateSetAccessor (fieldName: string, name: string, type: TypeNode, modifiers: ModifiersArray) { return createSetAccessor( /*decorators*/ undefined, modifiers, @@ -121,7 +135,7 @@ namespace ts.refactor.GenerateGetterAndSetter { /*dotDotDotToken*/ undefined, createIdentifier("value"), /*questionToken*/ undefined, - propertyDeclaration.type + type )], createBlock([ createStatement( @@ -137,15 +151,15 @@ namespace ts.refactor.GenerateGetterAndSetter { ); } - function updateOriginPropertyDeclaration (propertyDeclaration: PropertyDeclaration, fieldName: string, modifiers: ModifiersArray) { + function updateoriginalPropertyDeclaration (propertyDeclaration: PropertyDeclaration, fieldName: string, modifiers: ModifiersArray) { return updateProperty( propertyDeclaration, - /*decorators*/ undefined, + propertyDeclaration.decorators, modifiers, fieldName, - /*questionOrExclamationToken*/ undefined, + propertyDeclaration.questionToken || propertyDeclaration.exclamationToken, propertyDeclaration.type, - propertyDeclaration.initializer, + propertyDeclaration.initializer ); } } diff --git a/src/services/refactors/refactors.ts b/src/services/refactors/refactors.ts index 1a8af7d316b1e..f899c100ff52c 100644 --- a/src/services/refactors/refactors.ts +++ b/src/services/refactors/refactors.ts @@ -4,4 +4,4 @@ /// /// /// -/// +/// diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 589b030d68641..25a2e45c43cdc 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -269,6 +269,10 @@ namespace ts { getExternalModuleImportEqualsDeclarationExpression(node.parent.parent) === node; } + export function mergeTypeNodeToUnion(target: TypeNode, ...types: TypeNode[]) { + return createUnionTypeNode(isUnionTypeNode(target) ? target.types.concat(types) : [target].concat(types)); + } + export function getContainerNode(node: Node): Declaration { if (node.kind === SyntaxKind.JSDocTypedefTag) { // This doesn't just apply to the node immediately under the comment, but to everything in its parent's scope. diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess12.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess12.ts new file mode 100644 index 0000000000000..76afb375de1be --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess12.ts @@ -0,0 +1,23 @@ +/// + +//// class A { +//// @foo +//// /*a*/public a: string = "foo";/*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 { + @foo + private _a: string = "foo"; + public get a(): string { + return this._a; + } + public set a(value: string) { + this._a = value; + } +}`, +}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess13.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess13.ts new file mode 100644 index 0000000000000..b374bb1589bbb --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess13.ts @@ -0,0 +1,21 @@ +/// + +//// class A { +//// /*a*/public a?: string = "foo";/*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 _a?: string = "foo"; + public get a(): string | undefined { + return this._a; + } + public set a(value: string | undefined) { + this._a = value; + } +}`, +}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts new file mode 100644 index 0000000000000..39fda7a49f3f1 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts @@ -0,0 +1,21 @@ +/// + +//// class A { +//// /*a*/public a!: string = "foo";/*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 _a!: string = "foo"; + public get a(): string { + return this._a; + } + public set a(value: string) { + this._a = value; + } +}`, +}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess15.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess15.ts new file mode 100644 index 0000000000000..3cd29710fa4be --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess15.ts @@ -0,0 +1,16 @@ +/// + +//// class A { +//// /*a*/public "a": string = "foo";/*b*/ +//// /*c*/public get b/*d*/ () { return 1; } +//// /*e*/public set b/*f*/ (v) { } +//// } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Generate 'get' and 'set' accessors"); + +goTo.select("c", "d"); +verify.not.refactorAvailable("Generate 'get' and 'set' accessors"); + +goTo.select("e", "f"); +verify.not.refactorAvailable("Generate 'get' and 'set' accessors"); \ No newline at end of file diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts new file mode 100644 index 0000000000000..d1a8e23838429 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess16.ts @@ -0,0 +1,9 @@ +/// + +//// class A { +//// /*a*/public readonly a: string = "foo";/*b*/ +//// /*c*/public static a: string = "foo";/*d*/ +//// } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Generate 'get' and 'set' accessors"); \ No newline at end of file diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts new file mode 100644 index 0000000000000..5bbe0b62e785a --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess17.ts @@ -0,0 +1,11 @@ +/// + +//// class A { +//// public _a: number = 1; +//// /*a*/public a: string = "foo";/*b*/ +//// public b: number = 2; +//// /*c*/public _b: string = "foo";/*d*/ +//// } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Generate 'get' and 'set' accessors"); \ No newline at end of file diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess2.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess2.ts index aa8c77313dd97..fe3e06c996d54 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess2.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess2.ts @@ -10,11 +10,11 @@ edit.applyRefactor({ actionName: "Generate 'get' and 'set' accessors", actionDescription: "Generate 'get' and 'set' accessors", newContent: `class A { - protected _a: string; - public get a(): string { + private _a: string; + protected get a(): string { return this._a; } - public set a(value: string) { + protected set a(value: string) { this._a = value; } }`, diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess5.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess5.ts index 16edc3515d379..19e3f55ad20bb 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess5.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess5.ts @@ -10,11 +10,11 @@ edit.applyRefactor({ actionName: "Generate 'get' and 'set' accessors", actionDescription: "Generate 'get' and 'set' accessors", newContent: `class A { - protected _a: string; - public get a(): string { + private _a: string; + protected get a(): string { return this._a; } - public set a(value: string) { + protected set a(value: string) { this._a = value; } }`,