Breaking-Change-Request: Fix StreamIterator accepting null as stream.

[lrhn]

The StreamIterator class in dart:async accepts null as a constructor argument. This acts the same as an empty stream.
The constructor is not documented as accepting null, it was not intended and may be hiding bugs.

The scope of the bug is increased by the StreamIterator class being used internally by some platforms as part of the implementation of await for loops. This means that await for (var x in null) ... is accepted without error. Again, this was not intended, and may be hiding actual bugs.

When we introduce non-nullable types, the StreamIterator constructor will be changed to requiring a non-nullable argument, and await for will require a non-nullable type for its stream operand. That will turn the current (missing) run-time error into a compile-time error.
This also means that the current behavior cannot be retained in the longer term, even if we wanted to avoid changing the behavior right now. So, we choose to fix the bug as soon as possible.

We can delay the change until the launch of non-nullable types, but that would introduce one more thing to check as part of the non-null migration, a subtle problem that might drown in other migration issues.
We think it will be better to fix this up-front while the problem is easily detectable independently of other changes.

Planned Change

See planned fix CL: https://dart-review.googlesource.com/c/sdk/+/98001

The StreamIterator constructor will check its argument for being null, and throw an ArgumentError if it receives a null. This will turn the current behavior on a null stream into a run-time error.
This will also propagate to the language implementation uses, so await for with a null stream will also throw at run-time.

Expected Impact

It is not expected that there is any code deliberately depending on the incorrect behavior.
There is a CL ready to land which will fix the problem. We will run this against available source to see how much breakage it will cause.

Mitigation

Any place where a null value can be the argument to the StreamIterator constructor or the stream operand of an await for loop, just add ?? Stream.empty() to the expression.

[aadilmaan]

@dgrove @Hixie @matanlurey - Please review and approve this breaking-change-request.

[Hixie]

I have no opinion.

[matanlurey]

Seems fine to change, thanks.

[aadilmaan]

Waiting on approval from @dgrove who will take a look today.

[sigmundch]

In lieu of @dgrove, seems fine to change IMO as well.

@mit-mit - since @dgrove is OOO, could you
in addition provide your opinion as well or do you believe we should add anyone else to approve instead?

[mit-mit]

LGTM for making this change.

In terms of timing I would prefer to land this early in a release cycle though (i.e., wait until after 2.3 goes out).

[aadilmaan]

Lasse - This request is approved. Please note that we would like to target this for after Dart 2.3.

[natebosch]

Please note that we would like to target this for after Dart 2.3.

Note that in 2.3 await for will be supported inside collection literals, and will exhibit this broken behavior. Waiting until after 2.3 means we'll be allowing new places where folks could rely on this behavior before we change it.

[lrhn]

The CL has landed, but is not intended to be cherry-picked for 2.3.