microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.03k stars 12.49k forks source link

Bug: TS extract to constant in enclosing scope with lamda functions without curly braces #55323

Open hrueger opened 1 year ago

hrueger commented 1 year ago

Type: Bug

Hi, when using the intellisense command Extract to constant in enclosing scope while inside a lambda function without curly braces and while having an argument of that lambda function selected, the new constant is created outside if the function which generates a build error.

For example:

const doSomething = (i: number) => (i + 2) * 5;

when selecting i + 2 and running Extract to constant in enclosing scope, we get this:

const newLocal = i + 2;
const doSomething = (i: number) => (newLocal) * 5;

which is obviously wrong. We need to run Add braces to arrow function first, then the command above works correctly and generates

const doSomething = (i: number) => {
    const newLocal = i + 2;
    return (newLocal) * 5;
};

TS should please do this automatically, it bothers me every day ;-) Thanks in advance!

VS Code version: Code 1.81.0 (6445d93c81ebe42c4cbd7a60712e0b17d9463e97, 2023-08-02T12:37:13.485Z) OS version: Windows_NT x64 10.0.19045 Modes:

System Info |Item|Value| |---|---| |CPUs|Intel(R) Core(TM) i7-8700K CPU @ 3.70GHz (12 x 3696)| |GPU Status|2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: enabled| |Load (avg)|undefined| |Memory (System)|31.96GB (16.72GB free)| |Process Argv|| |Screen Reader|no| |VM|0%|
Extensions (60) Extension|Author (truncated)|Version ---|---|--- vscode-sql-formatter|adp|1.4.4 ng-template|Ang|16.1.4 vscode-intelephense-client|bme|1.9.5 dart-code|Dar|3.70.0 flutter|Dar|3.70.0 mustache|daw|1.1.1 vscode-eslint|dba|2.4.2 FreeMarker|dco|0.0.9 devicescript-vscode|dev|2.14.14 gitlens|eam|14.2.0 prettier-vscode|esb|10.1.0 php-intellisense|fel|2.3.14 auto-close-tag|for|0.5.14 auto-rename-tag|for|0.1.10 copilot|Git|1.101.317 copilot-labs|Git|0.14.884 vscode-github-actions|git|0.26.1 vscode-pull-request-github|Git|0.70.0 hcl|has|0.3.2 Angular2|joh|16.0.1 vscode-json5|mrm|1.0.0 vscode-docker|ms-|1.26.0 vscode-language-pack-de|MS-|1.81.2023080209 vscode-dotnet-runtime|ms-|1.6.0 playwright|ms-|1.0.15 isort|ms-|2023.10.1 python|ms-|2023.14.0 vscode-pylance|ms-|2023.8.10 remote-containers|ms-|0.304.0 remote-ssh|ms-|0.102.0 remote-ssh-edit|ms-|0.86.0 remote-wsl|ms-|0.81.0 vscode-remote-extensionpack|ms-|0.24.0 cpptools|ms-|1.16.3 hexeditor|ms-|1.9.12 remote-explorer|ms-|0.4.1 remote-server|ms-|1.4.0 vsliveshare|ms-|1.0.5877 vscode-streamdeck|nic|4.1.6 pico-w-go|pau|3.2.1 vscode-versionlens|pfl|1.5.0 platformio-ide|pla|3.3.1 prisma|Pri|5.1.0 sqlite-viewer|qwt|0.3.13 vscode-xml|red|0.26.1 vscode-sort-json|ric|1.20.0 markdown-preview-enhanced|shd|0.6.8 svg-preview|Sim|2.8.3 vscode-stripe|str|2.0.14 svelte-vscode|sve|107.9.0 even-better-toml|tam|0.19.2 simple-php-formatter|tob|0.0.1 sort-lines|Tyr|1.10.2 vscode-counter|uct|3.2.1 adb-interface-vscode|vin|0.22.4 keyoti-changeallendoflinesequence|vs-|0.0.3 vscode-icons|vsc|12.5.0 html-css-class-completion|Zig|1.20.0 linkerscript|Zix|1.0.3 vscode-proto3|zxh|0.5.5
Andarist commented 1 year ago

Duplicate of https://github.com/Microsoft/TypeScript/issues/18924 . What is interesting here is that this refactor shouldn't even succeed as per the diagnostic that could be raised here: https://github.com/microsoft/TypeScript/blob/634d3a1db5c69c1425119a74045790a4d1dc3046/src/services/refactors/extractSymbol.ts#L1918-L1924

However, in this specific scenario the source file becomes the "innermost scope" (since the body-less arrow gets ignored) and this code just assumes that it's fine to not even test this (as per the described special case in the code comment).

It's probably fine to ignore this since the support for body-less arrows should "fix" this special case to always be true.