denoland / deno_graph

The module graph logic for Deno CLI
https://docs.rs/deno_graph
MIT License
112 stars 39 forks source link

fix: handle return statements in functions correctly #470

Closed lucacasonato closed 5 months ago

lucacasonato commented 5 months ago

We now correctly make a distinction between expression like and declaration like functions when inferring return types based on return statements.

The biggest change here is that expression-like functions that have no return statements in the function body will now not infer to void anymore. This is because in some cases TypeScript will instead infer the return type to never. This applies to function expressions, arrow functions, and object literal function expression shorthand. It does not apply to function declarations and class members.

Fixes #468

lucacasonato commented 5 months ago

LGTM code wise! I'm admittedly a bit unsure of the implications from product a product perspective. We seem to be able to very accurately infer whether a function should return void to the point to where we could add the missing return type ourselves, rather than asking the user to do that.

In all the cases we are 100% sure, we do this inference already. However in the cases that we now disallow, TypeScript can infer both to void and never based on type information.

For example this code block infers never as the return type:

() => {
  Deno.exit();
}

But this example infers void:

() => {
  fetch()
}

We cannot make this distinction without type info, so we'll require the user to specify a return type explicitly.

This applies only to function bodies of function expression-like things, like function expressions, arrow functions, and function shorthand in object literals. I have included many other examples in the linked issue.