Enforce non-nullability in more cases during JSON parse (#367)

This reverts commit 8d275f0047.

CHANGELOG.md

@@ -1,5 +1,10 @@
 Notable changes are recorded here.
 
+# WoofWare.Myriad.Plugins 5.0.1
+
+We now enforce non-nullability on more types during JSON parse.
+We have always expected you to consume nullable types wrapped in an `option`, but now we enforce this in more cases by throwing `ArgumentNullException`.
+
 # WoofWare.Myriad.Plugins 3.0.1
 
 Semantics of `HttpClient`'s URI component composition changed:

ConsumePlugin/GeneratedJson.fs

@@ -71,7 +71,11 @@ module JsonRecordType =
                  )
              | v -> v)
                 .AsArray ()
-            |> Seq.map (fun elt -> elt.AsValue().GetValue<System.Int32> ())
+            |> Seq.map (fun elt ->
+                (match elt with
+                 | null -> raise (System.ArgumentNullException ())
+                 | elt -> elt.AsValue().GetValue<System.Int32> ())
+            )
             |> Array.ofSeq
 
         let arg_4 =
@@ -84,7 +88,11 @@ module JsonRecordType =
                  )
              | v -> v)
                 .AsArray ()
-            |> Seq.map (fun elt -> elt.AsValue().GetValue<System.String> ())
+            |> Seq.map (fun elt ->
+                (match elt with
+                 | null -> raise (System.ArgumentNullException ())
+                 | elt -> elt.AsValue().GetValue<System.String> ())
+            )
             |> Array.ofSeq
 
         let arg_3 =
@@ -109,7 +117,11 @@ module JsonRecordType =
                  )
              | v -> v)
                 .AsArray ()
-            |> Seq.map (fun elt -> elt.AsValue().GetValue<System.Int32> ())
+            |> Seq.map (fun elt ->
+                (match elt with
+                 | null -> raise (System.ArgumentNullException ())
+                 | elt -> elt.AsValue().GetValue<System.Int32> ())
+            )
             |> List.ofSeq
 
         let arg_1 =

ConsumePlugin/GeneratedPureGymDto.fs

@@ -60,7 +60,11 @@ module GymOpeningHours =
                  )
              | v -> v)
                 .AsArray ()
-            |> Seq.map (fun elt -> elt.AsValue().GetValue<System.String> ())
+            |> Seq.map (fun elt ->
+                (match elt with
+                 | null -> raise (System.ArgumentNullException ())
+                 | elt -> elt.AsValue().GetValue<System.String> ())
+            )
             |> List.ofSeq
 
         let arg_0 =
@@ -1038,7 +1042,11 @@ module Sessions =
                  )
              | v -> v)
                 .AsArray ()
-            |> Seq.map (fun elt -> Visit.jsonParse elt)
+            |> Seq.map (fun elt ->
+                (match elt with
+                 | null -> raise (System.ArgumentNullException ())
+                 | elt -> Visit.jsonParse elt)
+            )
             |> List.ofSeq
 
         let arg_0 =

ConsumePlugin/GeneratedRestClient.fs

@@ -48,7 +48,14 @@ module PureGymApi =
                         System.Text.Json.Nodes.JsonNode.ParseAsync (responseStream, cancellationToken = ct)
                         |> Async.AwaitTask
 
-                    return jsonNode.AsArray () |> Seq.map (fun elt -> Gym.jsonParse elt) |> List.ofSeq
+                    return
+                        jsonNode.AsArray ()
+                        |> Seq.map (fun elt ->
+                            (match elt with
+                             | null -> raise (System.ArgumentNullException ())
+                             | elt -> Gym.jsonParse elt)
+                        )
+                        |> List.ofSeq
                 }
                 |> (fun a -> Async.StartAsTask (a, ?cancellationToken = ct))
 

ConsumePlugin/GeneratedSerde.fs

@@ -408,7 +408,11 @@ module InnerTypeWithBothJsonParseExtension =
 
                     let value =
                         (kvp.Value).AsArray ()
-                        |> Seq.map (fun elt -> elt.AsValue().GetValue<System.Char> ())
+                        |> Seq.map (fun elt ->
+                            (match elt with
+                             | null -> raise (System.ArgumentNullException ())
+                             | elt -> elt.AsValue().GetValue<System.Char> ())
+                        )
                         |> List.ofSeq
 
                     key, value
@@ -676,7 +680,11 @@ module JsonRecordTypeWithBothJsonParseExtension =
                      )
                  | v -> v)
                     .AsArray ()
-                |> Seq.map (fun elt -> elt.AsValue().GetValue<System.Int32> ())
+                |> Seq.map (fun elt ->
+                    (match elt with
+                     | null -> raise (System.ArgumentNullException ())
+                     | elt -> elt.AsValue().GetValue<System.Int32> ())
+                )
                 |> Array.ofSeq
 
             let arg_4 =
@@ -689,7 +697,11 @@ module JsonRecordTypeWithBothJsonParseExtension =
                      )
                  | v -> v)
                     .AsArray ()
-                |> Seq.map (fun elt -> elt.AsValue().GetValue<System.String> ())
+                |> Seq.map (fun elt ->
+                    (match elt with
+                     | null -> raise (System.ArgumentNullException ())
+                     | elt -> elt.AsValue().GetValue<System.String> ())
+                )
                 |> Array.ofSeq
 
             let arg_3 =
@@ -714,7 +726,11 @@ module JsonRecordTypeWithBothJsonParseExtension =
                      )
                  | v -> v)
                     .AsArray ()
-                |> Seq.map (fun elt -> elt.AsValue().GetValue<System.Int32> ())
+                |> Seq.map (fun elt ->
+                    (match elt with
+                     | null -> raise (System.ArgumentNullException ())
+                     | elt -> elt.AsValue().GetValue<System.Int32> ())
+                )
                 |> List.ofSeq
 
             let arg_1 =

ConsumePlugin/GeneratedVault.fs

@@ -94,7 +94,11 @@ module JwtVaultAuthResponse =
                  )
              | v -> v)
                 .AsArray ()
-            |> Seq.map (fun elt -> elt.AsValue().GetValue<System.String> ())
+            |> Seq.map (fun elt ->
+                (match elt with
+                 | null -> raise (System.ArgumentNullException ())
+                 | elt -> elt.AsValue().GetValue<System.String> ())
+            )
             |> List.ofSeq
 
         let arg_3 =
@@ -107,7 +111,11 @@ module JwtVaultAuthResponse =
                  )
              | v -> v)
                 .AsArray ()
-            |> Seq.map (fun elt -> elt.AsValue().GetValue<System.String> ())
+            |> Seq.map (fun elt ->
+                (match elt with
+                 | null -> raise (System.ArgumentNullException ())
+                 | elt -> elt.AsValue().GetValue<System.String> ())
+            )
             |> List.ofSeq
 
         let arg_2 =
@@ -120,7 +128,11 @@ module JwtVaultAuthResponse =
                  )
              | v -> v)
                 .AsArray ()
-            |> Seq.map (fun elt -> elt.AsValue().GetValue<System.String> ())
+            |> Seq.map (fun elt ->
+                (match elt with
+                 | null -> raise (System.ArgumentNullException ())
+                 | elt -> elt.AsValue().GetValue<System.String> ())
+            )
             |> List.ofSeq
 
         let arg_1 =

WoofWare.Myriad.Plugins.Test/WoofWare.Myriad.Plugins.Test.fsproj

@@ -43,7 +43,7 @@
 
   <ItemGroup>
     <PackageReference Include="ApiSurface" Version="4.1.20"/>
-    <PackageReference Include="FsCheck" Version="3.1.0"/>
+    <PackageReference Include="FsCheck" Version="3.2.0"/>
     <PackageReference Include="FsUnit" Version="7.0.1"/>
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.13.0"/>
     <PackageReference Include="NUnit" Version="4.3.2"/>

WoofWare.Myriad.Plugins/ArgParserGenerator.fs

@@ -673,7 +673,7 @@ module internal ArgParserGenerator =
                 args
                 |> Map.toList
                 |> List.map (fun (ident, expr) -> SynLongIdent.create [ Ident.create ident ], expr)
-                |> AstHelper.instantiateRecord
+                |> SynExpr.createRecord None
             )
 
         tree, counter

WoofWare.Myriad.Plugins/AstHelper.fs

@@ -36,13 +36,6 @@ module internal AstHelper =
         | SynTypeDefnRepr.Simple (SynTypeDefnSimpleRepr.Enum _, _) -> true
         | _ -> false
 
-    let instantiateRecord (fields : (SynLongIdent * SynExpr) list) : SynExpr =
-        let fields =
-            fields
-            |> List.map (fun (rfn, synExpr) -> SynExprRecordField ((rfn, true), Some range0, Some synExpr, None))
-
-        SynExpr.Record (None, None, fields, range0)
-
     let defineRecordType (record : RecordType) : SynTypeDefn =
         let name =
             SynComponentInfo.create record.Name

WoofWare.Myriad.Plugins/InterfaceMockGenerator.fs

@@ -89,7 +89,7 @@ module internal InterfaceMockGenerator =
                      []
                  else
                      [ SynPat.unit ])
-                (AstHelper.instantiateRecord constructorFields)
+                (SynExpr.createRecord None constructorFields)
             |> SynBinding.withXmlDoc (PreXmlDoc.create "An implementation where every method throws.")
             |> SynBinding.withReturnAnnotation constructorReturnType
             |> SynMemberDefn.staticMember

WoofWare.Myriad.Plugins/JsonParseGenerator.fs

@@ -26,7 +26,7 @@ module internal JsonParseGenerator =
             }
 
     /// (match {indexed} with | null -> raise (System.Collections.Generic.KeyNotFoundException ({propertyName} not found)) | v -> v)
-    let assertNotNull (propertyName : SynExpr) (indexed : SynExpr) =
+    let assertPropertyExists (propertyName : SynExpr) (indexed : SynExpr) =
         let raiseExpr =
             SynExpr.applyFunction
                 (SynExpr.createIdent "sprintf")
@@ -46,28 +46,42 @@ module internal JsonParseGenerator =
         |> SynExpr.createMatch indexed
         |> SynExpr.paren
 
+    let assertNotNull (body : SynExpr) (boundIdent : Ident) : SynExpr =
+        let raiseExpr =
+            SynExpr.CreateConst ()
+            |> SynExpr.applyFunction (SynExpr.createLongIdent [ "System" ; "ArgumentNullException" ])
+            |> SynExpr.paren
+            |> SynExpr.applyFunction (SynExpr.createIdent "raise")
+
+        [
+            SynMatchClause.create SynPat.createNull raiseExpr
+            SynMatchClause.create (SynPat.namedI boundIdent) body
+        ]
+        |> SynExpr.createMatch (SynExpr.createIdent' boundIdent)
+        |> SynExpr.paren
+
     /// {node}.AsValue().GetValue<{typeName}> ()
-    /// If `propertyName` is Some, uses `assertNotNull {node}` instead of `{node}`.
+    /// If `propertyName` is Some, uses `assertPropertyExists {node}` instead of `{node}`.
     let asValueGetValue (propertyName : SynExpr option) (typeName : string) (node : SynExpr) : SynExpr =
         match propertyName with
         | None -> node
-        | Some propertyName -> assertNotNull propertyName node
+        | Some propertyName -> assertPropertyExists propertyName node
         |> SynExpr.callMethod "AsValue"
         |> SynExpr.callGenericMethod' "GetValue" typeName
 
     let asValueGetValueIdent (propertyName : SynExpr option) (typeName : LongIdent) (node : SynExpr) : SynExpr =
         match propertyName with
         | None -> node
-        | Some propertyName -> assertNotNull propertyName node
+        | Some propertyName -> assertPropertyExists propertyName node
         |> SynExpr.callMethod "AsValue"
         |> SynExpr.callGenericMethod (SynLongIdent.createS "GetValue") [ SynType.createLongIdent typeName ]
 
     /// {node}.AsObject()
-    /// If `propertyName` is Some, uses `assertNotNull {node}` instead of `{node}`.
+    /// If `propertyName` is Some, uses `assertPropertyExists {node}` instead of `{node}`.
     let asObject (propertyName : SynExpr option) (node : SynExpr) : SynExpr =
         match propertyName with
         | None -> node
-        | Some propertyName -> assertNotNull propertyName node
+        | Some propertyName -> assertPropertyExists propertyName node
         |> SynExpr.callMethod "AsObject"
 
     /// {type}.jsonParse {node}
@@ -77,8 +91,8 @@ module internal JsonParseGenerator =
 
     /// collectionType is e.g. "List"; we'll be calling `ofSeq` on it.
     /// body is the body of a lambda which takes a parameter `elt`.
-    /// {assertNotNull node}.AsArray()
-    /// |> Seq.map (fun elt -> {body})
+    /// {assertPropertyExists node}.AsArray()
+    /// |> Seq.map (fun elt -> {assertNotNull} {body})
     /// |> {collectionType}.ofSeq
     let asArrayMapped
         (propertyName : SynExpr option)
@@ -89,10 +103,12 @@ module internal JsonParseGenerator =
         =
         match propertyName with
         | None -> node
-        | Some propertyName -> assertNotNull propertyName node
+        | Some propertyName -> assertPropertyExists propertyName node
         |> SynExpr.callMethod "AsArray"
         |> SynExpr.pipeThroughFunction (
-            SynExpr.applyFunction (SynExpr.createLongIdent [ "Seq" ; "map" ]) (SynExpr.createLambda "elt" body)
+            SynExpr.applyFunction
+                (SynExpr.createLongIdent [ "Seq" ; "map" ])
+                (SynExpr.createLambda "elt" (assertNotNull body (Ident.create "elt")))
         )
         |> SynExpr.pipeThroughFunction (SynExpr.createLongIdent [ collectionType ; "ofSeq" ])
 
@@ -177,27 +193,29 @@ module internal JsonParseGenerator =
         =
         // TODO: parsing format for DateTime etc
         match fieldType with
+        // Struct types
         | DateOnly ->
             node
             |> asValueGetValue propertyName "string"
             |> SynExpr.pipeThroughFunction (SynExpr.createLongIdent [ "System" ; "DateOnly" ; "Parse" ])
-        | Uri ->
-            node
-            |> asValueGetValue propertyName "string"
-            |> SynExpr.pipeThroughFunction (SynExpr.createLongIdent [ "System" ; "Uri" ])
-        | Guid ->
-            node
-            |> asValueGetValue propertyName "string"
-            |> SynExpr.pipeThroughFunction (SynExpr.createLongIdent [ "System" ; "Guid" ; "Parse" ])
         | DateTime ->
             node
             |> asValueGetValue propertyName "string"
             |> SynExpr.pipeThroughFunction (SynExpr.createLongIdent [ "System" ; "DateTime" ; "Parse" ])
+        | NumberType typeName -> parseNumberType options propertyName node typeName
+        | Guid ->
+            node
+            |> asValueGetValue propertyName "string"
+            |> SynExpr.pipeThroughFunction (SynExpr.createLongIdent [ "System" ; "Guid" ; "Parse" ])
+        // Reference types
+        | Uri ->
+            node
+            |> asValueGetValue propertyName "string"
+            |> SynExpr.pipeThroughFunction (SynExpr.createLongIdent [ "System" ; "Uri" ])
         | DateTimeOffset ->
             node
             |> asValueGetValue propertyName "string"
             |> SynExpr.pipeThroughFunction (SynExpr.createLongIdent [ "System" ; "DateTimeOffset" ; "Parse" ])
-        | NumberType typeName -> parseNumberType options propertyName node typeName
         | PrimitiveType typeName -> asValueGetValueIdent propertyName typeName node
         | OptionType ty ->
             let someClause =
@@ -291,7 +309,7 @@ module internal JsonParseGenerator =
 
             match propertyName with
             | None -> node
-            | Some propertyName -> assertNotNull propertyName node
+            | Some propertyName -> assertPropertyExists propertyName node
             |> typeJsonParse typeName
 
     /// propertyName is probably a string literal, but it could be a [<Literal>] variable
@@ -480,7 +498,7 @@ module internal JsonParseGenerator =
         let finalConstruction =
             fields
             |> List.mapi (fun i fieldData -> SynLongIdent.createI fieldData.Ident, SynExpr.createIdent $"arg_%i{i}")
-            |> AstHelper.instantiateRecord
+            |> SynExpr.createRecord None
 
         (finalConstruction, assignments)
         ||> List.fold (fun final assignment -> SynExpr.createLet [ assignment ] final)
@@ -505,7 +523,7 @@ module internal JsonParseGenerator =
                     |> SynExpr.createLet
                         [
                             SynExpr.index (SynExpr.CreateConst "data") (SynExpr.createIdent "node")
-                            |> assertNotNull (SynExpr.CreateConst "data")
+                            |> assertPropertyExists (SynExpr.CreateConst "data")
                             |> SynBinding.basic [ Ident.create "node" ] []
                         ]
 
@@ -553,7 +571,7 @@ module internal JsonParseGenerator =
 
                 SynExpr.createIdent "node"
                 |> SynExpr.index property
-                |> assertNotNull property
+                |> assertPropertyExists property
                 |> SynExpr.pipeThroughFunction (
                     SynExpr.createLambda "v" (SynExpr.callGenericMethod' "GetValue" "string" (SynExpr.createIdent "v"))
                 )

WoofWare.Myriad.Plugins/RemoveOptionsGenerator.fs

@@ -97,7 +97,7 @@ module internal RemoveOptionsGenerator =
 
                 SynLongIdent.createI fieldData.Ident, body
             )
-            |> AstHelper.instantiateRecord
+            |> SynExpr.createRecord None
 
         SynBinding.basic
             [ functionName ]

WoofWare.Myriad.Plugins/WoofWare.Myriad.Plugins.fsproj

@@ -22,7 +22,7 @@
   <ItemGroup>
     <PackageReference Include="Myriad.Core" Version="0.8.3" />
     <PackageReference Include="TypeEquality" Version="0.3.0" />
-    <PackageReference Include="WoofWare.Whippet.Fantomas" Version="0.3.2" />
+    <PackageReference Include="WoofWare.Whippet.Fantomas" Version="0.5.1" />
     <!-- the lowest version allowed by Myriad.Core -->
     <PackageReference Update="FSharp.Core" Version="6.0.1" PrivateAssets="all"/>
   </ItemGroup>

WoofWare.Myriad.Plugins/version.json

@@ -1,5 +1,5 @@
 {
-  "version": "4.0",
+  "version": "5.0",
   "publicReleaseRefSpec": [
     "^refs/heads/main$"
   ],
@@ -11,4 +11,4 @@
     ":/README.md",
     ":/Directory.Build.props"
   ]
-}
+}

flake.lock

@@ -20,11 +20,11 @@
     },
     "nixpkgs": {
       "locked": {
-        "lastModified": 1744502386,
-        "narHash": "sha256-QAd1L37eU7ktL2WeLLLTmI6P9moz9+a/ONO8qNBYJgM=",
+        "lastModified": 1744868846,
+        "narHash": "sha256-5RJTdUHDmj12Qsv7XOhuospjAjATNiTMElplWnJE9Hs=",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "f6db44a8daa59c40ae41ba6e5823ec77fe0d2124",
+        "rev": "ebe4301cbd8f81c4f8d3244b3632338bbeb6d49c",
         "type": "github"
       },
       "original": {

nix/deps.json

@@ -21,8 +21,8 @@
   },
   {
     "pname": "FsCheck",
-    "version": "3.1.0",
-    "hash": "sha256-u0Ha94EjClJ8evNSHu8d6+Dx9qYM3kV3sl1PEq9gF8s="
+    "version": "3.2.0",
+    "hash": "sha256-ksZ4vLgWwyQOzFuK2BczdtDtWWYmedG7UBAg4pYuI8g="
   },
   {
     "pname": "fsharp-analyzers",
@@ -381,7 +381,7 @@
   },
   {
     "pname": "WoofWare.Whippet.Fantomas",
-    "version": "0.3.2",
-    "hash": "sha256-dhzp/ASz1dlROJAfLYRKOCRlmjLBoS00KYeWEkSVtMA="
+    "version": "0.5.1",
+    "hash": "sha256-59CwnOZQAq5ZJoUkd87OiP8KUwx8xYDLMimMMTlKeZA="
   }
 ]