Skip to content

add refactoring: inline local variable #28522

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e8d6e37
Add inline local all simple
D0nGiovanni Nov 7, 2018
bf2e7c8
Fix literals not being able to be replaced
D0nGiovanni Nov 8, 2018
baa5f43
Implement inline here
D0nGiovanni Nov 8, 2018
cc2348d
Clean style
D0nGiovanni Nov 8, 2018
dd9eb25
Clean style
D0nGiovanni Nov 8, 2018
ea35976
Add refactoring availability tests
D0nGiovanni Nov 8, 2018
14e67bb
Make inline local not available for edge cases
D0nGiovanni Nov 8, 2018
0d69ab1
Fix lint issues
D0nGiovanni Nov 8, 2018
6366971
Fix tests
D0nGiovanni Nov 9, 2018
b35a149
Remove default from prohibited modifiers
D0nGiovanni Nov 9, 2018
4907108
Split up test cases
D0nGiovanni Nov 9, 2018
76d7cce
Extract is assigned check
D0nGiovanni Nov 9, 2018
8170182
Add parentheses dynamically
D0nGiovanni Nov 9, 2018
063b864
Fix style
D0nGiovanni Nov 9, 2018
60e356a
Add precedence tests
D0nGiovanni Nov 9, 2018
6d7744e
Fix unnecessary parentheses in CallExpression
D0nGiovanni Nov 13, 2018
eab82d5
Implement review feedback
D0nGiovanni Nov 13, 2018
bf3a198
Update to new Refactor interface
D0nGiovanni Nov 14, 2018
fcb3747
Correct simple testcases
D0nGiovanni Nov 14, 2018
68f8e97
Add function call argument testcase
D0nGiovanni Nov 14, 2018
6f3b5d3
Remove dead code
D0nGiovanni Nov 14, 2018
e7f383a
Fix parenthesization
D0nGiovanni Nov 15, 2018
fd44e13
Fix lint errors
D0nGiovanni Nov 16, 2018
8f63309
Fix test breaking bug
D0nGiovanni Nov 16, 2018
a43fcdd
Merge remote-tracking branch 'upstream/master' into m-inline-local
D0nGiovanni Nov 16, 2018
383e559
Extract findDescendants function
D0nGiovanni Nov 16, 2018
1554d5d
Make getReferencesInScope more generic
D0nGiovanni Nov 16, 2018
4b47e3b
Fix parenthesization
D0nGiovanni Nov 29, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -4799,5 +4799,17 @@
"Add names to all parameters without names": {
"category": "Message",
"code": 95073
},
"Inline local": {
"category": "Message",
"code": 95080
},
"Inline here": {
"category": "Message",
"code": 95081
},
"Inline all": {
"category": "Message",
"code": 95082
}
}
182 changes: 182 additions & 0 deletions src/services/refactors/inlineLocal.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
/* @internal */
namespace ts.refactor.inlineLocal {
const refactorName = "Inline local";
const refactorDescription = getLocaleSpecificMessage(Diagnostics.Inline_local);

const inlineHereActionName = "Inline here";
const inlineAllActionName = "Inline all";

const inlineHereActionDescription = getLocaleSpecificMessage(Diagnostics.Inline_here);
const inlineAllActionDescription = getLocaleSpecificMessage(Diagnostics.Inline_all);


registerRefactor(refactorName, { getEditsForAction, getAvailableActions });

interface Info {
readonly declaration: VariableDeclaration;
readonly usages: ReadonlyArray<Identifier>;
readonly selectedUsage: Identifier | undefined;
}

function getAvailableActions(context: RefactorContext): ReadonlyArray<ApplicableRefactorInfo> {
const { file, program, startPosition } = context;
const info = getLocalInfo(file, program, startPosition);
if (!info) return emptyArray;
const { selectedUsage } = info;
const refactorInfo = {
name: refactorName,
description: refactorDescription,
actions: [{
name: inlineAllActionName,
description: inlineAllActionDescription
}]
};
if (selectedUsage) {
refactorInfo.actions.push({
name: inlineHereActionName,
description: inlineHereActionDescription
});
}
return [refactorInfo];
}

function getLocalInfo(file: SourceFile, program: Program, startPosition: number): Info | undefined {
const token = getTokenAtPosition(file, startPosition);
const maybeDeclaration = token.parent;
const checker = program.getTypeChecker();
if (isLocalVariable(maybeDeclaration)) {
return createInfo(checker, maybeDeclaration);
}
if (isIdentifier(token)) {
const symbol = checker.getSymbolAtLocation(token);
if (!symbol) return undefined;
const declaration = symbol.valueDeclaration;
if (!isLocalVariable(declaration)) return undefined;
return createInfo(checker, declaration, token);
}
return undefined;
}

function isLocalVariable(parent: Node): parent is VariableDeclaration {
return isVariableDeclaration(parent) && isVariableDeclarationInVariableStatement(parent);
}

function createInfo(checker: TypeChecker, declaration: VariableDeclaration, token?: Identifier): Info | undefined {
const name = declaration.name;
const usages = getReferencesInScope(getEnclosingBlockScopeContainer(name), name, checker, /* withDeclaration */ false);
return canInline(declaration, usages) ? {
declaration,
usages,
selectedUsage: token ? token : undefined
} : undefined;
}

function canInline(declaration: VariableDeclaration, usages: ReadonlyArray<Identifier>): boolean {
let hasErrors = false;
if (!declaration.initializer) hasErrors = true;
if (containsProhibitedModifiers(declaration.parent.parent.modifiers)) hasErrors = true;
forEach(usages, usage => {
if (isAssigned(usage)) hasErrors = true;
});
return !hasErrors;
}

function isAssigned(usage: Identifier): boolean {
type AssignExpr = AssignmentExpression<AssignmentOperatorToken>;
const assignment: AssignExpr = findAncestor(
usage,
ancestor => isAssignmentExpression(ancestor)) as AssignExpr;
return assignment && assignment.left === usage;
}

function containsProhibitedModifiers(modifiers?: NodeArray<Modifier>): boolean {
return !!modifiers && !!modifiers.find(mod => mod.kind === SyntaxKind.ExportKeyword);
}

function getEditsForAction(context: RefactorContext, actionName: string): RefactorEditInfo | undefined {
const { file, program, startPosition } = context;
const info = getLocalInfo(file, program, startPosition);
if (!info) return undefined;
const { declaration, usages, selectedUsage } = info;
switch (actionName) {
case inlineAllActionName:
return { edits: getInlineAllEdits(context, declaration, usages) };
case inlineHereActionName:
return { edits: getInlineHereEdits(context, declaration, usages, selectedUsage!) };
default:
return Debug.fail("invalid action");
}
}

function getInlineAllEdits(
context: RefactorContext,
declaration: VariableDeclaration,
usages: ReadonlyArray<Identifier>): FileTextChanges[] {
const { file } = context;
return textChanges.ChangeTracker.with(context, t => {
forEach(usages, oldNode => {
const { initializer } = declaration;
makeIdUnique(initializer!); // since there is no node-copying function
const expression = parenthesizeIfNecessary(oldNode, initializer!);
t.replaceNode(file, oldNode, expression);
});
t.delete(file, declaration);
});
}

function getInlineHereEdits(context: RefactorContext,
declaration: VariableDeclaration,
usages: ReadonlyArray<Identifier>,
selectedUsage: Identifier): FileTextChanges[] {
const { file } = context;
return textChanges.ChangeTracker.with(context, t => {
const { initializer } = declaration;
const expression = parenthesizeIfNecessary(selectedUsage, initializer!);
t.replaceNode(file, selectedUsage, expression);
if (usages.length === 1) t.delete(file, declaration);
});
}

function makeIdUnique(node: Node): void {
node.id = undefined;
getNodeId(node);
}

function parenthesizeIfNecessary(target: Node, expression: Expression): Expression {
const parent = target.parent;
if (isBinaryExpression(parent)) {
return parenthesizeBinaryOperand(
parent.operatorToken.kind,
expression,
target === parent.left,
parent.left);
}
if (isExpression(parent)) {
const parentPrecedence = getExpressionPrecedence(parent);
const expressionPrecedence = getExpressionPrecedence(expression);
if (parentPrecedence > expressionPrecedence) return createParen(expression);
else return expression;
}
return expression;
}

function getReferencesInScope(scope: Node, target: Node, checker: TypeChecker, withDeclaration: boolean): ReadonlyArray<Identifier> {
const symbol = checker.getSymbolAtLocation(target);
return findDescendants(scope, n =>
checker.getSymbolAtLocation(n) === symbol &&
(withDeclaration || !isDeclaration(n.parent))) as Identifier[];
}

function findDescendants(node: Node, predicate: (n: Node) => boolean) {
const nodes: Node[] = [];
visitDescendants(node);

function visitDescendants(node: Node) {
forEachChild(node, n => {
if (predicate(n)) nodes.push(n);
visitDescendants(n);
});
}
return nodes;
}
}
1 change: 1 addition & 0 deletions src/services/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"refactors/convertExport.ts",
"refactors/convertImport.ts",
"refactors/extractSymbol.ts",
"refactors/inlineLocal.ts",
"refactors/generateGetAccessorAndSetAccessor.ts",
"refactors/moveToNewFile.ts",
"refactors/addOrRemoveBracesToArrowFunction.ts",
Expand Down
12 changes: 12 additions & 0 deletions tests/cases/fourslash/refactorInlineLocal_Availability_Export.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/// <reference path='fourslash.ts' />

//// export const /*z*/a/*y*/ = 42-3;
//// const b = 2 * /*x*/a/*w*/;

goTo.select("z", "y");
verify.not.refactorAvailable("Inline local", "Inline all");
verify.not.refactorAvailable("Inline local", "Inline here");

goTo.select("x", "w");
verify.not.refactorAvailable("Inline local", "Inline all");
verify.not.refactorAvailable("Inline local", "Inline here");
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/// <reference path='fourslash.ts' />

//// let /*z*/a/*y*/ = 42-3;
//// a = 23;
//// const b = 2 * /*x*/a/*w*/;

goTo.select("z", "y");
verify.not.refactorAvailable("Inline local", "Inline all");
verify.not.refactorAvailable("Inline local", "Inline here");

goTo.select("x", "w");
verify.not.refactorAvailable("Inline local", "Inline all");
verify.not.refactorAvailable("Inline local", "Inline here");
12 changes: 12 additions & 0 deletions tests/cases/fourslash/refactorInlineLocal_Availability_Simple.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42-3;
//// const b = 2 * /*x*/a/*w*/;

goTo.select("z", "y");
verify.refactorAvailable("Inline local", "Inline all");
verify.not.refactorAvailable("Inline local", "Inline here");

goTo.select("x", "w");
verify.refactorAvailable("Inline local", "Inline all");
verify.refactorAvailable("Inline local", "Inline here");
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/// <reference path='fourslash.ts' />

//// let /*z*/a/*y*/;
//// const b = 2 * /*x*/a/*w*/;


goTo.select("z", "y");
verify.not.refactorAvailable("Inline local", "Inline all");
verify.not.refactorAvailable("Inline local", "Inline here");

goTo.select("x", "w");
verify.not.refactorAvailable("Inline local", "Inline all");
verify.not.refactorAvailable("Inline local", "Inline here");
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42;
//// function b (arg: number) { return arg; }
//// b(a);

goTo.select("z", "y");
edit.applyRefactor({
refactorName: "Inline local",
actionName: "Inline all",
actionDescription: "Inline all",
newContent: `function b (arg: number) { return arg; }
b(42);`
});
14 changes: 14 additions & 0 deletions tests/cases/fourslash/refactorInlineLocal_MultiUse_Declaration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42 - 3;
//// const b = 2 * /*x*/a/*w*/;
//// const c = 2 + a;

goTo.select("z", "y");
edit.applyRefactor({
refactorName: "Inline local",
actionName: "Inline all",
actionDescription: "Inline all",
newContent: `const b = 2 * (42 - 3);
const c = 2 + (42 - 3);`
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42 - 3;
//// const b = 2 * /*x*/a/*w*/;
//// const c = 2 + a;

goTo.select("x", "w");
edit.applyRefactor({
refactorName: "Inline local",
actionName: "Inline all",
actionDescription: "Inline all",
newContent: `const b = 2 * (42 - 3);
const c = 2 + (42 - 3);`
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42 - 3;
//// const b = 2 * /*x*/a/*w*/;
//// const c = 2 + a;

goTo.select("x", "w");
edit.applyRefactor({
refactorName: "Inline local",
actionName: "Inline here",
actionDescription: "Inline here",
newContent: `const a = 42 - 3;
const b = 2 * (42 - 3);
const c = 2 + a;`
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42 + 3;
//// const b = /*x*/a/*w*/ + 2;

goTo.select("z", "y");
edit.applyRefactor({
refactorName: "Inline local",
actionName: "Inline all",
actionDescription: "Inline all",
newContent: `const b = 42 + 3 + 2;`
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42 + 3;
//// const b = /*x*/a/*w*/ * 2;

goTo.select("z", "y");
edit.applyRefactor({
refactorName: "Inline local",
actionName: "Inline all",
actionDescription: "Inline all",
newContent: `const b = (42 + 3) * 2;`
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42 + 3;
//// const b = /*x*/a/*w*/ - 2;

goTo.select("z", "y");
edit.applyRefactor({
refactorName: "Inline local",
actionName: "Inline all",
actionDescription: "Inline all",
newContent: `const b = 42 + 3 - 2;`
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42 == 3;
//// const b = /*x*/a/*w*/ != false;

goTo.select("z", "y");
edit.applyRefactor({
refactorName: "Inline local",
actionName: "Inline all",
actionDescription: "Inline all",
newContent: `const b = 42 == 3 != false;`
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/// <reference path='fourslash.ts' />

//// const /*z*/a/*y*/ = 42 * 3;
//// const b = /*x*/a/*w*/ * 2;

goTo.select("z", "y");
edit.applyRefactor({
refactorName: "Inline local",
actionName: "Inline all",
actionDescription: "Inline all",
newContent: `const b = 42 * 3 * 2;`
});
Loading