Skip to content

Commit

Permalink
Follow up to compliant Int work (#3430)
Browse files Browse the repository at this point in the history 
* remove explicit uuid binding in the init template

* unify int / uint casting, sign and overflow errors with assertable types

* lint

* fix test

* revert init template change

* fix coverage gaps

* lint

* argh, int 64
  • Loading branch information
@phughes-scwx
phughes-scwx authored Dec 10, 2024
1 parent 7ba0197 commit 07c67c1
Show file tree
Hide file tree
Showing 5 changed files with 318 additions and 40 deletions.
2 changes: 1 addition & 1 deletion codegen/testserver/compliant-int/compliant_int_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func TestIntegration(t *testing.T) {
}
err := c.Post(`query { echoIntToInt(n: 2147483648) }`, &resp)
if tc.willError {
require.EqualError(t, err, `[{"message":"2147483648 overflows 32-bit integer","path":["echoIntToInt","n"]}]`)
require.EqualError(t, err, `[{"message":"2147483648 overflows signed 32-bit integer","path":["echoIntToInt","n"]}]`)
require.Equal(t, 0, resp.EchoIntToInt)
return
}
Expand Down
29 changes: 26 additions & 3 deletions graphql/int.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,40 @@ func UnmarshalInt32(v any) (int32, error) {
}
}

// IntegerError is an error type that allows users to identify errors associated
// with receiving an integer value that is not valid for the specific integer
// type designated by the API. IntegerErrors designate otherwise valid unsigned
// or signed 64-bit integers that are invalid in a specific context: they do not
// designate integers that overflow 64-bit versions of the current type.
type IntegerError struct {
Message string
}

func (e IntegerError) Error() string {
return e.Message
}

type Int32OverflowError struct {
Value int64
*IntegerError
}

func newInt32OverflowError(i int64) *Int32OverflowError {
return &Int32OverflowError{
Value: i,
IntegerError: &IntegerError{
Message: fmt.Sprintf("%d overflows signed 32-bit integer", i),
},
}
}

func (e *Int32OverflowError) Error() string {
return fmt.Sprintf("%d overflows 32-bit integer", e.Value)
func (e *Int32OverflowError) Unwrap() error {
return e.IntegerError
}

func safeCastInt32(i int64) (int32, error) {
if i > math.MaxInt32 || i < math.MinInt32 {
return 0, &Int32OverflowError{i}
return 0, newInt32OverflowError(i)
}
return int32(i), nil
}
42 changes: 32 additions & 10 deletions graphql/int_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ func TestInt(t *testing.T) {
})

t.Run("unmarshal", func(t *testing.T) {
assert.Equal(t, 0, mustUnmarshalInt(t, nil))
assert.Equal(t, 123, mustUnmarshalInt(t, 123))
assert.Equal(t, 123, mustUnmarshalInt(t, int64(123)))
assert.Equal(t, 123, mustUnmarshalInt(t, json.Number("123")))
Expand All @@ -35,6 +36,7 @@ func TestInt32(t *testing.T) {
})

t.Run("unmarshal", func(t *testing.T) {
assert.Equal(t, int32(0), mustUnmarshalInt32(t, nil))
assert.Equal(t, int32(123), mustUnmarshalInt32(t, 123))
assert.Equal(t, int32(123), mustUnmarshalInt32(t, int64(123)))
assert.Equal(t, int32(123), mustUnmarshalInt32(t, json.Number("123")))
Expand All @@ -48,23 +50,42 @@ func TestInt32(t *testing.T) {
v any
err string
}{
{"positive int overflow", math.MaxInt32 + 1, "2147483648 overflows 32-bit integer"},
{"negative int overflow", math.MinInt32 - 1, "-2147483649 overflows 32-bit integer"},
{"positive int overflow", int64(math.MaxInt32 + 1), "2147483648 overflows 32-bit integer"},
{"negative int overflow", int64(math.MinInt32 - 1), "-2147483649 overflows 32-bit integer"},
{"positive json.Number overflow", json.Number("2147483648"), "2147483648 overflows 32-bit integer"},
{"negative json.Number overflow", json.Number("-2147483649"), "-2147483649 overflows 32-bit integer"},
{"positive string overflow", "2147483648", "2147483648 overflows 32-bit integer"},
{"negative string overflow", "-2147483649", "-2147483649 overflows 32-bit integer"},
{"positive int overflow", math.MaxInt32 + 1, "2147483648 overflows signed 32-bit integer"},
{"negative int overflow", math.MinInt32 - 1, "-2147483649 overflows signed 32-bit integer"},
{"positive int64 overflow", int64(math.MaxInt32 + 1), "2147483648 overflows signed 32-bit integer"},
{"negative int64 overflow", int64(math.MinInt32 - 1), "-2147483649 overflows signed 32-bit integer"},
{"positive json.Number overflow", json.Number("2147483648"), "2147483648 overflows signed 32-bit integer"},
{"negative json.Number overflow", json.Number("-2147483649"), "-2147483649 overflows signed 32-bit integer"},
{"positive string overflow", "2147483648", "2147483648 overflows signed 32-bit integer"},
{"negative string overflow", "-2147483649", "-2147483649 overflows signed 32-bit integer"},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
var int32OverflowErr *Int32OverflowError
var intErr *IntegerError

res, err := UnmarshalInt32(tc.v)
assert.EqualError(t, err, tc.err) //nolint:testifylint // An error assertion makes more sense.
assert.EqualError(t, err, tc.err) //nolint:testifylint // An error assertion makes more sense.
assert.ErrorAs(t, err, &int32OverflowErr) //nolint:testifylint // An error assertion makes more sense.
assert.ErrorAs(t, err, &intErr) //nolint:testifylint // An error assertion makes more sense.
assert.Equal(t, int32(0), res)
})
}
})

t.Run("invalid string numbers are not integer errors", func(t *testing.T) {
var intErr *IntegerError

res, err := UnmarshalInt32("-1.03")
assert.EqualError(t, err, "strconv.ParseInt: parsing \"-1.03\": invalid syntax") //nolint:testifylint // An error assertion makes more sense.
assert.NotErrorAs(t, err, &intErr)
assert.Equal(t, int32(0), res)

res, err = UnmarshalInt32(json.Number(" 1"))
assert.EqualError(t, err, "strconv.ParseInt: parsing \" 1\": invalid syntax") //nolint:testifylint // An error assertion makes more sense.
assert.NotErrorAs(t, err, &intErr)
assert.Equal(t, int32(0), res)
})
}

func mustUnmarshalInt32(t *testing.T, v any) int32 {
Expand All @@ -75,10 +96,11 @@ func mustUnmarshalInt32(t *testing.T, v any) int32 {

func TestInt64(t *testing.T) {
t.Run("marshal", func(t *testing.T) {
assert.Equal(t, "123", m2s(MarshalInt32(123)))
assert.Equal(t, "123", m2s(MarshalInt64(123)))
})

t.Run("unmarshal", func(t *testing.T) {
assert.Equal(t, int64(0), mustUnmarshalInt64(t, nil))
assert.Equal(t, int64(123), mustUnmarshalInt64(t, 123))
assert.Equal(t, int64(123), mustUnmarshalInt64(t, int64(123)))
assert.Equal(t, int64(123), mustUnmarshalInt64(t, json.Number("123")))
Expand Down
127 changes: 107 additions & 20 deletions graphql/uint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"math"
"strconv"
)

Expand All @@ -18,21 +19,33 @@ func UnmarshalUint(v any) (uint, error) {
switch v := v.(type) {
case string:
u64, err := strconv.ParseUint(v, 10, 64)
if err != nil {
var strconvErr *strconv.NumError
if errors.As(err, &strconvErr) && isSignedInteger(v) {
return 0, newUintSignError(v)
}
return 0, err
}
return uint(u64), err
case int:
if v < 0 {
return 0, errors.New("cannot convert negative numbers to uint")
return 0, newUintSignError(strconv.FormatInt(int64(v), 10))
}

return uint(v), nil
case int64:
if v < 0 {
return 0, errors.New("cannot convert negative numbers to uint")
return 0, newUintSignError(strconv.FormatInt(v, 10))
}

return uint(v), nil
case json.Number:
u64, err := strconv.ParseUint(string(v), 10, 64)
if err != nil {
var strconvErr *strconv.NumError
if errors.As(err, &strconvErr) && isSignedInteger(string(v)) {
return 0, newUintSignError(string(v))
}
return 0, err
}
return uint(u64), err
case nil:
return 0, nil
Expand All @@ -50,21 +63,35 @@ func MarshalUint64(i uint64) Marshaler {
func UnmarshalUint64(v any) (uint64, error) {
switch v := v.(type) {
case string:
return strconv.ParseUint(v, 10, 64)
i, err := strconv.ParseUint(v, 10, 64)
if err != nil {
var strconvErr *strconv.NumError
if errors.As(err, &strconvErr) && isSignedInteger(v) {
return 0, newUintSignError(v)
}
return 0, err
}
return i, nil
case int:
if v < 0 {
return 0, errors.New("cannot convert negative numbers to uint64")
return 0, newUintSignError(strconv.FormatInt(int64(v), 10))
}

return uint64(v), nil
case int64:
if v < 0 {
return 0, errors.New("cannot convert negative numbers to uint64")
return 0, newUintSignError(strconv.FormatInt(v, 10))
}

return uint64(v), nil
case json.Number:
return strconv.ParseUint(string(v), 10, 64)
i, err := strconv.ParseUint(string(v), 10, 64)
if err != nil {
var strconvErr *strconv.NumError
if errors.As(err, &strconvErr) && isSignedInteger(string(v)) {
return 0, newUintSignError(string(v))
}
return 0, err
}
return i, nil
case nil:
return 0, nil
default:
Expand All @@ -81,32 +108,92 @@ func MarshalUint32(i uint32) Marshaler {
func UnmarshalUint32(v any) (uint32, error) {
switch v := v.(type) {
case string:
iv, err := strconv.ParseUint(v, 10, 32)
iv, err := strconv.ParseUint(v, 10, 64)
if err != nil {
var strconvErr *strconv.NumError
if errors.As(err, &strconvErr) && isSignedInteger(v) {
return 0, newUintSignError(v)
}
return 0, err
}
return uint32(iv), nil
return safeCastUint32(iv)
case int:
if v < 0 {
return 0, errors.New("cannot convert negative numbers to uint32")
return 0, newUintSignError(strconv.FormatInt(int64(v), 10))
}

return uint32(v), nil
return safeCastUint32(uint64(v))
case int64:
if v < 0 {
return 0, errors.New("cannot convert negative numbers to uint32")
return 0, newUintSignError(strconv.FormatInt(v, 10))
}

return uint32(v), nil
return safeCastUint32(uint64(v))
case json.Number:
iv, err := strconv.ParseUint(string(v), 10, 32)
iv, err := strconv.ParseUint(string(v), 10, 64)
if err != nil {
var strconvErr *strconv.NumError
if errors.As(err, &strconvErr) && isSignedInteger(string(v)) {
return 0, newUintSignError(string(v))
}
return 0, err
}
return uint32(iv), nil
return safeCastUint32(iv)
case nil:
return 0, nil
default:
return 0, fmt.Errorf("%T is not an uint", v)
}
}

type UintSignError struct {
*IntegerError
}

func newUintSignError(v string) *UintSignError {
return &UintSignError{
IntegerError: &IntegerError{
Message: fmt.Sprintf("%v is an invalid unsigned integer: includes sign", v),
},
}
}

func (e *UintSignError) Unwrap() error {
return e.IntegerError
}

func isSignedInteger(v string) bool {
if v == "" {
return false
}
if v[0] != '-' && v[0] != '+' {
return false
}
if _, err := strconv.ParseUint(v[1:], 10, 64); err == nil {
return true
}
return false
}

type Uint32OverflowError struct {
Value uint64
*IntegerError
}

func newUint32OverflowError(i uint64) *Uint32OverflowError {
return &Uint32OverflowError{
Value: i,
IntegerError: &IntegerError{
Message: fmt.Sprintf("%d overflows unsigned 32-bit integer", i),
},
}
}

func (e *Uint32OverflowError) Unwrap() error {
return e.IntegerError
}

func safeCastUint32(i uint64) (uint32, error) {
if i > math.MaxUint32 {
return 0, newUint32OverflowError(i)
}
return uint32(i), nil
}
Loading

0 comments on commit 07c67c1

Please sign in to comment.