Skip to content

FutureOr<void> function warns of missing return #35237

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
DanTup opened this issue Nov 21, 2018 · 7 comments
Closed

FutureOr<void> function warns of missing return #35237

DanTup opened this issue Nov 21, 2018 · 7 comments

Comments

@DanTup
Copy link
Collaborator

DanTup commented Nov 21, 2018

With the following code:

void a() {}
FutureOr<void> b() {}

The second function gives a hint about a missing return:

This function has a return type of 'FutureOr<void>', but doesn't end with a return statement.

It's not entirely obvious what I should do - it's not valid to write return void and I can't change the signature because it comes from a base class (the void is a type arg and has a real type for other implementations). It doesn't complain if I write return null though it feels weird (and I've not got as far as running it to confirm that's ok yet).

@DanTup
Copy link
Collaborator Author

DanTup commented Nov 21, 2018

Actually, I don't get any warnings if I change the return type to just void even though the method it's overriding is FutureOr<T> so maybe that's the expected solution?

@eernstg
Copy link
Member

eernstg commented Nov 21, 2018

The feature specification 'invalid_returns.md' does not make it explicit in which cases it is allowed to complete the body of a function normally (that is, to "fall through" at the end), and it allows only void, Null, and dynamic as the declared return type with return; (for this kind of function).

I can't confirm this from the text of the language specification, but it would be a reasonable rule to say that return; and falling out at the end of a function is an error or a warning in the same situations.

Basically, FutureOr<void> is outlawed because it is confusing and error prone to fall through (and implicitly return null) to that return type.

With the given information about the broader context, it sounds to me like void is a quite suitable return type here.

@DanTup
Copy link
Collaborator Author

DanTup commented Nov 21, 2018

So is it valid to override a Future<T>-returning method with just void when T is void (I guess the same applies to any other type - overriding a FutureOr<int> and just declaring int as the return type)?

I expected that the override would have to declare the exact same type, but certainly this seems to work in my tests (assuming my tests correctly cover this 🤷‍♂️).

@DanTup
Copy link
Collaborator Author

DanTup commented Nov 21, 2018

And a slight tangent, I guess the same question applies to arguments... If I use void as a typearg and it's used as a param, what's the best way to write it?

abstract class Foo<T> {
  doFoo(T thing);
}

class Bar extends Foo<void> {
  doFoo(void _) {
    // Is `void _` the best option here?
  }
}

@eernstg
Copy link
Member

eernstg commented Nov 21, 2018

Right, return type FutureOr<void> is a top type, so any return type can be used in an overriding declaration, including void. But that serves to tell the caller (whose receiver type is the subtype) that the returned value should be discarded, and this may or may not be what you intend to imply.

The type FutureOr<void> is not conceptually very useful, by the way: You are saying to the caller who receives such a value: "This is an object that you should discard (don't even look!!), or it's a future that will be completed with a value that should be discarded (but you can use that future for synchronization)". But the caller has no idea when you have the former and when you have the latter, because nothing stops the value from being obtained from an expression of type void (so it's the "discard this thing" case), so you may test whether it's a future and then be mislead into waiting for something that will only happen after a long delay, which wasn't intended to be awaited anyway. So whenever you want to use the type FutureOr<void>, think again. ;-)

Overriding return type FutureOr<int> by int is simpler: No problem! You are basically informing the caller that (for this subtype as the receiver) we no longer have a choice between a Future<int> and a plain int, it's guaranteed to always be the latter.

In general, Dart has covariant return types, that is, return type T can be overridden by return type S whenever S <: T, but not vice versa. This is sound, because the subtype can be checked to actually return an S (statically or via an implicit cast), and the caller who only knows the return type T will be happy receiving an S as well.

Tangent..

I think doFoo(void _) is fine. It means that you cannot use the value of that parameter in the body (unless you use an explicit as dynamic or something similar to force it into some other typing), and that's actually exactly what you have told the caller who knows that the receiver is a Bar, that is, this argument has type void.

Of course, having an parameter of type void does not provide any guarantees because it is possible to cast it away (and there are no technical problems with that object), it's just a developer intention statement that it should be discarded, and the checks on the use of _ in the body of doFoo will help respecting that wish.

@DanTup
Copy link
Collaborator Author

DanTup commented Nov 21, 2018

The type FutureOr is not conceptually very useful, by the way

Yeah, I didn't set out to use it, it just was a consequence of some generics. I have a base class for some handlers - some will return values and some will not. It feels clumsy making all those that do not return null; so using void as the return type arg seem (seemed?) like a reasonable thing.

I think doFoo(void _) is fine.

Cool :) It's similar to the above - my generic handler has a type arg for the parameter type, but one of the methods doesn't take the arg so I use void.

Unfortunately calling it isn't quite so simple, because it doesn't seem valid to pass void or not pass an argument:

abstract class Foo<T> {
  doFoo(T thing);
}

class Bar extends Foo<void> {
  doFoo(void _) {}
}

test() {
  final a = new Bar();
  a.doFoo(); // invalid
  a.doFoo(void); // invalid
  a.doFoo(null); // valid?;
}

It feels dirty passing null when the sig says void, but I'm guessing there's nothing better I can put there?

It seems like there's probably nothing to do here, so unless you think there's something that can/should be improved, feel free to close this. Thanks for the info!

@eernstg
Copy link
Member

eernstg commented Nov 21, 2018

doesn't seem valid to pass void or not pass an argument

The overriding declaration is allowed to make the argument optional: doFoo([void _]) {}, so that would enable the form doFoo(); (this is because Function([void]) is a subtype of Function(T) for all T). Passing void does not work because that's syntactically not an identifier (for historical reasons). Passing null is valid (and fine: It makes sense to use a value with no information in a situation where that value is intended to be discarded), but it is not so informative (because anyone who looks at the call site might think that this is an argument of some useful type T, and we're passing null in order to indicate that it's omitted, or something like that). So doFoo() looks best to me at this point. ;)

feel free to close this. Thanks for the info!

Will do, and thank you!

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

2 participants