dart-lang / dart-lang/language

Improve context schema rules for `await`

Open
#3,571 9 comments 0 reactions 0 assignees View on GitHub
request
Dominant language
TeX
Stars
2.9k
Forks
239
Avg merge
2d 18h
Merged PRs (30d)
14

Description

Imagine we have an asynchronous function that may or may not produce a result:
```dart
// Yields `null` if user name is not known
Future readUserNameFromDatabase() async {
UserEntry userEntry = await readUserEntryFromDatabase();
return userEntry.nameOrNull;
}
```

And we wrap it in an asynchronous function that supplies a default value:
```dart
Future getUserNameOrDefault() async {
return (await readUserNameFromDatabase()) ?? 'No data found';
}
```

Now imagine we want to make the original function more generic, e.g.:
```dart
Future readUserDataFromDatabase(
T Function(UserEntry) extractDataFromEntry) async {
UserEntry userEntry = await readUserEntryFromDatabase();
return extractDataFromEntry(userEntry);
}

Future getUserNameOrDefault() async {
return (await readUserDataFromDatabase((userEntry) => userEntry.nameOrNull)) ??
'Unknown name';
}
```

This no longer works, and it's not obvious why from the error messages. Here are the error messages from the CFE:
```
../../tmp/proj/test.dart:8:11: Warning: Operand of null-aware operation '??' has type 'String' which excludes null.
return (await readUserDataFromDatabase((userEntry) => userEntry.nameOrNull)) ??
^
../../tmp/proj/test.dart:8:67: Error: A value of type 'String?' can't be returned from a function with return type 'String' because 'String?' is nullable and 'String' isn't.
return (await readUserDataFromDatabase((userEntry) => userEntry.nameOrNull)) ??
^
```

It turns out that what's happening is this:
- The returned expression (`(await readUserDataFromDatabase(...)) ?? 'Unknown name'`) is being analyzed with a context schema of `FutureOr`.
- Therefore the left hand side of the `??` expression (`await readUserDataFromDatabase(...)`) is being analyzed with a context schema of `FutureOr?`.
- The context schema rules for analyzing an expression of the form `await e`, in context `K`, involve analyzing the subexpression `e` using the context `J`, where `J` is computed as follows:
- If `K` is `_` or `dynamic`*, then `J` is `_`.
- If `K` is `FutureOr`, `FutureOr?`, or `FutureOr*`, then `J` is `K`.*
- Otherwise, `J` is `FutureOr`.
- Therefore, `readUserDataFromDatabase(...)` is analyzed using a context schema of `FutureOr?`
- So the type parameter of `readUserDataFromDatabase` is being inferred as `String`, even though `String?` is what was intended.

_*Note that the behavior of star types is not really important anymore, because pre-null-safety code is no longer supported; but support for star types hasn't been removed from the CFE yet, so I'm including the full rule for completeness._

I would like to modify the context schema rules for `await` to this:
- If `K` is `_` or `dynamic`, then `J` is `_`.
- If `K` is `FutureOr`, then `J` is `K`.
- If `K` is `FutureOr?`, then `J` is `FutureOr`.
- If `K` is `FutureOr*`, then `J` is `FutureOr`.
- Otherwise, `J` is `FutureOr`.

_Once again I'm including information about star types for completeness._

This would allow the code example above to be accepted, because `readUserDataFromDatabase(...)` would be analyzed using a context schema of `FutureOr`.

Note that the only difference is in the behavior if `K` is `FutureOr?` (or `FutureOr*`). I think the change is justified, because in this situation, the `await e` expression is in a context that permits `null`, therefore it makes sense that the context for `e` should permit a future that completes with `null`.

As with any change to type inference, this would be a breaking change in theory, because any change to an inferred type could lead to cascading changes to other inferred types, which could in turn lead to a static error in code that would previously have been accepted. However, I've prototyped the change in google3 (in the analyzer only) and found no breakages. So I think the risk of breaking real-world code is low.

An additional reason we might want to consider this change is that if we ever decide to go ahead with https://github.com/dart-lang/language/issues/3471 (require every expression to satisfy its context), then we will need to revise the context schema rules for `await` in this way, in order to avoid serious breakage.

I do think that if we make this change, it would be reasonble to do it in a language-versioned way, so that after the change, if anyone writes code like the example above, they won't risk publishing it to pub in a form that will break with older versions of Dart.

@dart-lang/language-team what do you think?

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the current await context schema rules and the proposed FutureOr nullability cases described in the issue. Compare them with the generic asynchronous function example and the reported CFE diagnostics. Done means reaching a language-team decision on the rule change and its language-versioning implications; no implementation files or tests are named.

Written by the indexing model from the issue text.

Assessment

Tech stack
dart
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.