void_checks: Specify individual types which a FutureOr<void> target can accept

[srawlins]

Description

The type of () {} changed to Null Function() (from void Function() in Null Safety. The check performed in void_checks must then compare assignability with Future<dynamic>?.

Fixes dart-lang/sdk#58205

[scheglov]

Ah, if you need this only for FutureOr, instead use typeProvider.futureOrElement.instantiate and provide nullabilitySuffix. LUB is more expensive.

[scheglov]

Can you explain the portion of expecting FutureOr<void> and typeSystem.isAssignableTo(type, _futureDynamicType)?

[srawlins]

I'm not sure why futureDynamicType was chosen originally. Seems like a strange choice to me. This fix just makes it so that when Null is used where the expected type is FutureOr<void>, then the lint rule will not complain "Hey don't assign a Null to FutureOr<void>.

If I changed this check to "is the type assignable to Future", for example, then it no longer catches a case like:

FutureOr<void> a = 1;

I think because technically this is legal, int is assignable to FutureOr<void>, but the lint wishes to call it out as "Don't assign to void."

[scheglov]

Sorry, I don't understand which types we want to allow.

When the return/target type is void we allow only void and Null. BTW, it is already an error, but we also get a lint, which is probably not useful.

// @dart = 2.7
import 'dart:async';
void f(FutureOr<int> a) {
  return 0;
}

When the target type is FutureOr<void>, which types do we want to allow?

[srawlins]

If the return type of f in your example were FutureOr<void>, I think we want to allow:

• Future<dynamic> (tested on line 58),
• Future<void> (not tested?),
• Future<Null> (not tested?),
• FutureOr<dynamic> (not tested?),
• FutureOr<void> (line 71),
• FutureOr<Null> (newly tested in this PR, line 10)
• dynamic (line 72),
• void (untested?),
• Null (line 73),

and nothing else, I think. Right now the language allows, e.g. an int to be returned where FutureOr<void> is expected.

[scheglov]

If we can have this nice exhaustive list of types, I think it would be better to check for it explicitly, like we did for void, instead of using a more heavy isAssignableTo.

[srawlins]

Ah ha, good point. I've used isTypeAcceptableWhenExpectingVoid to write a isTypeAcceptableWhenExpectingFutureOrVoid.

[scheglov]

It seems to me that we are doing it the opposite way here - we should not construct FutureOr<void> and then use == so do force the analyzer to deconstruct it again. Instead check isDartAsyncFutureOr and that the type argument isVoid.

[scheglov]

I also looked at isTypeAcceptableWhenExpectingVoid and it seems that it can be improved by recursion instead of repeating the checks for void and Null.

[srawlins]

Ah I understand. OK fixed both issues.