From 734d4fbac18cb12e5974a3deca68fc91588b8feb Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Sun, 14 Jan 2024 02:44:55 +0200 Subject: [PATCH 1/4] feat(57028): support inlining variables that are used in object literal shorthand --- src/services/refactors/inlineVariable.ts | 12 ++++++++---- .../inlineVariableShorthandPropertyAssignment.ts | 13 +++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) create mode 100644 tests/cases/fourslash/inlineVariableShorthandPropertyAssignment.ts diff --git a/src/services/refactors/inlineVariable.ts b/src/services/refactors/inlineVariable.ts index 3b9b94c062424..217b7cc78f768 100644 --- a/src/services/refactors/inlineVariable.ts +++ b/src/services/refactors/inlineVariable.ts @@ -23,6 +23,7 @@ import { isNumericLiteral, isObjectLiteralExpression, isPropertyAccessExpression, + isShorthandPropertyAssignment, isTypeQueryNode, isVariableDeclarationInVariableStatement, isVariableStatement, @@ -102,7 +103,6 @@ registerRefactor(refactorName, { getEditsForAction(context, actionName) { Debug.assert(actionName === refactorName, "Unexpected refactor invoked"); - const { file, program, startPosition } = context; // tryWithReferenceToken is true below since here we're already performing the refactor. @@ -152,7 +152,7 @@ function getInliningInfo(file: SourceFile, startPosition: number, tryWithReferen } // Try finding the declaration and nodes to replace via the reference token. - if (tryWithReferenceToken) { + if (tryWithReferenceToken || isShorthandPropertyAssignment(parent)) { let definition = checker.resolveName(token.text, token, SymbolFlags.Value, /*excludeGlobals*/ false); definition = definition && checker.getMergedSymbol(definition); @@ -191,7 +191,7 @@ function getReferenceNodes(declaration: InitializedVariableDeclaration, checker: const cannotInline = FindAllReferences.Core.eachSymbolReferenceInFile(declaration.name as Identifier, checker, file, ref => { // Only inline if all references are reads. Else we might end up with an invalid scenario like: // const y = x++ + 1 -> const y = 2++ + 1 - if (FindAllReferences.isWriteAccessForReference(ref)) { + if (FindAllReferences.isWriteAccessForReference(ref) && !isShorthandPropertyAssignment(ref.parent)) { return true; } @@ -216,7 +216,7 @@ function getReferenceNodes(declaration: InitializedVariableDeclaration, checker: return references.length === 0 || cannotInline ? undefined : references; } -function getReplacementExpression(reference: Node, replacement: Expression): Expression { +function getReplacementExpression(reference: Node, replacement: Expression) { // Make sure each reference site gets its own copy of the replacement node. replacement = getSynthesizedDeepClone(replacement); const { parent } = reference; @@ -244,5 +244,9 @@ function getReplacementExpression(reference: Node, replacement: Expression): Exp return factory.createParenthesizedExpression(replacement); } + if (isIdentifier(reference) && isShorthandPropertyAssignment(parent)) { + return factory.createPropertyAssignment(reference, replacement); + } + return replacement; } diff --git a/tests/cases/fourslash/inlineVariableShorthandPropertyAssignment.ts b/tests/cases/fourslash/inlineVariableShorthandPropertyAssignment.ts new file mode 100644 index 0000000000000..adbee7cc55dfb --- /dev/null +++ b/tests/cases/fourslash/inlineVariableShorthandPropertyAssignment.ts @@ -0,0 +1,13 @@ +/// + +////const foo = 1; +////const bar = { /*a*/foo/*b*/ }; + +goTo.select("a", "b"); +verify.refactorAvailable("Inline variable"); +edit.applyRefactor({ + refactorName: "Inline variable", + actionName: "Inline variable", + actionDescription: "Inline variable", + newContent: "const bar = { foo: 1 };", +}); From 43eeda7d0205835f2aa8a87a410ae12b9a3d6dba Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Thu, 18 Jan 2024 03:04:22 +0200 Subject: [PATCH 2/4] update comments --- src/services/refactors/inlineVariable.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/services/refactors/inlineVariable.ts b/src/services/refactors/inlineVariable.ts index 217b7cc78f768..67b9a8c359a5d 100644 --- a/src/services/refactors/inlineVariable.ts +++ b/src/services/refactors/inlineVariable.ts @@ -189,8 +189,9 @@ function isDeclarationExported(declaration: InitializedVariableDeclaration): boo function getReferenceNodes(declaration: InitializedVariableDeclaration, checker: TypeChecker, file: SourceFile): Identifier[] | undefined { const references: Identifier[] = []; const cannotInline = FindAllReferences.Core.eachSymbolReferenceInFile(declaration.name as Identifier, checker, file, ref => { - // Only inline if all references are reads. Else we might end up with an invalid scenario like: - // const y = x++ + 1 -> const y = 2++ + 1 + // Only inline if all references are reads, or if it includes a shorthand property assignment. + // Else we might end up with an invalid scenario like: + // const y = x++ + 1 -> const y = 2++ + 1, if (FindAllReferences.isWriteAccessForReference(ref) && !isShorthandPropertyAssignment(ref.parent)) { return true; } From dbbed95b4daf5ee55d941fbd530f506ddf51e0a0 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Thu, 18 Jan 2024 10:52:20 +0200 Subject: [PATCH 3/4] allow inlining shorthand properties only when the trigger reason is invoked --- src/services/refactors/inlineVariable.ts | 2 +- .../fourslash/inlineVariableShorthandPropertyAssignment.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/refactors/inlineVariable.ts b/src/services/refactors/inlineVariable.ts index 67b9a8c359a5d..056cddb6a1945 100644 --- a/src/services/refactors/inlineVariable.ts +++ b/src/services/refactors/inlineVariable.ts @@ -152,7 +152,7 @@ function getInliningInfo(file: SourceFile, startPosition: number, tryWithReferen } // Try finding the declaration and nodes to replace via the reference token. - if (tryWithReferenceToken || isShorthandPropertyAssignment(parent)) { + if (tryWithReferenceToken) { let definition = checker.resolveName(token.text, token, SymbolFlags.Value, /*excludeGlobals*/ false); definition = definition && checker.getMergedSymbol(definition); diff --git a/tests/cases/fourslash/inlineVariableShorthandPropertyAssignment.ts b/tests/cases/fourslash/inlineVariableShorthandPropertyAssignment.ts index adbee7cc55dfb..b3fbe86f4ba56 100644 --- a/tests/cases/fourslash/inlineVariableShorthandPropertyAssignment.ts +++ b/tests/cases/fourslash/inlineVariableShorthandPropertyAssignment.ts @@ -4,10 +4,10 @@ ////const bar = { /*a*/foo/*b*/ }; goTo.select("a", "b"); -verify.refactorAvailable("Inline variable"); edit.applyRefactor({ refactorName: "Inline variable", actionName: "Inline variable", actionDescription: "Inline variable", newContent: "const bar = { foo: 1 };", + triggerReason: "invoked", }); From dd001de4af2b5ff1b12c8b06076ff523fe82e822 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Thu, 18 Jan 2024 20:09:29 +0200 Subject: [PATCH 4/4] add more comments --- src/services/refactors/inlineVariable.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/services/refactors/inlineVariable.ts b/src/services/refactors/inlineVariable.ts index 056cddb6a1945..bdd5af771e761 100644 --- a/src/services/refactors/inlineVariable.ts +++ b/src/services/refactors/inlineVariable.ts @@ -245,6 +245,10 @@ function getReplacementExpression(reference: Node, replacement: Expression) { return factory.createParenthesizedExpression(replacement); } + // Inline shorthand property assignment + // E.g.: + // const x = 1; + // const y = { x }; -> const y = { x: 1 }; if (isIdentifier(reference) && isShorthandPropertyAssignment(parent)) { return factory.createPropertyAssignment(reference, replacement); }