Change return value in Reader interface

[ry]

Currently Reader is defined as

export interface ReadResult {
  nread: number;
  eof: boolean;
}

// Reader is the interface that wraps the basic read() method.
// https://golang.org/pkg/io/#Reader
export interface Reader {
  /** Reads up to p.byteLength bytes into `p`. It resolves to the number
   * of bytes read (`0` <= `n` <= `p.byteLength`) and any error encountered.
   * Even if `read()` returns `n` < `p.byteLength`, it may use all of `p` as
   * scratch space during the call. If some data is available but not
   * `p.byteLength` bytes, `read()` conventionally returns what is available
   * instead of waiting for more.
   *
   * When `read()` encounters an error or end-of-file condition after
   * successfully reading `n` > `0` bytes, it returns the number of bytes read.
   * It may return the (non-nil) error from the same call or return the error
   * (and `n` == `0`) from a subsequent call. An instance of this general case
   * is that a `Reader` returning a non-zero number of bytes at the end of the
   * input stream may return either `err` == `EOF` or `err` == `null`. The next
   * `read()` should return `0`, `EOF`.
   *
   * Callers should always process the `n` > `0` bytes returned before
   * considering the `EOF`. Doing so correctly handles I/O errors that happen
   * after reading some bytes and also both of the allowed `EOF` behaviors.
   *
   * Implementations of `read()` are discouraged from returning a zero byte
   * count with a `null` error, except when `p.byteLength` == `0`. Callers
   * should treat a return of `0` and `null` as indicating that nothing
   * happened; in particular it does not indicate `EOF`.
   *
   * Implementations must not retain `p`.
   */
  read(p: Uint8Array): Promise<ReadResult>;
}

I think the ReadResult is overly complex. I propose returning a Promise<number> instead, with the typical convention that 0 = EOF.

One reason not to do this is for performance reasons. You can get an EOF and data in one op supposedly. Unfortunately this is not actually the case due to how AsyncRead() works in Rust. (In particular: Ok(Async::Ready(n)) means that n bytes of data was immediately read and placed into the output buffer, where n == 0 implies that EOF has been reached.)

Therefore I propose the following simplification:

// Reader is the interface that wraps the basic read() method.
// https://golang.org/pkg/io/#Reader
export interface Reader {
  /** Reads up to p.byteLength bytes into `p`. It resolves to the number
   * of bytes read (`0` < `n` <= `p.byteLength`) and any error encountered.
   * Even if `read()` returns `n` < `p.byteLength`, it may use all of `p` as
   * scratch space during the call. If some data is available but not
   * `p.byteLength` bytes, `read()` conventionally returns what is available
   * instead of waiting for more.
   *
   * When `read()` encounters an error or end-of-file condition after
   * successfully reading `n` > `0` bytes, it returns 0.
   *
   * Callers should always process the `n` > `0` bytes returned before
   * considering the EOF. Doing so correctly handles I/O errors that happen
   * after reading some bytes and also both of the allowed EOF behaviors.
   *
   * Implementations must not retain `p`.
   */
  read(p: Uint8Array): Promise<number>;
}

[bartlomieju]

I wonder if there are any objections to keeping the EOF symbol and type and moving it into core.
Originally I only added it as a "hack" to get typescript's help during this refactor, thinking that eventually I'd do a global search-and-replace and replace EOF by null, but I must say that it works rather well.

We could define EOF to be 0 or null but I see only disadvantages. Using 0 doesn't work when the return value is a number (e.g. BufReader.readByte() currently returns -1 on eof; I didn't touch it in this PR but I think we might want to change it over as well). Using null is useless in non-strict mode.

So what about just keeping it?

Originally posted by @piscisaureus in denoland/std#444 (comment)

So I guess scope of this issue changed a bit to introduce EOF type here and tweak interface to what we use in BufReader now.

CC @ry @piscisaureus