Typescript: add a flag to disable () => void being subtype of () => a

Created on 12 May 2016  ·  25Comments  ·  Source: microsoft/TypeScript

sometimes assuming that the result of a function might be ignored isn't safe, consider adding a flag that would disable such assumptions

followup of #8581

Revisit Suggestion

Most helpful comment

Adding to the discussion, this is especially annoying when working with lazy constructs such as Task in fp-ts.

When interoperating with callback-based API, you constantly risk to introduce subtle bugs:

// dummy declaration of Task
interface Task<A> { run(): Promise<A> };
declare function blippy(f: () => void);
declare function command(param: number): Task<string>;

blippy(() => command(42));       // nothing is being run, because `Task` is lazy
blippy(() => command(42).run()); // correct way of doing this

All 25 comments

Would we also ban function calls in expression statements unless those functions are void?

e.g. I imagine you have this problem

function doSomething(): Promise<number>;

// BUG
doSomething();

yes, you are right, expression statements other than void are a major pain

wish there was a flag that would require the void operator with them

unhandled Promise<r> (a result now or later) is a classic example, but it is also a misuse to ignore Optional<r> (a result or nothing) or Tried <r, e> (a result or an exception)

all in all TypeScript is clearly favoring OOP over FP being yet another mainstream languages with a low entry level

There are a lot of functions with infrequently-useful return values. They can't be declared as returning void, since you _sometimes_ want to use the return, but it would be quite burdensome to have to use the void operator whenever you don't. Node.prototype.appendChild comes to mind.

Whether a return value is "ignorable" or not isn't just determined by its type, either. People ignore the number setTimeout returns (a timer ID) all the time, but ignoring the number Math.floor returns, for example, can only be a bug:

var x = 3.14;
Math.floor(x);
console.log(x); // "Why isn't it 3?" ;)

So to properly detect bugs involving ignoring return values, the type system would have to distinguish between ignorable and unignorable return values, perhaps by intersection with void:

declare function setTimeout(...): number & void; // ignorable
declare function floor(...): number; // not ignorable

It's an interesting idea, but one the TypeScript people would probably reject.

People ignore the number setTimeout returns (a timer ID) this

if you are (like us) working on a serious UI heavy application with a possibility to cancel any long running opertaion you must never ignore it

for a jquery powered home page, sure go ahead

believe me the hassle of being exact about ignoring the return value is nothing compared to the benefits of being able to catch more bugs at compile time, if you value the quality and your time

this all comes down to an old idea of making typescript stricter to ones who needs it (minority): https://github.com/Microsoft/TypeScript/issues/274

it's about rephrasing the last line in 3.11.3 Subtypes and Supertypes

Instad:

the result type of M is Void, or the result type of N is a subtype of that of M.

Should be:

the result type of N is a subtype of that of M.

I will bring it up for discussion. though I do nothing think we should be adding flags to change the behavior of the type system, unless it is a transitional phase (e.g. noImplictAny, strictNullChecks, etc..).

Some points from discussion

  • No new flags unless absolutely unavoidable
  • This is a very useful modifier for _some_ functions

    • e.g. Array#reverse return value is always safe to ignore, but ignoring Array#concat's return value is 99.9% a bug (thank Array designers for confusing API design!)

    • Too cumbersome for the vast majority of people to enforce void func(); on _all_ non-void funcs

  • We could do this if we had function attributes, but we don't
  • Reconsider if we get attributes or other metadata system that would enable this

As I already said in #8240, this issue is must have to simplify work with all immutable data structures and immutable operations (concat is a good example of immutable operation). People quite often forget that they need to write

model = model.set('prop', 1)

instead of just

model.set('prop', 1)

And it's pretty hard to find the error in the large codebase. And it's also very annoying.

This kind of thing is also useful for scenarios where you want to disambiguate overloads. As an example, mocha's it function should take a callback and return nothing, or take no callback and return a promise, but not both. It'd be great to validate this using the type system, but since () => Promise is assignable to (cb: ...) => void, there's no way to validate this at compile time.

On another issue @aleksey-bykov raised the use-case:

/** @must_be_used */
function fn() { return {}; }

[].map(fn); // <-- a problem or not?

Array#map is pure, which is a superset of must-use anyway, regardless of if fn is must-use.

Redux store #dispatch<A>(action: A): A returns the argument, but store.dispatch(mustUse()); should remain non-must-use.

Are there cases where the must-use-ness of the result depends on the argument types?

So a rough design would be:

  • Add some syntax for declaring results as must-use. Strawman, add must_use before result type:

    • () => must_use number

    • interface { foo(): must_use number }

    • what about type inferrence?

  • must_use either makes the function type invariant in the result or simply disables the void conversion rule
  • Report a semantic error when the result of a must_use annotated method is used in a side-effect-only context:

    • expression statement,

    • LHS of , operator

    • increment clause of for (;;)

What about passing must-use function types as an argument?

declare function makeFoo(): must_use Foo;
declare function useFooMaker(maker: () => Foo): void;

useFooMaker(makeFoo()); // Is this an error? How do we know `useFooMaker()` actually uses `Foo`? Do we care?

I would probably say if it's declared as having to return a type, it's safe to assume it's actually used (otherwise why not void?)

there is no definition of pure, so it's hard to talk about what it is

[1, 2, 3].map((x, y, z) => { z.pop(); return x; }) // <-- pure?

but if we try: https://github.com/Microsoft/TypeScript/issues/7770

The callback is impure, which makes the call impure, but map() itself is reasonably considered pure. What I was referring to is that if you are calling Array#map() and ignoring the result, you could just as well use Array#forEach() (or for-of). That is, pure is a super-set of must-use, and in fact are the majority case (the only other I can think of is error code results, for which, why aren't you using exceptions?)

Allowing certain functions to insist on return value consumption would be valuable for greater error/exception type safety, and is a nice feature of Rust.

For instance, for any function that is used primarily for its side effects, you currently can only choose between:

createAccount(): void;
// Rely on exceptions, which are unchecked and lack compile time type safety.

createAccount(): Result<void, Disconnected | OrgNotFound | ConcurrentChange | ...>;
// Result may carry type-checked error info, but easy to simply not check this.

The current behaviour of treating () => void as () => any is a bug that should get addressed. We have a utility function always<T>(promise: Promise<T>, callback: () => void): Promise<T> which can be used to always call the callback for a Promise, in both resolve and reject case. If you pass in a callback that returns another Promise you might expect that the always invocation returns that Promise from the callback, but it does not. That is why callback is marked as returning void.

Imho TypeScript should help me by showing an error and not confuse me by allowing this. I was almost pushing code with this wrong assumption that would have been very hard to track down.

Two days back I also opened a issue similar to this #29173
I would like library author annotate his function that it's result should not be discarded

Also library author can make a type itself as non discard-able , so that any function returning that type is automatically non discard-able , without any annotation (thing like promises, observable , error codes)

also see https://en.cppreference.com/w/cpp/language/attributes/nodiscard for c++ syntax and behaviour.

Adding to the discussion, this is especially annoying when working with lazy constructs such as Task in fp-ts.

When interoperating with callback-based API, you constantly risk to introduce subtle bugs:

// dummy declaration of Task
interface Task<A> { run(): Promise<A> };
declare function blippy(f: () => void);
declare function command(param: number): Task<string>;

blippy(() => command(42));       // nothing is being run, because `Task` is lazy
blippy(() => command(42).run()); // correct way of doing this

There should be a new strict compiler option which does either of these:

  • Either treat return value of () => void as unknown, or
  • Make assigning () => not void to () => void an error

I came here from another issue which was discussing an option to mark a function such that it's return type cannot be unused. I'm unable to understand whether this issue is actually about it, so please forgive me if it isn't.

A useful scenario for the above mentioned feature is while using redux-thunk. We can create a thunk like subscribeToPlan(), but we might forget to actually dispatch it, leading to an error. This is usually not a problem when we create new thunks because we'll find that the new feature is not working as expected, but if we're migrating from a library which uses another convention to something like a thunk, we might miss a place if there are a lot of changes. If there was a way to mark a thunk such that it has to be used, we'd be able to catch them at compile time.

We've been spending hours dealing with bugs that would been caught with this check. as @simonbuchan had pointed out, this language feature is available in a lot of languages, and some even have it turned on by default. e.g. Swift

Would really like to see TypeScript adding support to this as well.

Also, if anyone knows if there is any workaround currently?

Either being eslint, tslint or anything that can help mitigate this?

F# has this turned on by default for all functions as well.

Instead of global flag could this be a new utility type than can be applied where we want?
At least it would not be breaking changes but still could help solve this issue in the long term
Like the opposite of readonly https://github.com/microsoft/TypeScript/issues/24509
If specified return or param should be checked

function compute(myNumber: readonly number): SideEffect<number>;
function mutate(myNumber: SideEffect<number>): void;

With readonly combined with SideEffect you would be able to know that a value has changed or not,
and if it could be a mistake.

Invalid

var x = 3.14;
Math.floor(readonly x);  // WARN: "math.floor() do not modify the input but returns it, did you mean to use the result?"
var x = {foo: 'bar'};
x = modify(x: SideEffect<object>): any;  // WARN: "compute() modifies the input but you reassigned the results to the original value, did you mean to check the input?"



md5-0c451355e4f084eb6aa2a5d11eba6ebe




### Valid



md5-d5625b84194c3202af764bbb90d4e8a0



```ts
var x: SideEffect<object> = {foo: 'bar'};
modify(x: SideEffect<object>): any; 
Was this page helpful?
0 / 5 - 0 ratings

Related issues

Roam-Cooper picture Roam-Cooper  ·  3Comments

fwanicka picture fwanicka  ·  3Comments

MartynasZilinskas picture MartynasZilinskas  ·  3Comments

Antony-Jones picture Antony-Jones  ·  3Comments

manekinekko picture manekinekko  ·  3Comments