bytes: (*Buffer).UnreadRune at EOF unexpectedly returns an error

[bcmills]

The documentation for (*bytes.Buffer).UnreadRune says:

UnreadRune unreads the last rune returned by ReadRune. If the most recent read or write operation on the buffer was not a ReadRune, UnreadRune returns an error.

From this description alone, one might reasonably expect that after ReadRune returns io.EOF, UnreadRune is a no-op and returns nil.

However, UnreadRune at EOF currently returns an error (https://play.golang.org/p/DyRzp8-W8q):
bytes.Buffer: UnreadRune: previous operation was not ReadRune

The error message is misleading: the previous operation was, in fact, ReadRune — it just happened to return io.EOF.

It is not obvious to me whether this is a bug in UnreadRune, an unclear error message, or unclear documentation.

[bradfitz]

/cc @robpike

[robpike]

Seems like it should be fine to unread a rune at EOF, so I would say "bug". But I am not the one who's going to fix it. @griesemer maybe?

[griesemer]

I am inclined to say that current behavior is the documented behavior. Note that the documentation says:

UnreadRune unreads the last rune returned by ReadRune.

If ReadRune was called at EOF, it didn't return a rune, so it cannot be unread. The issue is the error message which should be clearer.

UnreadByte behaves the same: Unless the previous Read returned at least one byte, UnreadByte won't work and return an error.

For UnreadRune it would be pretty easy to "fix" so that it can be called at EOF. A solution could be to introduce a new internal readOp with value 0, say opReadRune0, (and use -1, -2 for opInvalid and opRead) and then use opReadRune0 for a ReadRune call that returned no rune. But we also need to do the same for any byte reading operation and then it gets pretty complicated.

It doesn't seem worthwhile adding complexity for a corner case especially given that the current semantics is quite reasonable. It's also reasonable to expect that a well-written program doesn't call UnreadByte or UnreadRune after an unsuccessful read operation.

Fixing this by clarifying the documentation and error message, plus extra tests.

[gopherbot]

CL https://golang.org/cl/42020 mentions this issue.