Skip to content

Commit

Permalink
fix: encoding negative integers should keep their sign
Browse files Browse the repository at this point in the history
[core][javascript][python][newtonsoft]

Fix #187
  • Loading branch information
MangelMaxime committed May 4, 2024
1 parent c883b29 commit fb379ff
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 13 deletions.
13 changes: 7 additions & 6 deletions packages/Thoth.Json.Core/Encode.fs
Original file line number Diff line number Diff line change
Expand Up @@ -91,36 +91,37 @@ module Encode =
let inline sbyte (value: sbyte) =
{ new IEncodable with
member _.Encode(helpers) =
helpers.encodeIntegralNumber (uint32 value)
helpers.encodeSignedIntegralNumber (int32 value)
}

let inline byte (value: byte) =
{ new IEncodable with
member _.Encode(helpers) =
helpers.encodeIntegralNumber (uint32 value)
helpers.encodeUnsignedIntegralNumber (uint32 value)
}

let inline int16 (value: int16) =
{ new IEncodable with
member _.Encode(helpers) =
helpers.encodeIntegralNumber (uint32 value)
helpers.encodeSignedIntegralNumber (int32 value)
}

let inline uint16 (value: uint16) =
{ new IEncodable with
member _.Encode(helpers) =
helpers.encodeIntegralNumber (uint32 value)
helpers.encodeUnsignedIntegralNumber (uint32 value)
}

let inline int (value: int) =
{ new IEncodable with
member _.Encode(helpers) =
helpers.encodeIntegralNumber (uint32 value)
helpers.encodeSignedIntegralNumber value
}

let inline uint32 (value: uint32) =
{ new IEncodable with
member _.Encode(helpers) = helpers.encodeIntegralNumber (value)
member _.Encode(helpers) =
helpers.encodeUnsignedIntegralNumber value
}

let inline int64 (value: int64) =
Expand Down
10 changes: 6 additions & 4 deletions packages/Thoth.Json.Core/Types.fs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ type IEncoderHelpers<'JsonValue> =
abstract encodeArray: 'JsonValue array -> 'JsonValue
abstract encodeList: 'JsonValue list -> 'JsonValue
abstract encodeSeq: 'JsonValue seq -> 'JsonValue
abstract encodeIntegralNumber: uint32 -> 'JsonValue

// See https://github.com/thoth-org/Thoth.Json/issues/187 for more information
// about why we make a distinction between signed and unsigned integral numbers
// when encoding them.
abstract encodeSignedIntegralNumber: int32 -> 'JsonValue
abstract encodeUnsignedIntegralNumber: uint32 -> 'JsonValue

type ErrorReason<'JsonValue> =
| BadPrimitive of string * 'JsonValue
Expand Down Expand Up @@ -75,7 +78,6 @@ type Json =

type IEncodable =
abstract member Encode<'JsonValue> :
helpers: IEncoderHelpers<'JsonValue> ->
'JsonValue
helpers: IEncoderHelpers<'JsonValue> -> 'JsonValue

type Encoder<'T> = 'T -> IEncodable
3 changes: 2 additions & 1 deletion packages/Thoth.Json.JavaScript/Encode.fs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ module Encode =
member _.encodeList values = JS.Constructors.Array.from values
member _.encodeSeq values = JS.Constructors.Array.from values

member _.encodeIntegralNumber value = box value
member _.encodeSignedIntegralNumber value = box value
member _.encodeUnsignedIntegralNumber value = box value
}

let toString (space: int) (value: IEncodable) : string =
Expand Down
4 changes: 3 additions & 1 deletion packages/Thoth.Json.Newtonsoft/Encode.fs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ module Encode =
member _.encodeList values = JArray(values)
member _.encodeSeq values = JArray(values)

member _.encodeIntegralNumber(value: uint32) =
member _.encodeSignedIntegralNumber(value: int32) = JValue(value)

member _.encodeUnsignedIntegralNumber(value: uint32) =
// We need to force the cast to uint64 here, otherwise
// Newtonsoft resolve the constructor to JValue(decimal)
// when we actually want to output a number without decimals
Expand Down
3 changes: 2 additions & 1 deletion packages/Thoth.Json.Python/Encode.fs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ module Encode =
member this.encodeSeq values =
values |> Seq.toArray |> this.encodeArray

member _.encodeIntegralNumber value = box value
member _.encodeSignedIntegralNumber value = box value
member _.encodeUnsignedIntegralNumber value = box value
}

let toString (space: int) (value: IEncodable) : string =
Expand Down
24 changes: 24 additions & 0 deletions tests/Thoth.Json.Tests/Encoders.fs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ let tests (runner: TestRunner<_, _>) =
let actual = Encode.int 1 |> runner.Encode.toString 0
runner.equal actual expected

runner.testCase "negative int keeps the sign"
<| fun _ ->
let expected = "-1"
let actual = Encode.int -1 |> runner.Encode.toString 0
runner.equal actual expected

runner.testCase "a float works"
<| fun _ ->
let expected = "1.2"
Expand Down Expand Up @@ -305,6 +311,15 @@ let tests (runner: TestRunner<_, _>) =

runner.equal actual expected

runner.testCase "negative sbyte keeps the sign"
<| fun _ ->
let expected = "-99"

let actual =
-99y |> Encode.sbyte |> runner.Encode.toString 0

runner.equal actual expected

runner.testCase "an int16 works"
<| fun _ ->
let expected = "99"
Expand All @@ -314,6 +329,15 @@ let tests (runner: TestRunner<_, _>) =

runner.equal actual expected

runner.testCase "negative int16 keeps the sign"
<| fun _ ->
let expected = "-99"

let actual =
-99s |> Encode.int16 |> runner.Encode.toString 0

runner.equal actual expected

runner.testCase "an uint16 works"
<| fun _ ->
let expected = "99"
Expand Down

0 comments on commit fb379ff

Please sign in to comment.