From 88bf9277ff83f3474ad302e9cb579ba279dd24c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Tue, 8 May 2018 12:26:34 +0800 Subject: [PATCH] add support for readonly modifier --- .../generateGetAccessorAndSetAccessor.ts | 72 ++++++++++++++++--- ...efactorConvertToGetAccessAndSetAccess14.ts | 13 +++- ...efactorConvertToGetAccessAndSetAccess33.ts | 24 +++++++ ...efactorConvertToGetAccessAndSetAccess34.ts | 24 +++++++ ...efactorConvertToGetAccessAndSetAccess35.ts | 24 +++++++ ...efactorConvertToGetAccessAndSetAccess36.ts | 24 +++++++ ...efactorConvertToGetAccessAndSetAccess37.ts | 28 ++++++++ 7 files changed, 198 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess34.ts create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess35.ts create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess36.ts create mode 100644 tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess37.ts diff --git a/src/services/refactors/generateGetAccessorAndSetAccessor.ts b/src/services/refactors/generateGetAccessorAndSetAccessor.ts index 523ad97e5fa..799fa58d6d7 100644 --- a/src/services/refactors/generateGetAccessorAndSetAccessor.ts +++ b/src/services/refactors/generateGetAccessorAndSetAccessor.ts @@ -11,6 +11,7 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { interface Info { container: ContainerDeclaration; isStatic: boolean; + isReadonly: boolean; type: TypeNode | undefined; declaration: AcceptedDeclaration; fieldName: AcceptedNameType; @@ -41,21 +42,40 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { const isJS = isSourceFileJavaScript(file); const changeTracker = textChanges.ChangeTracker.fromContext(context); - const { isStatic, fieldName, accessorName, type, container, declaration } = fieldInfo; + const { isStatic, isReadonly, fieldName, accessorName, type, container, declaration } = fieldInfo; + + suppressLeadingAndTrailingTrivia(fieldName); + suppressLeadingAndTrailingTrivia(declaration); + suppressLeadingAndTrailingTrivia(container); const isInClassLike = isClassLike(container); + // avoid Readonly modifier because it will convert to get accessor + const modifierFlags = getModifierFlags(declaration) & ~ModifierFlags.Readonly; const accessorModifiers = isInClassLike - ? !declaration.modifiers || getModifierFlags(declaration) & ModifierFlags.Private ? getModifiers(isJS, isStatic, SyntaxKind.PublicKeyword) : declaration.modifiers + ? !modifierFlags || modifierFlags & ModifierFlags.Private + ? getModifiers(isJS, isStatic, SyntaxKind.PublicKeyword) + : createNodeArray(createModifiersFromModifierFlags(modifierFlags)) : undefined; const fieldModifiers = isInClassLike ? getModifiers(isJS, isStatic, SyntaxKind.PrivateKeyword) : undefined; updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers); const getAccessor = generateGetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container); - const setAccessor = generateSetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container); - + suppressLeadingAndTrailingTrivia(getAccessor); insertAccessor(changeTracker, file, getAccessor, declaration, container); - insertAccessor(changeTracker, file, setAccessor, declaration, container); + + if (isReadonly) { + // readonly modifier only existed in classLikeDeclaration + const constructor = getFirstConstructorWithBody(container); + if (constructor) { + updateReadonlyPropertyInitializerStatementConstructor(changeTracker, file, constructor, accessorName, fieldName); + } + } + else { + const setAccessor = generateSetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container); + suppressLeadingAndTrailingTrivia(setAccessor); + insertAccessor(changeTracker, file, setAccessor, declaration, container); + } const edits = changeTracker.getChanges(); const renameFilename = file.fileName; @@ -92,16 +112,15 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { function getConvertibleFieldAtPosition(file: SourceFile, startPosition: number): Info | undefined { const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false); const declaration = findAncestor(node.parent, isAcceptedDeclaration); - // make sure propertyDeclaration have AccessibilityModifier or Static Modifier - const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static; + // make sure declaration have AccessibilityModifier or Static Modifier or Readonly Modifier + const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static | ModifierFlags.Readonly; if (!declaration || !isConvertableName(declaration.name) || (getModifierFlags(declaration) | meaning) !== meaning) return undefined; const fieldName = createPropertyName(getUniqueName(`_${declaration.name.text}`, file.text), declaration.name); const accessorName = createPropertyName(declaration.name.text, declaration.name); - suppressLeadingAndTrailingTrivia(fieldName); - suppressLeadingAndTrailingTrivia(declaration); return { isStatic: hasStaticModifier(declaration), + isReadonly: hasReadonlyModifier(declaration), type: getTypeAnnotationNode(declaration), container: declaration.kind === SyntaxKind.Parameter ? declaration.parent.parent : declaration.parent, declaration, @@ -159,7 +178,6 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { declaration.type, declaration.initializer ); - changeTracker.replaceNode(file, declaration, property); } @@ -186,4 +204,38 @@ namespace ts.refactor.generateGetAccessorAndSetAccessor { ? changeTracker.insertNodeAtClassStart(file, container, accessor) : 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); + + 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) + )); + } + } + } + } } diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts index 36789b29826..095cc4fdd79 100644 --- a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess14.ts @@ -5,4 +5,15 @@ //// } goTo.select("a", "b"); -verify.not.refactorAvailable("Generate 'get' and 'set' accessors"); \ No newline at end of file +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"; + public get a(): string { + return this._a; + } +}`, +}); diff --git a/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts new file mode 100644 index 00000000000..0a2897212df --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess33.ts @@ -0,0 +1,24 @@ +/// + +//// 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/refactorConvertToGetAccessAndSetAccess34.ts b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess34.ts new file mode 100644 index 00000000000..170db65d21c --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess34.ts @@ -0,0 +1,24 @@ +/// + +//// 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 new file mode 100644 index 00000000000..36cc668ef69 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess35.ts @@ -0,0 +1,24 @@ +/// + +//// 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 new file mode 100644 index 00000000000..f75caa271db --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess36.ts @@ -0,0 +1,24 @@ +/// + +//// 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 new file mode 100644 index 00000000000..60ba795d448 --- /dev/null +++ b/tests/cases/fourslash/refactorConvertToGetAccessAndSetAccess37.ts @@ -0,0 +1,28 @@ +/// + +//// 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 + } + } +}`, +});