From 14551471e3cd33e520cf1a2c49be96632a3f9355 Mon Sep 17 00:00:00 2001 From: Ruben Bartelink Date: Wed, 26 Apr 2023 10:43:53 +0100 Subject: [PATCH 1/4] Newtonsoft: Sync TypeSafeEnum signatures --- CHANGELOG.md | 3 +++ src/FsCodec.NewtonsoftJson/OptionConverter.fs | 8 +++--- src/FsCodec.NewtonsoftJson/Options.fs | 20 +++++++------- .../TypeSafeEnumConverter.fs | 23 ++++++++-------- src/FsCodec.NewtonsoftJson/UnionConverter.fs | 25 +++++++++++------- src/FsCodec.SystemTextJson/Options.fs | 26 +++++++++---------- .../TypeSafeEnumConverter.fs | 24 ++++++++--------- src/FsCodec.SystemTextJson/UnionConverter.fs | 8 +++--- 8 files changed, 72 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43bee728..5a9d322f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ The `Unreleased` section name is replaced by the expected version of next releas ### Added ### Changed + +- `NewtonsoftJson.TypeSafeEnum`: Sync with `SystemTextJson.TypeSafeEnum` [#88](https://github.com/jet/FsCodec/pull/88) + ### Removed ### Fixed diff --git a/src/FsCodec.NewtonsoftJson/OptionConverter.fs b/src/FsCodec.NewtonsoftJson/OptionConverter.fs index 0b45ee29..d748258c 100755 --- a/src/FsCodec.NewtonsoftJson/OptionConverter.fs +++ b/src/FsCodec.NewtonsoftJson/OptionConverter.fs @@ -15,7 +15,7 @@ type OptionConverter() = if value = null then null else let _, fields = FSharpValue.GetUnionFields(value, value.GetType()) - fields.[0] + fields[0] serializer.Serialize(writer, value) @@ -26,8 +26,8 @@ type OptionConverter() = else innerType let cases = Union.getUnionCases t - if reader.TokenType = JsonToken.Null then FSharpValue.MakeUnion(cases.[0], Array.empty) + if reader.TokenType = JsonToken.Null then FSharpValue.MakeUnion(cases[0], Array.empty) else let value = serializer.Deserialize(reader, innerType) - if value = null then FSharpValue.MakeUnion(cases.[0], Array.empty) - else FSharpValue.MakeUnion(cases.[1], [|value|]) + if value = null then FSharpValue.MakeUnion(cases[0], Array.empty) + else FSharpValue.MakeUnion(cases[1], [|value|]) diff --git a/src/FsCodec.NewtonsoftJson/Options.fs b/src/FsCodec.NewtonsoftJson/Options.fs index 4ab31a52..16b1a60d 100755 --- a/src/FsCodec.NewtonsoftJson/Options.fs +++ b/src/FsCodec.NewtonsoftJson/Options.fs @@ -17,13 +17,13 @@ type Options private () = /// Creates a default set of serializer settings used by Json serialization. When used with no args, same as JsonSerializerSettings.CreateDefault() static member CreateDefault ( [] converters : JsonConverter[], - /// Use multi-line, indented formatting when serializing JSON; defaults to false. + // Use multi-line, indented formatting when serializing JSON; defaults to false. [] ?indent : bool, - /// Render idiomatic camelCase for PascalCase items by using `CamelCasePropertyNamesContractResolver`. Defaults to false. + // Render idiomatic camelCase for PascalCase items by using `CamelCasePropertyNamesContractResolver`. Defaults to false. [] ?camelCase : bool, - /// Ignore null values in input data; defaults to false. + // Ignore null values in input data; defaults to false. [] ?ignoreNulls : bool, - /// Error on missing values (as opposed to letting them just be default-initialized); defaults to false. + // Error on missing values (as opposed to letting them just be default-initialized); defaults to false. [] ?errorOnMissing : bool) = let indent = defaultArg indent false @@ -48,16 +48,16 @@ type Options private () = /// - Always prepends an OptionConverter() to any converters supplied /// - everything else is as per CreateDefault:- i.e. emit nulls instead of omitting fields etc static member Create - ( /// List of converters to apply. An implicit OptionConverter() will be prepended and/or be used as a default + ( // List of converters to apply. An implicit OptionConverter() will be prepended and/or be used as a default [] converters : JsonConverter[], - /// Use multi-line, indented formatting when serializing JSON; defaults to false. + // Use multi-line, indented formatting when serializing JSON; defaults to false. [] ?indent : bool, - /// Render idiomatic camelCase for PascalCase items by using `CamelCasePropertyNamesContractResolver`. - /// Defaults to false on basis that you'll use record and tuple field names that are camelCase (and hence not `CLSCompliant`). + // Render idiomatic camelCase for PascalCase items by using `CamelCasePropertyNamesContractResolver`. + // Defaults to false on basis that you'll use record and tuple field names that are camelCase (and hence not `CLSCompliant`). [] ?camelCase : bool, - /// Ignore null values in input data; defaults to `false`. + // Ignore null values in input data; defaults to `false`. [] ?ignoreNulls : bool, - /// Error on missing values (as opposed to letting them just be default-initialized); defaults to false + // Error on missing values (as opposed to letting them just be default-initialized); defaults to false [] ?errorOnMissing : bool) = Options.CreateDefault( diff --git a/src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs b/src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs index 8d56887e..1620ca15 100755 --- a/src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs +++ b/src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs @@ -1,21 +1,21 @@ namespace FsCodec.NewtonsoftJson open Newtonsoft.Json -open System.Collections.Generic open System +open System.Collections.Generic /// Utilities for working with DUs where none of the cases have a value module TypeSafeEnum = let isTypeSafeEnum (t : Type) = Union.isUnion t - && (Union.getUnion t).cases |> Seq.forall (fun case -> case.GetFields().Length = 0) + && Union.hasOnlyNullaryCases t let tryParseT (t : Type) (str : string) = - let union = Union.getUnion t - union.cases - |> Array.tryFindIndex (fun case -> case.Name = str) - |> Option.map (fun tag -> (union.caseConstructor.[tag] [||])) + let u = Union.getInfo t + u.cases + |> Array.tryFindIndex (fun c -> c.Name = str) + |> Option.map (fun tag -> u.caseConstructor[tag] [||]) let tryParse<'T> (str : string) = tryParseT typeof<'T> str |> Option.map (fun e -> e :?> 'T) let parseT (t : Type) (str : string) = @@ -26,16 +26,17 @@ module TypeSafeEnum = raise (KeyNotFoundException(sprintf "Could not find case '%s' for type '%s'" str t.FullName)) let parse<'T> (str : string) = parseT typeof<'T> str :?> 'T - let toString (x : obj) = - let union = Union.getUnion (x.GetType()) - let tag = union.tagReader x - union.cases.[tag].Name + let toString<'t> (x : 't) = + let u = Union.getInfo typeof<'t> + let tag = u.tagReader x + u.cases[tag].Name /// Maps strings to/from Union cases; refuses to convert for values not in the Union type TypeSafeEnumConverter() = inherit JsonConverter() - override _.CanConvert (t : Type) = TypeSafeEnum.isTypeSafeEnum t + override _.CanConvert(t : Type) = + TypeSafeEnum.isTypeSafeEnum t override _.WriteJson(writer : JsonWriter, value : obj, _ : JsonSerializer) = let str = TypeSafeEnum.toString value diff --git a/src/FsCodec.NewtonsoftJson/UnionConverter.fs b/src/FsCodec.NewtonsoftJson/UnionConverter.fs index 7fd18568..51037308 100755 --- a/src/FsCodec.NewtonsoftJson/UnionConverter.fs +++ b/src/FsCodec.NewtonsoftJson/UnionConverter.fs @@ -20,7 +20,7 @@ module private Union = let isUnion = memoize (fun t -> FSharpType.IsUnion(t, true)) let getUnionCases = memoize (fun t -> FSharpType.GetUnionCases(t, true)) - let private createUnion t = + let private createInfo t = let cases = getUnionCases t { cases = cases @@ -28,7 +28,12 @@ module private Union = fieldReader = cases |> Array.map (fun c -> FSharpValue.PreComputeUnionReader(c, true)) caseConstructor = cases |> Array.map (fun c -> FSharpValue.PreComputeUnionConstructor(c, true)) } - let getUnion = memoize createUnion + let getInfo = memoize createInfo + + /// Allows us to distinguish between Unions that have bodies and hence should use a UnionConverter + let hasOnlyNullaryCases (t : Type) = + let union = getInfo t + union.cases |> Seq.forall (fun case -> case.GetFields().Length = 0) /// Parallels F# behavior wrt how it generates a DU's underlying .NET Type let inline isInlinedIntoUnionItem (t : Type) = @@ -53,7 +58,7 @@ module private Union = [| inputJObject.ToObject(singleCaseArg.PropertyType, serializer) |] | multipleFieldsInCustomCaseType -> [| for fi in multipleFieldsInCustomCaseType -> - match inputJObject.[fi.Name] with + match inputJObject[fi.Name] with | null when // Afford converters an opportunity to handle the missing field in the best way I can figure out to signal that // The specific need being covered (see tests) is to ensure that, even with MissingMemberHandling=Ignore, @@ -79,10 +84,10 @@ type UnionConverter private (discriminator : string, ?catchAllCase) = override _.CanConvert (t : Type) = Union.isUnion t override _.WriteJson(writer : JsonWriter, value : obj, serializer : JsonSerializer) = - let union = Union.getUnion (value.GetType()) + let union = Union.getInfo (value.GetType()) let tag = union.tagReader value - let case = union.cases.[tag] - let fieldValues = union.fieldReader.[tag] value + let case = union.cases[tag] + let fieldValues = union.fieldReader[tag] value let fieldInfos = case.GetFields() writer.WriteStartObject() @@ -92,7 +97,7 @@ type UnionConverter private (discriminator : string, ?catchAllCase) = match fieldInfos with | [| fi |] when not (Union.typeIsUnionWithConverterAttribute fi.PropertyType) -> - match fieldValues.[0] with + match fieldValues[0] with | null when serializer.NullValueHandling = NullValueHandling.Ignore -> () | fv -> let token = if fv = null then JToken.Parse "null" else JToken.FromObject(fv, serializer) @@ -117,9 +122,9 @@ type UnionConverter private (discriminator : string, ?catchAllCase) = if token.Type <> JTokenType.Object then raise (FormatException(sprintf "Expected object token, got %O" token.Type)) let inputJObject = token :?> JObject - let union = Union.getUnion t + let union = Union.getInfo t let targetCaseIndex = - let inputCaseNameValue = inputJObject.[discriminator] |> string + let inputCaseNameValue = inputJObject[discriminator] |> string let findCaseNamed x = union.cases |> Array.tryFindIndex (fun case -> case.Name = x) match findCaseNamed inputCaseNameValue, catchAllCase with | None, None -> @@ -133,5 +138,5 @@ type UnionConverter private (discriminator : string, ?catchAllCase) = inputCaseNameValue catchAllCaseName t.FullName |> invalidOp | Some foundIndex -> foundIndex - let targetCaseFields, targetCaseCtor = union.cases.[targetCaseIndex].GetFields(), union.caseConstructor.[targetCaseIndex] + let targetCaseFields, targetCaseCtor = union.cases[targetCaseIndex].GetFields(), union.caseConstructor[targetCaseIndex] targetCaseCtor (Union.mapTargetCaseArgs inputJObject serializer targetCaseFields) diff --git a/src/FsCodec.SystemTextJson/Options.fs b/src/FsCodec.SystemTextJson/Options.fs index f23e1b08..c925fa75 100755 --- a/src/FsCodec.SystemTextJson/Options.fs +++ b/src/FsCodec.SystemTextJson/Options.fs @@ -17,13 +17,13 @@ type Options private () = /// Creates a default set of serializer options used by Json serialization. When used with no args, same as `JsonSerializerOptions()` static member CreateDefault ( [] converters : JsonConverter[], - /// Use multi-line, indented formatting when serializing JSON; defaults to false. + // Use multi-line, indented formatting when serializing JSON; defaults to false. [] ?indent : bool, - /// Render idiomatic camelCase for PascalCase items by using `PropertyNamingPolicy = CamelCase`. Defaults to false. + // Render idiomatic camelCase for PascalCase items by using `PropertyNamingPolicy = CamelCase`. Defaults to false. [] ?camelCase : bool, - /// Ignore null values in input data, don't render fields with null values; defaults to `false`. + // Ignore null values in input data, don't render fields with null values; defaults to `false`. [] ?ignoreNulls : bool, - /// Drop escaping of HTML-sensitive characters. defaults to `false`. + // Drop escaping of HTML-sensitive characters. defaults to `false`. [] ?unsafeRelaxedJsonEscaping : bool) = let indent = defaultArg indent false @@ -43,22 +43,22 @@ type Options private () = /// - renders values with `UnsafeRelaxedJsonEscaping` - i.e. minimal escaping as per `NewtonsoftJson`
/// Everything else is as per CreateDefault:- i.e. emit nulls instead of omitting fields, no indenting, no camelCase conversion static member Create - ( /// List of converters to apply. Implicit converters may be prepended and/or be used as a default + ( // List of converters to apply. Implicit converters may be prepended and/or be used as a default [] converters : JsonConverter[], - /// Use multi-line, indented formatting when serializing JSON; defaults to false. + // Use multi-line, indented formatting when serializing JSON; defaults to false. [] ?indent : bool, - /// Render idiomatic camelCase for PascalCase items by using `PropertyNamingPolicy = CamelCase`. - /// Defaults to false on basis that you'll use record and tuple field names that are camelCase (but thus not `CLSCompliant`). + // Render idiomatic camelCase for PascalCase items by using `PropertyNamingPolicy = CamelCase`. + // Defaults to false on basis that you'll use record and tuple field names that are camelCase (but thus not `CLSCompliant`). [] ?camelCase : bool, - /// Ignore null values in input data, don't render fields with null values; defaults to `false`. + // Ignore null values in input data, don't render fields with null values; defaults to `false`. [] ?ignoreNulls : bool, - /// Drop escaping of HTML-sensitive characters. defaults to `true`. + // Drop escaping of HTML-sensitive characters. defaults to `true`. [] ?unsafeRelaxedJsonEscaping : bool, - /// Apply TypeSafeEnumConverter if possible. Defaults to false. + // Apply TypeSafeEnumConverter if possible. Defaults to false. [] ?autoTypeSafeEnumToJsonString : bool, - /// Apply UnionConverter for all Discriminated Unions, if TypeSafeEnumConverter not possible. Defaults to false. + // Apply UnionConverter for all Discriminated Unions, if TypeSafeEnumConverter not possible. Defaults to false. [] ?autoUnionToJsonObject : bool, - /// Apply RejectNullStringConverter in order to have serialization throw on null strings. Use string option to represent strings that can potentially be null. + // Apply RejectNullStringConverter in order to have serialization throw on null strings. Use string option to represent strings that can potentially be null. [] ?rejectNullStrings: bool) = let autoTypeSafeEnumToJsonString = defaultArg autoTypeSafeEnumToJsonString false diff --git a/src/FsCodec.SystemTextJson/TypeSafeEnumConverter.fs b/src/FsCodec.SystemTextJson/TypeSafeEnumConverter.fs index bae51df3..bba30a42 100755 --- a/src/FsCodec.SystemTextJson/TypeSafeEnumConverter.fs +++ b/src/FsCodec.SystemTextJson/TypeSafeEnumConverter.fs @@ -7,20 +7,19 @@ open System.Text.Json /// Utilities for working with DUs where none of the cases have a value module TypeSafeEnum = - let isTypeSafeEnum (typ : Type) = - Union.isUnion typ - && Union.hasOnlyNullaryCases typ + let isTypeSafeEnum (t : Type) = + Union.isUnion t + && Union.hasOnlyNullaryCases t - let tryParseT (t : Type) predicate = + let tryParseT (t : Type) (str : string) = let u = Union.getInfo t u.cases - |> Array.tryFindIndex (fun c -> predicate c.Name) - |> Option.map (fun tag -> u.caseConstructor.[tag] [||]) - // TOCONSIDER memoize and/or push into `Union` https://github.com/jet/FsCodec/pull/41#discussion_r394473137 - let tryParse<'T> (str : string) = tryParseT typeof<'T> ((=) str) |> Option.map (fun e -> e :?> 'T) + |> Array.tryFindIndex (fun c -> c.Name = str) + |> Option.map (fun tag -> u.caseConstructor[tag] [||]) + let tryParse<'T> (str : string) = tryParseT typeof<'T> str |> Option.map (fun e -> e :?> 'T) - let parseT (t : Type) (str : string) = - match tryParseT t ((=) str) with + let parseT (t : Type) (str : string) = + match tryParseT t str with | Some e -> e | None -> // Keep exception compat, but augment with a meaningful message. @@ -29,9 +28,8 @@ module TypeSafeEnum = let toString<'t> (x : 't) = let u = Union.getInfo typeof<'t> - let tag = u.tagReader (box x) - // TOCONSIDER memoize and/or push into `Union` https://github.com/jet/FsCodec/pull/41#discussion_r394473137 - u.cases.[tag].Name + let tag = u.tagReader x + u.cases[tag].Name /// Maps strings to/from Union cases; refuses to convert for values not in the Union type TypeSafeEnumConverter<'T>() = diff --git a/src/FsCodec.SystemTextJson/UnionConverter.fs b/src/FsCodec.SystemTextJson/UnionConverter.fs index 4aac49be..91655f1f 100755 --- a/src/FsCodec.SystemTextJson/UnionConverter.fs +++ b/src/FsCodec.SystemTextJson/UnionConverter.fs @@ -50,7 +50,7 @@ module private Union = |> Option.map (fun a -> a :?> IUnionConverterOptions) } let getInfo : Type -> Union = memoize createInfo - /// Allows us to distinguish between Unions that have bodies and hence should UnionConverter + /// Allows us to distinguish between Unions that have bodies and hence should use a UnionConverter let hasOnlyNullaryCases (t : Type) = let union = getInfo t union.cases |> Seq.forall (fun case -> case.GetFields().Length = 0) @@ -69,8 +69,8 @@ type UnionConverter<'T>() = let union = Union.getInfo typeof<'T> let unionOptions = getOptions union let tag = union.tagReader value - let case = union.cases.[tag] - let fieldValues = union.fieldReader.[tag] value + let case = union.cases[tag] + let fieldValues = union.fieldReader[tag] value let fieldInfos = case.GetFields() writer.WriteStartObject() @@ -111,7 +111,7 @@ type UnionConverter<'T>() = inputCaseNameValue catchAllCaseName t.FullName |> invalidOp | Some foundIndex -> foundIndex - let targetCaseFields, targetCaseCtor = union.cases.[targetCaseIndex].GetFields(), union.caseConstructor.[targetCaseIndex] + let targetCaseFields, targetCaseCtor = union.cases[targetCaseIndex].GetFields(), union.caseConstructor[targetCaseIndex] let ctorArgs = [| for fieldInfo in targetCaseFields -> let t = fieldInfo.PropertyType From 58336bd80308361ff069ca39c2006ff8cb697a25 Mon Sep 17 00:00:00 2001 From: Ruben Bartelink Date: Wed, 26 Apr 2023 10:46:10 +0100 Subject: [PATCH 2/4] Fix comment --- src/FsCodec.NewtonsoftJson/UnionConverter.fs | 2 +- src/FsCodec.SystemTextJson/UnionConverter.fs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/FsCodec.NewtonsoftJson/UnionConverter.fs b/src/FsCodec.NewtonsoftJson/UnionConverter.fs index 51037308..09308f74 100755 --- a/src/FsCodec.NewtonsoftJson/UnionConverter.fs +++ b/src/FsCodec.NewtonsoftJson/UnionConverter.fs @@ -30,7 +30,7 @@ module private Union = } let getInfo = memoize createInfo - /// Allows us to distinguish between Unions that have bodies and hence should use a UnionConverter + /// Allows us to distinguish Unions that do not have bodies and hence should use a TypeSafeEnumConverter let hasOnlyNullaryCases (t : Type) = let union = getInfo t union.cases |> Seq.forall (fun case -> case.GetFields().Length = 0) diff --git a/src/FsCodec.SystemTextJson/UnionConverter.fs b/src/FsCodec.SystemTextJson/UnionConverter.fs index 91655f1f..637538a2 100755 --- a/src/FsCodec.SystemTextJson/UnionConverter.fs +++ b/src/FsCodec.SystemTextJson/UnionConverter.fs @@ -50,7 +50,7 @@ module private Union = |> Option.map (fun a -> a :?> IUnionConverterOptions) } let getInfo : Type -> Union = memoize createInfo - /// Allows us to distinguish between Unions that have bodies and hence should use a UnionConverter + /// Allows us to distinguish Unions that do not have bodies and hence should use a TypeSafeEnumConverter let hasOnlyNullaryCases (t : Type) = let union = getInfo t union.cases |> Seq.forall (fun case -> case.GetFields().Length = 0) From d7c68a7836c069a1a8093e3fda697d71f392b893 Mon Sep 17 00:00:00 2001 From: Ruben Bartelink Date: Wed, 26 Apr 2023 10:53:33 +0100 Subject: [PATCH 3/4] Fix --- src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs b/src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs index 1620ca15..3b45315a 100755 --- a/src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs +++ b/src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs @@ -26,10 +26,12 @@ module TypeSafeEnum = raise (KeyNotFoundException(sprintf "Could not find case '%s' for type '%s'" str t.FullName)) let parse<'T> (str : string) = parseT typeof<'T> str :?> 'T - let toString<'t> (x : 't) = - let u = Union.getInfo typeof<'t> + let toStringT (t : Type) (x : obj) = + let u = Union.getInfo t let tag = u.tagReader x u.cases[tag].Name + let toString<'t> (x : 't) = + toStringT typeof<'t> x /// Maps strings to/from Union cases; refuses to convert for values not in the Union type TypeSafeEnumConverter() = @@ -39,7 +41,7 @@ type TypeSafeEnumConverter() = TypeSafeEnum.isTypeSafeEnum t override _.WriteJson(writer : JsonWriter, value : obj, _ : JsonSerializer) = - let str = TypeSafeEnum.toString value + let str = TypeSafeEnum.toStringT (value.GetType()) value writer.WriteValue str override _.ReadJson(reader : JsonReader, t : Type, _ : obj, _ : JsonSerializer) = From 729cbfce6eccf47246c389ff079c37ef54fd0dbc Mon Sep 17 00:00:00 2001 From: Ruben Bartelink Date: Wed, 26 Apr 2023 11:44:13 +0100 Subject: [PATCH 4/4] Fix PR no --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a9d322f..4980c6bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ The `Unreleased` section name is replaced by the expected version of next releas ### Added ### Changed -- `NewtonsoftJson.TypeSafeEnum`: Sync with `SystemTextJson.TypeSafeEnum` [#88](https://github.com/jet/FsCodec/pull/88) +- `NewtonsoftJson.TypeSafeEnum`: Sync with `SystemTextJson.TypeSafeEnum` [#91](https://github.com/jet/FsCodec/pull/91) ### Removed ### Fixed