Skip to content

Commit f05a50a

Browse files
author
Maria Solano
committed
Do not offer refactor if there is a write
1 parent 6b93a15 commit f05a50a

File tree

2 files changed

+32
-13
lines changed

2 files changed

+32
-13
lines changed

src/services/findAllReferences.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -848,8 +848,12 @@ export function getTextSpanOfEntry(entry: Entry) {
848848
getTextSpan(entry.node, entry.node.getSourceFile());
849849
}
850850

851-
/** A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment */
852-
function isWriteAccessForReference(node: Node): boolean {
851+
/**
852+
* A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment.
853+
*
854+
* @internal
855+
*/
856+
export function isWriteAccessForReference(node: Node): boolean {
853857
const decl = getDeclarationFromName(node);
854858
return !!decl && declarationIsWriteAccess(decl) || node.kind === SyntaxKind.DefaultKeyword || isWriteAccess(node);
855859
}

src/services/refactors/inlineVariable.ts

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -120,13 +120,9 @@ function getInliningInfo(file: SourceFile, startPosition: number, tryWithReferen
120120
if (!referencedSymbols || referencedSymbols.length !== 1) {
121121
return undefined;
122122
}
123+
const referenceNodes = getReferenceNodes(referencedSymbols[0].references, name);
123124

124-
// Only replace node references, and exclude the original variable too.
125-
const references = mapDefined(referencedSymbols[0].references, entry =>
126-
entry.kind === FindAllReferences.EntryKind.Node ? entry.node === name ? undefined : entry.node : undefined
127-
);
128-
129-
return references.length === 0 ? undefined : { references, declaration: parent, replacement: parent.initializer };
125+
return referenceNodes && { references: referenceNodes, declaration: parent, replacement: parent.initializer };
130126
}
131127

132128
if (tryWithReferenceToken && isIdentifier(token)) {
@@ -136,21 +132,40 @@ function getInliningInfo(file: SourceFile, startPosition: number, tryWithReferen
136132
return undefined;
137133
}
138134

139-
const { definition } = referencedSymbols[0];
135+
const { definition, references } = referencedSymbols[0];
140136
if (definition?.type !== FindAllReferences.DefinitionKind.Symbol) {
141137
return undefined;
142138
}
143139

144140
const { valueDeclaration } = definition.symbol;
145141
if (valueDeclaration && isInitializedVariable(valueDeclaration) && isVariableDeclarationInVariableStatement(valueDeclaration)) {
146-
const references = mapDefined(referencedSymbols[0].references, entry =>
147-
entry.kind === FindAllReferences.EntryKind.Node ? entry.node === valueDeclaration.name ? undefined : entry.node : undefined
148-
);
142+
const referenceNodes = getReferenceNodes(references, valueDeclaration.name);
149143

150-
return references.length === 0 ? undefined : { references, declaration: valueDeclaration, replacement: valueDeclaration.initializer };
144+
return referenceNodes && { references: referenceNodes, declaration: valueDeclaration, replacement: valueDeclaration.initializer };
151145
}
152146
}
153147

154148
// TODO: Do we want to have other errors too?
155149
return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_variable_to_inline) };
150+
}
151+
152+
function getReferenceNodes(entries: readonly FindAllReferences.Entry[], declaration: Node) {
153+
const referenceNodes = mapDefined(entries, entry => {
154+
// Only replace node references, and exclude the original variable too.
155+
if (entry.kind !== FindAllReferences.EntryKind.Node || entry.node === declaration) {
156+
return undefined;
157+
}
158+
159+
// Only inline if all references are reads. Else we might end up with an invalid scenario like:
160+
// const y = x++ + 1 -> const y = 2++ + 1
161+
if (FindAllReferences.isWriteAccessForReference(entry.node)) {
162+
return undefined;
163+
}
164+
165+
return entry.node;
166+
});
167+
168+
// Return undefined if the only reference is the declaration itself, or if a reference
169+
// isn't applicable for inlining.
170+
return referenceNodes.length > 0 && referenceNodes.length === entries.length - 1 ? referenceNodes : undefined;
156171
}

0 commit comments

Comments
 (0)