Skip to content

TypeScript's type inference errors on unreachable return statements #51800

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

Closed
patricio-ezequiel-hondagneu-roig opened this issue Dec 7, 2022 · 3 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@patricio-ezequiel-hondagneu-roig
Copy link

patricio-ezequiel-hondagneu-roig commented Dec 7, 2022

Type: Bug

This snippet does not currently work:

function test() {
    return 1
    return 1 as unknown
}
test() + 1

Currently, the editor infers the return type of test to be unknown (when it should be number), which causes the value + 1 expression afterwards to be considered illegal.


This other snippet also fails:

function test() {
    return 1
    return ""
}
test() + 1

This time it's because the return type of test is inferred to be number | string, making the value + 1 expression illegal.


In general, VSCode appears to be inferring the return value of a function with unreachable return statements as a discriminated union of the types of the values being returned in them, even if they are unreachable.

For instance, this final snippet works because the return type of test is inferred to be any (or, rather, number | any which is simplified to any):

function test() {
    return 1
    return "" as any
}
test() + 1

VS Code version: Code 1.73.1 (Universal) (6261075646f055b99068d3688932416f2346dd3b, 2022-11-09T02:08:38.961Z)
OS version: Darwin x64 21.5.0
Modes:
Sandboxed: No

System Info
Item Value
CPUs Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz (12 x 2600)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_renderer: enabled_on
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: disabled_off
Load (avg) 5, 6, 5
Memory (System) 16.00GB (0.08GB free)
Process Argv --crash-reporter-id 4132e07f-b40c-40f2-9dcd-378cb5144d46
Screen Reader no
VM 0%
Extensions (26)
Extension Author (truncated) Version
Bookmarks ale 13.3.1
pug ama 1.0.1
ng-template Ang 15.0.2
npm-intellisense chr 1.4.3
vscode-eslint dba 2.2.6
pug-formatter duc 0.6.0
gitlens eam 13.1.1
EditorConfig Edi 0.16.4
vscode-npm-script eg2 0.3.29
auto-rename-tag for 0.1.10
vscode-pull-request-github Git 0.54.1
todo-tree Gru 0.0.220
terraform has 2.25.1
Angular2 joh 13.0.0
import-sorter mik 3.3.1
dotenv mik 1.0.1
vscode-scss mrm 0.10.0
vscode-docker ms- 1.23.1
vscode-typescript-tslint-plugin ms- 1.3.4
vscode-xml red 0.22.0
vscode-yaml red 1.10.1
LiveServer rit 5.7.9
santacodes-region-viewer San 1.1.1
code-settings-sync Sha 3.4.3
svg-preview Sim 2.8.3
vscode-icons vsc 12.0.1
A/B Experiments
vsliv368cf:30146710
vsreu685:30147344
python383cf:30185419
vspor879:30202332
vspor708:30202333
vspor363:30204092
vslsvsres303:30308271
pythonvspyl392:30443607
vserr242cf:30382550
pythontb:30283811
vsjup518:30340749
pythonptprofiler:30281270
vshan820:30294714
vstes263:30335439
vscoreces:30445986
pythondataviewer:30285071
vscod805cf:30301675
binariesv615:30325510
bridge0708:30335490
bridge0723:30353136
cmake_vspar411:30581797
vsaa593cf:30376535
pythonvs932:30410667
cppdebug:30492333
vsclangdc:30486549
c4g48928:30535728
dsvsc012:30540252
azure-dev_surveyone:30548225
vsccc:30610678
pyindex848:30577860
nodejswelcome1:30587005
3biah626:30602489
gswce1:30612156
iaj6b796:30613358
dbltrim-noruby:30604474
89544117:30613380

@mjbvz mjbvz transferred this issue from microsoft/vscode Dec 7, 2022
@mjbvz mjbvz removed their assignment Dec 7, 2022
@MartinJohns
Copy link
Contributor

IMO it's a dupliate of #26914. Unreachable code screws up the compiler.

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Dec 7, 2022
@RyanCavanaugh
Copy link
Member

This inference is still sound and I think there's a reasonable argument to be made that code like this is always transitory, so the question is "What is the final state of this code going to be?" - of which there are obviously two possible answers. With the current behavior, in the case where the second return is removed, then downstream errors go away, and if the second return becomes reachable then the downstream code gets the correct typechecking behavior the whole time. If unreachable returns are ignored then the reverse happens, and that seems worse IMO.

@RyanCavanaugh RyanCavanaugh closed this as not planned Won't fix, can't repro, duplicate, stale Dec 7, 2022
@fatcerberus
Copy link

in the case where the second return is removed, then downstream errors go away

Devil's advocate position is that the wider return type, while sound, has potentially detrimental downstream effects, e.g. being forced to write type guards and/or casts and then forgetting to remove them once the second return goes away. Not a Defect is definitely the appropriate response because there's no right or wrong answer here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

5 participants