Repository: BurntSushi/toml
Pull request #260 (2020-04-04 12:27:46)

Add support for base-prefixed integers: '0b', '0o', '0x'
Added to the language in toml-lang/toml#507

Fixes: #7

arp242: I made a change in #274 to disallow leading zeros in numbers (i.e. `i = 001` is specifically disallowed by the TOML specification) as tests were failing on that.

This conflicts with this patch a bit; sorry about that 😅 I think the patch looks mostly okay from a quick review; if you're still interested in working on this you can rebase on master, fix it up if needed, and I'll merge it. If not then that's okay and I'll fix it up myself.

Thanks!
usedbytes: Gosh I'd totally forgotten I typed this 😆 I'll have a look at rebasing this evening.
I also didn't notice all the other PRs implementing this when I did it! Woops.
arp242: > Gosh I'd totally forgotten I typed this

Yeah, it's been a while 😅 This project was unmaintained for a while and I took over day-to-day maintainership a few days ago, hence the late response.

No hurries getting it fixed; I'm still working on some of the bugs etc. that were reported. Just figured I'd give you the opportunity in case you were keen.

> I also didn't notice all the other PRs implementing this when I did it! Woops.

No worries, and this PR was the best one so there's that :-)
usedbytes: Well, your new test cases have shown some bugs in my implementation, and it's also apparently not spec conformant (`-` is not allowed on prefixed-integers) :blush: 

Do you have any preference for how much validation is done in `lex` versus `parse`? For example, on floats and dates, quite a lot of the validation is done in `parse` after `lex` has pulled out something "approximate". For integers, if `ParseInt()` fails, that's treated as a `p.bug()`.

It feels more in-line with the rest of the code if I don't be too strict in `lex`, and let `ParseInt()` fail for some malformed inputs (such as `0o8`), but let me know if you have a preference either way.
arp242: Only getting 100% valid numbers in lex is rather tricky as you need to look ahead or backtrack quite a bit to be sure. I suppose that this is why some of the number validation was already in parse rather than lex, and this is also why I added the leading zero check in there. I tried doing that in lex and it wasn't all that easy. Just seeing `0` doesn't mean it's a number, but can also be `0007-01-01` for the 1st of January in the year 7.

Getting an as clean as possible output from the lexer is nice, but it's main task is not to fully parse numbers and the like; just split things in to "this is a number (possible invalid, but a number nonetheless)", "this is a string", "this is a key", "this is a date", etc.

> For integers, if ParseInt() fails, that's treated as a p.bug().

I think that's okay, as there are a number of checks for this (`numPeriodsOK`, `numUnderscoresOK`, `numHasLeadingZero`). Any other invalid numbers should probably be detected in that way so a reasonably nice friendly error message can be presented. For example this:

    i, err := strconv.ParseInt("1__000", 0, 64)

Will give the error:

    strconv.ParseInt: parsing "1__000": invalid syntax

Which is kind of meh. "Invalid" in what way? With the check that's here now you get `Invalid integer "1__000": underscores must be surrounded by digits`. Much nicer ya?

If there are any unhandled situations that will result in `ParseInt` failing then that's probably a bug.

Anyway, in short, stuff like `0xzz` and `0b99` can just be detected by the lexer, as that's clearly not valid and not hard to detect. But more tricky stuff: just check for it in the parser. If you find yourself trying to get it done in the lexer and getting stuck then just move it to the parser.

Patch:
diff --git a/lex.go b/lex.go
index 47def13..b73b06a 100644
--- a/lex.go
+++ b/lex.go
@@ -388,8 +388,6 @@ func lexBareKey(lx *lexer) stateFn {
 		lx.backup()
 		lx.emit(itemText)
 		return lexKeyEnd
-	case r == eof:
-		return lx.errorf("unexpected EOF; expected key separator '='")
 	default:
 		return lx.errorf("bare keys cannot contain %q", r)
 	}
@@ -403,8 +401,6 @@ func lexKeyEnd(lx *lexer) stateFn {
 		return lexSkip(lx, lexValue)
 	case isWhitespace(r):
 		return lexSkip(lx, lexKeyEnd)
-	case r == eof:
-		return lx.errorf("unexpected EOF; expected key separator %q", keySep)
 	default:
 		return lx.errorf("expected key separator %q, but got %q instead",
 			keySep, r)
@@ -454,8 +450,8 @@ func lexValue(lx *lexer) stateFn {
 		}
 		lx.ignore() // ignore the "'"
 		return lexRawString
-	case '+', '-':
-		return lexNumberStart
+	case '-', '+':
+		return lexDecimalNumberStart
 	case '.': // special error case, be kind to users
 		return lx.errorf("floats must start with a digit, not '.'")
 	}
@@ -467,9 +463,6 @@ func lexValue(lx *lexer) stateFn {
 		lx.backup()
 		return lexBool
 	}
-	if r == eof {
-		return lx.errorf("unexpected EOF; expected value")
-	}
 	return lx.errorf("expected value but found %q instead", r)
 }
 
@@ -586,7 +579,7 @@ func lexString(lx *lexer) stateFn {
 	r := lx.next()
 	switch {
 	case r == eof:
-		return lx.errorf(`unexpected EOF; expected '"'`)
+		return lx.errorf("unexpected EOF")
 	case isNL(r):
 		return lx.errorf("strings cannot contain newlines")
 	case r == '\\':
@@ -607,7 +600,7 @@ func lexString(lx *lexer) stateFn {
 func lexMultilineString(lx *lexer) stateFn {
 	switch lx.next() {
 	case eof:
-		return lx.errorf(`unexpected EOF; expected '"""'`)
+		return lx.errorf("unexpected EOF")
 	case '\\':
 		return lexMultilineStringEscape
 	case stringEnd:
@@ -635,7 +628,7 @@ func lexRawString(lx *lexer) stateFn {
 	r := lx.next()
 	switch {
 	case r == eof:
-		return lx.errorf(`unexpected EOF; expected "'"`)
+		return lx.errorf("unexpected EOF")
 	case isNL(r):
 		return lx.errorf("strings cannot contain newlines")
 	case r == rawStringEnd:
@@ -654,7 +647,7 @@ func lexRawString(lx *lexer) stateFn {
 func lexMultilineRawString(lx *lexer) stateFn {
 	switch lx.next() {
 	case eof:
-		return lx.errorf(`unexpected EOF; expected "'''"`)
+		return lx.errorf("unexpected EOF")
 	case rawStringEnd:
 		if lx.accept(rawStringEnd) {
 			if lx.accept(rawStringEnd) {
@@ -737,21 +730,25 @@ func lexLongUnicodeEscape(lx *lexer) stateFn {
 	return lx.pop()
 }
 
-// lexNumberOrDateStart consumes either an integer, a float, or datetime.
+// lexNumberOrDateStart processes the first character of a value which begins
+// with a digit. It exists to catch values starting with '0', so that
+// lexBaseNumberOrDate can differentiate base prefixed integers from other
+// types.
 func lexNumberOrDateStart(lx *lexer) stateFn {
 	r := lx.next()
-	if isDigit(r) {
-		return lexNumberOrDate
-	}
 	switch r {
-	case '_':
-		return lexNumber
-	case 'e', 'E':
-		return lexFloat
-	case '.':
-		return lx.errorf("floats must start with a digit, not '.'")
+	case '0':
+		return lexBaseNumberOrDate
 	}
-	return lx.errorf("expected a digit but got %q", r)
+
+	if !isDigit(r) {
+		// The only way to reach this state is if the value starts
+		// with a digit, so specifically treat anything else as an
+		// error.
+		return lx.errorf("expected a digit but got %q", r)
+	}
+
+	return lexNumberOrDate
 }
 
 // lexNumberOrDate consumes either an integer, float or datetime.
@@ -761,10 +758,10 @@ func lexNumberOrDate(lx *lexer) stateFn {
 		return lexNumberOrDate
 	}
 	switch r {
-	case '-':
+	case '-', ':':
 		return lexDatetime
 	case '_':
-		return lexNumber
+		return lexDecimalNumber
 	case '.', 'e', 'E':
 		return lexFloat
 	}
@@ -791,32 +788,133 @@ func lexDatetime(lx *lexer) stateFn {
 	return lx.pop()
 }
 
-// lexNumberStart consumes either an integer or a float. It assumes that a sign
-// has already been read, but that *no* digits have been consumed.
-// lexNumberStart will move to the appropriate integer or float states.
-func lexNumberStart(lx *lexer) stateFn {
-	// We MUST see a digit. Even floats have to start with a digit.
+// lexHexInteger consumes a hexadecimal integer after seeing the '0x' prefix.
+func lexHexInteger(lx *lexer) stateFn {
 	r := lx.next()
-	if !isDigit(r) {
-		if r == '.' {
-			return lx.errorf("floats must start with a digit, not '.'")
+	if isHexadecimal(r) {
+		return lexHexInteger
+	}
+	switch r {
+	case '_':
+		return lexHexInteger
+	}
+
+	lx.backup()
+	lx.emit(itemInteger)
+	return lx.pop()
+}
+
+// lexOctalInteger consumes an octal integer after seeing the '0o' prefix.
+func lexOctalInteger(lx *lexer) stateFn {
+	r := lx.next()
+	if isOctal(r) {
+		return lexOctalInteger
+	}
+	switch r {
+	case '_':
+		return lexOctalInteger
+	}
+
+	lx.backup()
+	lx.emit(itemInteger)
+	return lx.pop()
+}
+
+// lexBinaryInteger consumes a binary integer after seeing the '0b' prefix.
+func lexBinaryInteger(lx *lexer) stateFn {
+	r := lx.next()
+	if isBinary(r) {
+		return lexBinaryInteger
+	}
+	switch r {
+	case '_':
+		return lexBinaryInteger
+	}
+
+	lx.backup()
+	lx.emit(itemInteger)
+	return lx.pop()
+}
+
+// lexDecimalNumber consumes a decimal float or integer.
+func lexDecimalNumber(lx *lexer) stateFn {
+	r := lx.next()
+	if isDigit(r) {
+		return lexDecimalNumber
+	}
+	switch r {
+	case '.', 'e', 'E':
+		return lexFloat
+	case '_':
+		return lexDecimalNumber
+	}
+
+	lx.backup()
+	lx.emit(itemInteger)
+	return lx.pop()
+}
+
+// lexDecimalNumber consumes the first digit of a number beginning with a sign.
+// It assumes the sign has already been consumed. Values which start with a sign
+// are only allowed to be decimal integers or floats.
+func lexDecimalNumberStart(lx *lexer) stateFn {
+	r := lx.next()
+	// special error cases to give users better error messages
+	switch r {
+	case '0':
+		// possibly surprising element of the spec
+		p := lx.peek()
+		switch p {
+		case 'b', 'o', 'x':
+			return lx.errorf("cannot use sign with non-decimal numbers: '%s%c'", lx.current(), p)
 		}
-		return lx.errorf("expected a digit but got %q", r)
+	case '.':
+		return lx.errorf("floats must start with a digit, not '.'")
 	}
-	return lexNumber
+
+	if isDigit(r) {
+		return lexDecimalNumber
+	}
+
+	return lx.errorf("expected a digit but got %q", r)
 }
 
-// lexNumber consumes an integer or a float after seeing the first digit.
-func lexNumber(lx *lexer) stateFn {
+// lexBaseNumberOrDate differentiates between the possible values which
+// start with '0'. It assumes that before reaching this state, the initial '0'
+// has been consumed.
+func lexBaseNumberOrDate(lx *lexer) stateFn {
 	r := lx.next()
+	// Note: All datetimes start with at least two digits, so we don't
+	// handle date characters (':', '-', etc.) here.
 	if isDigit(r) {
-		return lexNumber
+		return lexNumberOrDate
 	}
 	switch r {
 	case '_':
-		return lexNumber
+		// Can only be decimal, because there can't be an underscore
+		// between the '0' and the base designator, and dates can't
+		// contain underscores.
+		return lexDecimalNumber
 	case '.', 'e', 'E':
 		return lexFloat
+	case 'b':
+		r = lx.peek()
+		if !isBinary(r) {
+			lx.errorf("not a binary number: '%s%c'", lx.current(), r)
+		}
+		return lexBinaryInteger
+	case 'o':
+		r = lx.peek()
+		if !isOctal(r) {
+			lx.errorf("not an octal number: '%s%c'", lx.current(), r)
+		}
+		return lexOctalInteger
+	case 'x':
+		r = lx.peek()
+		if !isHexadecimal(r) {
+			lx.errorf("not a hexidecimal number: '%s%c'", lx.current(), r)
+		}
+		return lexHexInteger
 	}
 
 	lx.backup()
@@ -909,6 +1007,14 @@ func isHexadecimal(r rune) bool {
 		(r >= 'A' && r <= 'F')
 }
 
+func isOctal(r rune) bool {
+	return r >= '0' && r <= '7'
+}
+
+func isBinary(r rune) bool {
+	return r == '0' || r == '1'
+}
+
 func isBareKeyChar(r rune) bool {
 	return (r >= 'A' && r <= 'Z') ||
 		(r >= 'a' && r <= 'z') ||
diff --git a/parse.go b/parse.go
index bbd8687..d4b5066 100644
--- a/parse.go
+++ b/parse.go
@@ -208,11 +208,11 @@ func (p *parser) value(it item) (interface{}, tomlType) {
 			p.panicf("Invalid integer %q: underscores must be surrounded by digits",
 				it.val)
 		}
-		val := strings.Replace(it.val, "_", "", -1)
 		if numHasLeadingZero(it.val) {
 			p.panicf("Invalid integer %q: cannot have leading zeroes", it.val)
 		}
-		num, err := strconv.ParseInt(val, 10, 64)
+
+		num, err := strconv.ParseInt(it.val, 0, 64)
 		if err != nil {
 			// Distinguish integer values. Normally, it'd be a bug if the lexer
 			// provides an invalid integer, but it's possible that the number is
@@ -341,10 +341,10 @@ func (p *parser) value(it item) (interface{}, tomlType) {
 	panic("unreachable")
 }
 
-// numHasLeadingZero checks if this number has leading zeroes, allowing for '0'
-// and +/- signs.
+// numHasLeadingZero checks if this number has leading zeroes, allowing for '0',
+// +/- signs, and base prefixes.
 func numHasLeadingZero(s string) bool {
-	if len(s) > 1 && s[0] == '0' { // >1 to allow "0"
+	if len(s) > 1 && s[0] == '0' && isDigit(rune(s[1])) { // >1 to allow "0" and isDigit to allow 0x
 		return true
 	}
 	if len(s) > 2 && (s[0] == '-' || s[0] == '+') && s[1] == '0' {
@@ -362,10 +362,11 @@ func numUnderscoresOK(s string) bool {
 			if !accept {
 				return false
 			}
-			accept = false
-			continue
 		}
-		accept = true
+
+		// isHexadecimal is a superset of all the permissable characters
+		// surrounding an underscore.
+		accept = isHexadecimal(r)
 	}
 	return accept
 }


Test patch:
diff --git a/decode_test.go b/decode_test.go
index d18bff4..0bce6d1 100644
--- a/decode_test.go
+++ b/decode_test.go
@@ -552,12 +552,25 @@ func TestDecodeInts(t *testing.T) {
 		want int64
 	}{
 		{"0", 0},
+		{"0x0", 0},
+		{"0x00", 0},
+		{"0o0", 0},
+		{"0o00", 0},
+		{"0b0", 0},
+		{"0b00", 0},
 		{"+0", 0},
 		{"-0", 0},
 		{"+99", 99},
 		{"-10", -10},
 		{"1_234_567", 1234567},
 		{"1_2_3_4", 1234},
+		{"0xdead_BEEF", 0xdeadbeef},
+		{"0b0_1_1_0", 0b0110},
+		{"0o7_7_7", 0o777},
+		{"0x12345", 0x12345},
+		{"0x0987", 0x987},
+		{"0b1101", 0xd},
+		{"0o777", 0x1ff},
 		{"-9_223_372_036_854_775_808", math.MinInt64},
 		{"9_223_372_036_854_775_807", math.MaxInt64},
 	} {
@@ -583,6 +596,7 @@ func TestDecodeFloats(t *testing.T) {
 		{"+1.0", 1},
 		{"3.1415", 3.1415},
 		{"-0.01", -0.01},
+		{"0.1", 0.1},
 		{"5e+22", 5e22},
 		{"1e6", 1e6},
 		{"-2E-2", -2e-2},
@@ -615,16 +629,26 @@ func TestDecodeMalformedNumbers(t *testing.T) {
 		{"1e2e3", "Invalid float value"},
 		{"_123", "expected value"},
 		{"123_", "surrounded by digits"},
+		{"0b0_", "surrounded by digits"},
 		{"1._23", "surrounded by digits"},
 		{"1e__23", "surrounded by digits"},
 		{"123.", "must be followed by one or more digits"},
 		{"1.e2", "must be followed by one or more digits"},
+		{"00", "cannot have leading zeroes"},
 		{"01", "cannot have leading zeroes"},
 		{"+01", "cannot have leading zeroes"},
 		{"-01", "cannot have leading zeroes"},
 		{"01.2", "cannot have leading zeroes"},
 		{"-01.2", "cannot have leading zeroes"},
 		{"+01.2", "cannot have leading zeroes"},
+		{"0x_d00d", "not a hexidecimal number: '0x_'"},
+		{"0b_0", "not a binary number: '0b_'"},
+		{"0z", "but got 'z' instead"},
+		{"+0x3", "cannot use sign with non-decimal numbers: '+0x'"},
+		{"-0xf00", "cannot use sign with non-decimal numbers: '-0x'"},
+		{"0B0", "got 'B' instead"},
+		{"0X0", "got 'X' instead"},
+		{"0O0", "got 'O' instead"},
 	} {
 		t.Run(tt.s, func(t *testing.T) {
 			var x struct{ N interface{} }
@@ -634,7 +658,7 @@ func TestDecodeMalformedNumbers(t *testing.T) {
 				t.Fatalf("got nil, want error containing %q", tt.want)
 			}
 			if !strings.Contains(err.Error(), tt.want) {
-				t.Errorf("got %q, want error containing %q", err, tt.want)
+				t.Errorf("\nhave: %q\nwant: %q", err, tt.want)
 			}
 		})
 	}
@@ -989,14 +1013,17 @@ func TestDecodeErrors(t *testing.T) {
 		wantErr string
 		lastKey bool
 	}{
-		{`x`, "unexpected EOF; expected key separator '='", false},
-		{`x  `, "unexpected EOF; expected key separator '='", true},
-		{`x="`, `unexpected EOF; expected '"'`, true},
-		{`x="""`, `unexpected EOF; expected '"""'`, true},
-		{`x='`, `unexpected EOF; expected "'"`, true},
-		{`x='''`, `unexpected EOF; expected "'''"`, true},
-		{"x = ", "unexpected EOF; expected value", true},
-		{"x = \n", "expected value but found '\\n' instead", true},
+		{`x="`, "unexpected EOF", true},
+		{`x='`, "unexpected EOF", true},
+		{`x='''`, "unexpected EOF", true},
+
+		// TODO: Improve these
+		// {"x = ", "X", false},            // expected value but found '\x00' instead
+		// {"x = \n", "X", false},          // Near line 1 (last key parsed 'abc'): expected value but found '\n' instead
+		// {"x = \nzxc = 1\n", "X", false}, // Near line 1 (last key parsed 'abc'): expected value but found '\n' instead
+		// {`x  `, "X", false},             // expected key separator '=', but got '\x00' instead
+		// {`x`, "X", false},               // Near line 0 (last key parsed ''): bare keys cannot contain '\x00'
+		// {"x\n", "X", false},             // Near line 0 (last key parsed ''): bare keys cannot contain '\n'
 
 		// Cases found by fuzzing in #155
 		{`""` + "\ufffd", "expected key separator", false}, // used to panic with index out of range
@@ -1163,7 +1190,7 @@ c = 001  # invalid
 		LastKey: "c",
 		Message: `Invalid integer "001": cannot have leading zeroes`,
 	}
-	if !strings.Contains(pErr.Message, `Invalid integer "001"`) ||
+	if !strings.Contains(pErr.Message, want.Message) ||
 		pErr.Line != want.Line ||
 		pErr.LastKey != want.LastKey {
 		t.Errorf("unexpected data\nhave: %#v\nwant: %#v", pErr, want)