Skip to content

Suggestion: compiler flag to prevent functions returning non-void being assigned to functions returning void #53421

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
5 tasks done
OliverJAsh opened this issue Mar 21, 2023 · 6 comments

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Mar 21, 2023

Suggestion

πŸ” Search Terms

  • void
  • match
  • function return type
  • IO type
  • Lazy

βœ… Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

A compiler flag to prevent functions returning non-void being assigned to functions returning void.

πŸ“ƒ Motivating Example

Playground

type Lazy<A> = () => A;

declare const condition: boolean;

const match =
  <A>(x: () => A, y: () => A): Lazy<A> =>
  () =>
    condition ? x() : y();

const log =
  (msg: string): Lazy<void> =>
  () =>
    console.log(msg);

// ❌ This is a programmer error.
const fn1 = match(log("foo"), () => log("This will not be logged."));
const a = fn1();
//    ^?

// βœ… This is how it should have been written.
const fn2 = match(log("foo"), log("This will be logged."));
const b = fn2();
//    ^?
// => "This will be logged."

πŸ’» Use Cases

See the above motivating example. This is a reduced version of bug we had on Unsplash (using the IO type in fp-ts instead of Lazy).

It's possible to catch these bugs using no-discarded-pure-expression, however this lint rule is extremely slow.

@fatcerberus
Copy link

Is there a reason you don’t think this agrees with the design goals?

@OliverJAsh
Copy link
Contributor Author

I wasn't sure, but after thinking about it some more, I've now checked that box.

@RyanCavanaugh
Copy link
Member

There's no way this would be workable in practice. The defining feature of void (relative to undefined or unknown) is that it does this; I would say if you think you have a problem with void, the general advice would be to only ever use undefined or unknown in your return type annotations instead.

The example itself doesn't really manifest any type violations, so trying to fix it with a type system change seems off. You could be writing, for example, this:

const log =
  (msg: string): Lazy<undefined> => // <- more-precise return type
  () =>
    void console.log(msg);

// I did mean to do this; no error has occurred
const fn1 = match(log("foo"), () => log("This will not be logged."));
const a = fn1();
if (a) {
  a();
}

A possible flag would be to treat unreturning function bodies as returning undefined, though it wouldn't really affect anything in the given example.

@fatcerberus
Copy link

fatcerberus commented Mar 21, 2023

One big inconvenience with undefined returning functions is that they must have an explicit return undefined; IIRC

@MartinJohns
Copy link
Contributor

One big inconvenience with undefined returning functions is that they must have an explicit return undefined; IIRC

In TypeScript 5.1 not anymore, see #36288. It already works in nightly: Playground link

@OliverJAsh
Copy link
Contributor Author

OliverJAsh commented Mar 21, 2023

A possible flag would be to treat unreturning function bodies as returning undefined, though it wouldn't really affect anything in the given example.

Imagine we were annotating fn1, expecting that it will be Lazy<void> (not Lazy<Lazy<void>>). This does not error:

const log =
  (msg: string): Lazy<void> =>
  () =>
    void console.log(msg);

const fn1: Lazy<void> = match(log("foo"), () =>
  log("This will not be logged.")
);

If we replace void with undefined, it does (correctly) error:

const log =
  (msg: string): Lazy<undefined> =>
  () =>
    void console.log(msg);

// Type 'Lazy<undefined>' is not assignable to type 'undefined'.
const fn1: Lazy<undefined> = match(log("foo"), () =>
  log("This will not be logged.")
);

So, I think the flag you're describing would be useful (in the case where log was missing a function return type).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants