Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

io: documentation for ByteScanner and RuneScanner does not match common implementations #48449

Closed
bcmills opened this issue Sep 17, 2021 · 2 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Sep 17, 2021

What version of Go are you using (go version)?

go1.17.1

Does this issue reproduce with the latest release?

Yes.

What did you do?

Compare the documentation for io.ByteScanner and io.RuneScanner to the behavior of common standard-library implementations (strings.Reader, bytes.Reader, bytes.Buffer, bufio.Reader).

What did you expect to see?

Behavior consistent with the io interface documentation (https://pkg.go.dev/io@go1.17.1#ByteScanner):

UnreadByte causes the next call to ReadByte to return the same byte as the previous call to ReadByte. It may be an error to call UnreadByte twice without an intervening call to ReadByte.

UnreadRune causes the next call to ReadRune to return the same rune as the previous call to ReadRune. It may be an error to call UnreadRune twice without an intervening call to ReadRune.

What did you see instead?

A mix of reasonable and less-reasonable behaviors, none of which match the io interface documentation.

  • strings.Reader and bytes.Reader.
    • UnreadByte functions like Seek(-1, io.SeekCurrent). It returns nil as long as the current offset is ≥ 1, even if no read at all has occurred (such as if the offset has only been moved by Seek).
    • UnreadRune returns nil only if the last method call that adjusted the offset was UnreadRune, even if the portion of the buffer immediately ahead of the current offset contains a complete, valid rune.
  • bytes.Buffer:
    • UnreadByte returns nil as long as the last operation was any kind of Read, or Next (but not WriteTo, which is similarly Read-like).
    • UnreadRune returns nil only if the last operation was ReadRune.
  • bufio.Reader:

As in #48316, I think that given the long-established behaviors of these implementations we must fix the io interface documentation instead of the de-facto behavior. I suggest that we change the documentation to read:

UnreadByte causes the next call to ReadByte to return the last byte read. If the last operation was not a successful call to ReadByte, UnreadByte may return an error, unread the last byte read (or the byte prior to the last-unread byte), or (in implementations that support the Seeker interface) seek to one byte before the current offset.

UnreadRune causes the next call to ReadRune to return the last rune read. If the last operation was not a successful call to ReadRune, UnreadRune may return an error, unread the last rune read (or the rune prior to the last-unread rune), or (in implementations that support the Seeker interface) seek to the start of the rune before the current offset.

CC @robpike @griesemer @bradfitz @ianlancetaylor

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 17, 2021
@robpike
Copy link
Contributor

robpike commented Sep 18, 2021

SGTM

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 23, 2021
@bcmills bcmills self-assigned this Sep 23, 2021
@bcmills bcmills added this to the Backlog milestone Sep 23, 2021
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 23, 2021
@gopherbot
Copy link

Change https://golang.org/cl/351809 mentions this issue: io: update ByteScanner and RuneScanner docs to match long-standing implementations

@bcmills bcmills modified the milestones: Backlog, Go1.18 Sep 23, 2021
@rsc rsc unassigned bcmills Jun 23, 2022
@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants