-
Notifications
You must be signed in to change notification settings - Fork 12.8k
add refactor of convert private field to getter and setter #22143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 8 commits
bed084f
0e4d513
507e804
1af3f46
f60e7b2
56d4834
e5daa8c
fafdcf5
0da98bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,248 @@ | ||
/* @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 (isJS || !isClassLike) return undefined; | ||
|
||
if (!declaration.modifiers || getModifierFlags(declaration) & ModifierFlags.Private) { | ||
return createNodeArray( | ||
append<Modifier>([createToken(SyntaxKind.PublicKeyword)], | ||
isStatic ? createToken(SyntaxKind.StaticKeyword) : undefined)); | ||
} | ||
return declaration.modifiers; | ||
} | ||
|
||
function getFieldModifiers(isJS: boolean, isStatic: boolean, isClassLike: boolean): NodeArray<Modifier> | undefined { | ||
if (isJS || !isClassLike) return undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you still want to handel the return createNodeArray(
append<Modifier>(isJS ? [] : [createToken(SyntaxKind.PrivateKeyword)],
isStatic ? createToken(SyntaxKind.StaticKeyword) : undefined)); |
||
|
||
return createNodeArray( | ||
append<Modifier>([createToken(SyntaxKind.PrivateKeyword)], | ||
isStatic ? createToken(SyntaxKind.StaticKeyword) : undefined)); | ||
} | ||
|
||
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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what should i do with this type declaration, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, you are fine. the type will be inferred from the return type of the getter. |
||
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; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also keep in mind that also you should add a test for that. |
||
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); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
/// <reference path="extractSymbol.ts" /> | ||
/// <reference path="generateGetAccessorAndSetAccessor.ts" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: would move this closer to its use.