Skip to content

Commit

Permalink
add refactor of convert private field to getter and setter (#22143)
Browse files Browse the repository at this point in the history
* add refactor of convert private field to getter and setter

* fix refactor

* stash

* refactor accessor generate

* revert merge union type

* refeactor and accept baseline

* add support of PropertyAssignment and StringLiteral

* add support for js file

* allow static modifier in js file
  • Loading branch information
Kingwl authored and Andy committed Apr 10, 2018
1 parent 556a801 commit 9c0671d
Show file tree
Hide file tree
Showing 52 changed files with 1,608 additions and 459 deletions.
783 changes: 368 additions & 415 deletions package-lock.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3122,8 +3122,8 @@ namespace ts {
return (arg: T) => f(arg) && g(arg);
}

export function or<T>(f: (arg: T) => boolean, g: (arg: T) => boolean) {
return (arg: T) => f(arg) || g(arg);
export function or<T>(f: (arg: T) => boolean, g: (arg: T) => boolean, ...others: ((arg: T) => boolean)[]) {
return (arg: T) => f(arg) || g(arg) || others.some(f => f(arg));
}

export function assertTypeIsNever(_: never): void { } // tslint:disable-line no-empty
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4161,5 +4161,9 @@
"Convert all constructor functions to classes": {
"category": "Message",
"code": 95045
},
"Generate 'get' and 'set' accessors": {
"category": "Message",
"code": 95046
}
}
2 changes: 1 addition & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4273,7 +4273,7 @@ namespace ts {
}
}

export function isParameterPropertyDeclaration(node: Node): boolean {
export function isParameterPropertyDeclaration(node: Node): node is ParameterDeclaration {
return hasModifier(node, ModifierFlags.ParameterPropertyModifier) && node.parent.kind === SyntaxKind.Constructor && isClassLike(node.parent.parent);
}

Expand Down
39 changes: 0 additions & 39 deletions src/services/refactors/extractSymbol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,14 +701,6 @@ namespace ts.refactor.extractSymbol {
Global,
}

function getUniqueName(baseName: string, fileText: string): string {
let nameText = baseName;
for (let i = 1; stringContains(fileText, nameText); i++) {
nameText = `${baseName}_${i}`;
}
return nameText;
}

/**
* Result of 'extractRange' operation for a specific scope.
* Stores either a list of changes that should be applied to extract a range or a list of errors
Expand Down Expand Up @@ -1129,37 +1121,6 @@ namespace ts.refactor.extractSymbol {
}
}

/**
* @return The index of the (only) reference to the extracted symbol. We want the cursor
* to be on the reference, rather than the declaration, because it's closer to where the
* user was before extracting it.
*/
function getRenameLocation(edits: ReadonlyArray<FileTextChanges>, renameFilename: string, functionNameText: string, isDeclaredBeforeUse: boolean): number {
let delta = 0;
let lastPos = -1;
for (const { fileName, textChanges } of edits) {
Debug.assert(fileName === renameFilename);
for (const change of textChanges) {
const { span, newText } = change;
const index = newText.indexOf(functionNameText);
if (index !== -1) {
lastPos = span.start + delta + index;

// If the reference comes first, return immediately.
if (!isDeclaredBeforeUse) {
return lastPos;
}
}
delta += newText.length - span.length;
}
}

// If the declaration comes first, return the position of the last occurrence.
Debug.assert(isDeclaredBeforeUse);
Debug.assert(lastPos >= 0);
return lastPos;
}

function getFirstDeclaration(type: Type): Declaration | undefined {
let firstDeclaration;

Expand Down
252 changes: 252 additions & 0 deletions src/services/refactors/generateGetAccessorAndSetAccessor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,252 @@
/* @internal */
namespace ts.refactor.generateGetAccessorAndSetAccessor {
const actionName = "Generate 'get' and 'set' accessors";
const actionDescription = Diagnostics.Generate_get_and_set_accessors.message;
registerRefactor(actionName, { getEditsForAction, getAvailableActions });

type AccepedDeclaration = ParameterDeclaration | PropertyDeclaration | PropertyAssignment;
type AccepedNameType = Identifier | StringLiteral;
type ContainerDeclation = ClassLikeDeclaration | ObjectLiteralExpression;

interface DeclarationInfo {
container: ContainerDeclation;
isStatic: boolean;
type: TypeNode | undefined;
}

interface Info extends DeclarationInfo {
declaration: AccepedDeclaration;
fieldName: AccepedNameType;
accessorName: AccepedNameType;
}

function getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined {
const { file, startPosition } = context;
if (!getConvertibleFieldAtPosition(file, startPosition)) return undefined;

return [{
name: actionName,
description: actionDescription,
actions: [
{
name: actionName,
description: actionDescription
}
]
}];
}

function getEditsForAction(context: RefactorContext, _actionName: string): RefactorEditInfo | undefined {
const { file, startPosition } = context;

const fieldInfo = getConvertibleFieldAtPosition(file, startPosition);
if (!fieldInfo) return undefined;

const isJS = isSourceFileJavaScript(file);
const changeTracker = textChanges.ChangeTracker.fromContext(context);
const { isStatic, fieldName, accessorName, type, container, declaration } = fieldInfo;

const isInClassLike = isClassLike(container);
const accessorModifiers = getAccessorModifiers(isJS, declaration, isStatic, isInClassLike);
const fieldModifiers = getFieldModifiers(isJS, isStatic, isInClassLike);

updateFieldDeclaration(changeTracker, file, declaration, fieldName, fieldModifiers, container);

const getAccessor = generateGetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);
const setAccessor = generateSetAccessor(fieldName, accessorName, type, accessorModifiers, isStatic, container);

insertAccessor(changeTracker, file, getAccessor, declaration, container);
insertAccessor(changeTracker, file, setAccessor, declaration, container);

const edits = changeTracker.getChanges();
const renameFilename = file.fileName;
const renameLocationOffset = isIdentifier(fieldName) ? 0 : -1;
const renameLocation = renameLocationOffset + getRenameLocation(edits, renameFilename, fieldName.text, /*isDeclaredBeforeUse*/ false);
return { renameFilename, renameLocation, edits };
}

function isConvertableName (name: DeclarationName): name is AccepedNameType {
return isIdentifier(name) || isStringLiteral(name);
}

function createPropertyName (name: string, originalName: AccepedNameType) {
return isIdentifier(originalName) ? createIdentifier(name) : createLiteral(name);
}

function createAccessorAccessExpression (fieldName: AccepedNameType, isStatic: boolean, container: ContainerDeclation) {
const leftHead = isStatic ? (<ClassLikeDeclaration>container).name : createThis();
return isIdentifier(fieldName) ? createPropertyAccess(leftHead, fieldName) : createElementAccess(leftHead, createLiteral(fieldName));
}

function getAccessorModifiers(isJS: boolean, declaration: AccepedDeclaration, isStatic: boolean, isClassLike: boolean): NodeArray<Modifier> | undefined {
if (!isClassLike) return undefined;

if (!declaration.modifiers || getModifierFlags(declaration) & ModifierFlags.Private) {
const modifiers = append<Modifier>(
!isJS ? [createToken(SyntaxKind.PublicKeyword)] : undefined,
isStatic ? createToken(SyntaxKind.StaticKeyword) : undefined
);
return modifiers && createNodeArray(modifiers);
}
return declaration.modifiers;
}

function getFieldModifiers(isJS: boolean, isStatic: boolean, isClassLike: boolean): NodeArray<Modifier> | undefined {
if (!isClassLike) return undefined;

const modifiers = append<Modifier>(
!isJS ? [createToken(SyntaxKind.PrivateKeyword)] : undefined,
isStatic ? createToken(SyntaxKind.StaticKeyword) : undefined
);
return modifiers && createNodeArray(modifiers);
}

function getPropertyDeclarationInfo(propertyDeclaration: PropertyDeclaration): DeclarationInfo | undefined {
if (!isClassLike(propertyDeclaration.parent) || !propertyDeclaration.parent.members) return undefined;

return {
isStatic: hasStaticModifier(propertyDeclaration),
type: propertyDeclaration.type,
container: propertyDeclaration.parent
};
}

function getParameterPropertyDeclarationInfo(parameterDeclaration: ParameterDeclaration): DeclarationInfo | undefined {
if (!isClassLike(parameterDeclaration.parent.parent) || !parameterDeclaration.parent.parent.members) return undefined;

return {
isStatic: false,
type: parameterDeclaration.type,
container: parameterDeclaration.parent.parent
};
}

function getPropertyAssignmentDeclarationInfo(propertyAssignment: PropertyAssignment): DeclarationInfo | undefined {
return {
isStatic: false,
type: undefined,
container: propertyAssignment.parent
};
}

function getDeclarationInfo(declaration: AccepedDeclaration) {
if (isPropertyDeclaration(declaration)) {
return getPropertyDeclarationInfo(declaration);
}
else if (isPropertyAssignment(declaration)) {
return getPropertyAssignmentDeclarationInfo(declaration);
}
else {
return getParameterPropertyDeclarationInfo(declaration);
}
}

function getConvertibleFieldAtPosition(file: SourceFile, startPosition: number): Info | undefined {
const node = getTokenAtPosition(file, startPosition, /*includeJsDocComment*/ false);
const declaration = <AccepedDeclaration>findAncestor(node.parent, or(isParameterPropertyDeclaration, isPropertyDeclaration, isPropertyAssignment));
// make sure propertyDeclaration have AccessibilityModifier or Static Modifier
const meaning = ModifierFlags.AccessibilityModifier | ModifierFlags.Static;
if (!declaration || !isConvertableName(declaration.name) || (getModifierFlags(declaration) | meaning) !== meaning) return undefined;

const info = getDeclarationInfo(declaration);
const fieldName = createPropertyName(getUniqueName(`_${declaration.name.text}`, file.text), declaration.name);
suppressLeadingAndTrailingTrivia(fieldName);
suppressLeadingAndTrailingTrivia(declaration);
return {
...info,
declaration,
fieldName,
accessorName: createPropertyName(declaration.name.text, declaration.name)
};
}

function generateGetAccessor(fieldName: AccepedNameType, accessorName: AccepedNameType, type: TypeNode, modifiers: ModifiersArray | undefined, isStatic: boolean, container: ContainerDeclation) {
return createGetAccessor(
/*decorators*/ undefined,
modifiers,
accessorName,
/*parameters*/ undefined,
type,
createBlock([
createReturn(
createAccessorAccessExpression(fieldName, isStatic, container)
)
], /*multiLine*/ true)
);
}

function generateSetAccessor(fieldName: AccepedNameType, accessorName: AccepedNameType, type: TypeNode, modifiers: ModifiersArray | undefined, isStatic: boolean, container: ContainerDeclation) {
return createSetAccessor(
/*decorators*/ undefined,
modifiers,
accessorName,
[createParameter(
/*decorators*/ undefined,
/*modifiers*/ undefined,
/*dotDotDotToken*/ undefined,
createIdentifier("value"),
/*questionToken*/ undefined,
type
)],
createBlock([
createStatement(
createAssignment(
createAccessorAccessExpression(fieldName, isStatic, container),
createIdentifier("value")
)
)
], /*multiLine*/ true)
);
}

function updatePropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyDeclaration, fieldName: AccepedNameType, modifiers: ModifiersArray | undefined) {
const property = updateProperty(
declaration,
declaration.decorators,
modifiers,
fieldName,
declaration.questionToken || declaration.exclamationToken,
declaration.type,
declaration.initializer
);

changeTracker.replaceNode(file, declaration, property);
}

function updateParameterPropertyDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: ParameterDeclaration, fieldName: AccepedNameType, modifiers: ModifiersArray | undefined, classLikeContainer: ClassLikeDeclaration) {
const property = createProperty(
declaration.decorators,
modifiers,
fieldName,
declaration.questionToken,
declaration.type,
declaration.initializer
);

changeTracker.insertNodeAtClassStart(file, classLikeContainer, property);
changeTracker.deleteNodeInList(file, declaration);
}

function updatePropertyAssignmentDeclaration (changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: PropertyAssignment, fieldName: AccepedNameType) {
const assignment = updatePropertyAssignment(declaration, fieldName, declaration.initializer);
changeTracker.replacePropertyAssignment(file, declaration, assignment);
}

function updateFieldDeclaration(changeTracker: textChanges.ChangeTracker, file: SourceFile, declaration: AccepedDeclaration, fieldName: AccepedNameType, modifiers: ModifiersArray | undefined, container: ContainerDeclation) {
if (isPropertyDeclaration(declaration)) {
updatePropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers);
}
else if (isPropertyAssignment(declaration)) {
updatePropertyAssignmentDeclaration(changeTracker, file, declaration, fieldName);
}
else {
updateParameterPropertyDeclaration(changeTracker, file, declaration, fieldName, modifiers, <ClassLikeDeclaration>container);
}
}

function insertAccessor(changeTracker: textChanges.ChangeTracker, file: SourceFile, accessor: AccessorDeclaration, declaration: AccepedDeclaration, container: ContainerDeclation) {
isParameterPropertyDeclaration(declaration)
? changeTracker.insertNodeAtClassStart(file, <ClassLikeDeclaration>container, accessor)
: changeTracker.insertNodeAfter(file, declaration, accessor);
}
}
1 change: 1 addition & 0 deletions src/services/refactors/refactors.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
/// <reference path="extractSymbol.ts" />
/// <reference path="generateGetAccessorAndSetAccessor.ts" />
9 changes: 9 additions & 0 deletions src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,12 @@ namespace ts.textChanges {
return this.replaceRangeWithNodes(sourceFile, getAdjustedRange(sourceFile, startNode, endNode, options), newNodes, options);
}

public replacePropertyAssignment(sourceFile: SourceFile, oldNode: PropertyAssignment, newNode: PropertyAssignment) {
return this.replaceNode(sourceFile, oldNode, newNode, {
suffix: "," + this.newLineCharacter
});
}

private insertNodeAt(sourceFile: SourceFile, pos: number, newNode: Node, options: InsertNodeOptions = {}) {
this.replaceRange(sourceFile, createTextRange(pos), newNode, options);
}
Expand Down Expand Up @@ -468,6 +474,9 @@ namespace ts.textChanges {
else if (isVariableDeclaration(node)) {
return { prefix: ", " };
}
else if (isPropertyAssignment(node)) {
return { suffix: "," + this.newLineCharacter };
}
else if (isParameter(node)) {
return {};
}
Expand Down
Loading

0 comments on commit 9c0671d

Please sign in to comment.