refactor number parsing a bit

There are subtle inconsistencies, like "01" being a valid decimal but not
a valid integer, which vaguely bug me. Cleaning this up, and the corresponding
parser logic.

A number can't have leading spaces because the grammar doesn't
put spaces in them in the first place, so stop accepting them in the
number syntax. This should never have any impact on anything,
it's just simpler.

Update a couple of test cases to reflect this -- no longer testing
that trailing spaces are okay, now testing that they're not, for
instance.

pql/decimal.go

@@ -309,7 +309,6 @@ func ParseDecimal(s string) (Decimal, error) {
 	var err error
 
 	// General steps:
-	// - Trim leading whitespace/zeros
 	// - Get the sign value
 	// - Trim leading zeros
 	// - Push characters into a buffer
@@ -328,26 +327,23 @@ func ParseDecimal(s string) (Decimal, error) {
 		switch state {
 		case stateSign:
 			switch s[i] {
-			case ' ':
-				continue
 			case '-':
 				sign = true
 				fallthrough
 			case '+':
 				state = stateLeadingZeros
-			default:
-				state = stateLeadingZeros
-				i--
+				// Resume loop and look at next character
+				continue
 			}
+			state = stateLeadingZeros
+			fallthrough
 		case stateLeadingZeros:
-			switch s[i] {
-			case '0':
+			if s[i] == '0' {
 				foundLeadingZero = true
 				continue
-			default:
-				state = stateMantissa
-				i--
 			}
+			state = stateMantissa
+			fallthrough
 		case stateMantissa:
 			switch s[i] {
 			case '.':
@@ -371,23 +367,14 @@ func ParseDecimal(s string) (Decimal, error) {
 		return Decimal{}, errors.New("decimal string is empty")
 	}
 
-	// Trim trailing zeros/spaces of mantissa
-	// for any portion that would have been to the
-	// right of the decimal.
+	// Trim trailing zeros from mantissa. If we ended up with no
+	// characters at all in string, thus, pos == 0, the loop doesn't
+	// happen and we pick [:0], which is correct, probably.
 	trimZeroCnt := 0
-	trimSpaceCnt := 0
-	for i := len(mantissa) - 1; i >= 0; i-- {
-		switch mantissa[i] {
-		case uint8(0), uint8(32): // nil/space
-			trimSpaceCnt++
-			continue
-		case uint8(48): // zero
-			trimZeroCnt++
-			continue
-		}
-		break
+	for i := pos - 1; i >= 0 && mantissa[i] == '0'; i-- {
+		trimZeroCnt++
 	}
-	mantissa = mantissa[:len(mantissa)-trimSpaceCnt-trimZeroCnt]
+	mantissa = mantissa[:pos-trimZeroCnt]
 
 	// Based on where (or if) the decimal was found,
 	// calculate scale.

pql/decimal_test.go

@@ -34,10 +34,9 @@ func TestDecimal(t *testing.T) {
 			{"0", pql.Decimal{0, 0}, ""},
 			{"-0", pql.Decimal{0, 0}, ""},
 			{"0.0", pql.Decimal{0, 0}, ""},
+			{"0.", pql.Decimal{0, 0}, ""},
 			{"-0.00", pql.Decimal{0, 0}, ""},
 			{"123.4567", pql.Decimal{1234567, 4}, ""},
-			{"  123.4567", pql.Decimal{1234567, 4}, ""},
-			{"  123.4567  ", pql.Decimal{1234567, 4}, ""},
 			{"123.456700", pql.Decimal{1234567, 4}, ""},
 			{"00123.4567", pql.Decimal{1234567, 4}, ""},
 			{"+123.4567", pql.Decimal{1234567, 4}, ""},
@@ -56,8 +55,7 @@ func TestDecimal(t *testing.T) {
 			{".123", pql.Decimal{123, 3}, ""},
 			{"0.123", pql.Decimal{123, 3}, ""},
 			{"0.001230", pql.Decimal{123, 5}, ""},
-			{" 0.001230 ", pql.Decimal{123, 5}, ""},
-			{"-0.001230 ", pql.Decimal{-123, 5}, ""},
+			{"-0.001230", pql.Decimal{-123, 5}, ""},
 
 			// int64 edges.
 			{".000009223372036854775807", pql.Decimal{9223372036854775807, 24}, ""},
@@ -83,6 +81,10 @@ func TestDecimal(t *testing.T) {
 			{"abc", pql.Decimal{}, "invalid syntax"},
 			{"0.12.3", pql.Decimal{}, "invalid decimal string"},
 			{"--12300", pql.Decimal{}, "invalid syntax"},
+			{"  123.4567  ", pql.Decimal{}, "invalid syntax"},
+			{"  123.4567", pql.Decimal{}, "invalid syntax"},
+			{"123.4567 ", pql.Decimal{}, "invalid syntax"},
+			{"0.a", pql.Decimal{}, "invalid syntax"},
 
 			// These are no longer error cases since we introduced precision adjustment.
 			//{"922337203685477580.9", pql.Decimal{}, "value out of range"},
@@ -94,12 +96,12 @@ func TestDecimal(t *testing.T) {
 			dec, err := pql.ParseDecimal(test.s)
 			if test.expErr != "" {
 				if err == nil || !strings.Contains(err.Error(), test.expErr) {
-					t.Fatalf("test %d expected error to contain: %s, but got: %v", i, test.expErr, err)
+					t.Fatalf("test %d parsing string `%s`: expected error to contain: %s, but got: %v", i, test.s, test.expErr, err)
 				}
 			} else if err != nil {
 				t.Fatalf("test %d parsing string `%s`: %s", i, test.s, err)
 			} else if dec != test.exp {
-				t.Fatalf("test %d expected: %v, but got: %v", i, test.exp, dec)
+				t.Fatalf("test %d parsing string `%s`: expected: %v, but got: %v", i, test.s, test.exp, dec)
 			}
 		}
 	})
@@ -141,8 +143,6 @@ func TestDecimal(t *testing.T) {
 			exp string
 		}{
 			{"123.4567", "123.4567"},
-			{"  123.4567", "123.4567"},
-			{"  123.4567  ", "123.4567"},
 			{"123.456700", "123.4567"},
 			{"00123.4567", "123.4567"},
 			{"+123.4567", "123.4567"},
@@ -161,8 +161,8 @@ func TestDecimal(t *testing.T) {
 
 			{"0.123", "0.123"},
 			{"0.001230", "0.00123"},
-			{" 0.001230 ", "0.00123"},
-			{"-0.001230 ", "-0.00123"},
+			{"+0.001230", "0.00123"},
+			{"-0.001230", "-0.00123"},
 		}
 		for i, test := range tests {
 			dec, err := pql.ParseDecimal(test.s)

pql/pql.peg

@@ -4,6 +4,7 @@ type PQL Peg {
      Query
 }
 
+# All input queries consist of a sequence of calls, at the top level.
 Calls <- sp (Call sp)* !.
 Call <-  "Set" {p.startCall("Set")} open col comma args (comma timestamp)? close {p.endCall()}
        / "SetRowAttrs" {p.startCall("SetRowAttrs")} open posfield comma row comma args close {p.endCall()}
@@ -29,19 +30,18 @@ COND <-   '><' { p.addBTWN() }
         / '>' { p.addGT() }
 
 conditional <- {p.startConditional()} condint condLT condfield condLT condint {p.endConditional()}
-condint <- < '-'? [0-9]* '.' [0-9]+ / '0' / '-'? [1-9] [0-9]* > sp {p.condAdd(text)}
+condint <- < decimal > sp {p.condAdd(text)}
 condLT <- <('<=' / '<')> sp {p.condAdd(text)}
 condfield <- <fieldExpr> sp {p.condAdd(text)}
 
 value <-    item
-          / lbrack { p.startList() } list rbrack { p.endList() }
-list <- item (comma list)?
+          / lbrack { p.startList() } items rbrack { p.endList() }
+items <- item (comma items)?
 item <-   'null' &(comma / close) { p.addVal(nil) }
          / 'true' &(comma / close) { p.addVal(true) }
          / 'false' &(comma / close) { p.addVal(false) }
          / timestampfmt { p.addVal(text) }
-         / < '-'? [0-9]+ ('.'[0-9]*)? > { p.addNumVal(text) }
-         / < '-'? '.'[0-9]+ > { p.addNumVal(text) }
+         / < decimal > { p.addNumVal(text) }
          / < IDENT > { p.startCall(text) } open allargs comma? close { p.addVal(p.endCall()) }
          / < ([[A-Z]] / [0-9] / '-' / '_' / ':')+ > { p.addVal(text) }
          / < '"'  doublequotedstring '"' > { p.addVal(text) }
@@ -54,11 +54,10 @@ fieldExpr <- ( [[A-Z]] / '_' ) ( [[A-Z]] / [0-9] / '_' / '-' )*
 field <- <fieldExpr / reserved> { p.addField(text) }
 reserved <- '_row' / '_col' / '_start' / '_end' / '_timestamp' / '_field'
 posfield <- <fieldExpr> { p.addPosStr("_field", text) }
-uint <- [1-9] [0-9]* / '0'
-col <-   <uint> {p.addPosNum("_col", text)}
+col <-   < digits > {p.addPosNum("_col", text)}
         / < '\'' singlequotedstring '\'' > {p.addPosStr("_col", text)}
         / < '"' doublequotedstring '"' > {p.addPosStr("_col", text)}
-row <-   <uint> {p.addPosNum("_row", text)}
+row <-   < digits > {p.addPosNum("_row", text)}
         / < '\'' singlequotedstring '\'' > {p.addPosStr("_row", text)}
         / < '"' doublequotedstring '"' > {p.addPosStr("_row", text)}
 
@@ -70,7 +69,10 @@ comma <- sp ',' sp
 lbrack <- '[' sp
 rbrack <- sp ']' sp
 IDENT <- [[A-Z]] ([[A-Z]] / [0-9])*
-
+digits <- [0-9]+
+signedDigits <- '-'? digits
+decimal <- signedDigits ('.' digits?)?
+         / '-'? '.' digits
 
 timestampbasicfmt <- [0-9][0-9][0-9][0-9]'-'[01][0-9]'-'[0-3][0-9]'T'[0-9][0-9]':'[0-9][0-9]
 timestampfmt <- '"' <timestampbasicfmt> '"' / '\'' <timestampbasicfmt> '\'' / <timestampbasicfmt>