Skip to content

Commit b788987

Browse files
author
Maria Solano
committed
Add parenthesize logic
1 parent 542df60 commit b788987

File tree

1 file changed

+23
-2
lines changed

1 file changed

+23
-2
lines changed

src/services/refactors/inlineVariable.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@ import {
44
Diagnostics,
55
emptyArray,
66
Expression,
7+
factory,
78
FindAllReferences,
9+
getExpressionPrecedence,
810
getLocaleSpecificMessage,
911
getTokenAtPosition,
12+
isExpression,
1013
isIdentifier,
1114
isInitializedVariable,
1215
isVariableDeclarationInVariableStatement,
@@ -98,7 +101,7 @@ registerRefactor(refactorName, {
98101
const { references, declaration, replacement } = info;
99102
const edits = textChanges.ChangeTracker.with(context, tracker => {
100103
for (const node of references) {
101-
tracker.replaceNode(file, node, replacement);
104+
tracker.replaceNode(file, node, getReplacementExpression(node, replacement));
102105
}
103106
tracker.delete(file, declaration);
104107
});
@@ -149,7 +152,7 @@ function getInliningInfo(file: SourceFile, startPosition: number, tryWithReferen
149152
return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_variable_to_inline) };
150153
}
151154

152-
function getReferenceNodes(entries: readonly FindAllReferences.Entry[], declaration: Node) {
155+
function getReferenceNodes(entries: readonly FindAllReferences.Entry[], declaration: Node): Node[] | undefined {
153156
const referenceNodes = mapDefined(entries, entry => {
154157
// Only replace node references, and exclude the original variable too.
155158
if (entry.kind !== FindAllReferences.EntryKind.Node || entry.node === declaration) {
@@ -168,4 +171,22 @@ function getReferenceNodes(entries: readonly FindAllReferences.Entry[], declarat
168171
// Return undefined if the only reference is the declaration itself, or if a reference
169172
// isn't applicable for inlining.
170173
return referenceNodes.length > 0 && referenceNodes.length === entries.length - 1 ? referenceNodes : undefined;
174+
}
175+
176+
function getReplacementExpression(reference: Node, replacement: Expression): Expression {
177+
// Logic from binaryOperandNeedsParentheses: "If the operand has lower precedence,
178+
// then it needs to be parenthesized to preserve the intent of the expression.
179+
// If the operand has higher precedence, then it does not need to be parenthesized."
180+
//
181+
// Note that binaryOperandNeedsParentheses has further logic when the precedences
182+
// are equal, but for the purposes of this refactor we keep things simple and always
183+
// parenthesize.
184+
const { parent } = reference;
185+
if (isExpression(parent)) {
186+
if (getExpressionPrecedence(replacement) <= getExpressionPrecedence(parent)) {
187+
return factory.createParenthesizedExpression(replacement);
188+
}
189+
}
190+
191+
return replacement;
171192
}

0 commit comments

Comments
 (0)