Skip to content

Commit 00d3fee

Browse files
IlanayaIlanaya
authored
feat(refactoring): add/from destructure improvements (#182)
1 parent 3bc7159 commit 00d3fee

File tree

4 files changed

+337
-51
lines changed

4 files changed

+337
-51
lines changed

‎typescript/src/codeActions/custom/addDestructure.ts

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,4 @@
1-
import {
2-
findChildContainingExactPosition,
3-
getChangesTracker,
4-
getPositionHighlights,
5-
isValidInitializerForDestructure,
6-
isNameUniqueAtNodeClosestScope,
7-
} from '../../utils'
1+
import { findChildContainingExactPosition, getChangesTracker, getPositionHighlights, isValidInitializerForDestructure, makeUniqueName } from '../../utils'
82
import { CodeAction } from '../getCodeActions'
93

104
const createDestructuredDeclaration = (initializer: ts.Expression, type: ts.TypeNode | undefined, declarationName: ts.BindingName) => {
@@ -32,46 +26,86 @@ const addDestructureToVariableWithSplittedPropertyAccessors = (
3226
formatOptions: ts.FormatCodeSettings | undefined,
3327
languageService: ts.LanguageService,
3428
) => {
35-
if (!ts.isIdentifier(node) && !(ts.isPropertyAccessExpression(node.parent) || ts.isParameter(node.parent))) return
29+
if (!ts.isIdentifier(node) && !(ts.isPropertyAccessExpression(node.parent) || ts.isParameter(node.parent) || !ts.isElementAccessExpression(node.parent)))
30+
return
3631

3732
const highlightPositions = getPositionHighlights(node.getStart(), sourceFile, languageService)
3833

3934
if (!highlightPositions) return
4035
const tracker = getChangesTracker(formatOptions ?? {})
4136

42-
const propertyNames: Array<{ initial: string; unique: string | undefined }> = []
37+
const propertyNames: Array<{ initial: string; unique: string | undefined; dotDotDotToken?: ts.DotDotDotToken }> = []
4338
let nodeToReplaceWithBindingPattern: ts.Identifier | undefined
4439

4540
for (const pos of highlightPositions) {
4641
const highlightedNode = findChildContainingExactPosition(sourceFile, pos)
4742

4843
if (!highlightedNode) continue
4944

50-
if (ts.isIdentifier(highlightedNode) && ts.isPropertyAccessExpression(highlightedNode.parent)) {
51-
const propertyAccessorName = highlightedNode.parent.name.getText()
45+
if (
46+
ts.isIdentifier(highlightedNode) &&
47+
(ts.isPropertyAccessExpression(highlightedNode.parent) || ts.isElementAccessExpression(highlightedNode.parent))
48+
) {
49+
if (ts.isElementAccessExpression(highlightedNode.parent) && ts.isIdentifier(highlightedNode.parent.argumentExpression)) {
50+
const uniqueName = makeUniqueName('newVariable', node, languageService, sourceFile)
5251

53-
const uniquePropertyName = isNameUniqueAtNodeClosestScope(propertyAccessorName, node, languageService.getProgram()!.getTypeChecker())
54-
? undefined
55-
: tsFull.getUniqueName(propertyAccessorName, sourceFile as any)
52+
propertyNames.push({
53+
initial: 'newVariable',
54+
unique: uniqueName === 'newVariable' ? undefined : uniqueName,
55+
dotDotDotToken: ts.factory.createToken(ts.SyntaxKind.DotDotDotToken),
56+
})
5657

57-
propertyNames.push({ initial: propertyAccessorName, unique: uniquePropertyName })
58+
tracker.replaceRangeWithText(sourceFile, { pos, end: highlightedNode.end }, uniqueName)
5859

59-
tracker.replaceRangeWithText(sourceFile, { pos, end: highlightedNode.parent.end }, uniquePropertyName ?? propertyAccessorName)
60+
continue
61+
}
62+
const indexedAccessorName =
63+
ts.isElementAccessExpression(highlightedNode.parent) && ts.isStringLiteral(highlightedNode.parent.argumentExpression)
64+
? highlightedNode.parent.argumentExpression.text
65+
: undefined
66+
67+
const accessorName = ts.isPropertyAccessExpression(highlightedNode.parent) ? highlightedNode.parent.name.getText() : indexedAccessorName
68+
69+
if (!accessorName) continue
70+
71+
const uniqueName = makeUniqueName(accessorName, node, languageService, sourceFile)
72+
73+
propertyNames.push({ initial: accessorName, unique: uniqueName === accessorName ? undefined : uniqueName })
74+
75+
// Replace both variable and property access expression `a.fo|o` -> `foo`
76+
// if (ts.isIdentifier(highlightedNode.parent.expression)) {
77+
// tracker.replaceRangeWithText(
78+
// sourceFile,
79+
// { pos: highlightedNode.parent.end, end: highlightedNode.parent.expression.end },
80+
// uniquePropertyName || propertyAccessorName,
81+
// )
82+
// continue
83+
// }
84+
85+
tracker.replaceRangeWithText(sourceFile, { pos, end: highlightedNode.parent.end }, uniqueName)
6086
continue
6187
}
6288

6389
if (ts.isIdentifier(highlightedNode) && (ts.isVariableDeclaration(highlightedNode.parent) || ts.isParameter(highlightedNode.parent))) {
6490
nodeToReplaceWithBindingPattern = highlightedNode
6591
continue
6692
}
93+
// Support for `const a = { foo: 1 }; a.fo|o` refactor activation
94+
// if (ts.isIdentifier(highlightedNode) && ts.isPropertyAssignment(highlightedNode.parent)) {
95+
// const closestParent = ts.findAncestor(highlightedNode.parent, n => ts.isVariableDeclaration(n))
96+
97+
// if (!closestParent || !ts.isVariableDeclaration(closestParent) || !ts.isIdentifier(closestParent.name)) continue
98+
// nodeToReplaceWithBindingPattern = closestParent.name
99+
// }
67100
}
68101

69102
if (!nodeToReplaceWithBindingPattern || propertyNames.length === 0) return
70103

71-
const bindings = propertyNames.map(({ initial, unique }) => {
72-
return ts.factory.createBindingElement(undefined, unique ? initial : undefined, unique ?? initial)
104+
const bindings = propertyNames.map(({ initial, unique, dotDotDotToken }) => {
105+
return ts.factory.createBindingElement(dotDotDotToken, unique ? initial : undefined, unique ?? initial)
73106
})
74-
const bindingPattern = ts.factory.createObjectBindingPattern(bindings)
107+
const bindingsWithRestLast = bindings.sort((a, b) => (!a.dotDotDotToken && !b.dotDotDotToken ? 0 : -1))
108+
const bindingPattern = ts.factory.createObjectBindingPattern(bindingsWithRestLast)
75109
const { pos, end } = nodeToReplaceWithBindingPattern
76110

77111
tracker.replaceRange(

‎typescript/src/codeActions/custom/fromDestructure.ts

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,14 @@ const convertFromDestructureWithVariableNameReplacement = (
6666

6767
const BASE_VARIABLE_NAME = 'newVariable'
6868

69-
const variableName = isNameUniqueAtNodeClosestScope(BASE_VARIABLE_NAME, declarationName, languageService.getProgram()!.getTypeChecker())
69+
const uniqueVariableName = isNameUniqueAtNodeClosestScope(BASE_VARIABLE_NAME, declarationName, languageService.getProgram()!.getTypeChecker())
7070
? BASE_VARIABLE_NAME
7171
: tsFull.getUniqueName(BASE_VARIABLE_NAME, sourceFile as unknown as FullSourceFile)
7272

73+
const uniqueVariableIdentifier = ts.factory.createIdentifier(uniqueVariableName)
74+
7375
for (const binding of bindings) {
74-
const declaration = createFlattenedExpressionFromDestructuring(binding, ts.factory.createIdentifier(variableName))
76+
const declaration = createFlattenedExpressionFromDestructuring(binding, uniqueVariableIdentifier)
7577

7678
/** Important to use `getEnd()` here to get correct highlights for destructured and renamed binding, e.g. `{ bar: bar_1 }` */
7779
const bindingNameEndPos = binding.getEnd()
@@ -85,20 +87,18 @@ const convertFromDestructureWithVariableNameReplacement = (
8587
}
8688
const node = findChildContainingExactPosition(sourceFile, pos)
8789

88-
if (!node) continue
90+
if (!node || ts.isPropertyAssignment(node.parent)) continue
8991
const printer = ts.createPrinter()
9092

91-
tracker.replaceRangeWithText(sourceFile, { pos, end: node.end }, printer.printNode(ts.EmitHint.Unspecified, declaration, sourceFile))
93+
// If dotDotDotToken is present, we work with rest element, so we need to replace it with identifier
94+
const replacement = binding.dotDotDotToken ? uniqueVariableIdentifier : declaration
95+
tracker.replaceRangeWithText(sourceFile, { pos, end: node.end }, printer.printNode(ts.EmitHint.Unspecified, replacement, sourceFile))
9296
}
9397
}
9498

9599
const declarationNameLeadingTrivia = declarationName.getLeadingTriviaWidth(sourceFile)
96100

97-
tracker.replaceRange(
98-
sourceFile,
99-
{ pos: declarationName.pos + declarationNameLeadingTrivia, end: declarationName.end },
100-
ts.factory.createIdentifier(variableName),
101-
)
101+
tracker.replaceRange(sourceFile, { pos: declarationName.pos + declarationNameLeadingTrivia, end: declarationName.end }, uniqueVariableIdentifier)
102102
const changes = tracker.getChanges()
103103
return {
104104
edits: [

‎typescript/src/utils.ts

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ export const getIndentFromPos = (typescript: typeof ts, sourceFile: ts.SourceFil
8080
)
8181
}
8282

83-
export const findClosestParent = (node: ts.Node, stopKinds: ts.SyntaxKind[], rejectKinds: ts.SyntaxKind[]) => {
84-
rejectKinds = [...rejectKinds, ts.SyntaxKind.SourceFile]
83+
export const findClosestParent = (node: ts.Node, stopKinds: ts.SyntaxKind[], rejectKinds: ts.SyntaxKind[], skipSourceFile = true) => {
84+
rejectKinds = [...rejectKinds, ...(skipSourceFile ? [ts.SyntaxKind.SourceFile] : [])]
8585
while (node && !stopKinds.includes(node.kind)) {
8686
if (rejectKinds.includes(node.kind)) return
8787
node = node.parent
@@ -352,24 +352,79 @@ export const isValidInitializerForDestructure = (match: ts.Expression) => {
352352
}
353353
export const isNameUniqueAtLocation = (name: string, location: ts.Node | undefined, typeChecker: ts.TypeChecker) => {
354354
const checker = getFullTypeChecker(typeChecker)
355-
let result: boolean | undefined
355+
let hasCollision: boolean | undefined
356356

357357
const checkCollision = (childNode: ts.Node) => {
358-
if (result) return
359-
result = !!checker.resolveName(name, childNode as unknown as import('typescript-full').Node, ts.SymbolFlags.Value, true)
358+
if (hasCollision) return
359+
hasCollision = !!checker.resolveName(name, childNode as unknown as import('typescript-full').Node, ts.SymbolFlags.Value, true)
360360

361361
if (ts.isBlock(childNode)) {
362362
childNode.forEachChild(checkCollision)
363363
}
364364
}
365-
location?.forEachChild(checkCollision)
366-
return !result
365+
if (!location) return
366+
367+
if (ts.isSourceFile(location)) {
368+
hasCollision = createUniqueName(name, location as any) !== name
369+
} else {
370+
location.forEachChild(checkCollision)
371+
}
372+
return !hasCollision
367373
}
368374
export const isNameUniqueAtNodeClosestScope = (name: string, node: ts.Node, typeChecker: ts.TypeChecker) => {
369375
const closestScope = findClosestParent(
370376
node,
371-
[ts.SyntaxKind.Block, ts.SyntaxKind.FunctionDeclaration, ts.SyntaxKind.FunctionExpression, ts.SyntaxKind.ArrowFunction],
377+
[ts.SyntaxKind.Block, ts.SyntaxKind.FunctionDeclaration, ts.SyntaxKind.FunctionExpression, ts.SyntaxKind.ArrowFunction, ts.SyntaxKind.SourceFile],
372378
[],
379+
false,
373380
)
374381
return isNameUniqueAtLocation(name, closestScope, typeChecker)
375382
}
383+
384+
const createUniqueName = (name: string, sourceFile: ts.SourceFile) => {
385+
/**
386+
* A free identifier is an identifier that can be accessed through name lookup as a local variable.
387+
* In the expression `x.y`, `x` is a free identifier, but `y` is not.
388+
*/
389+
const forEachFreeIdentifier = (node: ts.Node, cb: (id: ts.Identifier) => void) => {
390+
if (ts.isIdentifier(node) && isFreeIdentifier(node)) cb(node)
391+
node.forEachChild(child => forEachFreeIdentifier(child, cb))
392+
}
393+
394+
const isFreeIdentifier = (node: ts.Identifier): boolean => {
395+
const { parent } = node
396+
switch (parent.kind) {
397+
case ts.SyntaxKind.PropertyAccessExpression:
398+
return (parent as ts.PropertyAccessExpression).name !== node
399+
case ts.SyntaxKind.BindingElement:
400+
return (parent as ts.BindingElement).propertyName !== node
401+
case ts.SyntaxKind.ImportSpecifier:
402+
return (parent as ts.ImportSpecifier).propertyName !== node
403+
case ts.SyntaxKind.PropertyAssignment:
404+
return (parent as ts.PropertyAssignment).name !== node
405+
default:
406+
return true
407+
}
408+
}
409+
const collectFreeIdentifiers = (file: ts.SourceFile) => {
410+
const arr: string[] = []
411+
forEachFreeIdentifier(file, id => arr.push(id.text))
412+
return arr
413+
}
414+
415+
const identifiers = collectFreeIdentifiers(sourceFile)
416+
while (identifiers.includes(name)) {
417+
name = `_${name}`
418+
}
419+
return name
420+
}
421+
422+
export const makeUniqueName = (accessorName: string, node: ts.Node, languageService: ts.LanguageService, sourceFile: ts.SourceFile) => {
423+
const isNameUniqueInScope = isNameUniqueAtNodeClosestScope(accessorName, node, languageService.getProgram()!.getTypeChecker())
424+
const isReservedWord = tsFull.isIdentifierANonContextualKeyword(tsFull.factory.createIdentifier(accessorName))
425+
426+
const uniquePropertyName = isNameUniqueInScope ? undefined : createUniqueName(accessorName, sourceFile)
427+
428+
const uniqueReservedPropName = isReservedWord ? createUniqueName(`_${accessorName}`, sourceFile) : undefined
429+
return uniqueReservedPropName || uniquePropertyName || accessorName
430+
}

0 commit comments

Comments
 (0)