diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index 102be920acf0e..65ccc4cb18fdc 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -7632,6 +7632,18 @@ "category": "Message", "code": 95183 }, + "Inline variable": { + "category": "Message", + "code": 95184 + }, + "Could not find variable to inline.": { + "category": "Message", + "code": 95185 + }, + "Variables with multiple declarations cannot be inlined.": { + "category": "Message", + "code": 95186 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/services/_namespaces/ts.refactor.ts b/src/services/_namespaces/ts.refactor.ts index 2edf81a5b8caa..e397f484766ba 100644 --- a/src/services/_namespaces/ts.refactor.ts +++ b/src/services/_namespaces/ts.refactor.ts @@ -5,6 +5,7 @@ export * from "../refactors/convertExport"; export * from "../refactors/convertImport"; export * from "../refactors/extractType"; export * from "../refactors/helpers"; +export * from "../refactors/inlineVariable"; export * from "../refactors/moveToNewFile"; export * from "../refactors/moveToFile"; import * as addOrRemoveBracesToArrowFunction from "./ts.refactor.addOrRemoveBracesToArrowFunction"; diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index e19d22cd509cc..1aa55a6cbc2cf 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -849,8 +849,12 @@ export function getTextSpanOfEntry(entry: Entry) { getTextSpan(entry.node, entry.node.getSourceFile()); } -/** A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment */ -function isWriteAccessForReference(node: Node): boolean { +/** + * A node is considered a writeAccess iff it is a name of a declaration or a target of an assignment. + * + * @internal + */ +export function isWriteAccessForReference(node: Node): boolean { const decl = getDeclarationFromName(node); return !!decl && declarationIsWriteAccess(decl) || node.kind === SyntaxKind.DefaultKeyword || isWriteAccess(node); } diff --git a/src/services/refactors/inlineVariable.ts b/src/services/refactors/inlineVariable.ts new file mode 100644 index 0000000000000..2b0b43bf426a4 --- /dev/null +++ b/src/services/refactors/inlineVariable.ts @@ -0,0 +1,236 @@ +import { + cast, + Debug, + Diagnostics, + emptyArray, + Expression, + factory, + FindAllReferences, + getExpressionPrecedence, + getLocaleSpecificMessage, + getSynthesizedDeepClone, + getTouchingPropertyName, + Identifier, + InitializedVariableDeclaration, + isCallLikeExpression, + isExportAssignment, + isExportModifier, + isExportSpecifier, + isExpression, + isFunctionLike, + isIdentifier, + isInitializedVariable, + isTypeQueryNode, + isVariableDeclarationInVariableStatement, + isVariableStatement, + needsParentheses, + Node, + Program, + refactor, + some, + SourceFile, + SymbolFlags, + textChanges, + textRangeContainsPositionInclusive, + TypeChecker, + VariableDeclaration, +} from "../_namespaces/ts"; +import { RefactorErrorInfo, registerRefactor } from "../_namespaces/ts.refactor"; + +const refactorName = "Inline variable"; +const refactorDescription = getLocaleSpecificMessage(Diagnostics.Inline_variable); + +const inlineVariableAction = { + name: refactorName, + description: refactorDescription, + kind: "refactor.inline.variable" +}; + +interface InliningInfo { + references: Node[]; + declaration: VariableDeclaration; + replacement: Expression; +} + +registerRefactor(refactorName, { + kinds: [inlineVariableAction.kind], + + getAvailableActions(context) { + const { + file, + program, + preferences, + startPosition, + triggerReason + } = context; + + // tryWithReferenceToken is true below when triggerReason === "invoked", since we want to + // always provide the refactor in the declaration site but only show it in references when + // the refactor is explicitly invoked. + const info = getInliningInfo(file, startPosition, triggerReason === "invoked", program); + if (!info) { + return emptyArray; + } + + if (!refactor.isRefactorErrorInfo(info)) { + return [{ + name: refactorName, + description: refactorDescription, + actions: [inlineVariableAction] + }]; + } + + if (preferences.provideRefactorNotApplicableReason) { + return [{ + name: refactorName, + description: refactorDescription, + actions: [{ + ...inlineVariableAction, + notApplicableReason: info.error + }] + }]; + } + + return emptyArray; + }, + + 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. + // The trigger kind was only relevant when checking the availability of the refactor. + const info = getInliningInfo(file, startPosition, /*tryWithReferenceToken*/ true, program); + if (!info || refactor.isRefactorErrorInfo(info)) { + return undefined; + } + + const { references, declaration, replacement } = info; + const edits = textChanges.ChangeTracker.with(context, tracker => { + for (const node of references) { + tracker.replaceNode(file, node, getReplacementExpression(node, replacement)); + } + tracker.delete(file, declaration); + }); + + return { edits }; + } +}); + +function getInliningInfo(file: SourceFile, startPosition: number, tryWithReferenceToken: boolean, program: Program): InliningInfo | RefactorErrorInfo | undefined { + const checker = program.getTypeChecker(); + const token = getTouchingPropertyName(file, startPosition); + const parent = token.parent; + + if (!isIdentifier(token)) { + return undefined; + } + + // If triggered in a variable declaration, make sure it's not in a catch clause or for-loop + // and that it has a value. + if (isInitializedVariable(parent) && isVariableDeclarationInVariableStatement(parent)) { + // Don't inline the variable if it has multiple declarations. + if (checker.getMergedSymbol(parent.symbol).declarations?.length !== 1) { + return { error: getLocaleSpecificMessage(Diagnostics.Variables_with_multiple_declarations_cannot_be_inlined) }; + } + + // Do not inline if the variable is exported. + if (isDeclarationExported(parent)) { + return undefined; + } + + // Find all references to the variable in the current file. + const references = getReferenceNodes(parent, checker, file); + return references && { references, declaration: parent, replacement: parent.initializer }; + } + + // Try finding the declaration and nodes to replace via the reference token. + if (tryWithReferenceToken) { + let definition = checker.resolveName(token.text, token, SymbolFlags.Value, /*excludeGlobals*/ false); + definition = definition && checker.getMergedSymbol(definition); + + // Don't inline the variable if it has multiple declarations. + if (definition?.declarations?.length !== 1) { + return { error: getLocaleSpecificMessage(Diagnostics.Variables_with_multiple_declarations_cannot_be_inlined) }; + } + + // Make sure we're not inlining something like "let foo;" or "for (let i = 0; i < 5; i++) {}". + const declaration = definition.declarations[0]; + if (!isInitializedVariable(declaration) || !isVariableDeclarationInVariableStatement(declaration) || !isIdentifier(declaration.name)) { + return undefined; + } + + // Do not inline if the variable is exported. + if (isDeclarationExported(declaration)) { + return undefined; + } + + const references = getReferenceNodes(declaration, checker, file); + return references && { references, declaration, replacement: declaration.initializer }; + } + + return { error: getLocaleSpecificMessage(Diagnostics.Could_not_find_variable_to_inline) }; +} + +function isDeclarationExported(declaration: InitializedVariableDeclaration): boolean { + // We use this function after isVariableDeclarationInVariableStatement, which ensures + // that the cast below succeeds. + const variableStatement = cast(declaration.parent.parent, isVariableStatement); + return some(variableStatement.modifiers, isExportModifier); +} + +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 + if (FindAllReferences.isWriteAccessForReference(ref)) { + return true; + } + + // We cannot inline exported variables like "export { x as y }" or "export default foo". + if (isExportSpecifier(ref.parent) || isExportAssignment(ref.parent)) { + return true; + } + + // typeof needs an identifier, so we can't inline a value in there. + if (isTypeQueryNode(ref.parent)) { + return true; + } + + // Cannot inline recursive declarations (e.g. const foo = () => foo();) + if (textRangeContainsPositionInclusive(declaration, ref.pos)) { + return true; + } + + references.push(ref); + }); + + return references.length === 0 || cannotInline ? undefined : references; +} + +function getReplacementExpression(reference: Node, replacement: Expression): Expression { + // Make sure each reference site gets its own copy of the replacement node. + replacement = getSynthesizedDeepClone(replacement); + const { parent } = reference; + + // Logic from binaryOperandNeedsParentheses: "If the operand has lower precedence, + // then it needs to be parenthesized to preserve the intent of the expression. + // If the operand has higher precedence, then it does not need to be parenthesized." + // + // Note that binaryOperandNeedsParentheses has further logic when the precedences + // are equal, but for the purposes of this refactor we keep things simple and + // instead just check for special cases with needsParentheses. + if (isExpression(parent) && (getExpressionPrecedence(replacement) < getExpressionPrecedence(parent) || needsParentheses(parent))) { + return factory.createParenthesizedExpression(replacement); + } + + // Functions also need to be parenthesized. + // E.g.: const f = () => {}; f(); -> (() => {})(); + if (isFunctionLike(replacement) && isCallLikeExpression(parent)) { + return factory.createParenthesizedExpression(replacement); + } + + return replacement; +} diff --git a/tests/cases/fourslash/inlineVariableExportedVariable.ts b/tests/cases/fourslash/inlineVariableExportedVariable.ts new file mode 100644 index 0000000000000..26ef4f7034b85 --- /dev/null +++ b/tests/cases/fourslash/inlineVariableExportedVariable.ts @@ -0,0 +1,18 @@ +/// + +////export const /*a1*/x/*b1*/ = 1; +////const /*a2*/y/*b2*/ = 2; +////const /*a3*/z/*b3*/ = 3; +////const u = x + 1; +////const v = 2 + y; +////export { y }; +////export { z as w }; + +goTo.select("a1", "b1"); +verify.not.refactorAvailable("Inline variable"); + +goTo.select("a2", "b2"); +verify.not.refactorAvailable("Inline variable"); + +goTo.select("a3", "b3"); +verify.not.refactorAvailable("Inline variable"); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableFunctionResult.ts b/tests/cases/fourslash/inlineVariableFunctionResult.ts new file mode 100644 index 0000000000000..e72dc8376fdae --- /dev/null +++ b/tests/cases/fourslash/inlineVariableFunctionResult.ts @@ -0,0 +1,17 @@ +/// + +////function inc(x: number) { return x + 1; } +////const /*a*/y/*b*/ = inc(1); +////const z = y + 1; +////const w = inc(y); + +goTo.select("a", "b"); +verify.refactorAvailable("Inline variable"); +edit.applyRefactor({ + refactorName: "Inline variable", + actionName: "Inline variable", + actionDescription: "Inline variable", + newContent: `function inc(x: number) { return x + 1; } +const z = inc(1) + 1; +const w = inc(inc(1));` +}); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableJsxCallback.ts b/tests/cases/fourslash/inlineVariableJsxCallback.ts new file mode 100644 index 0000000000000..a386f17ee6761 --- /dev/null +++ b/tests/cases/fourslash/inlineVariableJsxCallback.ts @@ -0,0 +1,35 @@ +/// + +//@module: commonjs +//@jsx: preserve + +// @Filename: file.tsx +////function Button() { +//// const /*a*/onClick/*b*/ = () => { +//// console.log("clicked"); +//// }; +//// +//// return ( +//// +//// ); +////} + +goTo.select("a", "b"); +verify.refactorAvailable("Inline variable"); +edit.applyRefactor({ + refactorName: "Inline variable", + actionName: "Inline variable", + actionDescription: "Inline variable", + newContent: `function Button() { + + return ( + + ); +}` +}); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableMultipleDeclarations.ts b/tests/cases/fourslash/inlineVariableMultipleDeclarations.ts new file mode 100644 index 0000000000000..704c076632234 --- /dev/null +++ b/tests/cases/fourslash/inlineVariableMultipleDeclarations.ts @@ -0,0 +1,11 @@ +/// + +////const /*a1*/foo/*b1*/ = "foo"; +////type /*a2*/foo/*b2*/ = string; +////type bar = foo; + +goTo.select("a1", "b1"); +verify.not.refactorAvailable("Inline variable"); + +goTo.select("a2", "b2"); +verify.not.refactorAvailable("Inline variable"); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableMultipleScopes.ts b/tests/cases/fourslash/inlineVariableMultipleScopes.ts new file mode 100644 index 0000000000000..56845b330098f --- /dev/null +++ b/tests/cases/fourslash/inlineVariableMultipleScopes.ts @@ -0,0 +1,19 @@ +/// + +////let /*a*/x/*b*/ = 1; +////function foo() { +//// console.log(x); +////} +////const y = x + 2; + +goTo.select("a", "b"); +verify.refactorAvailable("Inline variable"); +edit.applyRefactor({ + refactorName: "Inline variable", + actionName: "Inline variable", + actionDescription: "Inline variable", + newContent: `function foo() { + console.log(1); +} +const y = 1 + 2;` +}); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableNeedsParens.ts b/tests/cases/fourslash/inlineVariableNeedsParens.ts new file mode 100644 index 0000000000000..6d4edfa1288fe --- /dev/null +++ b/tests/cases/fourslash/inlineVariableNeedsParens.ts @@ -0,0 +1,27 @@ +/// + +////const /*a1*/x/*b1*/ = 1 + 2; +////const y = x * 3; +////const /*a2*/f/*b2*/ = () => { }; +////f(); + +goTo.select("a1", "b1"); +verify.refactorAvailable("Inline variable"); +edit.applyRefactor({ + refactorName: "Inline variable", + actionName: "Inline variable", + actionDescription: "Inline variable", + newContent: `const y = (1 + 2) * 3; +const f = () => { }; +f();` +}); + +goTo.select("a2", "b2"); +verify.refactorAvailable("Inline variable"); +edit.applyRefactor({ + refactorName: "Inline variable", + actionName: "Inline variable", + actionDescription: "Inline variable", + newContent: `const y = (1 + 2) * 3; +(() => { })();` +}); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableNoReplacementValue.ts b/tests/cases/fourslash/inlineVariableNoReplacementValue.ts new file mode 100644 index 0000000000000..7c7f2b9ce9a06 --- /dev/null +++ b/tests/cases/fourslash/inlineVariableNoReplacementValue.ts @@ -0,0 +1,7 @@ +/// + +////let /*a*/x/*b*/; +////const y = x + 1; + +goTo.select("a", "b"); +verify.not.refactorAvailable("Inline variable"); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableNotAVariableStatement.ts b/tests/cases/fourslash/inlineVariableNotAVariableStatement.ts new file mode 100644 index 0000000000000..b2efbaeec74bb --- /dev/null +++ b/tests/cases/fourslash/inlineVariableNotAVariableStatement.ts @@ -0,0 +1,8 @@ +/// + +////for (let /*a*/i/*b*/ = 0; i < 5; i++) { +//// console.log(i) +////} + +goTo.select("a", "b"); +verify.not.refactorAvailable("Inline variable"); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableNullCoalescing.ts b/tests/cases/fourslash/inlineVariableNullCoalescing.ts new file mode 100644 index 0000000000000..7efc9ace0a6f1 --- /dev/null +++ b/tests/cases/fourslash/inlineVariableNullCoalescing.ts @@ -0,0 +1,15 @@ +/// + +////function foo(): number | undefined { return Math.random() > 0.5 ? 1 : undefined; } +////const /*a*/x/*b*/ = foo(); +////const y = x?.toString(); + +goTo.select("a", "b"); +verify.refactorAvailable("Inline variable"); +edit.applyRefactor({ + refactorName: "Inline variable", + actionName: "Inline variable", + actionDescription: "Inline variable", + newContent: `function foo(): number | undefined { return Math.random() > 0.5 ? 1 : undefined; } +const y = (foo())?.toString();` +}); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableOnlyDeclaration.ts b/tests/cases/fourslash/inlineVariableOnlyDeclaration.ts new file mode 100644 index 0000000000000..cc31af2778d6e --- /dev/null +++ b/tests/cases/fourslash/inlineVariableOnlyDeclaration.ts @@ -0,0 +1,7 @@ +/// + +////const /*a*/x/*b*/ = 0; +////const y = 1 + 2; + +goTo.select("a", "b"); +verify.not.refactorAvailable("Inline variable"); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableRecursiveInitializer.ts b/tests/cases/fourslash/inlineVariableRecursiveInitializer.ts new file mode 100644 index 0000000000000..8ce59e9d77407 --- /dev/null +++ b/tests/cases/fourslash/inlineVariableRecursiveInitializer.ts @@ -0,0 +1,6 @@ +/// + +////const /*a*/foo/*b*/ = () => foo(); + +goTo.select("a", "b"); +verify.not.refactorAvailable("Inline variable"); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableRefactorAvailability.ts b/tests/cases/fourslash/inlineVariableRefactorAvailability.ts new file mode 100644 index 0000000000000..67348ba42fc54 --- /dev/null +++ b/tests/cases/fourslash/inlineVariableRefactorAvailability.ts @@ -0,0 +1,16 @@ +/// + +////const/*a*/ /*b*/x/*c*/ = /*d*/1; +////const y = x + 2; + +goTo.marker("a"); +verify.not.refactorAvailable("Inline variable"); + +goTo.marker("b"); +verify.refactorAvailable("Inline variable"); + +goTo.marker("c"); +verify.refactorAvailable("Inline variable"); + +goTo.marker("d"); +verify.not.refactorAvailable("Inline variable"); diff --git a/tests/cases/fourslash/inlineVariableSingleReference.ts b/tests/cases/fourslash/inlineVariableSingleReference.ts new file mode 100644 index 0000000000000..79e05c6d139bf --- /dev/null +++ b/tests/cases/fourslash/inlineVariableSingleReference.ts @@ -0,0 +1,13 @@ +/// + +////const /*a*/x/*b*/ = 0; +////const y = x + 1; + +goTo.select("a", "b"); +verify.refactorAvailable("Inline variable"); +edit.applyRefactor({ + refactorName: "Inline variable", + actionName: "Inline variable", + actionDescription: "Inline variable", + newContent: "const y = 0 + 1;" +}); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableTriggerInReference.ts b/tests/cases/fourslash/inlineVariableTriggerInReference.ts new file mode 100644 index 0000000000000..b192c7ab3cce8 --- /dev/null +++ b/tests/cases/fourslash/inlineVariableTriggerInReference.ts @@ -0,0 +1,15 @@ +/// + +////const x = 0; +////const y = /*a*/x/*b*/ + 1; + +goTo.select("a", "b"); +verify.not.refactorAvailableForTriggerReason("implicit", "Inline variable"); +verify.refactorAvailableForTriggerReason("invoked", "Inline variable"); +edit.applyRefactor({ + refactorName: "Inline variable", + actionName: "Inline variable", + actionDescription: "Inline variable", + newContent: "const y = 0 + 1;", + triggerReason: "invoked" +}); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableTypeof.ts b/tests/cases/fourslash/inlineVariableTypeof.ts new file mode 100644 index 0000000000000..9af8f951a33e9 --- /dev/null +++ b/tests/cases/fourslash/inlineVariableTypeof.ts @@ -0,0 +1,7 @@ +/// + +////const /*a*/Foo/*b*/ = class Foo {} +////type FooConstructor = typeof Foo; + +goTo.select("a", "b"); +verify.not.refactorAvailable("Inline variable"); \ No newline at end of file diff --git a/tests/cases/fourslash/inlineVariableWriteReference.ts b/tests/cases/fourslash/inlineVariableWriteReference.ts new file mode 100644 index 0000000000000..9c0228f6ce91d --- /dev/null +++ b/tests/cases/fourslash/inlineVariableWriteReference.ts @@ -0,0 +1,7 @@ +/// + +////const /*a*/x/*b*/ = 0; +////const y = x++ + 1; + +goTo.select("a", "b"); +verify.not.refactorAvailable("Inline variable"); \ No newline at end of file