Update documentation of StreamIterator, improve tests.

sdk/lib/async/stream.dart

Index: sdk/lib/async/stream.dart
diff --git a/sdk/lib/async/stream.dart b/sdk/lib/async/stream.dart
index 725644562145d09e9241df40ff67f0589aa11f17..c83170c1cad5e0f2653b6fcc07896bb855965b78 100644
--- a/sdk/lib/async/stream.dart
+++ b/sdk/lib/async/stream.dart
@@ -1673,16 +1673,18 @@ abstract class StreamTransformer<S, T> {
 abstract class StreamIterator<T> {
 
   /** Create a [StreamIterator] on [stream]. */
-  factory StreamIterator(Stream<T> stream)
-      // TODO(lrn): use redirecting factory constructor when type
-      // arguments are supported.
-      => new _StreamIteratorImpl<T>(stream);
+  factory StreamIterator(Stream<T> stream) = _StreamIteratorImpl<T>;
 
   /**
    * Wait for the next stream value to be available.
    *
-   * It is not allowed to call this function again until the future has
-   * completed. If the returned future completes with anything except `true`,
+   * The function must not be called again until the future returned by a

[Søren Gjesse, 2015/07/03 11:06:52]

"must not": What happens if that is violated? StateError?

[Lasse Reichstein Nielsen, 2015/07/03 11:21:17]

Yes, it throws StateError. We don't document that.

+   * previous call is completed.
+   * When the returned future completes with `true`, a new event is available
+   * and can be read from [current].
+   *
+   * If the returned future completes with anything except `true`
+   * (whether with another value or with an error),

[Søren Gjesse, 2015/07/03 11:06:52]

When can "another value" be anything but false?

[Lasse Reichstein Nielsen, 2015/07/03 11:21:17]

It *should* not, but this is an interface, but we can't guarantee that an
implementation won't do something else.

But let's state that it must complete with true or false, then it's the
implementation's fault if it doesn't.

    * the iterator is done, and no new value will ever be available.
    *
    * The future may complete with an error, if the stream produces an error.
@@ -1692,8 +1694,14 @@ abstract class StreamIterator<T> {
   /**
    * The current value of the stream.
    *
-   * Only valid when the future returned by [moveNext] completes with `true`
-   * as value, and only until the next call to [moveNext].
+   * Is `null` before the first call to [moveNext] and after a call to
+   * `moveNext` completes with a `false` result or an error.
+   *
+   * When a `moveNext` call completes with `true`, the `current` field holds
+   * the most recent event of the stream, and it stays like that until the next
+   * call to `moveNext`.
+   * Between a call to `moveNext` and when its returned future completes,
+   * the value is unspecified.
    */
   T get current;
 
@@ -1703,13 +1711,14 @@ abstract class StreamIterator<T> {
    * The stream iterator is automatically canceled if the [moveNext] future
    * completes with either `false` or an error.
    *
-   * If a [moveNext] call has been made, it will complete with `false` as value,
-   * as will all further calls to [moveNext].
-   *
    * If you need to stop listening for values before the stream iterator is
    * automatically closed, you must call [cancel] to ensure that the stream
    * is properly closed.
    *
+   * If [moveNext] has been called when the iterator is cancelled,
+   * its returned future will complete with `false` as value,
+   * as will all further calls to [moveNext].

[Søren Gjesse, 2015/07/03 11:06:52]

So moveNext cannot distinguish between end of stream and cancel. Should there be
a canceled exception in this case?

[Lasse Reichstein Nielsen, 2015/07/03 11:21:17]

I don't think it's worth it.
The code calling cancel should also be the code calling moveNext, so it should
be able to coordinate that. It should not need to introduce an error that you
have to handle as well. That means that any error would come from the stream.

+   *
    * Returns a future if the cancel-operation is not completed synchronously.
    * Otherwise returns `null`.
    */