Skip to content

Commit 6e44db7

Browse files
authored
Don't capture type parameters defined inside the extraction range with "Extract to function" (#53543)
1 parent 85ef01d commit 6e44db7

File tree

3 files changed

+47
-4
lines changed

3 files changed

+47
-4
lines changed

src/services/refactors/extractSymbol.ts

+5-4
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ import {
111111
LabeledStatement,
112112
last,
113113
map,
114+
mapDefined,
114115
MethodDeclaration,
115116
Modifier,
116117
ModifierFlags,
@@ -1070,12 +1071,12 @@ function extractFunctionInScope(
10701071
callArguments.push(factory.createIdentifier(name));
10711072
});
10721073

1073-
const typeParametersAndDeclarations = arrayFrom(typeParameterUsages.values(), type => ({ type, declaration: getFirstDeclaration(type) }));
1074+
const typeParametersAndDeclarations = arrayFrom(typeParameterUsages.values(), type => ({ type, declaration: getFirstDeclarationBeforePosition(type, context.startPosition) }));
10741075
const sortedTypeParametersAndDeclarations = typeParametersAndDeclarations.sort(compareTypesByDeclarationOrder);
10751076

10761077
const typeParameters: readonly TypeParameterDeclaration[] | undefined = sortedTypeParametersAndDeclarations.length === 0
10771078
? undefined
1078-
: sortedTypeParametersAndDeclarations.map(t => t.declaration as TypeParameterDeclaration);
1079+
: mapDefined(sortedTypeParametersAndDeclarations, ({ declaration }) => declaration as TypeParameterDeclaration);
10791080

10801081
// Strictly speaking, we should check whether each name actually binds to the appropriate type
10811082
// parameter. In cases of shadowing, they may not.
@@ -1547,13 +1548,13 @@ function getContainingVariableDeclarationIfInList(node: Node, scope: Scope) {
15471548
}
15481549
}
15491550

1550-
function getFirstDeclaration(type: Type): Declaration | undefined {
1551+
function getFirstDeclarationBeforePosition(type: Type, position: number): Declaration | undefined {
15511552
let firstDeclaration;
15521553

15531554
const symbol = type.symbol;
15541555
if (symbol && symbol.declarations) {
15551556
for (const declaration of symbol.declarations) {
1556-
if (firstDeclaration === undefined || declaration.pos < firstDeclaration.pos) {
1557+
if ((firstDeclaration === undefined || declaration.pos < firstDeclaration.pos) && declaration.pos < position) {
15571558
firstDeclaration = declaration;
15581559
}
15591560
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////function f() {
4+
//// let g = /*start*/<T>(x: T) => x/*end*/;
5+
//// return g;
6+
////}
7+
8+
goTo.select('start', 'end');
9+
edit.applyRefactor({
10+
refactorName: "Extract Symbol",
11+
actionName: "function_scope_1",
12+
actionDescription: "Extract to function in global scope",
13+
newContent:
14+
`function f() {
15+
let g = /*RENAME*/newFunction();
16+
return g;
17+
}
18+
19+
function newFunction() {
20+
return <T>(x: T) => x;
21+
}
22+
`});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
////function satisfies<A>() {
4+
//// return /*start*/<T extends A>(x: T) => x/*end*/;
5+
////}
6+
7+
goTo.select('start', 'end');
8+
edit.applyRefactor({
9+
refactorName: "Extract Symbol",
10+
actionName: "function_scope_1",
11+
actionDescription: "Extract to function in global scope",
12+
newContent:
13+
`function satisfies<A>() {
14+
return /*RENAME*/newFunction<A>();
15+
}
16+
17+
function newFunction<A>() {
18+
return <T extends A>(x: T) => x;
19+
}
20+
`});

0 commit comments

Comments
 (0)