Skip to content

Commit 31936ea

Browse files
authored
Add inline variable refactor (#54281)
1 parent 89cbea8 commit 31936ea

20 files changed

+483
-2
lines changed

src/compiler/diagnosticMessages.json

+12
Original file line numberDiff line numberDiff line change
@@ -7632,6 +7632,18 @@
76327632
"category": "Message",
76337633
"code": 95183
76347634
},
7635+
"Inline variable": {
7636+
"category": "Message",
7637+
"code": 95184
7638+
},
7639+
"Could not find variable to inline.": {
7640+
"category": "Message",
7641+
"code": 95185
7642+
},
7643+
"Variables with multiple declarations cannot be inlined.": {
7644+
"category": "Message",
7645+
"code": 95186
7646+
},
76357647

76367648
"No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
76377649
"category": "Error",

src/services/_namespaces/ts.refactor.ts

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ export * from "../refactors/convertExport";
55
export * from "../refactors/convertImport";
66
export * from "../refactors/extractType";
77
export * from "../refactors/helpers";
8+
export * from "../refactors/inlineVariable";
89
export * from "../refactors/moveToNewFile";
910
export * from "../refactors/moveToFile";
1011
import * as addOrRemoveBracesToArrowFunction from "./ts.refactor.addOrRemoveBracesToArrowFunction";

src/services/findAllReferences.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -849,8 +849,12 @@ export function getTextSpanOfEntry(entry: Entry) {
849849
getTextSpan(entry.node, entry.node.getSourceFile());
850850
}
851851

852-
/** A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment */
853-
function isWriteAccessForReference(node: Node): boolean {
852+
/**
853+
* A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment.
854+
*
855+
* @internal
856+
*/
857+
export function isWriteAccessForReference(node: Node): boolean {
854858
const decl = getDeclarationFromName(node);
855859
return !!decl && declarationIsWriteAccess(decl) || node.kind === SyntaxKind.DefaultKeyword || isWriteAccess(node);
856860
}
+236
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,236 @@
1+
import {
2+
cast,
3+
Debug,
4+
Diagnostics,
5+
emptyArray,
6+
Expression,
7+
factory,
8+
FindAllReferences,
9+
getExpressionPrecedence,
10+
getLocaleSpecificMessage,
11+
getSynthesizedDeepClone,
12+
getTouchingPropertyName,
13+
Identifier,
14+
InitializedVariableDeclaration,
15+
isCallLikeExpression,
16+
isExportAssignment,
17+
isExportModifier,
18+
isExportSpecifier,
19+
isExpression,
20+
isFunctionLike,
21+
isIdentifier,
22+
isInitializedVariable,
23+
isTypeQueryNode,
24+
isVariableDeclarationInVariableStatement,
25+
isVariableStatement,
26+
needsParentheses,
27+
Node,
28+
Program,
29+
refactor,
30+
some,
31+
SourceFile,
32+
SymbolFlags,
33+
textChanges,
34+
textRangeContainsPositionInclusive,
35+
TypeChecker,
36+
VariableDeclaration,
37+
} from "../_namespaces/ts";
38+
import { RefactorErrorInfo, registerRefactor } from "../_namespaces/ts.refactor";
39+
40+
const refactorName = "Inline variable";
41+
const refactorDescription = getLocaleSpecificMessage(Diagnostics.Inline_variable);
42+
43+
const inlineVariableAction = {
44+
name: refactorName,
45+
description: refactorDescription,
46+
kind: "refactor.inline.variable"
47+
};
48+
49+
interface InliningInfo {
50+
references: Node[];
51+
declaration: VariableDeclaration;
52+
replacement: Expression;
53+
}
54+
55+
registerRefactor(refactorName, {
56+
kinds: [inlineVariableAction.kind],
57+
58+
getAvailableActions(context) {
59+
const {
60+
file,
61+
program,
62+
preferences,
63+
startPosition,
64+
triggerReason
65+
} = context;
66+
67+
// tryWithReferenceToken is true below when triggerReason === "invoked", since we want to
68+
// always provide the refactor in the declaration site but only show it in references when
69+
// the refactor is explicitly invoked.
70+
const info = getInliningInfo(file, startPosition, triggerReason === "invoked", program);
71+
if (!info) {
72+
return emptyArray;
73+
}
74+
75+
if (!refactor.isRefactorErrorInfo(info)) {
76+
return [{
77+
name: refactorName,
78+
description: refactorDescription,
79+
actions: [inlineVariableAction]
80+
}];
81+
}
82+
83+
if (preferences.provideRefactorNotApplicableReason) {
84+
return [{
85+
name: refactorName,
86+
description: refactorDescription,
87+
actions: [{
88+
...inlineVariableAction,
89+
notApplicableReason: info.error
90+
}]
91+
}];
92+
}
93+
94+
return emptyArray;
95+
},
96+
97+
getEditsForAction(context, actionName) {
98+
Debug.assert(actionName === refactorName, "Unexpected refactor invoked");
99+
100+
const { file, program, startPosition } = context;
101+
102+
// tryWithReferenceToken is true below since here we're already performing the refactor.
103+
// The trigger kind was only relevant when checking the availability of the refactor.
104+
const info = getInliningInfo(file, startPosition, /*tryWithReferenceToken*/ true, program);
105+
if (!info || refactor.isRefactorErrorInfo(info)) {
106+
return undefined;
107+
}
108+
109+
const { references, declaration, replacement } = info;
110+
const edits = textChanges.ChangeTracker.with(context, tracker => {
111+
for (const node of references) {
112+
tracker.replaceNode(file, node, getReplacementExpression(node, replacement));
113+
}
114+
tracker.delete(file, declaration);
115+
});
116+
117+
return { edits };
118+
}
119+
});
120+
121+
function getInliningInfo(file: SourceFile, startPosition: number, tryWithReferenceToken: boolean, program: Program): InliningInfo | RefactorErrorInfo | undefined {
122+
const checker = program.getTypeChecker();
123+
const token = getTouchingPropertyName(file, startPosition);
124+
const parent = token.parent;
125+
126+
if (!isIdentifier(token)) {
127+
return undefined;
128+
}
129+
130+
// If triggered in a variable declaration, make sure it's not in a catch clause or for-loop
131+
// and that it has a value.
132+
if (isInitializedVariable(parent) && isVariableDeclarationInVariableStatement(parent)) {
133+
// Don't inline the variable if it has multiple declarations.
134+
if (checker.getMergedSymbol(parent.symbol).declarations?.length !== 1) {
135+
return { error: getLocaleSpecificMessage(Diagnostics.Variables_with_multiple_declarations_cannot_be_inlined) };
136+
}
137+
138+
// Do not inline if the variable is exported.
139+
if (isDeclarationExported(parent)) {
140+
return undefined;
141+
}
142+
143+
// Find all references to the variable in the current file.
144+
const references = getReferenceNodes(parent, checker, file);
145+
return references && { references, declaration: parent, replacement: parent.initializer };
146+
}
147+
148+
// Try finding the declaration and nodes to replace via the reference token.
149+
if (tryWithReferenceToken) {
150+
let definition = checker.resolveName(token.text, token, SymbolFlags.Value, /*excludeGlobals*/ false);
151+
definition = definition && checker.getMergedSymbol(definition);
152+
153+
// Don't inline the variable if it has multiple declarations.
154+
if (definition?.declarations?.length !== 1) {
155+
return { error: getLocaleSpecificMessage(Diagnostics.Variables_with_multiple_declarations_cannot_be_inlined) };
156+
}
157+
158+
// Make sure we're not inlining something like "let foo;" or "for (let i = 0; i < 5; i++) {}".
159+
const declaration = definition.declarations[0];
160+
if (!isInitializedVariable(declaration) || !isVariableDeclarationInVariableStatement(declaration) || !isIdentifier(declaration.name)) {
161+
return undefined;
162+
}
163+
164+
// Do not inline if the variable is exported.
165+
if (isDeclarationExported(declaration)) {
166+
return undefined;
167+
}
168+
169+
const references = getReferenceNodes(declaration, checker, file);
170+
return references && { references, declaration, replacement: declaration.initializer };
171+
}
172+
173+
return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_variable_to_inline) };
174+
}
175+
176+
function isDeclarationExported(declaration: InitializedVariableDeclaration): boolean {
177+
// We use this function after isVariableDeclarationInVariableStatement, which ensures
178+
// that the cast below succeeds.
179+
const variableStatement = cast(declaration.parent.parent, isVariableStatement);
180+
return some(variableStatement.modifiers, isExportModifier);
181+
}
182+
183+
function getReferenceNodes(declaration: InitializedVariableDeclaration, checker: TypeChecker, file: SourceFile): Identifier[] | undefined {
184+
const references: Identifier[] = [];
185+
const cannotInline = FindAllReferences.Core.eachSymbolReferenceInFile(declaration.name as Identifier, checker, file, ref => {
186+
// Only inline if all references are reads. Else we might end up with an invalid scenario like:
187+
// const y = x++ + 1 -> const y = 2++ + 1
188+
if (FindAllReferences.isWriteAccessForReference(ref)) {
189+
return true;
190+
}
191+
192+
// We cannot inline exported variables like "export { x as y }" or "export default foo".
193+
if (isExportSpecifier(ref.parent) || isExportAssignment(ref.parent)) {
194+
return true;
195+
}
196+
197+
// typeof needs an identifier, so we can't inline a value in there.
198+
if (isTypeQueryNode(ref.parent)) {
199+
return true;
200+
}
201+
202+
// Cannot inline recursive declarations (e.g. const foo = () => foo();)
203+
if (textRangeContainsPositionInclusive(declaration, ref.pos)) {
204+
return true;
205+
}
206+
207+
references.push(ref);
208+
});
209+
210+
return references.length === 0 || cannotInline ? undefined : references;
211+
}
212+
213+
function getReplacementExpression(reference: Node, replacement: Expression): Expression {
214+
// Make sure each reference site gets its own copy of the replacement node.
215+
replacement = getSynthesizedDeepClone(replacement);
216+
const { parent } = reference;
217+
218+
// Logic from binaryOperandNeedsParentheses: "If the operand has lower precedence,
219+
// then it needs to be parenthesized to preserve the intent of the expression.
220+
// If the operand has higher precedence, then it does not need to be parenthesized."
221+
//
222+
// Note that binaryOperandNeedsParentheses has further logic when the precedences
223+
// are equal, but for the purposes of this refactor we keep things simple and
224+
// instead just check for special cases with needsParentheses.
225+
if (isExpression(parent) && (getExpressionPrecedence(replacement) < getExpressionPrecedence(parent) || needsParentheses(parent))) {
226+
return factory.createParenthesizedExpression(replacement);
227+
}
228+
229+
// Functions also need to be parenthesized.
230+
// E.g.: const f = () => {}; f(); -> (() => {})();
231+
if (isFunctionLike(replacement) && isCallLikeExpression(parent)) {
232+
return factory.createParenthesizedExpression(replacement);
233+
}
234+
235+
return replacement;
236+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////export const /*a1*/x/*b1*/ = 1;
4+
////const /*a2*/y/*b2*/ = 2;
5+
////const /*a3*/z/*b3*/ = 3;
6+
////const u = x + 1;
7+
////const v = 2 + y;
8+
////export { y };
9+
////export { z as w };
10+
11+
goTo.select("a1", "b1");
12+
verify.not.refactorAvailable("Inline variable");
13+
14+
goTo.select("a2", "b2");
15+
verify.not.refactorAvailable("Inline variable");
16+
17+
goTo.select("a3", "b3");
18+
verify.not.refactorAvailable("Inline variable");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////function inc(x: number) { return x + 1; }
4+
////const /*a*/y/*b*/ = inc(1);
5+
////const z = y + 1;
6+
////const w = inc(y);
7+
8+
goTo.select("a", "b");
9+
verify.refactorAvailable("Inline variable");
10+
edit.applyRefactor({
11+
refactorName: "Inline variable",
12+
actionName: "Inline variable",
13+
actionDescription: "Inline variable",
14+
newContent: `function inc(x: number) { return x + 1; }
15+
const z = inc(1) + 1;
16+
const w = inc(inc(1));`
17+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
//@module: commonjs
4+
//@jsx: preserve
5+
6+
// @Filename: file.tsx
7+
////function Button() {
8+
//// const /*a*/onClick/*b*/ = () => {
9+
//// console.log("clicked");
10+
//// };
11+
////
12+
//// return (
13+
//// <button onClick={onClick}>
14+
//// Click me!
15+
//// </button>
16+
//// );
17+
////}
18+
19+
goTo.select("a", "b");
20+
verify.refactorAvailable("Inline variable");
21+
edit.applyRefactor({
22+
refactorName: "Inline variable",
23+
actionName: "Inline variable",
24+
actionDescription: "Inline variable",
25+
newContent: `function Button() {
26+
27+
return (
28+
<button onClick={() => {
29+
console.log("clicked");
30+
}}>
31+
Click me!
32+
</button>
33+
);
34+
}`
35+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////const /*a1*/foo/*b1*/ = "foo";
4+
////type /*a2*/foo/*b2*/ = string;
5+
////type bar = foo;
6+
7+
goTo.select("a1", "b1");
8+
verify.not.refactorAvailable("Inline variable");
9+
10+
goTo.select("a2", "b2");
11+
verify.not.refactorAvailable("Inline variable");
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////let /*a*/x/*b*/ = 1;
4+
////function foo() {
5+
//// console.log(x);
6+
////}
7+
////const y = x + 2;
8+
9+
goTo.select("a", "b");
10+
verify.refactorAvailable("Inline variable");
11+
edit.applyRefactor({
12+
refactorName: "Inline variable",
13+
actionName: "Inline variable",
14+
actionDescription: "Inline variable",
15+
newContent: `function foo() {
16+
console.log(1);
17+
}
18+
const y = 1 + 2;`
19+
});

0 commit comments

Comments
 (0)