diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 950bb078e06..859f7e1d3cc 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -1006,11 +1006,14 @@ namespace ts.refactor.extractSymbol { const localNameText = getUniqueName(isClassLike(scope) ? "newProperty" : "newLocal", file); const isJS = isInJSFile(scope); - const variableType = isJS || !checker.isContextSensitive(node) + let variableType = isJS || !checker.isContextSensitive(node) ? undefined : checker.typeToTypeNode(checker.getContextualType(node)!, scope, NodeBuilderFlags.NoTruncation); // TODO: GH#18217 - const initializer = transformConstantInitializer(node, substitutions); + let initializer = transformConstantInitializer(node, substitutions); + + ({ variableType, initializer } = transformFunctionInitializerAndType(variableType, initializer)); + suppressLeadingAndTrailingTrivia(initializer); const changeTracker = textChanges.ChangeTracker.fromContext(context); @@ -1102,6 +1105,73 @@ namespace ts.refactor.extractSymbol { const renameFilename = node.getSourceFile().fileName; const renameLocation = getRenameLocation(edits, renameFilename, localNameText, /*isDeclaredBeforeUse*/ true); return { renameFilename, renameLocation, edits }; + + function transformFunctionInitializerAndType(variableType: TypeNode | undefined, initializer: Expression): { variableType: TypeNode | undefined, initializer: Expression } { + // If no contextual type exists there is nothing to transfer to the function signature + if (variableType === undefined) return { variableType, initializer }; + // Only do this for function expressions and arrow functions that are not generic + if (!isFunctionExpression(initializer) && !isArrowFunction(initializer) || !!initializer.typeParameters) return { variableType, initializer }; + const functionType = checker.getTypeAtLocation(node); + const functionSignature = singleOrUndefined(checker.getSignaturesOfType(functionType, SignatureKind.Call)); + + // If no function signature, maybe there was an error, do nothing + if (!functionSignature) return { variableType, initializer }; + // If the function signature has generic type parameters we don't attempt to move the parameters + if (!!functionSignature.getTypeParameters()) return { variableType, initializer }; + + // We add parameter types if needed + const parameters: ParameterDeclaration[] = []; + let hasAny = false; + for (const p of initializer.parameters) { + if (p.type) { + parameters.push(p); + } + else { + const paramType = checker.getTypeAtLocation(p); + if (paramType === checker.getAnyType()) hasAny = true; + + parameters.push(updateParameter(p, + p.decorators, p.modifiers, p.dotDotDotToken, + p.name, p.questionToken, p.type || checker.typeToTypeNode(paramType, scope, NodeBuilderFlags.NoTruncation), p.initializer)); + } + } + // If a parameter was inferred as any we skip adding function parameters at all. + // Turning an implicit any (which under common settings is a error) to an explicit + // is probably actually a worse refactor outcome. + if (hasAny) return { variableType, initializer }; + variableType = undefined; + if (isArrowFunction(initializer)) { + initializer = updateArrowFunction(initializer, node.modifiers, initializer.typeParameters, + parameters, + initializer.type || checker.typeToTypeNode(functionSignature.getReturnType(), scope, NodeBuilderFlags.NoTruncation), + initializer.equalsGreaterThanToken, + initializer.body); + } + else { + if (functionSignature && !!functionSignature.thisParameter) { + const firstParameter = firstOrUndefined(parameters); + // If the function signature has a this parameter and if the first defined parameter is not the this parameter, we must add it + // Note: If this parameter was already there, it would have been previously updated with the type if not type was present + if ((!firstParameter || (isIdentifier(firstParameter.name) && firstParameter.name.escapedText !== "this"))) { + const thisType = checker.getTypeOfSymbolAtLocation(functionSignature.thisParameter, node); + parameters.splice(0, 0, createParameter( + /* decorators */ undefined, + /* modifiers */ undefined, + /* dotDotDotToken */ undefined, + "this", + /* questionToken */ undefined, + checker.typeToTypeNode(thisType, scope, NodeBuilderFlags.NoTruncation) + )); + } + } + initializer = updateFunctionExpression(initializer, node.modifiers, initializer.asteriskToken, + initializer.name, initializer.typeParameters, + parameters, + initializer.type || checker.typeToTypeNode(functionSignature.getReturnType(), scope, NodeBuilderFlags.NoTruncation), + initializer.body); + } + return { variableType, initializer }; + } } function getContainingVariableDeclarationIfInList(node: Node, scope: Scope) { diff --git a/tests/baselines/reference/extractConstant/extractConstant_ContextualType_Lambda.ts b/tests/baselines/reference/extractConstant/extractConstant_ContextualType_Lambda.ts index d5a5a55bc27..fae0a6812cd 100644 --- a/tests/baselines/reference/extractConstant/extractConstant_ContextualType_Lambda.ts +++ b/tests/baselines/reference/extractConstant/extractConstant_ContextualType_Lambda.ts @@ -5,7 +5,7 @@ const myObj: { member(x: number, y: string): void } = { } // ==SCOPE::Extract to constant in enclosing scope== -const newLocal: (x: number, y: string) => void = (x, y) => x + y; +const newLocal = (x: number, y: string): string => x + y; const myObj: { member(x: number, y: string): void } = { member: /*RENAME*/newLocal, } diff --git a/tests/cases/fourslash/extract-const-callback-function-generic.ts b/tests/cases/fourslash/extract-const-callback-function-generic.ts new file mode 100644 index 00000000000..cafba8f8245 --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback-function-generic.ts @@ -0,0 +1,15 @@ +/// + +////declare function fnUnion(fn: (a: T) => T): void +////fnUnion(/*a*/a => a/*b*/); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`declare function fnUnion(fn: (a: T) => T): void +const newLocal: (a: T) => T = a => a; +fnUnion(/*RENAME*/newLocal);` +}); \ No newline at end of file diff --git a/tests/cases/fourslash/extract-const-callback-function-no-context1.ts b/tests/cases/fourslash/extract-const-callback-function-no-context1.ts new file mode 100644 index 00000000000..4bd09076868 --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback-function-no-context1.ts @@ -0,0 +1,15 @@ +/// + +////declare function fnUnion(fn: ((a: number) => number) | ((a: string) => string)): void +////fnUnion(/*a*/(a: any) => a/*b*/); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`declare function fnUnion(fn: ((a: number) => number) | ((a: string) => string)): void +const newLocal = (a: any) => a; +fnUnion(/*RENAME*/newLocal);` +}); diff --git a/tests/cases/fourslash/extract-const-callback-function-no-context2.ts b/tests/cases/fourslash/extract-const-callback-function-no-context2.ts new file mode 100644 index 00000000000..eb392bcb445 --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback-function-no-context2.ts @@ -0,0 +1,15 @@ +/// + +////declare function fnUnion(fn: ((a: number) => number) | ((a: string) => string)): void +////fnUnion(/*a*/(a) => a/*b*/); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`declare function fnUnion(fn: ((a: number) => number) | ((a: string) => string)): void +const newLocal: ((a: number) => number) | ((a: string) => string) = (a) => a; +fnUnion(/*RENAME*/newLocal);` +}); diff --git a/tests/cases/fourslash/extract-const-callback-function-no-context3.ts b/tests/cases/fourslash/extract-const-callback-function-no-context3.ts new file mode 100644 index 00000000000..92835d0d38c --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback-function-no-context3.ts @@ -0,0 +1,15 @@ +/// + +////declare function fnUnion(fn: ((a: number) => number) | ((a: string) => string)): void +////fnUnion(/*a*/(a: string) => a/*b*/); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`declare function fnUnion(fn: ((a: number) => number) | ((a: string) => string)): void +const newLocal = (a: string) => a; +fnUnion(/*RENAME*/newLocal);` +}); diff --git a/tests/cases/fourslash/extract-const-callback-function-rest.ts b/tests/cases/fourslash/extract-const-callback-function-rest.ts new file mode 100644 index 00000000000..50feb63dd20 --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback-function-rest.ts @@ -0,0 +1,18 @@ +/// + + +////function fWithRest(fn: (...a: number[]) => number) { } +////fWithRest(/*a*/(a, b, c) => a + b + c/*b*/); + + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`function fWithRest(fn: (...a: number[]) => number) { } +const newLocal = (a: number, b: number, c: number): number => a + b + c; +fWithRest(/*RENAME*/newLocal);` +}); + diff --git a/tests/cases/fourslash/extract-const-callback-function-this1.ts b/tests/cases/fourslash/extract-const-callback-function-this1.ts new file mode 100644 index 00000000000..b0d269400bb --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback-function-this1.ts @@ -0,0 +1,15 @@ +/// + +////declare function fWithThis(fn: (this: { a: string }, a: string) => string): void; +////fWithThis(/*a*/function (a: string): string { return this.a; }/*b*/); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`declare function fWithThis(fn: (this: { a: string }, a: string) => string): void; +const newLocal = function(this: { a: string; }, a: string): string { return this.a; }; +fWithThis(/*RENAME*/newLocal);` +}); diff --git a/tests/cases/fourslash/extract-const-callback-function-this2.ts b/tests/cases/fourslash/extract-const-callback-function-this2.ts new file mode 100644 index 00000000000..518c39e26e5 --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback-function-this2.ts @@ -0,0 +1,15 @@ +/// + +////declare function fWithThis(fn: (this: { a: string }, a: string) => string): void; +////fWithThis(/*a*/function (a) { return this.a; }/*b*/); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`declare function fWithThis(fn: (this: { a: string }, a: string) => string): void; +const newLocal = function(this: { a: string; }, a: string): string { return this.a; }; +fWithThis(/*RENAME*/newLocal);` +}); diff --git a/tests/cases/fourslash/extract-const-callback-function-this3.ts b/tests/cases/fourslash/extract-const-callback-function-this3.ts new file mode 100644 index 00000000000..1d9d8ba59e7 --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback-function-this3.ts @@ -0,0 +1,17 @@ +/// + +////declare function fWithThis(fn: (this: { a: string }, a: string) => string): void; +////fWithThis(/*a*/function (this: { a: string }, a) { return this.a; }/*b*/); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`declare function fWithThis(fn: (this: { a: string }, a: string) => string): void; +const newLocal = function(this: { + a: string; +}, a: string): string { return this.a; }; +fWithThis(/*RENAME*/newLocal);` +}); diff --git a/tests/cases/fourslash/extract-const-callback-function.ts b/tests/cases/fourslash/extract-const-callback-function.ts new file mode 100644 index 00000000000..ccf05c6c1ad --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback-function.ts @@ -0,0 +1,13 @@ +/// + +////const x = [1,2,3].map(/*a*/function (x) { return x + 1 }/*b*/); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`const newLocal = function(x: number): number { return x + 1; }; +const x = [1,2,3].map(/*RENAME*/newLocal);` +}); diff --git a/tests/cases/fourslash/extract-const-callback.ts b/tests/cases/fourslash/extract-const-callback.ts new file mode 100644 index 00000000000..4bd71ba16b1 --- /dev/null +++ b/tests/cases/fourslash/extract-const-callback.ts @@ -0,0 +1,13 @@ +/// + +////const x = [1,2,3].map(/*a*/x => x + 1/*b*/); + +goTo.select("a", "b"); +edit.applyRefactor({ + refactorName: "Extract Symbol", + actionName: "constant_scope_0", + actionDescription: "Extract to constant in enclosing scope", + newContent: +`const newLocal = (x: number): number => x + 1; +const x = [1,2,3].map(/*RENAME*/newLocal);` +});