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

text/scanner: latest changes for Go2 numbers break existing behavior #30320

Closed
griesemer opened this issue Feb 19, 2019 · 2 comments
Closed

text/scanner: latest changes for Go2 numbers break existing behavior #30320

griesemer opened this issue Feb 19, 2019 · 2 comments
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@griesemer
Copy link
Contributor

The code https://play.golang.org/p/T55vse0RPga (running on pre-Go1.13 playground!) produces:

"foo01.bar31.xx-0-1-1-0" --> [01 31 0 1 1 0]
"foo0/12/0/5.67" --> [0 12 0 5 67]

while the same code running with the latest changes to text/scanner (https://golang.org/cl/161199) produces:

"foo01.bar31.xx-0-1-1-0" --> [0 1 1 0]
"foo0/12/0/5.67" --> [0 12 0]

The new code is too liberally accepting '.'.

@griesemer griesemer added NeedsFix The path to resolution is known, but the work has not been done. early-in-cycle A change that should be done early in the 3 month dev cycle. labels Feb 19, 2019
@griesemer griesemer added this to the Go1.13 milestone Feb 19, 2019
@griesemer griesemer self-assigned this Feb 19, 2019
@griesemer
Copy link
Contributor Author

griesemer commented Feb 20, 2019

Test case for text/scanner:

func TestIssue30320(t *testing.T) {
	for _, test := range []struct {
		in, want string
	}{
		{"foo01.bar31.xx-0-1-1-0", "01 31 0 1 1 0"},
		{"foo0/12/0/5.67", "0 12 0 5 67"},
	} {
		got := extractInts(test.in)
		if got != test.want {
			t.Errorf("%q: got %q; want %q", test.in, got, test.want)
		}
	}
}

func extractInts(t string) (res string) {
	var s Scanner
	s.Init(strings.NewReader(t))
	s.Mode = ScanInts
	for {
		switch tok := s.Scan(); tok {
		case Int:
			if len(res) > 0 {
				res += " "
			}
			res += s.TokenText()
		case EOF:
			return
		}
	}
}

@gopherbot
Copy link

Change https://golang.org/cl/163079 mentions this issue: text/scanner: don't liberally consume (invalid) floats or underbars

nebulabox pushed a commit to nebulabox/go that referenced this issue Feb 21, 2019
This is a follow-up on https://golang.org/cl/161199 which introduced
the new Go 2 number literals to text/scanner.

That change introduced a bug by allowing decimal and hexadecimal floats
to be consumed even if the scanner was not configured to accept floats.

This CL changes the code to not consume a radix dot '.' or exponent
unless the scanner is configured to accept floats.

This CL also introduces a new mode "AllowNumberbars" which controls
whether underbars '_' are permitted as digit separators in numbers
or not.

There is a possibility that we may need to refine text/scanner
further (e.g., the Float mode now includes hexadecimal floats
which it didn't recognize before). We're very early in the cycle,
so let's see how it goes.

RELNOTE=yes

Updates golang#12711.
Updates golang#19308.
Updates golang#28493.
Updates golang#29008.

Fixes golang#30320.

Change-Id: I6481d314f0384e09ef6803ffad38dc529b1e89a3
Reviewed-on: https://go-review.googlesource.com/c/163079
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Feb 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
early-in-cycle A change that should be done early in the 3 month dev cycle. FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants