Skip to content

fixes(44110): allow "extract to inner function" refactoring inside function expression #44206

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

Merged
merged 1 commit into from
Jun 10, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 1 addition & 5 deletions src/services/refactors/extractSymbol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ namespace ts.refactor.extractSymbol {
export const cannotExtractReadonlyPropertyInitializerOutsideConstructor = createMessage("Cannot move initialization of read-only class property outside of the constructor");
export const cannotExtractAmbientBlock = createMessage("Cannot extract code from ambient contexts");
export const cannotAccessVariablesFromNestedScopes = createMessage("Cannot access variables from nested scopes");
export const cannotExtractToOtherFunctionLike = createMessage("Cannot extract method to a function-like scope that is not a function");
export const cannotExtractToJSClass = createMessage("Cannot extract constant to a class scope in JS");
export const cannotExtractToExpressionArrowFunction = createMessage("Cannot extract constant to an arrow function without a block");
}
Expand Down Expand Up @@ -1624,10 +1623,7 @@ namespace ts.refactor.extractSymbol {
usagesPerScope.push({ usages: new Map<string, UsageEntry>(), typeParameterUsages: new Map<string, TypeParameter>(), substitutions: new Map<string, Expression>() });
substitutionsPerScope.push(new Map<string, Expression>());

functionErrorsPerScope.push(
isFunctionLikeDeclaration(scope) && scope.kind !== SyntaxKind.FunctionDeclaration
? [createDiagnosticForNode(scope, Messages.cannotExtractToOtherFunctionLike)]
: []);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sure there was a good reason not to allow extractions for functions which aren't declarations…

functionErrorsPerScope.push([]);

const constantErrors = [];
if (expressionDiagnostic) {
Expand Down
5 changes: 5 additions & 0 deletions src/testRunner/unittests/services/extract/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,11 @@ function parsePrimaryExpression(): any {
`function F() {
[#|function G() { }|]
}`);
// Arrow function
testExtractFunction("extractFunction34",
`const F = () => {
[#|function G() { }|]
};`);

testExtractFunction("extractFunction_RepeatedSubstitution",
`namespace X {
Expand Down
15 changes: 15 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction10.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,21 @@ namespace A {
}
}
}
// ==SCOPE::Extract to inner function in method 'a'==
namespace A {
export interface I { x: number };
class C {
a() {
let z = 1;
return /*RENAME*/newFunction();

function newFunction() {
let a1: I = { x: 1 };
return a1.x + 10;
}
}
}
}
// ==SCOPE::Extract to method in class 'C'==
namespace A {
export interface I { x: number };
Expand Down
17 changes: 17 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction11.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,23 @@ namespace A {
}
}
}
// ==SCOPE::Extract to inner function in method 'a'==
namespace A {
let y = 1;
class C {
a() {
let z = 1;
return /*RENAME*/newFunction();

function newFunction() {
let a1 = { x: 1 };
y = 10;
z = 42;
return a1.x + 10;
}
}
}
}
// ==SCOPE::Extract to method in class 'C'==
namespace A {
let y = 1;
Expand Down
19 changes: 19 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction12.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,25 @@ namespace A {
}
}
}
// ==SCOPE::Extract to inner function in method 'a'==
namespace A {
let y = 1;
class C {
b() {}
a() {
let z = 1;
return /*RENAME*/newFunction();

function newFunction() {
let a1 = { x: 1 };
y = 10;
z = 42;
this.b();
return a1.x + 10;
}
}
}
}
// ==SCOPE::Extract to method in class 'C'==
namespace A {
let y = 1;
Expand Down
20 changes: 20 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction13.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,26 @@
}
}
}
// ==SCOPE::Extract to inner function in arrow function==
<U1a, U1b>(u1a: U1a, u1b: U1b) => {
function F1<T1a, T1b>(t1a: T1a, t1b: T1b) {
<U2a, U2b>(u2a: U2a, u2b: U2b) => {
function F2<T2a, T2b>(t2a: T2a, t2b: T2b) {
<U3a, U3b>(u3a: U3a, u3b: U3b) => {
/*RENAME*/newFunction();

function newFunction() {
t1a.toString();
t2a.toString();
u1a.toString();
u2a.toString();
u3a.toString();
}
}
}
}
}
}
// ==SCOPE::Extract to inner function in function 'F2'==
<U1a, U1b>(u1a: U1a, u1b: U1b) => {
function F1<T1a, T1b>(t1a: T1a, t1b: T1b) {
Expand Down
10 changes: 10 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction17.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ class C<T1, T2> {
/*[#|*/t1.toString()/*|]*/;
}
}
// ==SCOPE::Extract to inner function in method 'M'==
class C<T1, T2> {
M(t1: T1, t2: T2) {
/*RENAME*/newFunction();

function newFunction() {
t1.toString();
}
}
}
// ==SCOPE::Extract to method in class 'C'==
class C<T1, T2> {
M(t1: T1, t2: T2) {
Expand Down
10 changes: 10 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction18.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ class C {
/*[#|*/t1.toString()/*|]*/;
}
}
// ==SCOPE::Extract to inner function in method 'M'==
class C {
M<T1, T2>(t1: T1, t2: T2) {
/*RENAME*/newFunction();

function newFunction() {
t1.toString();
}
}
}
// ==SCOPE::Extract to method in class 'C'==
class C {
M<T1, T2>(t1: T1, t2: T2) {
Expand Down
11 changes: 11 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction20.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@ const _ = class {
return a1.x + 10;/*|]*/
}
}
// ==SCOPE::Extract to inner function in method 'a'==
const _ = class {
a() {
return /*RENAME*/newFunction();

function newFunction() {
let a1 = { x: 1 };
return a1.x + 10;
}
}
}
// ==SCOPE::Extract to method in anonymous class expression==
const _ = class {
a() {
Expand Down
11 changes: 11 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction20.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@ const _ = class {
return a1.x + 10;/*|]*/
}
}
// ==SCOPE::Extract to inner function in method 'a'==
const _ = class {
a() {
return /*RENAME*/newFunction();

function newFunction() {
let a1 = { x: 1 };
return a1.x + 10;
}
}
}
// ==SCOPE::Extract to method in anonymous class expression==
const _ = class {
a() {
Expand Down
12 changes: 12 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction26.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ class C {
}
M3() { }
}
// ==SCOPE::Extract to inner function in method 'M2'==
class C {
M1() { }
M2() {
return /*RENAME*/newFunction();

function newFunction() {
return 1;
}
}
M3() { }
}
// ==SCOPE::Extract to method in class 'C'==
class C {
M1() { }
Expand Down
12 changes: 12 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction26.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@ class C {
}
M3() { }
}
// ==SCOPE::Extract to inner function in method 'M2'==
class C {
M1() { }
M2() {
return /*RENAME*/newFunction();

function newFunction() {
return 1;
}
}
M3() { }
}
// ==SCOPE::Extract to method in class 'C'==
class C {
M1() { }
Expand Down
13 changes: 13 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction27.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ class C {
constructor() { }
M3() { }
}
// ==SCOPE::Extract to inner function in method 'M2'==
class C {
M1() { }
M2() {
return /*RENAME*/newFunction();

function newFunction() {
return 1;
}
}
constructor() { }
M3() { }
}
// ==SCOPE::Extract to method in class 'C'==
class C {
M1() { }
Expand Down
13 changes: 13 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction27.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ class C {
constructor() { }
M3() { }
}
// ==SCOPE::Extract to inner function in method 'M2'==
class C {
M1() { }
M2() {
return /*RENAME*/newFunction();

function newFunction() {
return 1;
}
}
constructor() { }
M3() { }
}
// ==SCOPE::Extract to method in class 'C'==
class C {
M1() { }
Expand Down
13 changes: 13 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction28.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ class C {
M3() { }
constructor() { }
}
// ==SCOPE::Extract to inner function in method 'M2'==
class C {
M1() { }
M2() {
return /*RENAME*/newFunction();

function newFunction() {
return 1;
}
}
M3() { }
constructor() { }
}
// ==SCOPE::Extract to method in class 'C'==
class C {
M1() { }
Expand Down
13 changes: 13 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction28.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,19 @@ class C {
M3() { }
constructor() { }
}
// ==SCOPE::Extract to inner function in method 'M2'==
class C {
M1() { }
M2() {
return /*RENAME*/newFunction();

function newFunction() {
return 1;
}
}
M3() { }
constructor() { }
}
// ==SCOPE::Extract to method in class 'C'==
class C {
M1() { }
Expand Down
16 changes: 16 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction31.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,22 @@ namespace N {
}/*|]*/
}
}
// ==SCOPE::Extract to inner function in arrow function==
namespace N {

export const value = 1;

() => {
var f: () => number;
/*RENAME*/newFunction();

function newFunction() {
f = function(): number {
return value;
};
}
}
}
// ==SCOPE::Extract to function in namespace 'N'==
namespace N {

Expand Down
17 changes: 17 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction32.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,23 @@ namespace N {
}/*|]*/
}
}
// ==SCOPE::Extract to inner function in arrow function==
namespace N {

export const value = 1;

() => {
var c = /*RENAME*/newFunction()

function newFunction() {
return class {
M() {
return value;
}
};
}
}
}
// ==SCOPE::Extract to function in namespace 'N'==
namespace N {

Expand Down
20 changes: 20 additions & 0 deletions tests/baselines/reference/extractFunction/extractFunction34.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// ==ORIGINAL==
const F = () => {
/*[#|*/function G() { }/*|]*/
};
// ==SCOPE::Extract to inner function in arrow function==
const F = () => {
/*RENAME*/newFunction();

function newFunction() {
function G() { }
}
};
// ==SCOPE::Extract to function in global scope==
const F = () => {
/*RENAME*/newFunction();
};

function newFunction() {
function G() { }
}
Loading