Repository: alecthomas/kong
Pull request #338 (2022-10-17 12:58:58)

Fix large number decoding for unsigned types in JSON
I noticed that after merging [the fix for large numbers (#334)](https://github.com/alecthomas/kong/pull/334), the build was broken on the `master` branch [due to a linter complaint](https://github.com/alecthomas/kong/actions/runs/3101957356/jobs/5023801376).

Problem is that the `uintDecoder` is no longer a code duplication of `intDecoder`, because the adjustments by #334 only changed `intDecoder`. It’s not the linter directive that’s obsolete, but I rather think that `uintDecoder` must have that fix as well, because it suffers from the same problem.

In addition to the actual fix, I slightly adjusted the tests:

- I converted the Github link to an inlined explanation, just for convenience.
- Both cases for `int` and `uint` are now covered. I think it makes sense to have them together in one single test, since they are so closely related. I enclosed each one in a block, so that they have their own scope.
- For the sake of completeness/consistency, I added another test for floats, which covers the `floatDecoder`.
- Instead of having a helper variable `n` in the test, I think it’s more robust to inline the number. That way, we don’t rely on how `fmt.Sprintf` would serialise the number within the test.
- Writing `n := 100000` would infer the type to `int`, but [`int` is platform-dependent](https://go.dev/tour/basics/11). It’s safer to assign the type explicitly (e.g. `int64`), to make the test deterministic.

One note aside: the tests don’t seem to be run on PRs. Maybe that could be fixed, which would uncover build failures before they go into the `master` branch.

jotaen: > One note aside: the tests don’t seem to be run on PRs. Maybe that could be fixed, which would uncover build failures before they go into the master branch.

I just saw that is only [an issue for first-time contributors](https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks).
alecthomas: The tests do run, they just require approval first. This is a GitHub security measure.

Patch:
diff --git a/mapper.go b/mapper.go
index 6d36b15..65f9814 100644
--- a/mapper.go
+++ b/mapper.go
@@ -392,9 +392,12 @@ func uintDecoder(bits int) MapperFunc { // nolint: dupl
 		case string:
 			sv = v
 
-		case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64, float32, float64:
+		case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64:
 			sv = fmt.Sprintf("%v", v)
 
+		case float32, float64:
+			sv = fmt.Sprintf("%0.f", v)
+
 		default:
 			return fmt.Errorf("expected an int but got %q (%T)", t, t.Value)
 		}


Test patch:
diff --git a/mapper_test.go b/mapper_test.go
index 804ac39..e86fed7 100644
--- a/mapper_test.go
+++ b/mapper_test.go
@@ -401,18 +401,51 @@ func TestNumbers(t *testing.T) {
 }
 
 func TestJSONLargeNumber(t *testing.T) {
-	// https://github.com/alecthomas/kong/pull/334
-	const n = 1000000
-	var cli struct {
-		N int64
+	// Make sure that large numbers are not internally converted to
+	// scientific notation when the mapper parses the values.
+	// (Scientific notation is e.g. `1e+06` instead of `1000000`.)
+
+	// Large signed integers:
+	{
+		var cli struct {
+			N int64
+		}
+		json := `{"n": 1000000}`
+		r, err := kong.JSON(strings.NewReader(json))
+		assert.NoError(t, err)
+		parser := mustNew(t, &cli, kong.Resolvers(r))
+		_, err = parser.Parse([]string{})
+		assert.NoError(t, err)
+		assert.Equal(t, int64(1000000), cli.N)
+	}
+
+	// Large unsigned integers:
+	{
+		var cli struct {
+			N uint64
+		}
+		json := `{"n": 1000000}`
+		r, err := kong.JSON(strings.NewReader(json))
+		assert.NoError(t, err)
+		parser := mustNew(t, &cli, kong.Resolvers(r))
+		_, err = parser.Parse([]string{})
+		assert.NoError(t, err)
+		assert.Equal(t, uint64(1000000), cli.N)
+	}
+
+	// Large floats:
+	{
+		var cli struct {
+			N float64
+		}
+		json := `{"n": 1000000.1}`
+		r, err := kong.JSON(strings.NewReader(json))
+		assert.NoError(t, err)
+		parser := mustNew(t, &cli, kong.Resolvers(r))
+		_, err = parser.Parse([]string{})
+		assert.NoError(t, err)
+		assert.Equal(t, float64(1000000.1), cli.N)
 	}
-	json := fmt.Sprintf(`{"n": %d}`, n)
-	r, err := kong.JSON(strings.NewReader(json))
-	assert.NoError(t, err)
-	parser := mustNew(t, &cli, kong.Resolvers(r))
-	_, err = parser.Parse([]string{})
-	assert.NoError(t, err)
-	assert.Equal(t, n, cli.N)
 }
 
 func TestFileMapper(t *testing.T) {