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
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
is this the same as https://github.com/Microsoft/TypeScript/issues/8240?
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
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!)void func();
on _all_ non-void
func
sAs 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:
must_use
before result type:() => must_use number
interface { foo(): must_use number }
must_use
either makes the function type invariant in the result or simply disables the void
conversion rulemust_use
annotated method is used in a side-effect-only context:,
operatorfor (;;)
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:
() => void
as unknown
, or() => not void
to () => void
an errorI 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.
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;
Most helpful comment
Adding to the discussion, this is especially annoying when working with lazy constructs such as
Task
infp-ts
.When interoperating with callback-based API, you constantly risk to introduce subtle bugs: