proposal: `avoid_futureor_void`

[eernstg]

avoid_futureor_void

Description

Flag the type FutureOr<void> as questionable.

Details

The type FutureOr<void> is inherently a contradictory type.

It is essentially the union of Future<void> and void. However, void is a top type so it is never possible to discriminate the union: Any object whatsoever with static type FutureOr<void> can be intended as the void case, in which case the object should be ignored and discarded.

The other interpretation (which might always be wrong) may occur if we actually have a Future<void> (that is, a Future<T> for any type T). In this case the object should not be ignored. We might want to await it (or pass it on to some other location where it will be handled), or we might want to .ignore() it in order to handle the situation where it is completed with an error, but we should do something (cf., for example, https://dart.dev/tools/linter-rules/unawaited_futures and https://dart.dev/tools/linter-rules/discarded_futures).

This means that any expression whose type is FutureOr<void> is a dilemma, and, as far as possible, we shouldn't have expressions with that type.

The remedy depends on the situation: For return types, it might be possible to replace the type FutureOr<void> by Future<void>?. Future<void>? can be discriminated precisely: If we received null then we know that there is nothing to handle. Otherwise we would have received a Future<void>; the future should be awaited, but result that it is completed with should be discarded (or we could .ignore() the future, but that's in itself a way to handle it). The dilemma is gone!

For parameter types, FutureOr<void> can be replaced by void: This means that the function body will not use that argument (unless it goes to great lengths to cheat us), and the type of the function will remain unchanged (according to the subtype relationships), so it won't break any clients or overrides.

If the function might actually use the future (if any), it should be considered whether the parameter type can be Future<void>?. This is again a clear type because we can discriminate it precisely in the function body. This may be more difficult to do, because the change from parameter type FutureOr<void> to parameter type Future<void>? makes the function a supertype of what it was previously, which is a breaking change in many ways. With the old type we could pass anything whatsoever, with the new type we can only pass null or a future. However, this might arguably be a better type for that function, because it is a less ambiguous characterization of the intended use of that parameter.

Kind

Guard against logical errors where a future is ignored, but should be handled, or vice versa.

Bad Examples

FutureOr<void> f() {...}
SomeType g(FutureOr<void> arg) {...} // `arg` not used.
SomeType h(FutureOr<void> arg) {...} // `arg` used in some cases.

Good Examples

Future<void>? f() {...}
SomeType g(void _) {...} // Non-breaking change for callers of `g` above.
SomeType h(Future<void>? maybeFuture) {...} // Breaking change from `h` above.

Discussion

There may be situations where a legitimate signature using type variables yields the type FutureOr<void> somewhere, because a type argument turns out to have the value void. For instance, a visitor may yield a result, but some visitors exist only because of their side effects, so they could have Visitor<void> as a supertype, and they might have a resulting member signature where FutureOr<void> occurs.

This is probably not easy to avoid, but it might still be helpful to mark those occurrences as special (by ignoring the lint), and comment on the right interpretation for that particular part of the interface.

Discussion checklist

• List any existing rules this proposal modifies, complements, overlaps or conflicts with. None that I'm aware of.
• List any relevant issues (reported here, the [SDK Tracker], or elsewhere). Allow return; in function returning FutureOr<void>. language#3246, Future<void>/FutureOr<void> require returns? #32443, Analyzer reports a return-without-value in a FutureOr<void> function; does not report empty function #44480, Change Builder.build to FutureOr<void> build#535, Bogus void_checks warning when a FutureOr<void> callback contains a throw #58621, FutureOr<void> function warns of missing return #35237, void_checks false positive with FutureOr<void> and NNBD #58205, Analyzer: Future<void> requiring return statement? #31278, value-free return should be allowed in function which returns FutureOr<void> language#1370, Meta issue for returns in functions with void related return types #33218.
• If there's any prior art (e.g., in other linters), please add references here. None that I'm aware of.
• If this proposal corresponds to [Effective Dart] or [Flutter Style Guide] advice, please call it out. (If there isn't any corresponding advice, should there be?) The guides do not address this issue as far as I can see.
• Real-world examples: Please check the issues mentioned above.

[lrhn]

We should change void to be equivalent to Null, with implicit coercion fun any type to void which replaces the value with null.

Then FutureOr<void> would not be a to type, or equivalent to void any more.

It would be the dual of dynamic, which can be used for anything and assigned to anything, where void would be unusable and everything is assignable to it. The universal sink, instead of the universal source.

(Also wouldn't allow overriding a void returning function with a non-void return type. That's pretty dangerous anyway, but if we want it, we could just special case it, and throw away the value of any void typed expression, to ensure safety.)

[eernstg]

We should change void to be equivalent to Null, with implicit coercion fun any type to void which replaces the value with null.

That sounds like a severely breaking change. Of course, it would bring up the old issue that, say, a Visitor<void> is now a Visitor<Null>, and hence it can be provided whenever a Visitor<T> is expected where T is a nullable type. If we have special rules about assignability of types involving void then they would surely go away as soon as void occurs as the value of a type variable, rather than explicitly.

This means that we may accidentally consider a consistently-null source (intended to be a non-source because it has the type argument void) as an optional source, which seems to be rather bug prone. For example, using Iterable which makes the data flow even more evident:

Iterable<Y> f<X extends Y, Y>(Iterable<X> iter) => iter;

void main() {
  var dontLook = <void>[null]; // Or a thousand nulls.
  Iterable<int?> maybeInts = f(dontLook);

  for (int? i in maybeInts) {
    ... // workworkwork. But we shouldn't work on `dontLook` in the first place!
  }

  dontLook.add(1); // OK, adds another null.
  (dontLook as dynamic).add(1); // Throws? Checks the actual type argument and changes 1 to null?
}

However, it might then be simpler to change the usage, and say that Null is the correct type to use in every case where the result of a computation is known to have no value (it carries no information). No more void return types, just put Null instead. We wouldn't need the nullification step then, and the overall treatment would be less magic.

In any case, the proposal to lint against using FutureOr<void> is associated with the current rules.

[lrhn]

No easy fixes 😥

Anyway, my ramblings aside, I think this is a good warning to give.

While you can write FutureOr<void>, it's really just a way to write void (it NORM-alizes to void, so it's indistinguishable from void in most ways at runtime), while turning off the void-ness checks, which are there for your protection.

I'd be fine with allowing return; from a function returning that, but I'd still recommend changing to Future<void>? instead.
(But we'd have to allow the occasional inferred FutureOr<void>, or type parameter instantiated FutureOr<X>, where the author didn't write the type explicitly.)

[srawlins]

I support this.

[eernstg]

Here is one way to make progress on this issue: https://dart-review.googlesource.com/c/sdk/+/382403.

[johnniwinther]

I support this.

[lrhn]

Coming back to this, I still see reasonable uses of FutureOr<void>. I have had this issue in the back of my head for a year, and I still end up writing FutureOr<void> anyway, so I am not sure it really is a type I'd want to discourage entirely.

I mostly use it in a contravariant position, allowing clients of my API to pass one of void Function(...) or Future<void> Function, while signaling that a future will be awaited.

It's not the same type as Future<void>?. A type of Future<void>? Function(String) would not accept something with type bool Function(String).

Having something like Future<void> doEach<T>(Iterable<T> values, Future<void>? Function(T) action) { ... }
will not allow me to do

var ints = <int>{}; 
var stringLengths = strings.map((s) => s.length); 
doEach(stringLengths, ints.add);

because the return type of ints.add is bool.

I can see the reasoning here, but I think there is enough reasonable uses for type FutureOr<void> that I will not endorse a lint that discourages it completely (and not support it being in the recommended set).

If the lint can allow the type in contravariant positions, and if working with the result of something that comes from such a contravariant position, then I think it's fine. It's introducing new values of that type that is questionable. Don't declare a function returning FutureOr<void>, but if someone gives you one (or that's as much as you know about it), you are free to use the result. At least briefly.

A FutureOr being ignored should be as bad as a Future being ignored, so that should be handled by unawaited_futures or a similar lint. Throwing away the entire type shouldn't be needed for that.

[eernstg]

Makes sense! I'll adjust the lint.

[eernstg]

@lrhn, thinking about this one more time I'm still not convinced. Let's consider the case where FutureOr<void> is used as the return type of a formal parameter which is a callback. (I won't go into the parameter types of that callback, I'll just assume that it doesn't accept any parameters, because it makes no difference for this discussion.)

What do we gain by using a parameter type like FutureOr<void> Function()? It is a mutual subtype with, e.g., Object? Function() and FutureOr<Object?> Function(), which means that we might as well use one of the latter ones because it would permit the exact same set of actual arguments. (It could matter for type inference, but I won't go into that discussion here, I don't think there's anything really important about type inference in this context).

The answer might be that it sends an informal signal to the client who is going to pass an actual argument to this parameter.

// Tell the caller that we will treat `fun` in a certain way.
void doTask(FutureOr<void> Function() fun) {...}

The signal cannot be "when we call your function we will ignore the result", because that's much better modeled by giving fun the type void Function().

The signal could be "if you provide a function that declares that it returns a future then we will await it and discard the result of awaiting, otherwise we will discard the result". This matches the meaning of a hypothetical union type like Future<void> Function() | Object? Function(). It could be implemented because it is possible to test whether fun is Future<void> Function(). However, in this case we want the (hypothetical) function type union, not the (currently expressible) function type whose return type is a union, and we can't express the former using FutureOr. So it's a stretch to say that this is the intended meaning, and that the type FutureOr<void> Function() is a suitable way to describe the situation.

The signal could be "when we call your function and it returns a result, any Future<void> (plus, probably, any Future<T> for any T) will be awaited, and the result will be ignored; all other objects will be ignored". This wouldn't require a dynamic check on the type of the function fun itself, but the value of the invocation fun() would be tested, and it would be awaited iff it's a Future. (We might await the result in any case, depending on whether we want the asynchronous suspension, but it is not unlikely that clients would prefer to avoid the suspension when possible, so we might just as well not await the non-futures.)

The crucial part is that this is a dynamic criterion, ignoring the statements of intent that the given actual argument may provide by means of its static type. Here is an example why it is possible, and not unlikely, that a future may be returned in a situation where it should not be awaited:

import 'dart:async';

// We need to log a few lines, but it doesn't have to be right now.
void logSomethingButDoNotWait() =>
    Future<void>.delayed(Duration(seconds: 5), () => print('Logging!'));

void work() => print('Work!');

Future<void> doTasks(List<FutureOr<void> Function()> tasks) async {
  for (var task in tasks) {
    var result = task();
    if (result is Future<void>) await result;
  }
}

void main() {
  doTasks([work, work, logSomethingButDoNotWait, work, work]);
}

Running it (e.g., in DartPad), we can see that it waits for a long time, for something that we shouldn't await in the first place. Basically, we are ignoring that logSomethingButDoNotWait has return type void rather than Future<void>.

To me it looks like a step backwards to ignore the static types and return to a dynamic test, in order to decide whether the result returned by a callback should be awaited or not, especially because there's no way we can make that decision unambiguously at run time. In the example, we're blocked for a while, despite the stated intention for the function logSomethingButDoNotWait, because we rely on a dynamic check.

You could say that we shouldn't write functions like logSomethingButDoNotWait, but we do have the rule that a void arrow function can return anything whatsoever, and it is being used in practice. Similarly, we could use void to indicate the same intent for a function with an async body:

void logSomethingButDoNotWait() async {
  await Future<void>.delayed(Duration(seconds: 5));
  // ... lots of hard work that doesn't have to happen right now.
  print('Logging!');
}

In both cases the returned object is a future, in both cases it is explicitly indicated using types that it should not be awaited, and in both cases the dynamic test on the returned object would ignore this intention.

(Compare: With FutureOr<int> Function() fun we can test whether fun() returns an int or a Future<int>, and we can rely on the assumption that the former should have the int treatment and the latter should have the Future<int> treatment, and it's going to be well aligned with the intentions of the caller. For some types T we can also have a run-time type which is a subtype of T and also a subtype of Future<T>, but that's really, really unlikely to happen, except if T is a top type or Object.)

I do recognize that we can't pass a function of type bool Function() if the parameter type is Future<void>? Function(), but you could also claim that we're using a very "loose" type of FutureOr<void> Function(), just so we can accept a bool Function(), and we might then just as well admit how loos the type is an declare it as Object? Function(), and document that the returned object will be awaited if and only if it is a future.

For local documentation, we could use a type alias:

// This can be any object whatsoever, but a `Future<void>` will get a special treatment.
typedef FutureVoidOrAny = Object?;

I think it's more reasonable to use FutureVoidOrAny than FutureOr<void> in order to inform developers that this particular entity can be anything whatsoever, but if it is a Future then it will get a special treatment.

Of course, we still have the error prone behavior of relying on a dynamic check (and ignoring that some futures are delivered with a type that says "even though you can, you should not await this!"). For that, I'd recommend that we use a test on the dynamic type of the function rather than on the returned result:

/// ... document that a `Future<void> Function()` is given special treatment ...
Future<void> doTasks(List<Object? Function()> tasks) async {
  for (var task in tasks) {
    if (task is Future<void> Function()) {
      await task();
    } else {
      task();
    }
  }
}

With this version of doTasks, the execution isn't disrupted by the unwanted await.

[lrhn]

The signal is indeed that the function can either return a future whose value will be ignored, or a non-future value which will then be ignored.
No more and no less. There is no other type which reflects that message.

I'd be happy to use void Function() | Future<void> Function() if that was available, but it is not.
(If FutureOr didn't exist, I wouldn't be creating APIs that depend on it, or ask for it to be introduced, but it exists and existing APIs use it, and new APIs are written to be compatible and consistent with existing APIs.)

The crucial part is that this is a dynamic criterion

It's a union type. Static union types are always distinguished dynamically, it's how you use them. So, yes.

(and ignoring that some futures are delivered with a type that says "even though you can, you should not await this!").

That should never be the case.
A fundamental assumption is that a result can be either synchronous or asynchronous. There is no such thing as an asynchronous result that should be treated as synchronous in code that is asynchrony-aware.
And definitely never mixed with actual futures.

I don't see a big difference between the last example and:

/// ... document that a `Future<void> Function()` is given special treatment ...
Future<void> doTasks(List<FutureOr<void> Function()> tasks) async {
  for (var task in tasks) {  
    if (task() case Future<void> future) await future;
  }
}

other than it being more complicated, and that it risks mishandling a function with actual return type FutureOr<Object?>.
Which can exist, fx as the result of Zone.bind which wraps a function based on its static type.

[eernstg]

The crucial part is that this is a dynamic criterion

It's a union type. Static union types are always distinguished dynamically, it's how you use them. So, yes.

Union types can be discriminated by a run-time test if the distinction has a run-time representation.

For instance, FutureOr<int> can be discriminated at run time because because some objects have a run-time type which is a subtype of Future<int>, other objects similarly have type int, and no object has both types.

With almost any other type A, FutureOr<A> can be discriminated because some objects have type Future<A>, others have type A, and perhaps, once in a weird blue moon, some objects have both types (so we need to have a policy for which perspective to prefer when both types are present).

However, when T is Object or a top type (such as Object?, dynamic, or void), FutureOr<T> can always be considered to have taken the T branch of the union, because it's a union of two types such that every object that has type Future<T> also has type T. So we're down to a pure convention about the treatment, and the reasonable convention is probably that we consider the branch Future<T> to be taken by every object that has this type. This means that we ignore that it is also a T, effectively making the T branch a default branch which is only used when nothing else works.

This kind of discrimination is weakly typed (I used the word 'dynamic', but the point I'm making is that it is weakly typed, not that it happens at run time), because there's no way the provider of the given value can use static typing (like return types on functions or declared types of variables) to influence the chosen branch.

It's the fact that we are deliberately ignoring the static types that makes me say that this is a step backwards. We did a similar thing about futures in general a long time ago: The function flatten (which is the core part of the semantics of await) used to await a variable number of futures (as in while (x is Future) x = await x;). The destruction of static analyzability which is implied by this kind of "flattening" is deep and serious, and it's great that we're now flattening at most once, and the result has a type which is well-understood. Using FutureOr<void> in prominent APIs is a smaller elimination of static typing than the repeating flatten, but I still prefer to avoid it as much as possible.

On top of this, the type FutureOr<void> has the special wart that it should not be discriminated: The type void is used in Dart to indicate that a particular expression has a value which should be discarded. It is possible to do use the value of an expression whose static type is void (e.g., print(print('Hello, world!') as Object?) will print the value which is returned by print, and print has return type void), but we have to use a special exception like a type cast, and it is definitely not good style to do it.

In other words, if we're using the type FutureOr<void> then we are actively promoting the bad style of not ignoring the value of an expression whose type is void.

(and ignoring that some futures are delivered with a type that says "even though you can, you should not await this!").

That should never be the case.

"Should" doesn't matter much if reality is different. I mentioned async functions and => functions with return type void, and they do exist (and you argued that they should be allowed, exactly in order to be able to say "don't await this").

[comment on doTasks]
... it risks mishandling a function with actual return type FutureOr<Object?>

As I said, FutureOr<T> where Object <: T is a weak kind of typing. Beware.