From cde9b9f35eb98a469f79eddb9acac8d7073f46b6 Mon Sep 17 00:00:00 2001 From: kingwl Date: Thu, 30 Jul 2020 18:15:10 +0800 Subject: [PATCH 01/28] wip --- .../refactors/convertObjectDestruction.ts | 29 +++++++++++++++++++ src/services/tsconfig.json | 1 + 2 files changed, 30 insertions(+) create mode 100644 src/services/refactors/convertObjectDestruction.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts new file mode 100644 index 0000000000000..c26570dcf5967 --- /dev/null +++ b/src/services/refactors/convertObjectDestruction.ts @@ -0,0 +1,29 @@ +/* @internal */ +namespace ts.refactor { + const refactorName = "Introduce Destruction"; + const actionNameIntroduceObjectDestruction = "Convert property access to Object destruction"; + registerRefactor(refactorName, { getAvailableActions, getEditsForAction }); + + function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { + const { file, startPosition } = context; + const isJSFile = isSourceFileJS(file); + if (isJSFile) return emptyArray; + + const functionDeclaration = getFunctionDeclarationAtPosition(file, startPosition, context.program.getTypeChecker()); + if (!functionDeclaration) return emptyArray; + + const description = getLocaleSpecificMessage(Diagnostics.Convert_parameters_to_destructured_object); + return [{ + name: refactorName, + description, + actions: [{ + name: refactorName, + description + }] + }]; + } + + function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { + return undefined; + } +} \ No newline at end of file diff --git a/src/services/tsconfig.json b/src/services/tsconfig.json index cc24ac7f9c405..5d07ce67476e2 100644 --- a/src/services/tsconfig.json +++ b/src/services/tsconfig.json @@ -107,6 +107,7 @@ "codefixes/fixExpectedComma.ts", "refactors/convertExport.ts", "refactors/convertImport.ts", + "refactors/convertObjectDestruction.ts", "refactors/convertToOptionalChainExpression.ts", "refactors/convertOverloadListToSingleSignature.ts", "refactors/extractSymbol.ts", From 35667744135e57c3cb1637ebb522b0f5c65b4784 Mon Sep 17 00:00:00 2001 From: kingwl Date: Thu, 30 Jul 2020 21:24:52 +0800 Subject: [PATCH 02/28] wip --- .../refactors/convertObjectDestruction.ts | 46 +++++++++++++++++-- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index c26570dcf5967..9203a75bbe3d9 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -5,12 +5,12 @@ namespace ts.refactor { registerRefactor(refactorName, { getAvailableActions, getEditsForAction }); function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { - const { file, startPosition } = context; + const { file, startPosition, program, cancellationToken } = context; const isJSFile = isSourceFileJS(file); - if (isJSFile) return emptyArray; + if (isJSFile || !cancellationToken) return emptyArray; - const functionDeclaration = getFunctionDeclarationAtPosition(file, startPosition, context.program.getTypeChecker()); - if (!functionDeclaration) return emptyArray; + const info = getInfo(file, startPosition, program.getTypeChecker(), program, cancellationToken) + if (!info) return emptyArray; const description = getLocaleSpecificMessage(Diagnostics.Convert_parameters_to_destructured_object); return [{ @@ -24,6 +24,42 @@ namespace ts.refactor { } function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { + if (context && actionName && actionNameIntroduceObjectDestruction) { + + } return undefined; } -} \ No newline at end of file + + interface Info { + valueDeclaration: Exclude + referencedExpression: Expression[] + } + + function getInfo(file: SourceFile, startPosition: number, checker: TypeChecker, program: Program, cancellationToken: CancellationToken): Info | undefined { + const node = getTouchingToken(file, startPosition); + if (!isAccessExpression(node.parent) || node.parent.expression !== node) return undefined + const symbol = checker.getSymbolAtLocation(node); + if (!symbol || !symbol.valueDeclaration || !isVariableLike(symbol.valueDeclaration) || isEnumMember(symbol.valueDeclaration)) return undefined; + + // Find only current file + const references = FindAllReferences.getReferenceEntriesForNode(-1, node, program, [file], cancellationToken); + const accessExprReferences = mapDefined(references, reference => { + if (reference.kind !== FindAllReferences.EntryKind.Node) { + return undefined; + } + + if (!isAccessExpression(reference.node.parent) || reference.node.parent.expression !== reference.node) { + return undefined; + } + + return reference.node; + }) + const referencedExpression = map(accessExprReferences, x => cast(x, isExpression)); + if (!accessExprReferences || !accessExprReferences.length) return undefined; + + return { + referencedExpression, + valueDeclaration: symbol.valueDeclaration, + } + } +} From deac7f65f59d729aca2c4829b3f776e5d06e1c24 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 31 Jul 2020 15:28:27 +0800 Subject: [PATCH 03/28] add desruction refactor --- .../refactors/convertObjectDestruction.ts | 140 +++++++++++++++--- .../refactorIntroduceDestruction1.ts | 22 +++ 2 files changed, 141 insertions(+), 21 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction1.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 9203a75bbe3d9..7e914e29e76ae 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -9,57 +9,155 @@ namespace ts.refactor { const isJSFile = isSourceFileJS(file); if (isJSFile || !cancellationToken) return emptyArray; - const info = getInfo(file, startPosition, program.getTypeChecker(), program, cancellationToken) + const info = getInfo(file, startPosition, program.getTypeChecker(), program, cancellationToken); if (!info) return emptyArray; - const description = getLocaleSpecificMessage(Diagnostics.Convert_parameters_to_destructured_object); return [{ name: refactorName, - description, + description: actionNameIntroduceObjectDestruction, actions: [{ name: refactorName, - description + description: actionNameIntroduceObjectDestruction }] }]; } - function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { - if (context && actionName && actionNameIntroduceObjectDestruction) { + function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { + Debug.assert(actionName === refactorName); + const { file, startPosition, program, cancellationToken } = context; + const isJSFile = isSourceFileJS(file); + + const emptyResult: RefactorEditInfo = { edits: [] }; + if (isJSFile || !cancellationToken) return emptyResult; + + const info = getInfo(file, startPosition, program.getTypeChecker(), program, cancellationToken); + if (!info) return emptyResult; + + const edits = textChanges.ChangeTracker.with(context, t => doChange(t, file, info)); + return { renameFilename: undefined, renameLocation: undefined, edits }; + } + + function doChange(changeTracker: textChanges.ChangeTracker, file: SourceFile, info: Info) { + const nameMap = new Map(); + const bindingElements: BindingElement[] = []; + info.referencedAccessExpression.forEach(([expr, name]) => { + if (!nameMap.has(name)) { + const temp = factory.createUniqueName(name, GeneratedIdentifierFlags.Optimistic); + nameMap.set(name, temp.text); + bindingElements.push(getUniqueDestructionName(expr, name, temp.text)); + } + const newName = nameMap.get(name)!; + + changeTracker.replaceNode( + file, + expr, + factory.createIdentifier(newName) + ); + }); + + const newBinding = factory.createVariableStatement( + /* modifiers*/ undefined, + factory.createVariableDeclarationList( + [ + factory.createVariableDeclaration( + factory.createObjectBindingPattern(bindingElements), + /*exclamationToken*/ undefined, + /*type*/ undefined, + info.replacementExpression + ) + ], + NodeFlags.Const + ), + ); + + const firstReferencedStatement = findAncestor(info.firstReferenced, isStatement); + Debug.assertIsDefined(firstReferencedStatement); + changeTracker.insertNodeBefore( + file, + firstReferencedStatement, + newBinding + ); + } + function getUniqueDestructionName(expr: AccessExpression, name: string, newName: string) { + const needRename = name !== newName; + if (isPropertyAccessExpression(expr)) { + const bindingName = cast(expr.name, isIdentifier); + return factory.createBindingElement( + /* dotDotDotToken*/ undefined, + needRename ? expr.name : undefined, + needRename ? newName : bindingName, + ); + } + else { + const argExpr = cast(expr.argumentExpression, isPropertyName); + return factory.createBindingElement( + /* dotDotDotToken*/ undefined, + argExpr, + newName, + ); } - return undefined; } interface Info { - valueDeclaration: Exclude - referencedExpression: Expression[] + replacementExpression: Expression, + referencedAccessExpression: [AccessExpression, string][] + firstReferenced: Expression } function getInfo(file: SourceFile, startPosition: number, checker: TypeChecker, program: Program, cancellationToken: CancellationToken): Info | undefined { const node = getTouchingToken(file, startPosition); - if (!isAccessExpression(node.parent) || node.parent.expression !== node) return undefined + if (!isAccessExpression(node.parent) || node.parent.expression !== node) return undefined; const symbol = checker.getSymbolAtLocation(node); - if (!symbol || !symbol.valueDeclaration || !isVariableLike(symbol.valueDeclaration) || isEnumMember(symbol.valueDeclaration)) return undefined; + if (!symbol || checker.isUnknownSymbol(symbol) || !symbol.valueDeclaration || !isVariableLike(symbol.valueDeclaration) || isEnumMember(symbol.valueDeclaration)) return undefined; // Find only current file const references = FindAllReferences.getReferenceEntriesForNode(-1, node, program, [file], cancellationToken); - const accessExprReferences = mapDefined(references, reference => { + let firstReferenced: Expression | undefined; + const referencedAccessExpression: [AccessExpression, string][] = mapDefined(references, reference => { if (reference.kind !== FindAllReferences.EntryKind.Node) { return undefined; } - + if (node !== symbol.valueDeclaration && isExpression(node) && (!firstReferenced || node.pos < firstReferenced.pos)) { + firstReferenced = node; + } if (!isAccessExpression(reference.node.parent) || reference.node.parent.expression !== reference.node) { return undefined; } - return reference.node; - }) - const referencedExpression = map(accessExprReferences, x => cast(x, isExpression)); - if (!accessExprReferences || !accessExprReferences.length) return undefined; + if (isElementAccessExpression(reference.node.parent)) { + if (!isStringLiteralLike(reference.node.parent.argumentExpression)) { + return undefined; + } + + return [reference.node.parent, reference.node.parent.argumentExpression.text]; + } + + return [reference.node.parent, reference.node.parent.name.text]; + }); + if (!referencedAccessExpression.length || !firstReferenced) return undefined; + + const type = checker.getTypeOfSymbolAtLocation(symbol, firstReferenced); + let hasUnconvertableReference = false; + forEach(referencedAccessExpression, ([expr, name]) => { + const referenceType = checker.getTypeAtLocation(expr); + if (referenceType !== type) { + const accessType = checker.getTypeAtLocation(expr); + const prop = checker.getPropertyOfType(type, name); + + if (!prop || checker.getTypeOfSymbolAtLocation(prop, expr) !== accessType) { + hasUnconvertableReference = true; + return "quit"; + } + } + }); + + if (hasUnconvertableReference) return undefined; return { - referencedExpression, - valueDeclaration: symbol.valueDeclaration, - } + replacementExpression: node.parent.expression, + firstReferenced, + referencedAccessExpression, + }; } -} +} \ No newline at end of file diff --git a/tests/cases/fourslash/refactorIntroduceDestruction1.ts b/tests/cases/fourslash/refactorIntroduceDestruction1.ts new file mode 100644 index 0000000000000..27e3227b9e694 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction1.ts @@ -0,0 +1,22 @@ +/// + +//// declare const un: { type: 'number'; payload: number } | { type: 'string', payload: number } +//// if(/*a*/un/*b*/.type === "number") { +//// un.payload.toExponential +//// } else { +//// un.payload.toFixed +//// } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `declare const un: { type: 'number'; payload: number } | { type: 'string', payload: number } +const { type, payload } = un +if(type === "number") { + payload.toExponential +} else { + payload.toFixed +}`, +}); From b146289c384315aa0fc7661e518f3e5b99c5bc99 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 31 Jul 2020 17:58:27 +0800 Subject: [PATCH 04/28] Add smart selection and unique name detection --- .../refactors/convertObjectDestruction.ts | 71 +++++++++++++------ src/services/refactors/extractType.ts | 4 -- src/services/utilities.ts | 5 ++ 3 files changed, 53 insertions(+), 27 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 7e914e29e76ae..f21cf3f35d82c 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -5,11 +5,11 @@ namespace ts.refactor { registerRefactor(refactorName, { getAvailableActions, getEditsForAction }); function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { - const { file, startPosition, program, cancellationToken } = context; + const { file, program, cancellationToken } = context; const isJSFile = isSourceFileJS(file); if (isJSFile || !cancellationToken) return emptyArray; - const info = getInfo(file, startPosition, program.getTypeChecker(), program, cancellationToken); + const info = getInfo(context, file, program.getTypeChecker(), program, cancellationToken, /*resolveUniqueName*/ false); if (!info) return emptyArray; return [{ @@ -24,13 +24,13 @@ namespace ts.refactor { function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { Debug.assert(actionName === refactorName); - const { file, startPosition, program, cancellationToken } = context; + const { file, program, cancellationToken } = context; const isJSFile = isSourceFileJS(file); - const emptyResult: RefactorEditInfo = { edits: [] }; + const emptyResult: RefactorEditInfo = { edits: [] }; if (isJSFile || !cancellationToken) return emptyResult; - const info = getInfo(file, startPosition, program.getTypeChecker(), program, cancellationToken); + const info = getInfo(context, file, program.getTypeChecker(), program, cancellationToken, /*resolveUniqueName*/ true); if (!info) return emptyResult; const edits = textChanges.ChangeTracker.with(context, t => doChange(t, file, info)); @@ -42,11 +42,14 @@ namespace ts.refactor { const bindingElements: BindingElement[] = []; info.referencedAccessExpression.forEach(([expr, name]) => { if (!nameMap.has(name)) { - const temp = factory.createUniqueName(name, GeneratedIdentifierFlags.Optimistic); - nameMap.set(name, temp.text); - bindingElements.push(getUniqueDestructionName(expr, name, temp.text)); + const needAlias = info.namesNeedUniqueName.has(name); + const uniqueName = needAlias ? getUniqueName(name, file) : name; + nameMap.set(name, uniqueName); + bindingElements.push(getUniqueDestructionName(expr, needAlias, uniqueName)); } - const newName = nameMap.get(name)!; + + const newName = nameMap.get(name); + Debug.assertIsDefined(newName); changeTracker.replaceNode( file, @@ -79,14 +82,13 @@ namespace ts.refactor { ); } - function getUniqueDestructionName(expr: AccessExpression, name: string, newName: string) { - const needRename = name !== newName; + function getUniqueDestructionName(expr: AccessExpression, needAlias: boolean, newName: string) { if (isPropertyAccessExpression(expr)) { const bindingName = cast(expr.name, isIdentifier); return factory.createBindingElement( /* dotDotDotToken*/ undefined, - needRename ? expr.name : undefined, - needRename ? newName : bindingName, + needAlias ? expr.name : undefined, + needAlias ? newName : bindingName, ); } else { @@ -103,11 +105,16 @@ namespace ts.refactor { replacementExpression: Expression, referencedAccessExpression: [AccessExpression, string][] firstReferenced: Expression + namesNeedUniqueName: Set } - function getInfo(file: SourceFile, startPosition: number, checker: TypeChecker, program: Program, cancellationToken: CancellationToken): Info | undefined { - const node = getTouchingToken(file, startPosition); - if (!isAccessExpression(node.parent) || node.parent.expression !== node) return undefined; + function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean): Info | undefined { + const current = getTokenAtPosition(file, context.startPosition); + const range = createTextRangeFromSpan(getRefactorContextSpan(context)); + + const node = findAncestor(current, (node => node.parent && isAccessExpression(node.parent) && !rangeContainsSkipTrivia(range, node.parent, file))); + + if (!node || !isAccessExpression(node.parent) || node.parent.expression !== node) return undefined; const symbol = checker.getSymbolAtLocation(node); if (!symbol || checker.isUnknownSymbol(symbol) || !symbol.valueDeclaration || !isVariableLike(symbol.valueDeclaration) || isEnumMember(symbol.valueDeclaration)) return undefined; @@ -121,24 +128,35 @@ namespace ts.refactor { if (node !== symbol.valueDeclaration && isExpression(node) && (!firstReferenced || node.pos < firstReferenced.pos)) { firstReferenced = node; } - if (!isAccessExpression(reference.node.parent) || reference.node.parent.expression !== reference.node) { + + let lastChild: Node | undefined; + const topReferencedAccessExpression = findAncestor(reference.node, n => { + if (isAccessExpression(n) && n.expression === lastChild) { + return true; + } + lastChild = n; + return false; + }); + if (!topReferencedAccessExpression) { return undefined; } - if (isElementAccessExpression(reference.node.parent)) { - if (!isStringLiteralLike(reference.node.parent.argumentExpression)) { + const accessExpression = cast(topReferencedAccessExpression, isAccessExpression); + if (isElementAccessExpression(accessExpression)) { + if (!isStringLiteralLike(accessExpression.argumentExpression)) { return undefined; } - return [reference.node.parent, reference.node.parent.argumentExpression.text]; + return [accessExpression, accessExpression.argumentExpression.text]; } - return [reference.node.parent, reference.node.parent.name.text]; + return [accessExpression, accessExpression.name.text]; }); if (!referencedAccessExpression.length || !firstReferenced) return undefined; - const type = checker.getTypeOfSymbolAtLocation(symbol, firstReferenced); let hasUnconvertableReference = false; + const namesNeedUniqueName = new Set(); + const type = checker.getTypeOfSymbolAtLocation(symbol, firstReferenced); forEach(referencedAccessExpression, ([expr, name]) => { const referenceType = checker.getTypeAtLocation(expr); if (referenceType !== type) { @@ -150,14 +168,21 @@ namespace ts.refactor { return "quit"; } } + + if (resolveUniqueName) { + const symbol = checker.resolveName(name, /*location*/ undefined, SymbolFlags.Value, /*excludeGlobals*/ false); + if (symbol) { + namesNeedUniqueName.add(name); + } + } }); if (hasUnconvertableReference) return undefined; - return { replacementExpression: node.parent.expression, firstReferenced, referencedAccessExpression, + namesNeedUniqueName }; } } \ No newline at end of file diff --git a/src/services/refactors/extractType.ts b/src/services/refactors/extractType.ts index 6f144cfe40b42..8cf64aa4cf8ea 100644 --- a/src/services/refactors/extractType.ts +++ b/src/services/refactors/extractType.ts @@ -125,10 +125,6 @@ namespace ts.refactor { return undefined; } - function rangeContainsSkipTrivia(r1: TextRange, node: Node, file: SourceFile): boolean { - return rangeContainsStartEnd(r1, skipTrivia(file.text, node.pos), node.end); - } - function collectTypeParameters(checker: TypeChecker, selection: TypeNode, statement: Statement, file: SourceFile): TypeParameterDeclaration[] | undefined { const result: TypeParameterDeclaration[] = []; return visitor(selection) ? undefined : result; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index c11c65d076dd6..4360dfff9d6f0 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2863,6 +2863,11 @@ namespace ts { return isArray(valueOrArray) ? first(valueOrArray) : valueOrArray; } + /* @internal */ + export function rangeContainsSkipTrivia(r1: TextRange, node: Node, file: SourceFile): boolean { + return rangeContainsStartEnd(r1, skipTrivia(file.text, node.pos), node.end); + } + export function getNameForExportedSymbol(symbol: Symbol, scriptTarget: ScriptTarget) { if (symbol.escapedName === InternalSymbolName.ExportEquals || symbol.escapedName === InternalSymbolName.Default) { // Name of "export default foo;" is "foo". Name of "export default 0" is the filename converted to camelCase. From 9c72d1c28769a776b8a056f7c99792888dc5cc5d Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 31 Jul 2020 18:15:58 +0800 Subject: [PATCH 05/28] add call and assignment detection --- src/compiler/utilities.ts | 2 +- src/services/refactors/convertObjectDestruction.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 41d513a252303..341f9979b573a 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -2708,7 +2708,7 @@ namespace ts { return skipOuterExpressions(node, OuterExpressionKinds.Parentheses); } - function skipParenthesesUp(node: Node): Node { + export function skipParenthesesUp(node: Node): Node { while (node.kind === SyntaxKind.ParenthesizedExpression) { node = node.parent; } diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index f21cf3f35d82c..d0bcd8ddb62e4 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -137,7 +137,7 @@ namespace ts.refactor { lastChild = n; return false; }); - if (!topReferencedAccessExpression) { + if (!topReferencedAccessExpression || isAssignmentTarget(topReferencedAccessExpression) || isCallLikeExpression(skipParenthesesUp(topReferencedAccessExpression.parent))) { return undefined; } From de086004afb6de212734388be2f918f27916872f Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 31 Jul 2020 19:21:35 +0800 Subject: [PATCH 06/28] fix incorrect convert --- .../refactors/convertObjectDestruction.ts | 69 ++++++++++++------- 1 file changed, 45 insertions(+), 24 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index d0bcd8ddb62e4..c416cae6d3126 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -121,61 +121,82 @@ namespace ts.refactor { // Find only current file const references = FindAllReferences.getReferenceEntriesForNode(-1, node, program, [file], cancellationToken); let firstReferenced: Expression | undefined; - const referencedAccessExpression: [AccessExpression, string][] = mapDefined(references, reference => { + const referencedAccessExpression: [AccessExpression, string][] = []; + const allReferencedAcccessExpression: AccessExpression[] = []; + forEach(references, reference => { if (reference.kind !== FindAllReferences.EntryKind.Node) { return undefined; } - if (node !== symbol.valueDeclaration && isExpression(node) && (!firstReferenced || node.pos < firstReferenced.pos)) { - firstReferenced = node; - } - let lastChild: Node | undefined; - const topReferencedAccessExpression = findAncestor(reference.node, n => { - if (isAccessExpression(n) && n.expression === lastChild) { - return true; + let lastChild = reference.node; + const topReferencedAccessExpression = findAncestor(reference.node.parent, n => { + if (isAccessExpression(n) || isParenthesizedExpression(n)) { + if (isAccessExpression(n) && n.expression === lastChild) { + return true; + } + lastChild = n; + return false; } - lastChild = n; - return false; + return "quit"; }); - if (!topReferencedAccessExpression || isAssignmentTarget(topReferencedAccessExpression) || isCallLikeExpression(skipParenthesesUp(topReferencedAccessExpression.parent))) { - return undefined; + if (!topReferencedAccessExpression) { + return; } const accessExpression = cast(topReferencedAccessExpression, isAccessExpression); + allReferencedAcccessExpression.push(accessExpression); + + if (isAssignmentTarget(accessExpression)) { + return; + } + + const referencedParentMaybeCall = skipParenthesesUp(accessExpression.parent); + if (isCallOrNewExpression(referencedParentMaybeCall) && !(referencedParentMaybeCall.arguments && rangeContainsRange(referencedParentMaybeCall.arguments, topReferencedAccessExpression))) { + return; + } + + if (accessExpression !== symbol.valueDeclaration && isExpression(accessExpression) && (!firstReferenced || reference.node.pos < firstReferenced.pos)) { + firstReferenced = accessExpression; + } + if (isElementAccessExpression(accessExpression)) { if (!isStringLiteralLike(accessExpression.argumentExpression)) { - return undefined; + return; } - return [accessExpression, accessExpression.argumentExpression.text]; + referencedAccessExpression.push([accessExpression, accessExpression.argumentExpression.text]); + return; } - return [accessExpression, accessExpression.name.text]; + referencedAccessExpression.push([accessExpression, accessExpression.name.text]); }); - if (!referencedAccessExpression.length || !firstReferenced) return undefined; + if (!firstReferenced || !referencedAccessExpression.length || !some(referencedAccessExpression, ([r]) => rangeContainsRange(r, current))) return undefined; let hasUnconvertableReference = false; const namesNeedUniqueName = new Set(); const type = checker.getTypeOfSymbolAtLocation(symbol, firstReferenced); - forEach(referencedAccessExpression, ([expr, name]) => { + forEach(allReferencedAcccessExpression, expr => { const referenceType = checker.getTypeAtLocation(expr); if (referenceType !== type) { + const accessSymbol = checker.getSymbolAtLocation(expr); const accessType = checker.getTypeAtLocation(expr); - const prop = checker.getPropertyOfType(type, name); + const prop = accessSymbol?.name && checker.getPropertyOfType(type, accessSymbol.name); if (!prop || checker.getTypeOfSymbolAtLocation(prop, expr) !== accessType) { hasUnconvertableReference = true; return "quit"; } } + }); - if (resolveUniqueName) { + if (resolveUniqueName) { + forEach(referencedAccessExpression, ([, name]) => { const symbol = checker.resolveName(name, /*location*/ undefined, SymbolFlags.Value, /*excludeGlobals*/ false); - if (symbol) { - namesNeedUniqueName.add(name); - } - } - }); + if (symbol) { + namesNeedUniqueName.add(name); + } + }); + } if (hasUnconvertableReference) return undefined; return { From bf481c149a51dd62ebdf7dda0eaf7d0d5196bfb7 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 31 Jul 2020 19:21:57 +0800 Subject: [PATCH 07/28] fix lint --- src/services/refactors/convertObjectDestruction.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index c416cae6d3126..88c5399b6b7e1 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -192,9 +192,9 @@ namespace ts.refactor { if (resolveUniqueName) { forEach(referencedAccessExpression, ([, name]) => { const symbol = checker.resolveName(name, /*location*/ undefined, SymbolFlags.Value, /*excludeGlobals*/ false); - if (symbol) { - namesNeedUniqueName.add(name); - } + if (symbol) { + namesNeedUniqueName.add(name); + } }); } From 784692304b38cc297531c04388ab6326cfad450e Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 31 Jul 2020 20:24:06 +0800 Subject: [PATCH 08/28] add some tests --- .../refactorIntroduceDestruction1.ts | 20 ++++----- .../refactorIntroduceDestruction10.ts | 16 +++++++ .../refactorIntroduceDestruction11.ts | 18 ++++++++ .../refactorIntroduceDestruction12.ts | 10 +++++ .../refactorIntroduceDestruction13.ts | 20 +++++++++ .../refactorIntroduceDestruction14.ts | 28 +++++++++++++ .../refactorIntroduceDestruction15.ts | 29 +++++++++++++ .../refactorIntroduceDestruction16.ts | 28 +++++++++++++ .../refactorIntroduceDestruction17.ts | 23 ++++++++++ .../refactorIntroduceDestruction18.ts | 42 +++++++++++++++++++ .../refactorIntroduceDestruction2.ts | 18 ++++++++ .../refactorIntroduceDestruction3.ts | 9 ++++ .../refactorIntroduceDestruction4.ts | 15 +++++++ .../refactorIntroduceDestruction5.ts | 22 ++++++++++ .../refactorIntroduceDestruction6.ts | 18 ++++++++ .../refactorIntroduceDestruction7.ts | 20 +++++++++ .../refactorIntroduceDestruction8.ts | 14 +++++++ .../refactorIntroduceDestruction9.ts | 10 +++++ 18 files changed, 348 insertions(+), 12 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction10.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction11.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction12.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction13.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction14.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction15.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction16.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction17.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction18.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction2.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction3.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction4.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction5.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction6.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction7.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction8.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction9.ts diff --git a/tests/cases/fourslash/refactorIntroduceDestruction1.ts b/tests/cases/fourslash/refactorIntroduceDestruction1.ts index 27e3227b9e694..064fef5823849 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction1.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction1.ts @@ -1,22 +1,18 @@ /// -//// declare const un: { type: 'number'; payload: number } | { type: 'string', payload: number } -//// if(/*a*/un/*b*/.type === "number") { -//// un.payload.toExponential -//// } else { -//// un.payload.toFixed +//// const item = { +//// a: 1, b: 2 //// } +//// call(/*a*/item/*b*/.a, item.b) goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce Destruction", actionName: "Introduce Destruction", actionDescription: "Convert property access to Object destruction", - newContent: `declare const un: { type: 'number'; payload: number } | { type: 'string', payload: number } -const { type, payload } = un -if(type === "number") { - payload.toExponential -} else { - payload.toFixed -}`, + newContent: `const item = { + a: 1, b: 2 +} +const { a, b } = item +call(a, b)`, }); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction10.ts b/tests/cases/fourslash/refactorIntroduceDestruction10.ts new file mode 100644 index 0000000000000..51d0a196ee4fb --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction10.ts @@ -0,0 +1,16 @@ +/// + +//// function foo (item: { a: string, b: number }) { +//// call(/*a*/item/*b*/.a, item.b) +//// } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `function foo (item: { a: string, b: number }) { + const { a, b } = item; + call(a, b) +}`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction11.ts b/tests/cases/fourslash/refactorIntroduceDestruction11.ts new file mode 100644 index 0000000000000..f6193d26acffb --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction11.ts @@ -0,0 +1,18 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// call(/*a*/item/*b*/["a"], item.b) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: 2 +} +const { "a": a_1, b } = item +call(a_1, b)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction12.ts b/tests/cases/fourslash/refactorIntroduceDestruction12.ts new file mode 100644 index 0000000000000..4ce582d9b09b2 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction12.ts @@ -0,0 +1,10 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// const key = "a" +//// call(/*a*/item/*b*/[key], item.b) + +goTo.select("a", "b"); +verify.not.refactorAvailable("Introduce Destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction13.ts b/tests/cases/fourslash/refactorIntroduceDestruction13.ts new file mode 100644 index 0000000000000..cb491c0e8c938 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction13.ts @@ -0,0 +1,20 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// const a = false +//// call(/*a*/item/*b*/.a, item.b) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: 2 +} +const a = false +const { a: a_1, b } = item +call(a_1, b)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction14.ts b/tests/cases/fourslash/refactorIntroduceDestruction14.ts new file mode 100644 index 0000000000000..a42e0461bd2e7 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction14.ts @@ -0,0 +1,28 @@ +/// + +//// const item = { +//// a: 1, b: { +//// c: { +//// d: 1, +//// e: 2 +//// } +//// } +//// } +//// call(/*a*/item/*b*/.a, item.b, item.b.c, item.b.c.d, item.b.c.e) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: { + c: { + d: 1, + e: 2 + } + } +} +const { a, b } = item +call(a, b, b.c, b.c.d, b.c.e)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction15.ts b/tests/cases/fourslash/refactorIntroduceDestruction15.ts new file mode 100644 index 0000000000000..fa28aa91b70e6 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction15.ts @@ -0,0 +1,29 @@ +/// + +//// const item = { +//// a: 1, b: { +//// c: { +//// d: 1, +//// e: 2 +//// } +//// } +//// } +//// call(item.a, item.b, /*a*/item.b/*b*/.c, item.b.c.d, item.b.c.e) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: { + c: { + d: 1, + e: 2 + } + } +} +const { c } = item.b +call(item.a, item.b, c, c.d, c.e)`, +}); + diff --git a/tests/cases/fourslash/refactorIntroduceDestruction16.ts b/tests/cases/fourslash/refactorIntroduceDestruction16.ts new file mode 100644 index 0000000000000..458407dcbba38 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction16.ts @@ -0,0 +1,28 @@ +/// + +//// const item = { +//// a: 1, b: { +//// c: { +//// d: 1, +//// e: 2 +//// } +//// } +//// } +//// call(item.a, item.b, item.b.c, /*a*/item.b.c/*b*/.d, item.b.c.e) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: { + c: { + d: 1, + e: 2 + } + } +} +const { d, e } = item.b.c +call(item.a, item.b, item.b.c, d, e)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction17.ts b/tests/cases/fourslash/refactorIntroduceDestruction17.ts new file mode 100644 index 0000000000000..a1ea0be88580a --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction17.ts @@ -0,0 +1,23 @@ +/// + +//// interface A { +//// f: 1, +//// b: number +//// c: () => void +//// } +//// interface B { +//// f: 2, +//// b: number +//// c: () => string +//// } +//// declare const a: A | B +//// if (/*a*/a/*b*/.f === 1) { +//// a.b = 1 +//// a.c() +//// } else { +//// a.b = 2 +//// a.c() +//// } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Introduce Destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction18.ts b/tests/cases/fourslash/refactorIntroduceDestruction18.ts new file mode 100644 index 0000000000000..c910c69807d42 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction18.ts @@ -0,0 +1,42 @@ +/// + +//// interface A { +//// f: 1, +//// b: number +//// c: () => void +//// } +//// interface B { +//// f: 2, +//// b: number +//// c: () => string +//// } +//// declare const a: A | B +//// if (/*a*/a/*b*/.f === 1) { +//// a.b = 1 +//// } else { +//// a.b = 2 +//// } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `interface A { + f: 1, + b: number + c: () => void +} +interface B { + f: 2, + b: number + c: () => string +} +declare const a: A | B +const { f } = a +if (f === 1) { + a.b = 1 +} else { + a.b = 2 +}`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction2.ts b/tests/cases/fourslash/refactorIntroduceDestruction2.ts new file mode 100644 index 0000000000000..1ccec06c849cd --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction2.ts @@ -0,0 +1,18 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// call(item.a, /*a*/item/*b*/.b) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: 2 +} +const { a, b } = item +call(a, b)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction3.ts b/tests/cases/fourslash/refactorIntroduceDestruction3.ts new file mode 100644 index 0000000000000..5a84e07029035 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction3.ts @@ -0,0 +1,9 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// call(/*a*/item.a/*b*/, item.b) + +goTo.select("a", "b"); +verify.not.refactorAvailable("Introduce Destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction4.ts b/tests/cases/fourslash/refactorIntroduceDestruction4.ts new file mode 100644 index 0000000000000..320aa5da76d92 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction4.ts @@ -0,0 +1,15 @@ +/// + +//// declare const u: { type: 'number'; payload: number } | { type: 'string', payload: string } +//// if(/*a*/u/*b*/.type === "number") { +//// /*c*/u/*d*/.payload.toExponential +//// } else { +//// /*e*/u/*f*/.payload.big +//// } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Introduce Destruction") +goTo.select("c", "d"); +verify.not.refactorAvailable("Introduce Destruction") +goTo.select("e", "f"); +verify.not.refactorAvailable("Introduce Destruction") \ No newline at end of file diff --git a/tests/cases/fourslash/refactorIntroduceDestruction5.ts b/tests/cases/fourslash/refactorIntroduceDestruction5.ts new file mode 100644 index 0000000000000..f91e379332a7b --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction5.ts @@ -0,0 +1,22 @@ +/// + +//// declare const u: { type: 'number'; payload: number } | { type: 'string', payload: number } +//// if(/*a*/u/*b*/.type === "number") { +//// u.payload.toExponential +//// } else { +//// u.payload.toFixed +//// } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `declare const u: { type: 'number'; payload: number } | { type: 'string', payload: number } +const { type, payload } = u +if(type === "number") { + payload.toExponential +} else { + payload.toFixed +}`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction6.ts b/tests/cases/fourslash/refactorIntroduceDestruction6.ts new file mode 100644 index 0000000000000..576b097b40a53 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction6.ts @@ -0,0 +1,18 @@ +/// + +//// const item = { +//// a: 1, b: () => void +//// } +//// call(/*a*/item/*b*/.a, item.b()) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: () => void +} +const { a } = item +call(a, item.b())`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction7.ts b/tests/cases/fourslash/refactorIntroduceDestruction7.ts new file mode 100644 index 0000000000000..5a6a88cb5c26d --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction7.ts @@ -0,0 +1,20 @@ +/// + +//// const item = { +//// a: 1, b: () => void +//// } +//// item.a = 2 +//// call(/*a*/item/*b*/.a, item.b()) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: () => void +} +item.a = 2 +const { a } = item +call(a, item.b())`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction8.ts b/tests/cases/fourslash/refactorIntroduceDestruction8.ts new file mode 100644 index 0000000000000..c2ed806992328 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction8.ts @@ -0,0 +1,14 @@ +/// + +//// const item = { +//// a: 1, b: () => void +//// } +//// /*a*/item/*b*/.a = 2 +//// call(item.a, /*c*/item/*d*/.b(), (((/*e*/item/*f*/.b)))()) + +goTo.select("a", "b"); +verify.not.refactorAvailable("Introduce Destruction") +goTo.select("c", "d"); +verify.not.refactorAvailable("Introduce Destruction") +goTo.select("e", "f"); +verify.not.refactorAvailable("Introduce Destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction9.ts b/tests/cases/fourslash/refactorIntroduceDestruction9.ts new file mode 100644 index 0000000000000..c5b682e63ac7d --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction9.ts @@ -0,0 +1,10 @@ +/// + +//// enum E { +//// A, +//// B +//// } +//// /*a*/E/*b*/.A + +goTo.select("a", "b"); +verify.not.refactorAvailable("Introduce Destruction") From fa63cbeb72a59617d281141b575e636703e4a676 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 31 Jul 2020 20:34:12 +0800 Subject: [PATCH 09/28] fix element access --- src/services/refactors/convertObjectDestruction.ts | 7 +++++-- tests/cases/fourslash/refactorIntroduceDestruction11.ts | 4 ++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 88c5399b6b7e1..a7840f5f4d449 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -178,9 +178,12 @@ namespace ts.refactor { forEach(allReferencedAcccessExpression, expr => { const referenceType = checker.getTypeAtLocation(expr); if (referenceType !== type) { - const accessSymbol = checker.getSymbolAtLocation(expr); + const propName = isElementAccessExpression(expr) ? + cast(expr.argumentExpression, isStringLiteralLike).text : + checker.getSymbolAtLocation(expr)?.name; + const accessType = checker.getTypeAtLocation(expr); - const prop = accessSymbol?.name && checker.getPropertyOfType(type, accessSymbol.name); + const prop = propName && checker.getPropertyOfType(type, propName); if (!prop || checker.getTypeOfSymbolAtLocation(prop, expr) !== accessType) { hasUnconvertableReference = true; diff --git a/tests/cases/fourslash/refactorIntroduceDestruction11.ts b/tests/cases/fourslash/refactorIntroduceDestruction11.ts index f6193d26acffb..8c859ba9c0d64 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction11.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction11.ts @@ -13,6 +13,6 @@ edit.applyRefactor({ newContent: `const item = { a: 1, b: 2 } -const { "a": a_1, b } = item -call(a_1, b)`, +const { "a": a, b } = item +call(a, b)`, }); From 8465df63635582b4fef0ceaafc451b5f7207e72a Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 31 Jul 2020 20:41:00 +0800 Subject: [PATCH 10/28] fix element access collapse --- .../refactors/convertObjectDestruction.ts | 2 +- .../refactorIntroduceDestruction11.ts | 2 +- .../refactorIntroduceDestruction19.ts | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction19.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index a7840f5f4d449..69c74a291ec53 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -95,7 +95,7 @@ namespace ts.refactor { const argExpr = cast(expr.argumentExpression, isPropertyName); return factory.createBindingElement( /* dotDotDotToken*/ undefined, - argExpr, + getTextOfPropertyName(argExpr) !== newName ? argExpr : undefined, newName, ); } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction11.ts b/tests/cases/fourslash/refactorIntroduceDestruction11.ts index 8c859ba9c0d64..efce9de031c36 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction11.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction11.ts @@ -13,6 +13,6 @@ edit.applyRefactor({ newContent: `const item = { a: 1, b: 2 } -const { "a": a, b } = item +const { a, b } = item call(a, b)`, }); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction19.ts b/tests/cases/fourslash/refactorIntroduceDestruction19.ts new file mode 100644 index 0000000000000..5b1b5f0862d9c --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction19.ts @@ -0,0 +1,18 @@ +/// + +//// const item = { +//// "a-a-a": 1, b: 2 +//// } +//// call(/*a*/item/*b*/["a-a-a"], item.b) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: 2 +} +const { "a-a-a": a, b } = item +call(a, b)`, +}); From 9f37ce9e14a62aa4d2e827ca435eb3f0ad3213d9 Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 3 Aug 2020 12:09:05 +0800 Subject: [PATCH 11/28] Do not allow invalid identifier name --- src/services/refactors/convertObjectDestruction.ts | 9 ++++++++- .../cases/fourslash/refactorIntroduceDestruction19.ts | 11 +---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 69c74a291ec53..206cca0acf510 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -95,7 +95,7 @@ namespace ts.refactor { const argExpr = cast(expr.argumentExpression, isPropertyName); return factory.createBindingElement( /* dotDotDotToken*/ undefined, - getTextOfPropertyName(argExpr) !== newName ? argExpr : undefined, + needAlias ? factory.cloneNode(argExpr) : undefined, newName, ); } @@ -110,6 +110,7 @@ namespace ts.refactor { function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean): Info | undefined { const current = getTokenAtPosition(file, context.startPosition); + const compilerOptions = context.program.getCompilerOptions(); const range = createTextRangeFromSpan(getRefactorContextSpan(context)); const node = findAncestor(current, (node => node.parent && isAccessExpression(node.parent) && !rangeContainsSkipTrivia(range, node.parent, file))); @@ -163,10 +164,16 @@ namespace ts.refactor { if (!isStringLiteralLike(accessExpression.argumentExpression)) { return; } + if (!isIdentifierText(accessExpression.argumentExpression.text, compilerOptions.target, compilerOptions.jsx ? LanguageVariant.JSX : LanguageVariant.Standard)) { + return; + } referencedAccessExpression.push([accessExpression, accessExpression.argumentExpression.text]); return; } + if (!isIdentifierText(accessExpression.name.text, compilerOptions.target, compilerOptions.jsx ? LanguageVariant.JSX : LanguageVariant.Standard)) { + return; + } referencedAccessExpression.push([accessExpression, accessExpression.name.text]); }); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction19.ts b/tests/cases/fourslash/refactorIntroduceDestruction19.ts index 5b1b5f0862d9c..d00b384b0d659 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction19.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction19.ts @@ -6,13 +6,4 @@ //// call(/*a*/item/*b*/["a-a-a"], item.b) goTo.select("a", "b"); -edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", - newContent: `const item = { - a: 1, b: 2 -} -const { "a-a-a": a, b } = item -call(a, b)`, -}); +verify.not.refactorAvailable("Introduce Destruction") From 87facd220bfcb2af53bae0f3285ad0037f19b0d3 Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 3 Aug 2020 16:42:49 +0800 Subject: [PATCH 12/28] Fix hosting --- .../refactors/convertObjectDestruction.ts | 39 +++++++++++++++---- .../refactorIntroduceDestruction20.ts | 26 +++++++++++++ .../refactorIntroduceDestruction21.ts | 20 ++++++++++ .../refactorIntroduceDestruction22.ts | 11 ++++++ .../refactorIntroduceDestruction23.ts | 37 ++++++++++++++++++ .../refactorIntroduceDestruction24.ts | 26 +++++++++++++ .../refactorIntroduceDestruction25.ts | 18 +++++++++ 7 files changed, 169 insertions(+), 8 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction20.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction21.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction22.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction23.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction24.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction25.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 206cca0acf510..32032f401df6c 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -73,11 +73,9 @@ namespace ts.refactor { ), ); - const firstReferencedStatement = findAncestor(info.firstReferenced, isStatement); - Debug.assertIsDefined(firstReferencedStatement); changeTracker.insertNodeBefore( file, - firstReferencedStatement, + info.firstReferencedStatement, newBinding ); } @@ -105,6 +103,7 @@ namespace ts.refactor { replacementExpression: Expression, referencedAccessExpression: [AccessExpression, string][] firstReferenced: Expression + firstReferencedStatement: Statement namesNeedUniqueName: Set } @@ -122,8 +121,14 @@ namespace ts.refactor { // Find only current file const references = FindAllReferences.getReferenceEntriesForNode(-1, node, program, [file], cancellationToken); let firstReferenced: Expression | undefined; + let firstReferencedStatement: Statement | undefined; const referencedAccessExpression: [AccessExpression, string][] = []; const allReferencedAcccessExpression: AccessExpression[] = []; + const container = isParameter(symbol.valueDeclaration) ? + symbol.valueDeclaration : + findAncestor(symbol.valueDeclaration, or(isStatement, isSourceFile)); + Debug.assertIsDefined(container); + forEach(references, reference => { if (reference.kind !== FindAllReferences.EntryKind.Node) { return undefined; @@ -131,8 +136,12 @@ namespace ts.refactor { let lastChild = reference.node; const topReferencedAccessExpression = findAncestor(reference.node.parent, n => { - if (isAccessExpression(n) || isParenthesizedExpression(n)) { - if (isAccessExpression(n) && n.expression === lastChild) { + if (isParenthesizedExpression(n)) { + lastChild = n; + return false; + } + else if (isAccessExpression(n)) { + if (n.expression === lastChild) { return true; } lastChild = n; @@ -156,8 +165,21 @@ namespace ts.refactor { return; } - if (accessExpression !== symbol.valueDeclaration && isExpression(accessExpression) && (!firstReferenced || reference.node.pos < firstReferenced.pos)) { - firstReferenced = accessExpression; + if (reference.node.pos < symbol.valueDeclaration.pos) { + return; + } + + if (accessExpression !== symbol.valueDeclaration && isExpression(accessExpression)) { + if (!firstReferenced || accessExpression.pos < firstReferenced.pos) { + firstReferenced = accessExpression; + } + const referencedStatement = findAncestor(accessExpression, n => { + const parent = n.parent && isBlock(n.parent) && isFunctionLikeDeclaration(n.parent.parent) ? n.parent.parent : n.parent; + return isStatement(n) && parent === container.parent; + }); + if (referencedStatement && (!firstReferencedStatement || referencedStatement.pos < firstReferencedStatement.pos)) { + firstReferencedStatement = cast(referencedStatement, isStatement); + } } if (isElementAccessExpression(accessExpression)) { @@ -177,7 +199,7 @@ namespace ts.refactor { referencedAccessExpression.push([accessExpression, accessExpression.name.text]); }); - if (!firstReferenced || !referencedAccessExpression.length || !some(referencedAccessExpression, ([r]) => rangeContainsRange(r, current))) return undefined; + if (!firstReferenced || !firstReferencedStatement || !referencedAccessExpression.length || !some(referencedAccessExpression, ([r]) => rangeContainsRange(r, current))) return undefined; let hasUnconvertableReference = false; const namesNeedUniqueName = new Set(); @@ -212,6 +234,7 @@ namespace ts.refactor { return { replacementExpression: node.parent.expression, firstReferenced, + firstReferencedStatement, referencedAccessExpression, namesNeedUniqueName }; diff --git a/tests/cases/fourslash/refactorIntroduceDestruction20.ts b/tests/cases/fourslash/refactorIntroduceDestruction20.ts new file mode 100644 index 0000000000000..97ca595c81d0d --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction20.ts @@ -0,0 +1,26 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// if (Math.random() < 0.5) { +//// call(/*a*/item/*b*/.a) +//// } else { +//// call(item.b) +//// } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: 2 +} +const { a, b } = item +if (Math.random() < 0.5) { + call(a) +} else { + call(b) +}`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction21.ts b/tests/cases/fourslash/refactorIntroduceDestruction21.ts new file mode 100644 index 0000000000000..bf348c3e77022 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction21.ts @@ -0,0 +1,20 @@ +/// + +//// function foo (item: { a: string, b: number }) { +//// function bar () { +//// call(/*a*/item/*b*/.a, item.b) +//// } +//// } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `function foo (item: { a: string, b: number }) { + const { a, b } = item; + function bar () { + call(a, b) + } +}`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction22.ts b/tests/cases/fourslash/refactorIntroduceDestruction22.ts new file mode 100644 index 0000000000000..d96a089bab98a --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction22.ts @@ -0,0 +1,11 @@ +/// + +//// function foo () { +//// function bar () { +//// call(/*a*/item/*b*/.a, item.b) +//// } +//// const item = { a: 1, b: 2 } +//// } + +goTo.select("a", "b"); +verify.not.refactorAvailable("Introduce Destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction23.ts b/tests/cases/fourslash/refactorIntroduceDestruction23.ts new file mode 100644 index 0000000000000..ace9b6e391ca1 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction23.ts @@ -0,0 +1,37 @@ +/// + +//// const item = { a: 1, b: 2 } +//// function foo() { +//// call(/*a*/item/*b*/.a) +//// } +//// function bar() { +//// call(item.b) +//// } +//// if (Math.max() < 0.5) { +//// call(item.a) +//// } +//// else { +//// call(item.b) +//// } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { a: 1, b: 2 } +const { a, b } = item +function foo() { + call(a) +} +function bar() { + call(b) +} +if (Math.max() < 0.5) { + call(a) +} +else { + call(b) +}`, +}); + diff --git a/tests/cases/fourslash/refactorIntroduceDestruction24.ts b/tests/cases/fourslash/refactorIntroduceDestruction24.ts new file mode 100644 index 0000000000000..2b39a2e3cb636 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction24.ts @@ -0,0 +1,26 @@ +/// + +//// function foo () { +//// call(item.a) +//// } +//// const item = { a: 1, b: 2 } +//// function bar () { +//// call(/*a*/item/*b*/.a) +//// } +//// const aa = item.b + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `function foo () { + call(item.a) +} +const item = { a: 1, b: 2 } +const { a, b } = item +function bar () { + call(a) +} +const aa = b`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction25.ts b/tests/cases/fourslash/refactorIntroduceDestruction25.ts new file mode 100644 index 0000000000000..a6c14070b69a1 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction25.ts @@ -0,0 +1,18 @@ +/// + +//// const item = { +//// a: 1, b: 2, c: 3 +//// } +//// call(/*a*/item/*b*/.a, item.a) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = { + a: 1, b: 2, c: 3 +} +const { a } = item +call(a, a)`, +}); From 7133140d9eea4c31a83aac18bc3965e90b0dc08a Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 3 Aug 2020 16:56:28 +0800 Subject: [PATCH 13/28] Add more cases --- .../refactors/convertObjectDestruction.ts | 3 +++ .../fourslash/refactorIntroduceDestruction26.ts | 7 +++++++ .../fourslash/refactorIntroduceDestruction27.ts | 16 ++++++++++++++++ 3 files changed, 26 insertions(+) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction26.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction27.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 32032f401df6c..d457fbaa763b6 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -168,6 +168,9 @@ namespace ts.refactor { if (reference.node.pos < symbol.valueDeclaration.pos) { return; } + if (isFunctionLikeDeclaration(container.parent) && container.parent.body && !rangeContainsRange(container.parent.body, reference.node)) { + return; + } if (accessExpression !== symbol.valueDeclaration && isExpression(accessExpression)) { if (!firstReferenced || accessExpression.pos < firstReferenced.pos) { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction26.ts b/tests/cases/fourslash/refactorIntroduceDestruction26.ts new file mode 100644 index 0000000000000..ad1cd452c0074 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction26.ts @@ -0,0 +1,7 @@ +/// + +//// const f = (item: { a: 1, b: 2}) => /*a*/item/*b*/.a + item.b + +goTo.select("a", "b"); +verify.not.refactorAvailable("Introduce Destruction") + diff --git a/tests/cases/fourslash/refactorIntroduceDestruction27.ts b/tests/cases/fourslash/refactorIntroduceDestruction27.ts new file mode 100644 index 0000000000000..acd11ccaf45d4 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction27.ts @@ -0,0 +1,16 @@ +/// + +//// function f(item: { a: 1 }, b = item.a) { +//// call(/*a*/item/*b*/.a, b) +//// } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `function f(item: { a: 1 }, b = item.a) { + const { a } = item; + call(a, b) +}`, +}); From bfb782ae54b681b72bfc232f781bc0a6cff565e3 Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 3 Aug 2020 17:07:30 +0800 Subject: [PATCH 14/28] refactor some code --- .../refactors/convertObjectDestruction.ts | 86 ++++++++++--------- 1 file changed, 46 insertions(+), 40 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index d457fbaa763b6..a85a575294e30 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -124,51 +124,15 @@ namespace ts.refactor { let firstReferencedStatement: Statement | undefined; const referencedAccessExpression: [AccessExpression, string][] = []; const allReferencedAcccessExpression: AccessExpression[] = []; - const container = isParameter(symbol.valueDeclaration) ? - symbol.valueDeclaration : - findAncestor(symbol.valueDeclaration, or(isStatement, isSourceFile)); + const container = isParameter(symbol.valueDeclaration) ? symbol.valueDeclaration : findAncestor(symbol.valueDeclaration, or(isStatement, isSourceFile)); Debug.assertIsDefined(container); - forEach(references, reference => { if (reference.kind !== FindAllReferences.EntryKind.Node) { - return undefined; - } - - let lastChild = reference.node; - const topReferencedAccessExpression = findAncestor(reference.node.parent, n => { - if (isParenthesizedExpression(n)) { - lastChild = n; - return false; - } - else if (isAccessExpression(n)) { - if (n.expression === lastChild) { - return true; - } - lastChild = n; - return false; - } - return "quit"; - }); - if (!topReferencedAccessExpression) { return; } - const accessExpression = cast(topReferencedAccessExpression, isAccessExpression); - allReferencedAcccessExpression.push(accessExpression); - - if (isAssignmentTarget(accessExpression)) { - return; - } - - const referencedParentMaybeCall = skipParenthesesUp(accessExpression.parent); - if (isCallOrNewExpression(referencedParentMaybeCall) && !(referencedParentMaybeCall.arguments && rangeContainsRange(referencedParentMaybeCall.arguments, topReferencedAccessExpression))) { - return; - } - - if (reference.node.pos < symbol.valueDeclaration.pos) { - return; - } - if (isFunctionLikeDeclaration(container.parent) && container.parent.body && !rangeContainsRange(container.parent.body, reference.node)) { + const accessExpression = getAccessExpressionIfValidReference(reference.node, symbol, container, allReferencedAcccessExpression); + if (!accessExpression) { return; } @@ -223,6 +187,7 @@ namespace ts.refactor { } } }); + if (hasUnconvertableReference) return undefined; if (resolveUniqueName) { forEach(referencedAccessExpression, ([, name]) => { @@ -233,7 +198,6 @@ namespace ts.refactor { }); } - if (hasUnconvertableReference) return undefined; return { replacementExpression: node.parent.expression, firstReferenced, @@ -242,4 +206,46 @@ namespace ts.refactor { namesNeedUniqueName }; } + + function getAccessExpressionIfValidReference(node: Node, symbol: Symbol, container: Node, allReferencedAcccessExpression: Node[]): AccessExpression | undefined { + let lastChild = node; + const topReferencedAccessExpression = findAncestor(node.parent, n => { + if (isParenthesizedExpression(n)) { + lastChild = n; + return false; + } + else if (isAccessExpression(n)) { + if (n.expression === lastChild) { + return true; + } + lastChild = n; + return false; + } + return "quit"; + }); + if (!topReferencedAccessExpression) { + return undefined; + } + + const accessExpression = cast(topReferencedAccessExpression, isAccessExpression); + allReferencedAcccessExpression.push(accessExpression); + + if (isAssignmentTarget(accessExpression)) { + return undefined; + } + + const referencedParentMaybeCall = skipParenthesesUp(accessExpression.parent); + if (isCallOrNewExpression(referencedParentMaybeCall) && !(referencedParentMaybeCall.arguments && rangeContainsRange(referencedParentMaybeCall.arguments, topReferencedAccessExpression))) { + return undefined; + } + + if (node.pos < symbol.valueDeclaration.pos) { + return undefined; + } + if (isFunctionLikeDeclaration(container.parent) && container.parent.body && !rangeContainsRange(container.parent.body, node)) { + return undefined; + } + + return accessExpression; + } } \ No newline at end of file From 39d2ccb078cba8ee095051522143dc19881286b0 Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 3 Aug 2020 18:43:31 +0800 Subject: [PATCH 15/28] Add array like destruction --- .../refactors/convertObjectDestruction.ts | 165 ++++++++++++++---- .../refactorIntroduceDestruction28.ts | 14 ++ .../refactorIntroduceDestruction29.ts | 14 ++ .../refactorIntroduceDestruction30.ts | 14 ++ .../refactorIntroduceDestruction31.ts | 14 ++ 5 files changed, 191 insertions(+), 30 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction28.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction29.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction30.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction31.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index a85a575294e30..b3b69a30631f0 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -37,33 +37,49 @@ namespace ts.refactor { return { renameFilename: undefined, renameLocation: undefined, edits }; } - function doChange(changeTracker: textChanges.ChangeTracker, file: SourceFile, info: Info) { - const nameMap = new Map(); - const bindingElements: BindingElement[] = []; - info.referencedAccessExpression.forEach(([expr, name]) => { - if (!nameMap.has(name)) { - const needAlias = info.namesNeedUniqueName.has(name); - const uniqueName = needAlias ? getUniqueName(name, file) : name; - nameMap.set(name, uniqueName); - bindingElements.push(getUniqueDestructionName(expr, needAlias, uniqueName)); + function getUniqueNumericAccessVariable(name: string | number, file: SourceFile) { + const tempName = `index_${name}`; + return isFileLevelUniqueName(file, tempName) ? tempName : getUniqueName(tempName, file); + } + + /** + * `Dense` means we use array literal pattern to destruction the expression. + * We allowed index up to 15 to avoid many omit expression. + */ + function getDenseNumericAccessInfo(infos: ReferencedAccessInfo[]): [max: number, indexSet: Set] | undefined { + let min = Infinity; + let max = -Infinity; + const indexSet = new Set(); + for (const info of infos) { + if (!info.isNumericAccess) { + return undefined; } - const newName = nameMap.get(name); - Debug.assertIsDefined(newName); + const value = parseInt(info.name); + min = Math.min(min, value); + max = Math.max(max, value); + indexSet.add(value); - changeTracker.replaceNode( - file, - expr, - factory.createIdentifier(newName) - ); - }); + if (isNaN(min) || isNaN(max) || min < 0 || max < 0) { + return undefined; + } + } + + if (max > 15) { + return undefined; + } + return [max, indexSet]; + } + + function doChange(changeTracker: textChanges.ChangeTracker, file: SourceFile, info: Info) { + const bindingPattern = getBindingPattern(info, file, changeTracker); const newBinding = factory.createVariableStatement( /* modifiers*/ undefined, factory.createVariableDeclarationList( [ factory.createVariableDeclaration( - factory.createObjectBindingPattern(bindingElements), + bindingPattern, /*exclamationToken*/ undefined, /*type*/ undefined, info.replacementExpression @@ -80,6 +96,80 @@ namespace ts.refactor { ); } + function getBindingPattern(info: Info, file: SourceFile, changeTracker: textChanges.ChangeTracker): BindingPattern { + const denseNumericInfo = getDenseNumericAccessInfo(info.referencedAccessExpression); + if (denseNumericInfo) { + const [max, indexSet] = denseNumericInfo; + return getdenseNumericBindingPattern(info, file, max, indexSet, changeTracker); + } + return getObjectBindingPattern(info, file, changeTracker); + } + + function getObjectBindingPattern(info: Info, file: SourceFile, changeTracker: textChanges.ChangeTracker) { + const nameMap = new Map(); + const bindingElements: BindingElement[] = []; + info.referencedAccessExpression.forEach(({ expression, name, isNumericAccess }) => { + if (!nameMap.has(name)) { + const needAlias = isNumericAccess || info.namesNeedUniqueName.has(name); + const uniqueName = isNumericAccess ? getUniqueNumericAccessVariable(name, file) : + needAlias ? getUniqueName(name, file) : name; + nameMap.set(name, uniqueName); + bindingElements.push(getUniqueDestructionName(expression, needAlias, uniqueName)); + } + + const newName = nameMap.get(name); + Debug.assertIsDefined(newName); + + changeTracker.replaceNode( + file, + expression, + factory.createIdentifier(newName) + ); + }); + return factory.createObjectBindingPattern(bindingElements); + } + + function getdenseNumericBindingPattern(info: Info, file: SourceFile, max: number, indexSet: Set, changeTracker: textChanges.ChangeTracker): ArrayBindingPattern { + const nameMap = new Map(); + const bindingElements: ArrayBindingElement[] = []; + + for (let i = 0; i <= max; ++i) { + if (indexSet.has(i)) { + if (!nameMap.has(i)) { + const name = getUniqueNumericAccessVariable(i, file); + nameMap.set(i, name); + } + + const name = nameMap.get(i); + Debug.assertIsDefined(name); + + bindingElements.push(factory.createBindingElement( + /* dotDotDotToken*/ undefined, + /*propertyName*/ undefined, + factory.createIdentifier(name) + )); + } + else { + bindingElements.push(factory.createOmittedExpression()); + } + } + + info.referencedAccessExpression.forEach(({ expression, name }) => { + const index = parseInt(name); + + const newName = nameMap.get(index); + Debug.assertIsDefined(newName); + + changeTracker.replaceNode( + file, + expression, + factory.createIdentifier(newName) + ); + }); + + return factory.createArrayBindingPattern(bindingElements); + } + function getUniqueDestructionName(expr: AccessExpression, needAlias: boolean, newName: string) { if (isPropertyAccessExpression(expr)) { const bindingName = cast(expr.name, isIdentifier); @@ -99,9 +189,15 @@ namespace ts.refactor { } } + interface ReferencedAccessInfo { + expression: AccessExpression + name: string, + isNumericAccess: boolean + } + interface Info { replacementExpression: Expression, - referencedAccessExpression: [AccessExpression, string][] + referencedAccessExpression: ReferencedAccessInfo[] firstReferenced: Expression firstReferencedStatement: Statement namesNeedUniqueName: Set @@ -122,7 +218,7 @@ namespace ts.refactor { const references = FindAllReferences.getReferenceEntriesForNode(-1, node, program, [file], cancellationToken); let firstReferenced: Expression | undefined; let firstReferencedStatement: Statement | undefined; - const referencedAccessExpression: [AccessExpression, string][] = []; + const referencedAccessExpression: ReferencedAccessInfo[] = []; const allReferencedAcccessExpression: AccessExpression[] = []; const container = isParameter(symbol.valueDeclaration) ? symbol.valueDeclaration : findAncestor(symbol.valueDeclaration, or(isStatement, isSourceFile)); Debug.assertIsDefined(container); @@ -150,23 +246,32 @@ namespace ts.refactor { } if (isElementAccessExpression(accessExpression)) { - if (!isStringLiteralLike(accessExpression.argumentExpression)) { + let isNumericAccess = false; + if (!isStringLiteralLike(accessExpression.argumentExpression) && !isNumericLiteral(accessExpression.argumentExpression)) { return; } - if (!isIdentifierText(accessExpression.argumentExpression.text, compilerOptions.target, compilerOptions.jsx ? LanguageVariant.JSX : LanguageVariant.Standard)) { + if (isNumericLiteral(accessExpression.argumentExpression)) { + isNumericAccess = true; + } + else if (!isIdentifierText(accessExpression.argumentExpression.text, compilerOptions.target, compilerOptions.jsx ? LanguageVariant.JSX : LanguageVariant.Standard)) { return; } - referencedAccessExpression.push([accessExpression, accessExpression.argumentExpression.text]); - return; - } - if (!isIdentifierText(accessExpression.name.text, compilerOptions.target, compilerOptions.jsx ? LanguageVariant.JSX : LanguageVariant.Standard)) { + referencedAccessExpression.push({ + expression: accessExpression, + name: accessExpression.argumentExpression.text, + isNumericAccess + }); return; } - referencedAccessExpression.push([accessExpression, accessExpression.name.text]); + referencedAccessExpression.push({ + expression: accessExpression, + name: accessExpression.name.text, + isNumericAccess: false + }); }); - if (!firstReferenced || !firstReferencedStatement || !referencedAccessExpression.length || !some(referencedAccessExpression, ([r]) => rangeContainsRange(r, current))) return undefined; + if (!firstReferenced || !firstReferencedStatement || !referencedAccessExpression.length || !some(referencedAccessExpression, ({ expression }) => rangeContainsRange(expression, current))) return undefined; let hasUnconvertableReference = false; const namesNeedUniqueName = new Set(); @@ -175,7 +280,7 @@ namespace ts.refactor { const referenceType = checker.getTypeAtLocation(expr); if (referenceType !== type) { const propName = isElementAccessExpression(expr) ? - cast(expr.argumentExpression, isStringLiteralLike).text : + cast(expr.argumentExpression, isStringOrNumericLiteralLike).text : checker.getSymbolAtLocation(expr)?.name; const accessType = checker.getTypeAtLocation(expr); @@ -190,7 +295,7 @@ namespace ts.refactor { if (hasUnconvertableReference) return undefined; if (resolveUniqueName) { - forEach(referencedAccessExpression, ([, name]) => { + forEach(referencedAccessExpression, ({ name }) => { const symbol = checker.resolveName(name, /*location*/ undefined, SymbolFlags.Value, /*excludeGlobals*/ false); if (symbol) { namesNeedUniqueName.add(name); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction28.ts b/tests/cases/fourslash/refactorIntroduceDestruction28.ts new file mode 100644 index 0000000000000..e6cf212504bfd --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction28.ts @@ -0,0 +1,14 @@ +/// + +//// const item = [ 1, 2, 3 ] as const +//// call(/*a*/item/*b*/[0], item[1], item[2]) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = [ 1, 2, 3 ] as const +const [index_0, index_1, index_2] = item +call(index_0, index_1, index_2)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction29.ts b/tests/cases/fourslash/refactorIntroduceDestruction29.ts new file mode 100644 index 0000000000000..6f43dc6128cea --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction29.ts @@ -0,0 +1,14 @@ +/// + +//// const item = [ 1, 2, 3 ] as const +//// call(/*a*/item/*b*/[0], item[2], item[2]) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = [ 1, 2, 3 ] as const +const [index_0, , index_2] = item +call(index_0, index_2, index_2)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction30.ts b/tests/cases/fourslash/refactorIntroduceDestruction30.ts new file mode 100644 index 0000000000000..a8830d1e322a9 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction30.ts @@ -0,0 +1,14 @@ +/// + +//// const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 ] as const +//// call(/*a*/item/*b*/[14], item[8], item[3]) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 ] as const +const [, , , index_3, , , , , index_8, , , , , , index_14] = item +call(index_14, index_8, index_3)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction31.ts b/tests/cases/fourslash/refactorIntroduceDestruction31.ts new file mode 100644 index 0000000000000..138bb3e35f02d --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction31.ts @@ -0,0 +1,14 @@ +/// + +//// const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 ] as const +//// call(/*a*/item/*b*/[14], item[8], item[16]) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 ] as const +const { 14: index_14, 8: index_8, 16: index_16 } = item +call(index_14, index_8, index_16)`, +}); From 9fb1d62486b964bfebffe53aa3ceccd7fb1fa4f9 Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 3 Aug 2020 19:33:31 +0800 Subject: [PATCH 16/28] Add more case --- .../refactors/convertObjectDestruction.ts | 9 ++++++--- .../refactorIntroduceDestruction32.ts | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction32.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index b3b69a30631f0..be546c5f4ee45 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -97,7 +97,7 @@ namespace ts.refactor { } function getBindingPattern(info: Info, file: SourceFile, changeTracker: textChanges.ChangeTracker): BindingPattern { - const denseNumericInfo = getDenseNumericAccessInfo(info.referencedAccessExpression); + const denseNumericInfo = info.isArrayLikeType && getDenseNumericAccessInfo(info.referencedAccessExpression); if (denseNumericInfo) { const [max, indexSet] = denseNumericInfo; return getdenseNumericBindingPattern(info, file, max, indexSet, changeTracker); @@ -201,6 +201,7 @@ namespace ts.refactor { firstReferenced: Expression firstReferencedStatement: Statement namesNeedUniqueName: Set + isArrayLikeType: boolean } function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean): Info | undefined { @@ -276,8 +277,9 @@ namespace ts.refactor { let hasUnconvertableReference = false; const namesNeedUniqueName = new Set(); const type = checker.getTypeOfSymbolAtLocation(symbol, firstReferenced); + const isArrayLikeType = checker.isArrayLikeType(type); forEach(allReferencedAcccessExpression, expr => { - const referenceType = checker.getTypeAtLocation(expr); + const referenceType = checker.getTypeAtLocation(expr.expression); if (referenceType !== type) { const propName = isElementAccessExpression(expr) ? cast(expr.argumentExpression, isStringOrNumericLiteralLike).text : @@ -308,7 +310,8 @@ namespace ts.refactor { firstReferenced, firstReferencedStatement, referencedAccessExpression, - namesNeedUniqueName + namesNeedUniqueName, + isArrayLikeType }; } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction32.ts b/tests/cases/fourslash/refactorIntroduceDestruction32.ts new file mode 100644 index 0000000000000..5e2c7e674a022 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction32.ts @@ -0,0 +1,20 @@ +/// + +//// interface A { +//// [k: string]: number +//// } +//// declare const foo: A +//// call(/*a*/foo/*b*/[0], foo[1]) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce Destruction", + actionName: "Introduce Destruction", + actionDescription: "Convert property access to Object destruction", + newContent: `interface A { + [k: string]: number +} +declare const foo: A +const { 0: index_0, 1: index_1 } = foo +call(index_0, index_1)`, +}); From 5dcbc3f5b1a70f03ae1f958cd16f98ab1fec6fce Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 3 Aug 2020 19:40:40 +0800 Subject: [PATCH 17/28] Refactor common logic --- .../refactors/convertObjectDestruction.ts | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index be546c5f4ee45..1443e076e49a2 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -102,7 +102,7 @@ namespace ts.refactor { const [max, indexSet] = denseNumericInfo; return getdenseNumericBindingPattern(info, file, max, indexSet, changeTracker); } - return getObjectBindingPattern(info, file, changeTracker); + return getObjectBindingPattern(info, file, changeTracker); } function getObjectBindingPattern(info: Info, file: SourceFile, changeTracker: textChanges.ChangeTracker) { @@ -119,12 +119,7 @@ namespace ts.refactor { const newName = nameMap.get(name); Debug.assertIsDefined(newName); - - changeTracker.replaceNode( - file, - expression, - factory.createIdentifier(newName) - ); + replaceBindingPatternReferenced(file, changeTracker, expression, newName); }); return factory.createObjectBindingPattern(bindingElements); } @@ -159,17 +154,24 @@ namespace ts.refactor { const newName = nameMap.get(index); Debug.assertIsDefined(newName); - - changeTracker.replaceNode( - file, - expression, - factory.createIdentifier(newName) - ); + replaceBindingPatternReferenced(file, changeTracker, expression, newName); }); return factory.createArrayBindingPattern(bindingElements); } + function replaceBindingPatternReferenced(file: SourceFile, changeTracker: textChanges.ChangeTracker, expression: Expression, newName: string) { + const newIdentifier = factory.createIdentifier(newName); + suppressLeadingAndTrailingTrivia(expression); + copyComments(expression, newIdentifier); + + changeTracker.replaceNode( + file, + expression, + newIdentifier + ); + } + function getUniqueDestructionName(expr: AccessExpression, needAlias: boolean, newName: string) { if (isPropertyAccessExpression(expr)) { const bindingName = cast(expr.name, isIdentifier); From 53a955bc76bbd9fe259f413d613afc696df9652e Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 3 Aug 2020 19:47:46 +0800 Subject: [PATCH 18/28] Update description --- src/services/refactors/convertObjectDestruction.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction1.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction10.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction11.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction12.ts | 2 +- tests/cases/fourslash/refactorIntroduceDestruction13.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction14.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction15.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction16.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction17.ts | 2 +- tests/cases/fourslash/refactorIntroduceDestruction18.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction19.ts | 2 +- tests/cases/fourslash/refactorIntroduceDestruction2.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction20.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction21.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction22.ts | 2 +- tests/cases/fourslash/refactorIntroduceDestruction23.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction24.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction25.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction26.ts | 2 +- tests/cases/fourslash/refactorIntroduceDestruction27.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction28.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction29.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction3.ts | 2 +- tests/cases/fourslash/refactorIntroduceDestruction30.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction31.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction32.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction4.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction5.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction6.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction7.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction8.ts | 6 +++--- tests/cases/fourslash/refactorIntroduceDestruction9.ts | 2 +- 33 files changed, 85 insertions(+), 85 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 1443e076e49a2..7facfa9445ef0 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -1,7 +1,7 @@ /* @internal */ namespace ts.refactor { - const refactorName = "Introduce Destruction"; - const actionNameIntroduceObjectDestruction = "Convert property access to Object destruction"; + const refactorName = "Introduce destruction"; + const actionNameIntroduceObjectDestruction = "Convert access to destruction"; registerRefactor(refactorName, { getAvailableActions, getEditsForAction }); function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { @@ -102,7 +102,7 @@ namespace ts.refactor { const [max, indexSet] = denseNumericInfo; return getdenseNumericBindingPattern(info, file, max, indexSet, changeTracker); } - return getObjectBindingPattern(info, file, changeTracker); + return getObjectBindingPattern(info, file, changeTracker); } function getObjectBindingPattern(info: Info, file: SourceFile, changeTracker: textChanges.ChangeTracker) { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction1.ts b/tests/cases/fourslash/refactorIntroduceDestruction1.ts index 064fef5823849..87989f9f88e65 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction1.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction1.ts @@ -7,9 +7,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction10.ts b/tests/cases/fourslash/refactorIntroduceDestruction10.ts index 51d0a196ee4fb..fee45509aed5b 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction10.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction10.ts @@ -6,9 +6,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `function foo (item: { a: string, b: number }) { const { a, b } = item; call(a, b) diff --git a/tests/cases/fourslash/refactorIntroduceDestruction11.ts b/tests/cases/fourslash/refactorIntroduceDestruction11.ts index efce9de031c36..92bd68e6c2843 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction11.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction11.ts @@ -7,9 +7,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction12.ts b/tests/cases/fourslash/refactorIntroduceDestruction12.ts index 4ce582d9b09b2..75c07f8385b5c 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction12.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction12.ts @@ -7,4 +7,4 @@ //// call(/*a*/item/*b*/[key], item.b) goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction13.ts b/tests/cases/fourslash/refactorIntroduceDestruction13.ts index cb491c0e8c938..835221414e04c 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction13.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction13.ts @@ -8,9 +8,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction14.ts b/tests/cases/fourslash/refactorIntroduceDestruction14.ts index a42e0461bd2e7..2d5b4a920c678 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction14.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction14.ts @@ -12,9 +12,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: { c: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction15.ts b/tests/cases/fourslash/refactorIntroduceDestruction15.ts index fa28aa91b70e6..4a1a7b74fa535 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction15.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction15.ts @@ -12,9 +12,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: { c: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction16.ts b/tests/cases/fourslash/refactorIntroduceDestruction16.ts index 458407dcbba38..2167ce0cfa3b3 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction16.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction16.ts @@ -12,9 +12,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: { c: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction17.ts b/tests/cases/fourslash/refactorIntroduceDestruction17.ts index a1ea0be88580a..2f138767b47a2 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction17.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction17.ts @@ -20,4 +20,4 @@ //// } goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction18.ts b/tests/cases/fourslash/refactorIntroduceDestruction18.ts index c910c69807d42..74da124409bb5 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction18.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction18.ts @@ -19,9 +19,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `interface A { f: 1, b: number diff --git a/tests/cases/fourslash/refactorIntroduceDestruction19.ts b/tests/cases/fourslash/refactorIntroduceDestruction19.ts index d00b384b0d659..2a01f5368c8a8 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction19.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction19.ts @@ -6,4 +6,4 @@ //// call(/*a*/item/*b*/["a-a-a"], item.b) goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction2.ts b/tests/cases/fourslash/refactorIntroduceDestruction2.ts index 1ccec06c849cd..a6e9005d3c960 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction2.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction2.ts @@ -7,9 +7,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction20.ts b/tests/cases/fourslash/refactorIntroduceDestruction20.ts index 97ca595c81d0d..4af5a28bd76c5 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction20.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction20.ts @@ -11,9 +11,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction21.ts b/tests/cases/fourslash/refactorIntroduceDestruction21.ts index bf348c3e77022..6a0883e8c9177 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction21.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction21.ts @@ -8,9 +8,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `function foo (item: { a: string, b: number }) { const { a, b } = item; function bar () { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction22.ts b/tests/cases/fourslash/refactorIntroduceDestruction22.ts index d96a089bab98a..0c9346fb14bb9 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction22.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction22.ts @@ -8,4 +8,4 @@ //// } goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction23.ts b/tests/cases/fourslash/refactorIntroduceDestruction23.ts index ace9b6e391ca1..8bfca56a72e80 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction23.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction23.ts @@ -16,9 +16,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: 2 } const { a, b } = item function foo() { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction24.ts b/tests/cases/fourslash/refactorIntroduceDestruction24.ts index 2b39a2e3cb636..7edbf43e62bc6 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction24.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction24.ts @@ -11,9 +11,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `function foo () { call(item.a) } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction25.ts b/tests/cases/fourslash/refactorIntroduceDestruction25.ts index a6c14070b69a1..4dac7cc337cfa 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction25.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction25.ts @@ -7,9 +7,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: 2, c: 3 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction26.ts b/tests/cases/fourslash/refactorIntroduceDestruction26.ts index ad1cd452c0074..520755e20703b 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction26.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction26.ts @@ -3,5 +3,5 @@ //// const f = (item: { a: 1, b: 2}) => /*a*/item/*b*/.a + item.b goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction27.ts b/tests/cases/fourslash/refactorIntroduceDestruction27.ts index acd11ccaf45d4..a67b64a461a30 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction27.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction27.ts @@ -6,9 +6,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `function f(item: { a: 1 }, b = item.a) { const { a } = item; call(a, b) diff --git a/tests/cases/fourslash/refactorIntroduceDestruction28.ts b/tests/cases/fourslash/refactorIntroduceDestruction28.ts index e6cf212504bfd..27c9c9f7235c4 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction28.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction28.ts @@ -5,9 +5,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = [ 1, 2, 3 ] as const const [index_0, index_1, index_2] = item call(index_0, index_1, index_2)`, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction29.ts b/tests/cases/fourslash/refactorIntroduceDestruction29.ts index 6f43dc6128cea..d77466324ed1c 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction29.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction29.ts @@ -5,9 +5,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = [ 1, 2, 3 ] as const const [index_0, , index_2] = item call(index_0, index_2, index_2)`, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction3.ts b/tests/cases/fourslash/refactorIntroduceDestruction3.ts index 5a84e07029035..efbfa2d8b4a8e 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction3.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction3.ts @@ -6,4 +6,4 @@ //// call(/*a*/item.a/*b*/, item.b) goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction30.ts b/tests/cases/fourslash/refactorIntroduceDestruction30.ts index a8830d1e322a9..c989dbe1dd3b0 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction30.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction30.ts @@ -5,9 +5,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 ] as const const [, , , index_3, , , , , index_8, , , , , , index_14] = item call(index_14, index_8, index_3)`, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction31.ts b/tests/cases/fourslash/refactorIntroduceDestruction31.ts index 138bb3e35f02d..ce8df9fb53340 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction31.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction31.ts @@ -5,9 +5,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 ] as const const { 14: index_14, 8: index_8, 16: index_16 } = item call(index_14, index_8, index_16)`, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction32.ts b/tests/cases/fourslash/refactorIntroduceDestruction32.ts index 5e2c7e674a022..da931dace895f 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction32.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction32.ts @@ -8,9 +8,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `interface A { [k: string]: number } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction4.ts b/tests/cases/fourslash/refactorIntroduceDestruction4.ts index 320aa5da76d92..dc33d8cc481e4 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction4.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction4.ts @@ -8,8 +8,8 @@ //// } goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") goTo.select("c", "d"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") goTo.select("e", "f"); -verify.not.refactorAvailable("Introduce Destruction") \ No newline at end of file +verify.not.refactorAvailable("Introduce destruction") \ No newline at end of file diff --git a/tests/cases/fourslash/refactorIntroduceDestruction5.ts b/tests/cases/fourslash/refactorIntroduceDestruction5.ts index f91e379332a7b..d2f57d8a151ab 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction5.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction5.ts @@ -9,9 +9,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `declare const u: { type: 'number'; payload: number } | { type: 'string', payload: number } const { type, payload } = u if(type === "number") { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction6.ts b/tests/cases/fourslash/refactorIntroduceDestruction6.ts index 576b097b40a53..98224c2ae4bab 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction6.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction6.ts @@ -7,9 +7,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: () => void } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction7.ts b/tests/cases/fourslash/refactorIntroduceDestruction7.ts index 5a6a88cb5c26d..5b0dd275f4aad 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction7.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction7.ts @@ -8,9 +8,9 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce Destruction", - actionName: "Introduce Destruction", - actionDescription: "Convert property access to Object destruction", + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", newContent: `const item = { a: 1, b: () => void } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction8.ts b/tests/cases/fourslash/refactorIntroduceDestruction8.ts index c2ed806992328..6f72a0c5169d6 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction8.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction8.ts @@ -7,8 +7,8 @@ //// call(item.a, /*c*/item/*d*/.b(), (((/*e*/item/*f*/.b)))()) goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") goTo.select("c", "d"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") goTo.select("e", "f"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction9.ts b/tests/cases/fourslash/refactorIntroduceDestruction9.ts index c5b682e63ac7d..55fb490956880 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction9.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction9.ts @@ -7,4 +7,4 @@ //// /*a*/E/*b*/.A goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce Destruction") +verify.not.refactorAvailable("Introduce destruction") From 5a9faecd6e87131d46108a1d9b17bfd71dbde3b9 Mon Sep 17 00:00:00 2001 From: kingwl Date: Mon, 3 Aug 2020 21:57:10 +0800 Subject: [PATCH 19/28] Add some comment test --- .../refactors/convertObjectDestruction.ts | 3 +- .../refactorIntroduceDestruction33.ts | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction33.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 7facfa9445ef0..00c18586139d9 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -74,6 +74,7 @@ namespace ts.refactor { function doChange(changeTracker: textChanges.ChangeTracker, file: SourceFile, info: Info) { const bindingPattern = getBindingPattern(info, file, changeTracker); + suppressLeadingAndTrailingTrivia(info.replacementExpression); const newBinding = factory.createVariableStatement( /* modifiers*/ undefined, factory.createVariableDeclarationList( @@ -162,8 +163,6 @@ namespace ts.refactor { function replaceBindingPatternReferenced(file: SourceFile, changeTracker: textChanges.ChangeTracker, expression: Expression, newName: string) { const newIdentifier = factory.createIdentifier(newName); - suppressLeadingAndTrailingTrivia(expression); - copyComments(expression, newIdentifier); changeTracker.replaceNode( file, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction33.ts b/tests/cases/fourslash/refactorIntroduceDestruction33.ts new file mode 100644 index 0000000000000..68e45b4a5c615 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction33.ts @@ -0,0 +1,30 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// call( +//// // l +//// /*a*/item/*b*/.a, +//// // t +//// item.b +//// ) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", + newContent: `const item = { + a: 1, b: 2 +} +const { a, b } = item +call( + // l + a, + // t + b +)`, +}); + + From f9a082894bb10b241727cd7da0b2462ff1a48186 Mon Sep 17 00:00:00 2001 From: kingwl Date: Wed, 28 Oct 2020 16:35:11 +0800 Subject: [PATCH 20/28] update refactor behavior --- .../refactors/convertObjectDestruction.ts | 44 +++++++++++++------ .../refactorIntroduceDestruction34.ts | 20 +++++++++ .../refactorIntroduceDestruction35.ts | 30 +++++++++++++ .../refactorIntroduceDestruction36.ts | 30 +++++++++++++ .../refactorIntroduceDestruction37.ts | 16 +++++++ .../refactorIntroduceDestruction38.ts | 16 +++++++ .../refactorIntroduceDestruction6.ts | 4 +- .../refactorIntroduceDestruction7.ts | 4 +- .../refactorIntroduceDestruction8.ts | 2 +- 9 files changed, 148 insertions(+), 18 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction34.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction35.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction36.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction37.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction38.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 00c18586139d9..abdac67b2a439 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -212,8 +212,11 @@ namespace ts.refactor { const node = findAncestor(current, (node => node.parent && isAccessExpression(node.parent) && !rangeContainsSkipTrivia(range, node.parent, file))); - if (!node || !isAccessExpression(node.parent) || node.parent.expression !== node) return undefined; - const symbol = checker.getSymbolAtLocation(node); + if (!node || !isAccessExpression(node.parent)) return undefined; + + const isLeftOfAccess = node.parent.expression === node; + + const symbol = checker.getSymbolAtLocation(isLeftOfAccess ? node.parent.expression : node.parent); if (!symbol || checker.isUnknownSymbol(symbol) || !symbol.valueDeclaration || !isVariableLike(symbol.valueDeclaration) || isEnumMember(symbol.valueDeclaration)) return undefined; // Find only current file @@ -229,7 +232,7 @@ namespace ts.refactor { return; } - const accessExpression = getAccessExpressionIfValidReference(reference.node, symbol, container, allReferencedAcccessExpression); + const accessExpression = getAccessExpressionIfValidReference(reference.node, symbol, container, allReferencedAcccessExpression, isLeftOfAccess); if (!accessExpression) { return; } @@ -282,16 +285,24 @@ namespace ts.refactor { forEach(allReferencedAcccessExpression, expr => { const referenceType = checker.getTypeAtLocation(expr.expression); if (referenceType !== type) { - const propName = isElementAccessExpression(expr) ? - cast(expr.argumentExpression, isStringOrNumericLiteralLike).text : - checker.getSymbolAtLocation(expr)?.name; - const accessType = checker.getTypeAtLocation(expr); - const prop = propName && checker.getPropertyOfType(type, propName); + if (!isLeftOfAccess) { + if (accessType !== type) { + hasUnconvertableReference = true; + return "quit"; + } + } + else { + const propName = isElementAccessExpression(expr) ? + cast(expr.argumentExpression, isStringOrNumericLiteralLike).text : + checker.getSymbolAtLocation(expr)?.name; + + const prop = propName && checker.getPropertyOfType(type, propName); - if (!prop || checker.getTypeOfSymbolAtLocation(prop, expr) !== accessType) { - hasUnconvertableReference = true; - return "quit"; + if (!prop || checker.getTypeOfSymbolAtLocation(prop, expr) !== accessType) { + hasUnconvertableReference = true; + return "quit"; + } } } }); @@ -316,7 +327,7 @@ namespace ts.refactor { }; } - function getAccessExpressionIfValidReference(node: Node, symbol: Symbol, container: Node, allReferencedAcccessExpression: Node[]): AccessExpression | undefined { + function getAccessExpressionIfValidReference(node: Node, symbol: Symbol, container: Node, allReferencedAcccessExpression: Node[], isLeftOfAccess: boolean): AccessExpression | undefined { let lastChild = node; const topReferencedAccessExpression = findAncestor(node.parent, n => { if (isParenthesizedExpression(n)) { @@ -324,9 +335,16 @@ namespace ts.refactor { return false; } else if (isAccessExpression(n)) { - if (n.expression === lastChild) { + if (isLeftOfAccess && n.expression === lastChild) { return true; } + else if (!isLeftOfAccess && (isPropertyAccessExpression(n) ? n.name === lastChild : n.argumentExpression === lastChild)) { + return true; + } + lastChild = n; + return false; + } + else if (isIdentifier(n) && isAccessExpression(n.parent)) { lastChild = n; return false; } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction34.ts b/tests/cases/fourslash/refactorIntroduceDestruction34.ts new file mode 100644 index 0000000000000..b3abc74bfc7fb --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction34.ts @@ -0,0 +1,20 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// item./*a*/a/*b*/ + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", + newContent: `const item = { + a: 1, b: 2 +} +const { a } = item +a`, +}); + + diff --git a/tests/cases/fourslash/refactorIntroduceDestruction35.ts b/tests/cases/fourslash/refactorIntroduceDestruction35.ts new file mode 100644 index 0000000000000..2c58261e977ab --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction35.ts @@ -0,0 +1,30 @@ +/// + +//// const item = { +//// a: { +//// b: { +//// c: 1 +//// } +//// } +//// } +//// item./*a*/a/*b*/.b.c // right of item +//// item.a.b.c // another reference + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", + newContent: `const item = { + a: { + b: { + c: 1 + } + } +} +const { a } = item +a.b.c // right of item +a.b.c // another reference`, +}); + + diff --git a/tests/cases/fourslash/refactorIntroduceDestruction36.ts b/tests/cases/fourslash/refactorIntroduceDestruction36.ts new file mode 100644 index 0000000000000..912f340680f74 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction36.ts @@ -0,0 +1,30 @@ +/// + +//// const item = { +//// a: { +//// b: { +//// c: 1 +//// } +//// } +//// } +//// /*a*/item.a/*b*/.b.c // left of b.c +//// item.a.b.c // another reference + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", + newContent: `const item = { + a: { + b: { + c: 1 + } + } +} +const { b } = item.a +b.c // left of b.c +b.c // another reference`, +}); + + diff --git a/tests/cases/fourslash/refactorIntroduceDestruction37.ts b/tests/cases/fourslash/refactorIntroduceDestruction37.ts new file mode 100644 index 0000000000000..f2c1aa71b6a6b --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction37.ts @@ -0,0 +1,16 @@ +/// + +//// interface I { foo: string; bar: number } +//// declare const a: I +//// console.log(a./*a*/foo/*b*/, a.bar, a.foo, a.bar) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", + newContent: `interface I { foo: string; bar: number } +declare const a: I +const { foo } = a +console.log(foo, a.bar, foo, a.bar)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction38.ts b/tests/cases/fourslash/refactorIntroduceDestruction38.ts new file mode 100644 index 0000000000000..7551e25abae93 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction38.ts @@ -0,0 +1,16 @@ +/// + +//// interface I { foo: string; bar: number } +//// declare const a: I +//// console.log(/*a*/a/*b*/.foo, a.bar, a.foo, a.bar) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", + newContent: `interface I { foo: string; bar: number } +declare const a: I +const { foo, bar } = a +console.log(foo, bar, foo, bar)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction6.ts b/tests/cases/fourslash/refactorIntroduceDestruction6.ts index 98224c2ae4bab..ac0a3779cc2ae 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction6.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction6.ts @@ -1,7 +1,7 @@ /// //// const item = { -//// a: 1, b: () => void +//// a: 1, b: () => 1 //// } //// call(/*a*/item/*b*/.a, item.b()) @@ -11,7 +11,7 @@ edit.applyRefactor({ actionName: "Introduce destruction", actionDescription: "Convert access to destruction", newContent: `const item = { - a: 1, b: () => void + a: 1, b: () => 1 } const { a } = item call(a, item.b())`, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction7.ts b/tests/cases/fourslash/refactorIntroduceDestruction7.ts index 5b0dd275f4aad..d36917e09ec76 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction7.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction7.ts @@ -1,7 +1,7 @@ /// //// const item = { -//// a: 1, b: () => void +//// a: 1, b: () => 1 //// } //// item.a = 2 //// call(/*a*/item/*b*/.a, item.b()) @@ -12,7 +12,7 @@ edit.applyRefactor({ actionName: "Introduce destruction", actionDescription: "Convert access to destruction", newContent: `const item = { - a: 1, b: () => void + a: 1, b: () => 1 } item.a = 2 const { a } = item diff --git a/tests/cases/fourslash/refactorIntroduceDestruction8.ts b/tests/cases/fourslash/refactorIntroduceDestruction8.ts index 6f72a0c5169d6..cfff37bed723f 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction8.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction8.ts @@ -1,7 +1,7 @@ /// //// const item = { -//// a: 1, b: () => void +//// a: 1, b: () => 1 //// } //// /*a*/item/*b*/.a = 2 //// call(item.a, /*c*/item/*d*/.b(), (((/*e*/item/*f*/.b)))()) From ad8349b3d95b6706a32e983142187ce7b972d782 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 8 Jan 2021 16:39:51 +0800 Subject: [PATCH 21/28] Add kind and support empty span --- .../refactors/convertObjectDestruction.ts | 15 ++++++++++----- .../refactorIntroduceDestruction39.ts | 19 +++++++++++++++++++ .../refactorIntroduceDestruction40.ts | 9 +++++++++ 3 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction39.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction40.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index abdac67b2a439..0269be4adf070 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -1,15 +1,16 @@ /* @internal */ namespace ts.refactor { const refactorName = "Introduce destruction"; + const refactorKind = "refactor.rewrite.expression.toDestructured"; const actionNameIntroduceObjectDestruction = "Convert access to destruction"; - registerRefactor(refactorName, { getAvailableActions, getEditsForAction }); + registerRefactor(refactorName, { getAvailableActions, getEditsForAction, kinds: [refactorKind] }); function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { const { file, program, cancellationToken } = context; const isJSFile = isSourceFileJS(file); if (isJSFile || !cancellationToken) return emptyArray; - const info = getInfo(context, file, program.getTypeChecker(), program, cancellationToken, /*resolveUniqueName*/ false); + const info = getInfo(context, file, program.getTypeChecker(), program, cancellationToken, /*resolveUniqueName*/ false, context.triggerReason === "invoked"); if (!info) return emptyArray; return [{ @@ -17,7 +18,8 @@ namespace ts.refactor { description: actionNameIntroduceObjectDestruction, actions: [{ name: refactorName, - description: actionNameIntroduceObjectDestruction + description: actionNameIntroduceObjectDestruction, + kind: refactorKind }] }]; } @@ -205,12 +207,15 @@ namespace ts.refactor { isArrayLikeType: boolean } - function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean): Info | undefined { + function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean, considerEmptySpans = true): Info | undefined { const current = getTokenAtPosition(file, context.startPosition); const compilerOptions = context.program.getCompilerOptions(); const range = createTextRangeFromSpan(getRefactorContextSpan(context)); + const cursorRequest = range.pos === range.end && considerEmptySpans; - const node = findAncestor(current, (node => node.parent && isAccessExpression(node.parent) && !rangeContainsSkipTrivia(range, node.parent, file))); + const node = findAncestor(current, (node => node.parent && isAccessExpression(node.parent) && !rangeContainsSkipTrivia(range, node.parent, file) && ( + cursorRequest || nodeOverlapsWithStartEnd(node, file, range.pos, range.end) + ))); if (!node || !isAccessExpression(node.parent)) return undefined; diff --git a/tests/cases/fourslash/refactorIntroduceDestruction39.ts b/tests/cases/fourslash/refactorIntroduceDestruction39.ts new file mode 100644 index 0000000000000..efb0fdb8daa94 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction39.ts @@ -0,0 +1,19 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// call(it/*a*/em.a, item.b) + +goTo.marker('a'); +edit.applyRefactor({ + refactorName: "Introduce destruction", + actionName: "Introduce destruction", + actionDescription: "Convert access to destruction", + triggerReason: 'invoked', + newContent: `const item = { + a: 1, b: 2 +} +const { a, b } = item +call(a, b)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction40.ts b/tests/cases/fourslash/refactorIntroduceDestruction40.ts new file mode 100644 index 0000000000000..3ad66ef81100b --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction40.ts @@ -0,0 +1,9 @@ +/// + +//// const item = { +//// a: 1, b: 2 +//// } +//// call(it/*a*/em.a, item.b) + +goTo.marker('a'); +verify.not.refactorAvailableForTriggerReason('implicit', 'Introduce destruction'); \ No newline at end of file From 9ac8cbc2b957becf16c12c645582df7e711a9d07 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 8 Jan 2021 17:16:01 +0800 Subject: [PATCH 22/28] Add support for notApplicableReason --- src/compiler/diagnosticMessages.json | 20 ++++++ .../refactors/convertObjectDestruction.ts | 61 +++++++++++++------ src/services/refactors/helpers.ts | 29 +++++++++ .../refactorIntroduceDestruction1.ts | 2 +- .../refactorIntroduceDestruction10.ts | 2 +- .../refactorIntroduceDestruction11.ts | 2 +- .../refactorIntroduceDestruction13.ts | 2 +- .../refactorIntroduceDestruction14.ts | 2 +- .../refactorIntroduceDestruction15.ts | 2 +- .../refactorIntroduceDestruction16.ts | 2 +- .../refactorIntroduceDestruction18.ts | 2 +- .../refactorIntroduceDestruction2.ts | 2 +- .../refactorIntroduceDestruction20.ts | 2 +- .../refactorIntroduceDestruction21.ts | 2 +- .../refactorIntroduceDestruction23.ts | 2 +- .../refactorIntroduceDestruction24.ts | 2 +- .../refactorIntroduceDestruction25.ts | 2 +- .../refactorIntroduceDestruction27.ts | 2 +- .../refactorIntroduceDestruction28.ts | 2 +- .../refactorIntroduceDestruction29.ts | 2 +- .../refactorIntroduceDestruction30.ts | 2 +- .../refactorIntroduceDestruction31.ts | 2 +- .../refactorIntroduceDestruction32.ts | 2 +- .../refactorIntroduceDestruction33.ts | 2 +- .../refactorIntroduceDestruction34.ts | 2 +- .../refactorIntroduceDestruction35.ts | 2 +- .../refactorIntroduceDestruction36.ts | 2 +- .../refactorIntroduceDestruction37.ts | 2 +- .../refactorIntroduceDestruction38.ts | 2 +- .../refactorIntroduceDestruction39.ts | 2 +- .../refactorIntroduceDestruction5.ts | 2 +- .../refactorIntroduceDestruction6.ts | 2 +- .../refactorIntroduceDestruction7.ts | 2 +- 33 files changed, 120 insertions(+), 50 deletions(-) diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index a2c2524efd95f..9be7498d321c5 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -6195,6 +6195,10 @@ "category": "Message", "code": 95155 }, + "Convert access expression to destruction": { + "category": "Message", + "code": 95156 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", @@ -6303,5 +6307,21 @@ "Invalid value for 'jsxFragmentFactory'. '{0}' is not a valid identifier or qualified-name.": { "category": "Error", "code": 18035 + }, + "No convertible access expression at location.": { + "category": "Error", + "code": 18036 + }, + "Cannot find convertible value declaration.": { + "category": "Error", + "code": 18037 + }, + "Cannot find convertible references.": { + "category": "Error", + "code": 18038 + }, + "Some references are un-convertible.": { + "category": "Error", + "code": 18039 } } diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 0269be4adf070..786049c975762 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -2,26 +2,40 @@ namespace ts.refactor { const refactorName = "Introduce destruction"; const refactorKind = "refactor.rewrite.expression.toDestructured"; - const actionNameIntroduceObjectDestruction = "Convert access to destruction"; registerRefactor(refactorName, { getAvailableActions, getEditsForAction, kinds: [refactorKind] }); + const introduceObjectDestructionAction = { + name: refactorName, + description: getLocaleSpecificMessage(Diagnostics.Convert_access_expression_to_destruction), + kind: refactorKind + } + + function makeRefactorActionWithErrorReason (reason: string) { + return { ...introduceObjectDestructionAction, notApplicableReason: reason }; + } + function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { const { file, program, cancellationToken } = context; const isJSFile = isSourceFileJS(file); if (isJSFile || !cancellationToken) return emptyArray; const info = getInfo(context, file, program.getTypeChecker(), program, cancellationToken, /*resolveUniqueName*/ false, context.triggerReason === "invoked"); - if (!info) return emptyArray; + if (!isErrorResult(info)) { + return [{ + name: refactorName, + description: getLocaleSpecificMessage(Diagnostics.Convert_access_expression_to_destruction), + actions: [introduceObjectDestructionAction] + }]; + } - return [{ - name: refactorName, - description: actionNameIntroduceObjectDestruction, - actions: [{ + if (context.preferences.provideRefactorNotApplicableReason) { + return [{ name: refactorName, - description: actionNameIntroduceObjectDestruction, - kind: refactorKind - }] - }]; + description: getLocaleSpecificMessage(Diagnostics.Convert_access_expression_to_destruction), + actions: [makeRefactorActionWithErrorReason(info.reason)] + }]; + } + return emptyArray; } function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { @@ -33,9 +47,9 @@ namespace ts.refactor { if (isJSFile || !cancellationToken) return emptyResult; const info = getInfo(context, file, program.getTypeChecker(), program, cancellationToken, /*resolveUniqueName*/ true); - if (!info) return emptyResult; + if (isErrorResult(info)) return emptyResult; - const edits = textChanges.ChangeTracker.with(context, t => doChange(t, file, info)); + const edits = textChanges.ChangeTracker.with(context, t => doChange(t, file, info.value)); return { renameFilename: undefined, renameLocation: undefined, edits }; } @@ -206,8 +220,7 @@ namespace ts.refactor { namesNeedUniqueName: Set isArrayLikeType: boolean } - - function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean, considerEmptySpans = true): Info | undefined { + function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean, considerEmptySpans = true): Result { const current = getTokenAtPosition(file, context.startPosition); const compilerOptions = context.program.getCompilerOptions(); const range = createTextRangeFromSpan(getRefactorContextSpan(context)); @@ -217,12 +230,16 @@ namespace ts.refactor { cursorRequest || nodeOverlapsWithStartEnd(node, file, range.pos, range.end) ))); - if (!node || !isAccessExpression(node.parent)) return undefined; + if (!node || !isAccessExpression(node.parent)) { + return Err(Diagnostics.No_convertible_access_expression_at_location.message); + } const isLeftOfAccess = node.parent.expression === node; const symbol = checker.getSymbolAtLocation(isLeftOfAccess ? node.parent.expression : node.parent); - if (!symbol || checker.isUnknownSymbol(symbol) || !symbol.valueDeclaration || !isVariableLike(symbol.valueDeclaration) || isEnumMember(symbol.valueDeclaration)) return undefined; + if (!symbol || checker.isUnknownSymbol(symbol) || !symbol.valueDeclaration || !isVariableLike(symbol.valueDeclaration) || isEnumMember(symbol.valueDeclaration)) { + return Err(Diagnostics.Cannot_find_convertible_value_declaration.message); + } // Find only current file const references = FindAllReferences.getReferenceEntriesForNode(-1, node, program, [file], cancellationToken); @@ -281,7 +298,9 @@ namespace ts.refactor { isNumericAccess: false }); }); - if (!firstReferenced || !firstReferencedStatement || !referencedAccessExpression.length || !some(referencedAccessExpression, ({ expression }) => rangeContainsRange(expression, current))) return undefined; + if (!firstReferenced || !firstReferencedStatement || !referencedAccessExpression.length || !some(referencedAccessExpression, ({ expression }) => rangeContainsRange(expression, current))) { + return Err(Diagnostics.Cannot_find_convertible_references.message); + } let hasUnconvertableReference = false; const namesNeedUniqueName = new Set(); @@ -311,7 +330,9 @@ namespace ts.refactor { } } }); - if (hasUnconvertableReference) return undefined; + if (hasUnconvertableReference) { + return Err(Diagnostics.Some_references_are_un_convertible.message); + } if (resolveUniqueName) { forEach(referencedAccessExpression, ({ name }) => { @@ -322,14 +343,14 @@ namespace ts.refactor { }); } - return { + return Ok({ replacementExpression: node.parent.expression, firstReferenced, firstReferencedStatement, referencedAccessExpression, namesNeedUniqueName, isArrayLikeType - }; + }); } function getAccessExpressionIfValidReference(node: Node, symbol: Symbol, container: Node, allReferencedAcccessExpression: Node[], isLeftOfAccess: boolean): AccessExpression | undefined { diff --git a/src/services/refactors/helpers.ts b/src/services/refactors/helpers.ts index 59d9c085f2fc9..0c2bf9ce919b6 100644 --- a/src/services/refactors/helpers.ts +++ b/src/services/refactors/helpers.ts @@ -22,4 +22,33 @@ namespace ts.refactor { if(!requested) return true; return known.substr(0, requested.length) === requested; } + + export const enum ResultStatus { + Ok, + Err + } + + export interface OkResult { + status: ResultStatus.Ok; + value: T; + } + + export interface ErrResult { + status: ResultStatus.Err; + reason: string; + } + + export type Result = OkResult | ErrResult; + + export function isErrorResult(result: Result): result is ErrResult { + return result.status === ResultStatus.Err; + } + + export function Err(reason: string): ErrResult { + return { status: ResultStatus.Err, reason }; + } + + export function Ok(value: T): OkResult { + return { status: ResultStatus.Ok, value }; + } } \ No newline at end of file diff --git a/tests/cases/fourslash/refactorIntroduceDestruction1.ts b/tests/cases/fourslash/refactorIntroduceDestruction1.ts index 87989f9f88e65..433150a15d102 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction1.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction1.ts @@ -9,7 +9,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction10.ts b/tests/cases/fourslash/refactorIntroduceDestruction10.ts index fee45509aed5b..3c4751dfa275e 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction10.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction10.ts @@ -8,7 +8,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `function foo (item: { a: string, b: number }) { const { a, b } = item; call(a, b) diff --git a/tests/cases/fourslash/refactorIntroduceDestruction11.ts b/tests/cases/fourslash/refactorIntroduceDestruction11.ts index 92bd68e6c2843..818615e6ffeb0 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction11.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction11.ts @@ -9,7 +9,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction13.ts b/tests/cases/fourslash/refactorIntroduceDestruction13.ts index 835221414e04c..eca73d5e1538a 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction13.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction13.ts @@ -10,7 +10,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction14.ts b/tests/cases/fourslash/refactorIntroduceDestruction14.ts index 2d5b4a920c678..5ca5f58294591 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction14.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction14.ts @@ -14,7 +14,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: { c: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction15.ts b/tests/cases/fourslash/refactorIntroduceDestruction15.ts index 4a1a7b74fa535..f789c0333335d 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction15.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction15.ts @@ -14,7 +14,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: { c: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction16.ts b/tests/cases/fourslash/refactorIntroduceDestruction16.ts index 2167ce0cfa3b3..f46ff78f928a7 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction16.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction16.ts @@ -14,7 +14,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: { c: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction18.ts b/tests/cases/fourslash/refactorIntroduceDestruction18.ts index 74da124409bb5..4768637dafba2 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction18.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction18.ts @@ -21,7 +21,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `interface A { f: 1, b: number diff --git a/tests/cases/fourslash/refactorIntroduceDestruction2.ts b/tests/cases/fourslash/refactorIntroduceDestruction2.ts index a6e9005d3c960..c4d74db124ae8 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction2.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction2.ts @@ -9,7 +9,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction20.ts b/tests/cases/fourslash/refactorIntroduceDestruction20.ts index 4af5a28bd76c5..be382138c6ec9 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction20.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction20.ts @@ -13,7 +13,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction21.ts b/tests/cases/fourslash/refactorIntroduceDestruction21.ts index 6a0883e8c9177..93951b0d8dcb4 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction21.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction21.ts @@ -10,7 +10,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `function foo (item: { a: string, b: number }) { const { a, b } = item; function bar () { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction23.ts b/tests/cases/fourslash/refactorIntroduceDestruction23.ts index 8bfca56a72e80..126b09d4ecac4 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction23.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction23.ts @@ -18,7 +18,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 } const { a, b } = item function foo() { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction24.ts b/tests/cases/fourslash/refactorIntroduceDestruction24.ts index 7edbf43e62bc6..5e0113a8eed53 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction24.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction24.ts @@ -13,7 +13,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `function foo () { call(item.a) } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction25.ts b/tests/cases/fourslash/refactorIntroduceDestruction25.ts index 4dac7cc337cfa..6f1be56ae15a9 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction25.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction25.ts @@ -9,7 +9,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2, c: 3 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction27.ts b/tests/cases/fourslash/refactorIntroduceDestruction27.ts index a67b64a461a30..9f06d13f84d7c 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction27.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction27.ts @@ -8,7 +8,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `function f(item: { a: 1 }, b = item.a) { const { a } = item; call(a, b) diff --git a/tests/cases/fourslash/refactorIntroduceDestruction28.ts b/tests/cases/fourslash/refactorIntroduceDestruction28.ts index 27c9c9f7235c4..8d5b5b1df2093 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction28.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction28.ts @@ -7,7 +7,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3 ] as const const [index_0, index_1, index_2] = item call(index_0, index_1, index_2)`, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction29.ts b/tests/cases/fourslash/refactorIntroduceDestruction29.ts index d77466324ed1c..bef68ab833b60 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction29.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction29.ts @@ -7,7 +7,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3 ] as const const [index_0, , index_2] = item call(index_0, index_2, index_2)`, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction30.ts b/tests/cases/fourslash/refactorIntroduceDestruction30.ts index c989dbe1dd3b0..fc9c70b2d49ea 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction30.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction30.ts @@ -7,7 +7,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 ] as const const [, , , index_3, , , , , index_8, , , , , , index_14] = item call(index_14, index_8, index_3)`, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction31.ts b/tests/cases/fourslash/refactorIntroduceDestruction31.ts index ce8df9fb53340..49934b4c3ae22 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction31.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction31.ts @@ -7,7 +7,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 ] as const const { 14: index_14, 8: index_8, 16: index_16 } = item call(index_14, index_8, index_16)`, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction32.ts b/tests/cases/fourslash/refactorIntroduceDestruction32.ts index da931dace895f..63d95bd50aeaf 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction32.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction32.ts @@ -10,7 +10,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `interface A { [k: string]: number } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction33.ts b/tests/cases/fourslash/refactorIntroduceDestruction33.ts index 68e45b4a5c615..cd1afc34a4544 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction33.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction33.ts @@ -14,7 +14,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction34.ts b/tests/cases/fourslash/refactorIntroduceDestruction34.ts index b3abc74bfc7fb..e88c20dc8218a 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction34.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction34.ts @@ -9,7 +9,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction35.ts b/tests/cases/fourslash/refactorIntroduceDestruction35.ts index 2c58261e977ab..a553a6f01f5fa 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction35.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction35.ts @@ -14,7 +14,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: { b: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction36.ts b/tests/cases/fourslash/refactorIntroduceDestruction36.ts index 912f340680f74..bfde615b5e818 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction36.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction36.ts @@ -14,7 +14,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: { b: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction37.ts b/tests/cases/fourslash/refactorIntroduceDestruction37.ts index f2c1aa71b6a6b..06de1ac6f17d4 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction37.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction37.ts @@ -8,7 +8,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `interface I { foo: string; bar: number } declare const a: I const { foo } = a diff --git a/tests/cases/fourslash/refactorIntroduceDestruction38.ts b/tests/cases/fourslash/refactorIntroduceDestruction38.ts index 7551e25abae93..2add26a6c92fe 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction38.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction38.ts @@ -8,7 +8,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `interface I { foo: string; bar: number } declare const a: I const { foo, bar } = a diff --git a/tests/cases/fourslash/refactorIntroduceDestruction39.ts b/tests/cases/fourslash/refactorIntroduceDestruction39.ts index efb0fdb8daa94..67e16bdfe2144 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction39.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction39.ts @@ -9,7 +9,7 @@ goTo.marker('a'); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, triggerReason: 'invoked', newContent: `const item = { a: 1, b: 2 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction5.ts b/tests/cases/fourslash/refactorIntroduceDestruction5.ts index d2f57d8a151ab..53fef96065a48 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction5.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction5.ts @@ -11,7 +11,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `declare const u: { type: 'number'; payload: number } | { type: 'string', payload: number } const { type, payload } = u if(type === "number") { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction6.ts b/tests/cases/fourslash/refactorIntroduceDestruction6.ts index ac0a3779cc2ae..8712afc22197a 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction6.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction6.ts @@ -9,7 +9,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: () => 1 } diff --git a/tests/cases/fourslash/refactorIntroduceDestruction7.ts b/tests/cases/fourslash/refactorIntroduceDestruction7.ts index d36917e09ec76..62df926a408fc 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction7.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction7.ts @@ -10,7 +10,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Introduce destruction", actionName: "Introduce destruction", - actionDescription: "Convert access to destruction", + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: () => 1 } From 8ba4ad9f3f7dd5cbb4a0aeec8d402e2f77212ae7 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 8 Jan 2021 17:19:23 +0800 Subject: [PATCH 23/28] Rename all code --- src/services/refactors/convertObjectDestruction.ts | 14 +++++++------- .../fourslash/refactorIntroduceDestruction1.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction10.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction11.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction12.ts | 2 +- .../fourslash/refactorIntroduceDestruction13.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction14.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction15.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction16.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction17.ts | 2 +- .../fourslash/refactorIntroduceDestruction18.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction19.ts | 2 +- .../fourslash/refactorIntroduceDestruction2.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction20.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction21.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction22.ts | 2 +- .../fourslash/refactorIntroduceDestruction23.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction24.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction25.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction26.ts | 2 +- .../fourslash/refactorIntroduceDestruction27.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction28.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction29.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction3.ts | 2 +- .../fourslash/refactorIntroduceDestruction30.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction31.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction32.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction33.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction34.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction35.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction36.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction37.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction38.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction39.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction4.ts | 6 +++--- .../fourslash/refactorIntroduceDestruction40.ts | 2 +- .../fourslash/refactorIntroduceDestruction5.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction6.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction7.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction8.ts | 4 ++-- .../fourslash/refactorIntroduceDestruction9.ts | 2 +- 41 files changed, 80 insertions(+), 80 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 786049c975762..e8188fc0be51f 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -1,17 +1,17 @@ /* @internal */ namespace ts.refactor { - const refactorName = "Introduce destruction"; + const refactorName = "Convert to destruction"; const refactorKind = "refactor.rewrite.expression.toDestructured"; registerRefactor(refactorName, { getAvailableActions, getEditsForAction, kinds: [refactorKind] }); - const introduceObjectDestructionAction = { + const convertToDestructionAction = { name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Convert_access_expression_to_destruction), kind: refactorKind - } + }; - function makeRefactorActionWithErrorReason (reason: string) { - return { ...introduceObjectDestructionAction, notApplicableReason: reason }; + function makeRefactorActionWithErrorReason(reason: string) { + return { ...convertToDestructionAction, notApplicableReason: reason }; } function getAvailableActions(context: RefactorContext): readonly ApplicableRefactorInfo[] { @@ -24,7 +24,7 @@ namespace ts.refactor { return [{ name: refactorName, description: getLocaleSpecificMessage(Diagnostics.Convert_access_expression_to_destruction), - actions: [introduceObjectDestructionAction] + actions: [convertToDestructionAction] }]; } @@ -35,7 +35,7 @@ namespace ts.refactor { actions: [makeRefactorActionWithErrorReason(info.reason)] }]; } - return emptyArray; + return emptyArray; } function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction1.ts b/tests/cases/fourslash/refactorIntroduceDestruction1.ts index 433150a15d102..ee1573cc1484b 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction1.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction1.ts @@ -7,8 +7,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction10.ts b/tests/cases/fourslash/refactorIntroduceDestruction10.ts index 3c4751dfa275e..b7612edc2da60 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction10.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction10.ts @@ -6,8 +6,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `function foo (item: { a: string, b: number }) { const { a, b } = item; diff --git a/tests/cases/fourslash/refactorIntroduceDestruction11.ts b/tests/cases/fourslash/refactorIntroduceDestruction11.ts index 818615e6ffeb0..7778e537c7619 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction11.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction11.ts @@ -7,8 +7,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction12.ts b/tests/cases/fourslash/refactorIntroduceDestruction12.ts index 75c07f8385b5c..5537d1f37dcbd 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction12.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction12.ts @@ -7,4 +7,4 @@ //// call(/*a*/item/*b*/[key], item.b) goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction13.ts b/tests/cases/fourslash/refactorIntroduceDestruction13.ts index eca73d5e1538a..4f59c8a8486fd 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction13.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction13.ts @@ -8,8 +8,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction14.ts b/tests/cases/fourslash/refactorIntroduceDestruction14.ts index 5ca5f58294591..0d1512fc0b658 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction14.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction14.ts @@ -12,8 +12,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction15.ts b/tests/cases/fourslash/refactorIntroduceDestruction15.ts index f789c0333335d..b8d2def8b4572 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction15.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction15.ts @@ -12,8 +12,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction16.ts b/tests/cases/fourslash/refactorIntroduceDestruction16.ts index f46ff78f928a7..33d813d6af3cf 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction16.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction16.ts @@ -12,8 +12,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction17.ts b/tests/cases/fourslash/refactorIntroduceDestruction17.ts index 2f138767b47a2..e00a3ddad5ea2 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction17.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction17.ts @@ -20,4 +20,4 @@ //// } goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction18.ts b/tests/cases/fourslash/refactorIntroduceDestruction18.ts index 4768637dafba2..3a47f73efa932 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction18.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction18.ts @@ -19,8 +19,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `interface A { f: 1, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction19.ts b/tests/cases/fourslash/refactorIntroduceDestruction19.ts index 2a01f5368c8a8..812d7b3e5b135 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction19.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction19.ts @@ -6,4 +6,4 @@ //// call(/*a*/item/*b*/["a-a-a"], item.b) goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction2.ts b/tests/cases/fourslash/refactorIntroduceDestruction2.ts index c4d74db124ae8..182e231b344fe 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction2.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction2.ts @@ -7,8 +7,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction20.ts b/tests/cases/fourslash/refactorIntroduceDestruction20.ts index be382138c6ec9..81e990b49c9c3 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction20.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction20.ts @@ -11,8 +11,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction21.ts b/tests/cases/fourslash/refactorIntroduceDestruction21.ts index 93951b0d8dcb4..ad027ed4c0e48 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction21.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction21.ts @@ -8,8 +8,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `function foo (item: { a: string, b: number }) { const { a, b } = item; diff --git a/tests/cases/fourslash/refactorIntroduceDestruction22.ts b/tests/cases/fourslash/refactorIntroduceDestruction22.ts index 0c9346fb14bb9..15e9c1d54e613 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction22.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction22.ts @@ -8,4 +8,4 @@ //// } goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction23.ts b/tests/cases/fourslash/refactorIntroduceDestruction23.ts index 126b09d4ecac4..2f96021de8be2 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction23.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction23.ts @@ -16,8 +16,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 } const { a, b } = item diff --git a/tests/cases/fourslash/refactorIntroduceDestruction24.ts b/tests/cases/fourslash/refactorIntroduceDestruction24.ts index 5e0113a8eed53..177100023cae6 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction24.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction24.ts @@ -11,8 +11,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `function foo () { call(item.a) diff --git a/tests/cases/fourslash/refactorIntroduceDestruction25.ts b/tests/cases/fourslash/refactorIntroduceDestruction25.ts index 6f1be56ae15a9..ab359a600ee81 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction25.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction25.ts @@ -7,8 +7,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2, c: 3 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction26.ts b/tests/cases/fourslash/refactorIntroduceDestruction26.ts index 520755e20703b..f8349b470f0d8 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction26.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction26.ts @@ -3,5 +3,5 @@ //// const f = (item: { a: 1, b: 2}) => /*a*/item/*b*/.a + item.b goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction27.ts b/tests/cases/fourslash/refactorIntroduceDestruction27.ts index 9f06d13f84d7c..2a2efcd7a97c8 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction27.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction27.ts @@ -6,8 +6,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `function f(item: { a: 1 }, b = item.a) { const { a } = item; diff --git a/tests/cases/fourslash/refactorIntroduceDestruction28.ts b/tests/cases/fourslash/refactorIntroduceDestruction28.ts index 8d5b5b1df2093..6702c65a61962 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction28.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction28.ts @@ -5,8 +5,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3 ] as const const [index_0, index_1, index_2] = item diff --git a/tests/cases/fourslash/refactorIntroduceDestruction29.ts b/tests/cases/fourslash/refactorIntroduceDestruction29.ts index bef68ab833b60..938c2474b6ccc 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction29.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction29.ts @@ -5,8 +5,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3 ] as const const [index_0, , index_2] = item diff --git a/tests/cases/fourslash/refactorIntroduceDestruction3.ts b/tests/cases/fourslash/refactorIntroduceDestruction3.ts index efbfa2d8b4a8e..2d8fc07bcda0c 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction3.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction3.ts @@ -6,4 +6,4 @@ //// call(/*a*/item.a/*b*/, item.b) goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction30.ts b/tests/cases/fourslash/refactorIntroduceDestruction30.ts index fc9c70b2d49ea..9591a782d32c0 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction30.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction30.ts @@ -5,8 +5,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 ] as const const [, , , index_3, , , , , index_8, , , , , , index_14] = item diff --git a/tests/cases/fourslash/refactorIntroduceDestruction31.ts b/tests/cases/fourslash/refactorIntroduceDestruction31.ts index 49934b4c3ae22..1988b72ae39c7 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction31.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction31.ts @@ -5,8 +5,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 ] as const const { 14: index_14, 8: index_8, 16: index_16 } = item diff --git a/tests/cases/fourslash/refactorIntroduceDestruction32.ts b/tests/cases/fourslash/refactorIntroduceDestruction32.ts index 63d95bd50aeaf..c83876a9f4b12 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction32.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction32.ts @@ -8,8 +8,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `interface A { [k: string]: number diff --git a/tests/cases/fourslash/refactorIntroduceDestruction33.ts b/tests/cases/fourslash/refactorIntroduceDestruction33.ts index cd1afc34a4544..ebce88d4340f9 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction33.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction33.ts @@ -12,8 +12,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction34.ts b/tests/cases/fourslash/refactorIntroduceDestruction34.ts index e88c20dc8218a..b69cd94c60d6c 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction34.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction34.ts @@ -7,8 +7,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction35.ts b/tests/cases/fourslash/refactorIntroduceDestruction35.ts index a553a6f01f5fa..48525f36e9dea 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction35.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction35.ts @@ -12,8 +12,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction36.ts b/tests/cases/fourslash/refactorIntroduceDestruction36.ts index bfde615b5e818..bd96cd3cbc5f1 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction36.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction36.ts @@ -12,8 +12,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction37.ts b/tests/cases/fourslash/refactorIntroduceDestruction37.ts index 06de1ac6f17d4..26ee130bb982f 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction37.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction37.ts @@ -6,8 +6,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `interface I { foo: string; bar: number } declare const a: I diff --git a/tests/cases/fourslash/refactorIntroduceDestruction38.ts b/tests/cases/fourslash/refactorIntroduceDestruction38.ts index 2add26a6c92fe..35841747e3050 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction38.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction38.ts @@ -6,8 +6,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `interface I { foo: string; bar: number } declare const a: I diff --git a/tests/cases/fourslash/refactorIntroduceDestruction39.ts b/tests/cases/fourslash/refactorIntroduceDestruction39.ts index 67e16bdfe2144..473f043d1ff70 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction39.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction39.ts @@ -7,8 +7,8 @@ goTo.marker('a'); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, triggerReason: 'invoked', newContent: `const item = { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction4.ts b/tests/cases/fourslash/refactorIntroduceDestruction4.ts index dc33d8cc481e4..3062d38143a9a 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction4.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction4.ts @@ -8,8 +8,8 @@ //// } goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") goTo.select("c", "d"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") goTo.select("e", "f"); -verify.not.refactorAvailable("Introduce destruction") \ No newline at end of file +verify.not.refactorAvailable("Convert to destruction") \ No newline at end of file diff --git a/tests/cases/fourslash/refactorIntroduceDestruction40.ts b/tests/cases/fourslash/refactorIntroduceDestruction40.ts index 3ad66ef81100b..63619820f640b 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction40.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction40.ts @@ -6,4 +6,4 @@ //// call(it/*a*/em.a, item.b) goTo.marker('a'); -verify.not.refactorAvailableForTriggerReason('implicit', 'Introduce destruction'); \ No newline at end of file +verify.not.refactorAvailableForTriggerReason('implicit', 'Convert to destruction'); \ No newline at end of file diff --git a/tests/cases/fourslash/refactorIntroduceDestruction5.ts b/tests/cases/fourslash/refactorIntroduceDestruction5.ts index 53fef96065a48..202af42a3a79a 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction5.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction5.ts @@ -9,8 +9,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `declare const u: { type: 'number'; payload: number } | { type: 'string', payload: number } const { type, payload } = u diff --git a/tests/cases/fourslash/refactorIntroduceDestruction6.ts b/tests/cases/fourslash/refactorIntroduceDestruction6.ts index 8712afc22197a..52071947d5b6b 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction6.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction6.ts @@ -7,8 +7,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: () => 1 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction7.ts b/tests/cases/fourslash/refactorIntroduceDestruction7.ts index 62df926a408fc..22537dfa4a779 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction7.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction7.ts @@ -8,8 +8,8 @@ goTo.select("a", "b"); edit.applyRefactor({ - refactorName: "Introduce destruction", - actionName: "Introduce destruction", + refactorName: "Convert to destruction", + actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: () => 1 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction8.ts b/tests/cases/fourslash/refactorIntroduceDestruction8.ts index cfff37bed723f..b00b13b68e818 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction8.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction8.ts @@ -7,8 +7,8 @@ //// call(item.a, /*c*/item/*d*/.b(), (((/*e*/item/*f*/.b)))()) goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") goTo.select("c", "d"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") goTo.select("e", "f"); verify.not.refactorAvailable("Introduce destruction") diff --git a/tests/cases/fourslash/refactorIntroduceDestruction9.ts b/tests/cases/fourslash/refactorIntroduceDestruction9.ts index 55fb490956880..2bb1234f2890e 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction9.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction9.ts @@ -7,4 +7,4 @@ //// /*a*/E/*b*/.A goTo.select("a", "b"); -verify.not.refactorAvailable("Introduce destruction") +verify.not.refactorAvailable("Convert to destruction") From 12c73ae0055125a0eb14f5cca42b13c17346c81d Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 8 Jan 2021 17:20:17 +0800 Subject: [PATCH 24/28] Fix typo --- src/services/refactors/convertObjectDestruction.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index e8188fc0be51f..32ec91361312d 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -117,7 +117,7 @@ namespace ts.refactor { const denseNumericInfo = info.isArrayLikeType && getDenseNumericAccessInfo(info.referencedAccessExpression); if (denseNumericInfo) { const [max, indexSet] = denseNumericInfo; - return getdenseNumericBindingPattern(info, file, max, indexSet, changeTracker); + return getDenseNumericBindingPattern(info, file, max, indexSet, changeTracker); } return getObjectBindingPattern(info, file, changeTracker); } @@ -141,7 +141,7 @@ namespace ts.refactor { return factory.createObjectBindingPattern(bindingElements); } - function getdenseNumericBindingPattern(info: Info, file: SourceFile, max: number, indexSet: Set, changeTracker: textChanges.ChangeTracker): ArrayBindingPattern { + function getDenseNumericBindingPattern(info: Info, file: SourceFile, max: number, indexSet: Set, changeTracker: textChanges.ChangeTracker): ArrayBindingPattern { const nameMap = new Map(); const bindingElements: ArrayBindingElement[] = []; From adba14f3ec3c41066b17bd5343b4697e03cbd6c6 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 8 Jan 2021 17:56:06 +0800 Subject: [PATCH 25/28] Provide array to object destruction if invoked --- src/compiler/diagnosticMessages.json | 4 +++ .../refactors/convertObjectDestruction.ts | 29 ++++++++++++++----- .../refactorIntroduceDestruction30.ts | 10 +++---- .../refactorIntroduceDestruction31.ts | 1 + .../refactorIntroduceDestruction41.ts | 15 ++++++++++ .../refactorIntroduceDestruction42.ts | 15 ++++++++++ .../refactorIntroduceDestruction43.ts | 7 +++++ .../refactorIntroduceDestruction44.ts | 7 +++++ .../refactorIntroduceDestruction45.ts | 7 +++++ 9 files changed, 82 insertions(+), 13 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction41.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction42.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction43.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction44.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction45.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 9be7498d321c5..6d6a36ab5cdf6 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -6323,5 +6323,9 @@ "Some references are un-convertible.": { "category": "Error", "code": 18039 + }, + "Too many empty array item.": { + "category": "Error", + "code": 18040 } } diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 32ec91361312d..d762c3692a8e0 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -47,7 +47,7 @@ namespace ts.refactor { if (isJSFile || !cancellationToken) return emptyResult; const info = getInfo(context, file, program.getTypeChecker(), program, cancellationToken, /*resolveUniqueName*/ true); - if (isErrorResult(info)) return emptyResult; + Debug.assert(!isErrorResult(info)); const edits = textChanges.ChangeTracker.with(context, t => doChange(t, file, info.value)); return { renameFilename: undefined, renameLocation: undefined, edits }; @@ -71,17 +71,22 @@ namespace ts.refactor { return undefined; } - const value = parseInt(info.name); + // Check for integer. + const value = Number(info.name); + if (isNaN(value) || parseInt(value.toString(), 10) !== value) { + return undefined; + } + min = Math.min(min, value); max = Math.max(max, value); indexSet.add(value); - if (isNaN(min) || isNaN(max) || min < 0 || max < 0) { + if (min < 0 || max < 0) { return undefined; } } - if (max > 15) { + if (indexSet.size <= Math.floor(max / 2)) { return undefined; } @@ -90,6 +95,8 @@ namespace ts.refactor { function doChange(changeTracker: textChanges.ChangeTracker, file: SourceFile, info: Info) { const bindingPattern = getBindingPattern(info, file, changeTracker); + Debug.assertIsDefined(bindingPattern); + suppressLeadingAndTrailingTrivia(info.replacementExpression); const newBinding = factory.createVariableStatement( /* modifiers*/ undefined, @@ -113,7 +120,7 @@ namespace ts.refactor { ); } - function getBindingPattern(info: Info, file: SourceFile, changeTracker: textChanges.ChangeTracker): BindingPattern { + function getBindingPattern(info: Info, file: SourceFile, changeTracker: textChanges.ChangeTracker): BindingPattern | undefined { const denseNumericInfo = info.isArrayLikeType && getDenseNumericAccessInfo(info.referencedAccessExpression); if (denseNumericInfo) { const [max, indexSet] = denseNumericInfo; @@ -220,11 +227,11 @@ namespace ts.refactor { namesNeedUniqueName: Set isArrayLikeType: boolean } - function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean, considerEmptySpans = true): Result { + function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean, triggerByInvoked = true): Result { const current = getTokenAtPosition(file, context.startPosition); const compilerOptions = context.program.getCompilerOptions(); const range = createTextRangeFromSpan(getRefactorContextSpan(context)); - const cursorRequest = range.pos === range.end && considerEmptySpans; + const cursorRequest = range.pos === range.end && triggerByInvoked; const node = findAncestor(current, (node => node.parent && isAccessExpression(node.parent) && !rangeContainsSkipTrivia(range, node.parent, file) && ( cursorRequest || nodeOverlapsWithStartEnd(node, file, range.pos, range.end) @@ -245,6 +252,7 @@ namespace ts.refactor { const references = FindAllReferences.getReferenceEntriesForNode(-1, node, program, [file], cancellationToken); let firstReferenced: Expression | undefined; let firstReferencedStatement: Statement | undefined; + let hasNumericAccess = false; const referencedAccessExpression: ReferencedAccessInfo[] = []; const allReferencedAcccessExpression: AccessExpression[] = []; const container = isParameter(symbol.valueDeclaration) ? symbol.valueDeclaration : findAncestor(symbol.valueDeclaration, or(isStatement, isSourceFile)); @@ -278,7 +286,7 @@ namespace ts.refactor { return; } if (isNumericLiteral(accessExpression.argumentExpression)) { - isNumericAccess = true; + hasNumericAccess = isNumericAccess = true; } else if (!isIdentifierText(accessExpression.argumentExpression.text, compilerOptions.target, compilerOptions.jsx ? LanguageVariant.JSX : LanguageVariant.Standard)) { return; @@ -306,6 +314,11 @@ namespace ts.refactor { const namesNeedUniqueName = new Set(); const type = checker.getTypeOfSymbolAtLocation(symbol, firstReferenced); const isArrayLikeType = checker.isArrayLikeType(type); + + if (hasNumericAccess && isArrayLikeType && !getDenseNumericAccessInfo(referencedAccessExpression) && !triggerByInvoked) { + return Err(Diagnostics.Too_many_empty_array_item.message); + } + forEach(allReferencedAcccessExpression, expr => { const referenceType = checker.getTypeAtLocation(expr.expression); if (referenceType !== type) { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction30.ts b/tests/cases/fourslash/refactorIntroduceDestruction30.ts index 9591a782d32c0..09884e806f2bd 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction30.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction30.ts @@ -1,14 +1,14 @@ /// -//// const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 ] as const -//// call(/*a*/item/*b*/[14], item[8], item[3]) +//// const item = [ 1, 2, 3 ] as const +//// call(/*a*/item/*b*/[0]) goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Convert to destruction", actionName: "Convert to destruction", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, - newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 ] as const -const [, , , index_3, , , , , index_8, , , , , , index_14] = item -call(index_14, index_8, index_3)`, + newContent: `const item = [ 1, 2, 3 ] as const +const [index_0] = item +call(index_0)`, }); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction31.ts b/tests/cases/fourslash/refactorIntroduceDestruction31.ts index 1988b72ae39c7..20573d940f438 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction31.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction31.ts @@ -7,6 +7,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Convert to destruction", actionName: "Convert to destruction", + triggerReason: 'invoked', actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 ] as const const { 14: index_14, 8: index_8, 16: index_16 } = item diff --git a/tests/cases/fourslash/refactorIntroduceDestruction41.ts b/tests/cases/fourslash/refactorIntroduceDestruction41.ts new file mode 100644 index 0000000000000..4019b52d1adf7 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction41.ts @@ -0,0 +1,15 @@ +/// + +//// const item = [ 1, 2, 3 ] as const +//// call(/*a*/item/*b*/[3]) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert to destruction", + actionName: "Convert to destruction", + triggerReason: 'invoked', + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, + newContent: `const item = [ 1, 2, 3 ] as const +const { 3: index_3 } = item +call(index_3)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction42.ts b/tests/cases/fourslash/refactorIntroduceDestruction42.ts new file mode 100644 index 0000000000000..30ab388f0ec35 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction42.ts @@ -0,0 +1,15 @@ +/// + +//// const item = [ 1, 2, 3 ] as const +//// call(/*a*/item/*b*/[2]) + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert to destruction", + actionName: "Convert to destruction", + triggerReason: 'invoked', + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, + newContent: `const item = [ 1, 2, 3 ] as const +const { 2: index_2 } = item +call(index_2)`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction43.ts b/tests/cases/fourslash/refactorIntroduceDestruction43.ts new file mode 100644 index 0000000000000..fd4751725617a --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction43.ts @@ -0,0 +1,7 @@ +/// + +//// const item = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17 ] as const +//// call(/*a*/item/*b*/[14], item[8], item[16]) + +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason('implicit', 'Convert to destruction'); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction44.ts b/tests/cases/fourslash/refactorIntroduceDestruction44.ts new file mode 100644 index 0000000000000..8f665e80a9d9f --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction44.ts @@ -0,0 +1,7 @@ +/// + +//// const item = [ 1, 2, 3 ] as const +//// call(/*a*/item/*b*/[3]) + +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason('implicit', 'Convert to destruction'); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction45.ts b/tests/cases/fourslash/refactorIntroduceDestruction45.ts new file mode 100644 index 0000000000000..20d23b8be6337 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction45.ts @@ -0,0 +1,7 @@ +/// + +//// const item = [ 1, 2, 3 ] as const +//// call(/*a*/item/*b*/[2]) + +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason('implicit', 'Convert to destruction'); From a66d51d1a6390693c0fbd3e236833192f1c0154c Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 8 Jan 2021 18:18:03 +0800 Subject: [PATCH 26/28] Avoid refactor if only one props or one ref --- src/compiler/diagnosticMessages.json | 4 +++ .../refactors/convertObjectDestruction.ts | 29 ++++++++++++------- .../refactorIntroduceDestruction18.ts | 1 + .../refactorIntroduceDestruction27.ts | 1 + .../refactorIntroduceDestruction30.ts | 1 + .../refactorIntroduceDestruction34.ts | 1 + .../refactorIntroduceDestruction46.ts | 8 +++++ .../refactorIntroduceDestruction47.ts | 20 +++++++++++++ .../refactorIntroduceDestruction48.ts | 7 +++++ .../refactorIntroduceDestruction6.ts | 1 + .../refactorIntroduceDestruction7.ts | 1 + 11 files changed, 63 insertions(+), 11 deletions(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction46.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction47.ts create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction48.ts diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 6d6a36ab5cdf6..06a3a007bdf2d 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -6327,5 +6327,9 @@ "Too many empty array item.": { "category": "Error", "code": 18040 + }, + "At least '{0}' references are required.": { + "category": "Error", + "code": 18041 } } diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index d762c3692a8e0..eacc8c67bb487 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -113,11 +113,13 @@ namespace ts.refactor { ), ); - changeTracker.insertNodeBefore( - file, - info.firstReferencedStatement, - newBinding - ); + if (info.firstReferencedStatement) { + changeTracker.insertNodeBefore( + file, + info.firstReferencedStatement, + newBinding + ); + } } function getBindingPattern(info: Info, file: SourceFile, changeTracker: textChanges.ChangeTracker): BindingPattern | undefined { @@ -220,12 +222,12 @@ namespace ts.refactor { } interface Info { - replacementExpression: Expression, - referencedAccessExpression: ReferencedAccessInfo[] - firstReferenced: Expression - firstReferencedStatement: Statement - namesNeedUniqueName: Set - isArrayLikeType: boolean + replacementExpression: Expression; + referencedAccessExpression: ReferencedAccessInfo[]; + firstReferenced: Expression | undefined; + firstReferencedStatement: Statement | undefined; + namesNeedUniqueName: Set; + isArrayLikeType: boolean; } function getInfo(context: RefactorContext, file: SourceFile, checker: TypeChecker, program: Program, cancellationToken: CancellationToken, resolveUniqueName: boolean, triggerByInvoked = true): Result { const current = getTokenAtPosition(file, context.startPosition); @@ -310,6 +312,11 @@ namespace ts.refactor { return Err(Diagnostics.Cannot_find_convertible_references.message); } + const minimumReferenceCount = 2; + if (!triggerByInvoked && referencedAccessExpression.length < minimumReferenceCount) { + return Err(formatMessage(/*_dummy*/ undefined, Diagnostics.At_least_0_references_are_required, minimumReferenceCount)); + } + let hasUnconvertableReference = false; const namesNeedUniqueName = new Set(); const type = checker.getTypeOfSymbolAtLocation(symbol, firstReferenced); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction18.ts b/tests/cases/fourslash/refactorIntroduceDestruction18.ts index 3a47f73efa932..d29257d6e0d4d 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction18.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction18.ts @@ -21,6 +21,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Convert to destruction", actionName: "Convert to destruction", + triggerReason: "invoked", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `interface A { f: 1, diff --git a/tests/cases/fourslash/refactorIntroduceDestruction27.ts b/tests/cases/fourslash/refactorIntroduceDestruction27.ts index 2a2efcd7a97c8..43f16f09eb265 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction27.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction27.ts @@ -8,6 +8,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Convert to destruction", actionName: "Convert to destruction", + triggerReason: 'invoked', actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `function f(item: { a: 1 }, b = item.a) { const { a } = item; diff --git a/tests/cases/fourslash/refactorIntroduceDestruction30.ts b/tests/cases/fourslash/refactorIntroduceDestruction30.ts index 09884e806f2bd..8dc50f2196cd0 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction30.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction30.ts @@ -7,6 +7,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Convert to destruction", actionName: "Convert to destruction", + triggerReason: "invoked", actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = [ 1, 2, 3 ] as const const [index_0] = item diff --git a/tests/cases/fourslash/refactorIntroduceDestruction34.ts b/tests/cases/fourslash/refactorIntroduceDestruction34.ts index b69cd94c60d6c..5ce02c0d541fe 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction34.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction34.ts @@ -9,6 +9,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Convert to destruction", actionName: "Convert to destruction", + triggerReason: 'invoked', actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: 2 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction46.ts b/tests/cases/fourslash/refactorIntroduceDestruction46.ts new file mode 100644 index 0000000000000..0e75aaea46b4d --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction46.ts @@ -0,0 +1,8 @@ +/// + +//// function f(item: { a: 1 }, b = item.a) { +//// call(/*a*/item/*b*/.a, b) +//// } + +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason('implicit', 'Convert to destruction'); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction47.ts b/tests/cases/fourslash/refactorIntroduceDestruction47.ts new file mode 100644 index 0000000000000..8bada444b598a --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction47.ts @@ -0,0 +1,20 @@ +/// + +//// function f(item: { a: 1 }, b = item.a) { +//// call(/*a*/item/*b*/.a, b) +//// call(item.a, b) +//// } + + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert to destruction", + actionName: "Convert to destruction", + triggerReason: 'invoked', + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, + newContent: `function f(item: { a: 1 }, b = item.a) { + const { a } = item + call(a, b) + call(a, b) +}`, +}); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction48.ts b/tests/cases/fourslash/refactorIntroduceDestruction48.ts new file mode 100644 index 0000000000000..7fbf99544f388 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction48.ts @@ -0,0 +1,7 @@ +/// + +//// const item = [ 1, 2, 3 ] as const +//// call(/*a*/item/*b*/[0]) + +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason('implicit', 'Convert to destruction'); diff --git a/tests/cases/fourslash/refactorIntroduceDestruction6.ts b/tests/cases/fourslash/refactorIntroduceDestruction6.ts index 52071947d5b6b..97a2caec0c453 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction6.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction6.ts @@ -9,6 +9,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Convert to destruction", actionName: "Convert to destruction", + triggerReason: 'invoked', actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: () => 1 diff --git a/tests/cases/fourslash/refactorIntroduceDestruction7.ts b/tests/cases/fourslash/refactorIntroduceDestruction7.ts index 22537dfa4a779..e32be45eb3195 100644 --- a/tests/cases/fourslash/refactorIntroduceDestruction7.ts +++ b/tests/cases/fourslash/refactorIntroduceDestruction7.ts @@ -10,6 +10,7 @@ goTo.select("a", "b"); edit.applyRefactor({ refactorName: "Convert to destruction", actionName: "Convert to destruction", + triggerReason: 'invoked', actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, newContent: `const item = { a: 1, b: () => 1 From 71210ac71c2ca38113d63f19480affdf768fc765 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 8 Jan 2021 18:50:49 +0800 Subject: [PATCH 27/28] Care deeper for container --- .../refactors/convertObjectDestruction.ts | 6 +++++- .../fourslash/refactorIntroduceDestruction49.ts | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/refactorIntroduceDestruction49.ts diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index eacc8c67bb487..61cda2c4530c3 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -257,7 +257,7 @@ namespace ts.refactor { let hasNumericAccess = false; const referencedAccessExpression: ReferencedAccessInfo[] = []; const allReferencedAcccessExpression: AccessExpression[] = []; - const container = isParameter(symbol.valueDeclaration) ? symbol.valueDeclaration : findAncestor(symbol.valueDeclaration, or(isStatement, isSourceFile)); + const container = getContainingParameterDeclaration(symbol.valueDeclaration) || findAncestor(symbol.valueDeclaration, or(isStatement, isSourceFile)); Debug.assertIsDefined(container); forEach(references, reference => { if (reference.kind !== FindAllReferences.EntryKind.Node) { @@ -373,6 +373,10 @@ namespace ts.refactor { }); } + function getContainingParameterDeclaration(decl: Declaration) { + return findAncestor(decl, node => isFunctionLike(node) ? "quit" : isParameter(node)) as ParameterDeclaration | undefined; + } + function getAccessExpressionIfValidReference(node: Node, symbol: Symbol, container: Node, allReferencedAcccessExpression: Node[], isLeftOfAccess: boolean): AccessExpression | undefined { let lastChild = node; const topReferencedAccessExpression = findAncestor(node.parent, n => { diff --git a/tests/cases/fourslash/refactorIntroduceDestruction49.ts b/tests/cases/fourslash/refactorIntroduceDestruction49.ts new file mode 100644 index 0000000000000..b770578d778a9 --- /dev/null +++ b/tests/cases/fourslash/refactorIntroduceDestruction49.ts @@ -0,0 +1,17 @@ +/// + +//// function foo (item: { a: string, b: number }) { +//// call(item./*a*/a/*b*/, item.b) +//// } + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Convert to destruction", + actionName: "Convert to destruction", + triggerReason: 'invoked', + actionDescription: ts.Diagnostics.Convert_access_expression_to_destruction.message, + newContent: `function foo (item: { a: string, b: number }) { + const { a } = item; + call(a, item.b) +}`, +}); From 38e874f44bd1283cc702211f4958e616700db132 Mon Sep 17 00:00:00 2001 From: kingwl Date: Wed, 16 Mar 2022 13:36:37 +0800 Subject: [PATCH 28/28] fix missing merge --- src/services/refactors/convertObjectDestruction.ts | 1 + src/services/utilities.ts | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/src/services/refactors/convertObjectDestruction.ts b/src/services/refactors/convertObjectDestruction.ts index 61cda2c4530c3..ae487ac6450a4 100644 --- a/src/services/refactors/convertObjectDestruction.ts +++ b/src/services/refactors/convertObjectDestruction.ts @@ -416,6 +416,7 @@ namespace ts.refactor { return undefined; } + Debug.assertIsDefined(symbol.valueDeclaration); if (node.pos < symbol.valueDeclaration.pos) { return undefined; } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 88836f59c3428..166b88a383cf3 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -532,6 +532,13 @@ namespace ts { return start < end; } + /** + * @internal + */ + export function rangeContainsSkipTrivia(r1: TextRange, node: Node, file: SourceFile): boolean { + return rangeContainsStartEnd(r1, skipTrivia(file.text, node.pos), node.end); + } + /** * Assumes `candidate.start <= position` holds. */