fix: null target tryGet and exists, and explicitConvert of null

CHANGELOG.md

@@ -4,6 +4,10 @@ Each release has a `## [version]` section. When a `v*.*.*` tag is pushed, the Pu
 
 ## [Unreleased]
 
+### Fixed
+
+- `Dyn.tryGet` and `Dyn.exists` on a null target return `None` and `false`. `Dyn.explicitConvert` and `explicitConvertTo` of null follow `implicitConvert` (null for a reference result type; a value type still fails). `Dyn.set` on a null target still throws `NullReferenceException` (#111).
+
 ### Docs
 
 - Caveats document 15-argument `TypeLoadException`, C# optional parameters, and `tryGet` / `exists` on a null target (#110).

FSharp.Interop.Dynamic/Dyn.fs

@@ -40,7 +40,10 @@ module Dyn =
         implicitConvertTo typeof<'TResult> target
 
     let explicitConvertTo (convertType:Type) (target:obj) : 'TResult  =
-        Dynamic.InvokeConvert(target, convertType, explicit = true) |> unbox<'TResult>
+        match target with
+        | null -> target
+        | t -> Dynamic.InvokeConvert(t, convertType, explicit = true)
+        |> unbox<'TResult>
 
     let explicitConvert (target:obj) : 'TResult  =
         explicitConvertTo typeof<'TResult> target
@@ -145,23 +148,29 @@ module Dyn =
         invocation target (Member propertyName)
 
     let tryGet (propertyName:string) (target:obj) : 'T option =
-        let found =
+        match target with
+        | null -> None
+        | target ->
+            let found =
+                try
+                    Some (Dynamic.InvokeGet(target, propertyName))
+                with
+                | :? RuntimeBinderException -> None
+                | _ -> reraise()
+            match found with
+            | None -> None
+            | Some value -> Some (invocation value Direct)
+
+    let exists (propertyName:string) (target:obj) : bool =
+        match target with
+        | null -> false
+        | target ->
             try
-                Some (Dynamic.InvokeGet(target, propertyName))
+                Dynamic.InvokeGet(target, propertyName) |> ignore
+                true
             with
-            | :? RuntimeBinderException -> None
+            | :? RuntimeBinderException -> false
             | _ -> reraise()
-        match found with
-        | None -> None
-        | Some value -> Some (invocation value Direct)
-
-    let exists (propertyName:string) (target:obj) : bool =
-        try
-            Dynamic.InvokeGet(target, propertyName) |> ignore
-            true
-        with
-        | :? RuntimeBinderException -> false
-        | _ -> reraise()
 
     let getChain (chainOfMembers:string seq) (target:obj) : 'TResult =
         let chainOfMembers' = String.concat "." chainOfMembers

FSharp.Interop.Dynamic/Dyn.fsi

@@ -7,7 +7,7 @@ namespace FSharp.Interop.Dynamic
     | Member of string
     /// Invoke the target itself, not a named member.
     | Direct
-
+
   /// <summary>
   /// Function surface behind the operators. Use when you want piping, a name as data, or an operation <c>?</c> does not spell. Target is last on most members so <c>target |> Dyn.get "Name"</c> works; <c>invocation</c> is the exception.
   /// </summary>
@@ -21,9 +21,9 @@ namespace FSharp.Interop.Dynamic
     /// <summary>Implicit conversion of <paramref name="target"/> to the inferred result type.</summary>
     val implicitConvert : target:obj -> 'TResult
 
-    /// <summary>Explicit conversion of <paramref name="target"/> to <paramref name="convertType"/>.</summary>
+    /// <summary>Explicit conversion of <paramref name="target"/> to <paramref name="convertType"/>. A null <paramref name="target"/> follows <c>implicitConvertTo</c>: null for a reference result type.</summary>
     val explicitConvertTo : convertType:System.Type -> target:obj -> 'TResult
-    /// <summary>Explicit conversion of <paramref name="target"/> to the inferred result type.</summary>
+    /// <summary>Explicit conversion of <paramref name="target"/> to the inferred result type. A null <paramref name="target"/> follows <c>implicitConvert</c>: null for a reference result type.</summary>
     val explicitConvert : target:obj -> 'TResult
     /// Marks an argument with a name for DLR named-argument invoke.
     val namedArg : name:string -> argValue:obj -> Dynamitey.InvokeArg
@@ -72,7 +72,7 @@ namespace FSharp.Interop.Dynamic
     /// <param name="propertyName">Member name to look up.</param>
     /// <param name="target">Object to read from.</param>
     /// <returns>
-    /// <c>None</c> if the binder cannot find the member.
+    /// <c>None</c> if <paramref name="target"/> is null or the binder cannot find the member.
     /// <c>Some</c> value if the member is present and converts to <c>'T</c>
     /// (a present null is <c>Some null</c> for a reference <c>'T</c>).
     /// Throws if the member is present but cannot convert to <c>'T</c>.
@@ -87,7 +87,7 @@ namespace FSharp.Interop.Dynamic
     /// <param name="target">Object to read from.</param>
     /// <returns>
     /// <c>true</c> if <c>InvokeGet</c> succeeds, including when the value is null.
-    /// <c>false</c> if the binder cannot find the member.
+    /// <c>false</c> if <paramref name="target"/> is null or the binder cannot find the member.
     /// Does not convert the value, so this can be <c>true</c> while <c>tryGet</c> as a given type throws.
     /// A getter that throws something other than a binder miss is not reported as missing.
     /// </returns>

README.md

@@ -128,7 +128,7 @@ You can still `open Dynamitey` for `Build`, `Dynamic.Curry`, `DynamicObjects.Dic
 - **Not trim-safe or NativeAOT-safe.**
 - **15 or more arguments** throw `TypeLoadException` (ImpromptuInterface was never shipped). Fourteen work.
 - **C# optional parameters** must all be passed. The DLR does not fill defaults.
-- `tryGet` / `exists` on a **null target** throw `NullReferenceException` (not `None` / `false`).
+- `tryGet` / `exists` on a **null target** are `None` / `false`. `Dyn.set` on a null target still throws `NullReferenceException`.
 - Do not build member names from untrusted input. [SECURITY.md](SECURITY.md).
 - Historical: .NET Core 2.0.0–2.0.2 broke `dynamic` on nested types inside generics ([#11](https://github.com/fsprojects/FSharp.Interop.Dynamic/issues/11)). Current TFMs are fine.
 

Tests/Invocation.fs

@@ -111,6 +111,24 @@ module Invocation =
         let toDecimal: int -> decimal = Dyn.implicitConvert
         toDecimal 50 |> should equal 50M
 
+    [<Fact>]
+    let ``explicitConvert of null matches implicitConvert for a reference type`` () =
+        let viaImplicit: string = null |> Dyn.implicitConvert
+        let viaExplicit: string = null |> Dyn.explicitConvert
+        let viaExplicitTo: string = null |> Dyn.explicitConvertTo typeof<string>
+        Assert.Null viaImplicit
+        Assert.Null viaExplicit
+        Assert.Null viaExplicitTo
+
+    [<Fact>]
+    let ``explicitConvert of null to a value type fails like implicitConvert`` () =
+        let implicitFail () : int = null |> Dyn.implicitConvert
+        let explicitFail () : int = null |> Dyn.explicitConvert
+        let explicitToFail () : int = null |> Dyn.explicitConvertTo typeof<int>
+        (fun () -> implicitFail () |> ignore) |> should throw typeof<NullReferenceException>
+        (fun () -> explicitFail () |> ignore) |> should throw typeof<NullReferenceException>
+        (fun () -> explicitToFail () |> ignore) |> should throw typeof<NullReferenceException>
+
     [<Fact>]
     let ``getChain on a missing link raises a binder exception`` () =
         let target = ExpandoObject()

Tests/TryGet.fs

@@ -78,4 +78,13 @@ module TryGet =
     let ``tryGet returns None for a missing member even when the result type is a function`` () =
         let o = ExpandoObject()
         let missing: (int -> int) option = o |> Dyn.tryGet "nope"
-        missing |> should equal None
+        missing |> should equal None
+
+    [<Fact>]
+    let ``tryGet on a null target is None`` () =
+        let missing: string option = (null: obj) |> Dyn.tryGet "X"
+        missing |> should equal None
+
+    [<Fact>]
+    let ``exists on a null target is false`` () =
+        (null: obj) |> Dyn.exists "X" |> should equal false

docfx/docs/caveats.md

@@ -20,7 +20,7 @@ Annotate `'T` / `unit` so the binder and the conversion agree.
 
 ## Null target
 
-`Dyn.tryGet` and `Dyn.exists` on a **null target** throw `NullReferenceException`. That is not a binder miss, so it is not `None` / `false`. Same for `Dyn.set`. A present null *value* is still `Some null` / `exists` true. Changing the null-target contract is [#111](https://github.com/fsprojects/FSharp.Interop.Dynamic/issues/111).
+`Dyn.tryGet` on a null target is `None`. `Dyn.exists` is `false`. That is the same answer as a missing member. `Dyn.set` on a null target still throws `NullReferenceException`. A present null *value* is still `Some null` / `exists` true.
 
 ## More than 14 arguments
 

docfx/docs/tryget.md

@@ -27,7 +27,7 @@ Lookup is `Dynamic.InvokeGet` only. Then the existing conversion / callable wrap
 | Member present, value cannot convert to `'T` | throws (`InvalidCastException` or a binder exception) — **not** `None` |
 | Member missing, `'T` is a function type | `None` immediately (does not return a lazy callable) |
 | Getter throws something else | that exception propagates |
-| Target is null | `NullReferenceException` (not `None`). [#110](https://github.com/fsprojects/FSharp.Interop.Dynamic/issues/110), [#111](https://github.com/fsprojects/FSharp.Interop.Dynamic/issues/111) |
+| Target is null | `None` |
 
 ## `Dyn.exists`
 
@@ -36,7 +36,7 @@ o |> Dyn.exists "myProp"   // true
 o |> Dyn.exists "nope"     // false
 ```
 
-True when `InvokeGet` succeeds. A present null is still present. A throwing getter is not reported as missing. A **null target** throws `NullReferenceException`, not `false`.
+True when `InvokeGet` succeeds. A present null is still present. A throwing getter is not reported as missing. A null target is `false`.
 
 `exists` does not convert the value. That is the difference from `tryGet >> Option.isSome` when conversion would fail: `exists` can be true while `tryGet` as `int` throws.