From e02998f70d254ef8e4d8e115e525dc1770f4c588 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Mon, 4 Apr 2022 22:27:49 +0300 Subject: [PATCH] fix(48541): forbid function extraction to arrow function with expression body (#48548) --- src/harness/fourslashImpl.ts | 5 +++-- src/harness/fourslashInterfaceImpl.ts | 4 ++-- src/services/refactors/extractSymbol.ts | 3 ++- .../extractConstant_ArrowFunction_Expression.js | 2 +- .../extractConstant_ArrowFunction_Expression.ts | 2 +- .../cases/fourslash/extract-method-two-type-literals.ts | 2 +- tests/cases/fourslash/extract-method44.ts | 9 +++++++++ tests/cases/fourslash/fourslash.ts | 2 +- 8 files changed, 20 insertions(+), 9 deletions(-) create mode 100644 tests/cases/fourslash/extract-method44.ts diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 408785ccf00..dac3d672d75 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -3529,9 +3529,10 @@ namespace FourSlash { }; } - public verifyRefactorAvailable(negative: boolean, triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) { + public verifyRefactorAvailable(negative: boolean, triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string, actionDescription?: string) { let refactors = this.getApplicableRefactorsAtSelection(triggerReason); - refactors = refactors.filter(r => r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName))); + refactors = refactors.filter(r => + r.name === name && (actionName === undefined || r.actions.some(a => a.name === actionName)) && (actionDescription === undefined || r.actions.some(a => a.description === actionDescription))); const isAvailable = refactors.length > 0; if (negative) { diff --git a/src/harness/fourslashInterfaceImpl.ts b/src/harness/fourslashInterfaceImpl.ts index dfe3808c8e5..064ec8b745a 100644 --- a/src/harness/fourslashInterfaceImpl.ts +++ b/src/harness/fourslashInterfaceImpl.ts @@ -215,8 +215,8 @@ namespace FourSlashInterface { this.state.verifyRefactorsAvailable(names); } - public refactorAvailable(name: string, actionName?: string) { - this.state.verifyRefactorAvailable(this.negative, "implicit", name, actionName); + public refactorAvailable(name: string, actionName?: string, actionDescription?: string) { + this.state.verifyRefactorAvailable(this.negative, "implicit", name, actionName, actionDescription); } public refactorAvailableForTriggerReason(triggerReason: ts.RefactorTriggerReason, name: string, actionName?: string) { diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts index 36571bbf1c1..f9a790e80ac 100644 --- a/src/services/refactors/extractSymbol.ts +++ b/src/services/refactors/extractSymbol.ts @@ -639,7 +639,8 @@ namespace ts.refactor.extractSymbol { } function isScope(node: Node): node is Scope { - return isFunctionLikeDeclaration(node) || isSourceFile(node) || isModuleBlock(node) || isClassLike(node); + return isArrowFunction(node) ? isFunctionBody(node.body) : + isFunctionLikeDeclaration(node) || isSourceFile(node) || isModuleBlock(node) || isClassLike(node); } /** diff --git a/tests/baselines/reference/extractConstant/extractConstant_ArrowFunction_Expression.js b/tests/baselines/reference/extractConstant/extractConstant_ArrowFunction_Expression.js index ac6de28d3c4..3313cc2e50d 100644 --- a/tests/baselines/reference/extractConstant/extractConstant_ArrowFunction_Expression.js +++ b/tests/baselines/reference/extractConstant/extractConstant_ArrowFunction_Expression.js @@ -1,5 +1,5 @@ // ==ORIGINAL== const f = () => /*[#|*/2 + 1/*|]*/; -// ==SCOPE::Extract to constant in global scope== +// ==SCOPE::Extract to constant in enclosing scope== const newLocal = 2 + 1; const f = () => /*RENAME*/newLocal; \ No newline at end of file diff --git a/tests/baselines/reference/extractConstant/extractConstant_ArrowFunction_Expression.ts b/tests/baselines/reference/extractConstant/extractConstant_ArrowFunction_Expression.ts index ac6de28d3c4..3313cc2e50d 100644 --- a/tests/baselines/reference/extractConstant/extractConstant_ArrowFunction_Expression.ts +++ b/tests/baselines/reference/extractConstant/extractConstant_ArrowFunction_Expression.ts @@ -1,5 +1,5 @@ // ==ORIGINAL== const f = () => /*[#|*/2 + 1/*|]*/; -// ==SCOPE::Extract to constant in global scope== +// ==SCOPE::Extract to constant in enclosing scope== const newLocal = 2 + 1; const f = () => /*RENAME*/newLocal; \ No newline at end of file diff --git a/tests/cases/fourslash/extract-method-two-type-literals.ts b/tests/cases/fourslash/extract-method-two-type-literals.ts index f8885d58bea..df502c5f9eb 100644 --- a/tests/cases/fourslash/extract-method-two-type-literals.ts +++ b/tests/cases/fourslash/extract-method-two-type-literals.ts @@ -5,7 +5,7 @@ goTo.select("1", "2"); edit.applyRefactor({ refactorName: "Extract Symbol", - actionName: "function_scope_1", + actionName: "function_scope_0", actionDescription: "Extract to function in global scope", newContent: `(x: {}, y: {}) => (/*RENAME*/newFunction(x, y)); diff --git a/tests/cases/fourslash/extract-method44.ts b/tests/cases/fourslash/extract-method44.ts new file mode 100644 index 00000000000..1fcdfb857f8 --- /dev/null +++ b/tests/cases/fourslash/extract-method44.ts @@ -0,0 +1,9 @@ +/// + +////function foo() { +//// let x = [1, 2, 3]; +//// let y = x.map(e => /*a*/e + e/*b*/); +////} + +goTo.select("a", "b"); +verify.not.refactorAvailable("Extract Symbol", "function_scope_0", "Extract to inner function in arrow function"); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index a02e06a574c..d6e0d3ce80c 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -271,7 +271,7 @@ declare namespace FourSlashInterface { codeFixDiagnosticsAvailableAtMarkers(markerNames: string[], diagnosticCode?: number): void; applicableRefactorAvailableForRange(): void; - refactorAvailable(name: string, actionName?: string): void; + refactorAvailable(name: string, actionName?: string, actionDescription?: string): void; refactorAvailableForTriggerReason(triggerReason: RefactorTriggerReason, name: string, action?: string): void; refactorKindAvailable(refactorKind: string, expected: string[], preferences?: {}): void; }