diff --git a/CHANGELOG.md b/CHANGELOG.md index 3466d2f..90bde09 100644 --- a/CHANGELOG.md +++ b/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). diff --git a/FSharp.Interop.Dynamic/Dyn.fs b/FSharp.Interop.Dynamic/Dyn.fs index 03d8dec..33052c9 100644 --- a/FSharp.Interop.Dynamic/Dyn.fs +++ b/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 diff --git a/FSharp.Interop.Dynamic/Dyn.fsi b/FSharp.Interop.Dynamic/Dyn.fsi index 2c5302d..0c76b3f 100644 --- a/FSharp.Interop.Dynamic/Dyn.fsi +++ b/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 - + /// /// Function surface behind the operators. Use when you want piping, a name as data, or an operation ? does not spell. Target is last on most members so target |> Dyn.get "Name" works; invocation is the exception. /// @@ -21,9 +21,9 @@ namespace FSharp.Interop.Dynamic /// Implicit conversion of to the inferred result type. val implicitConvert : target:obj -> 'TResult - /// Explicit conversion of to . + /// Explicit conversion of to . A null follows implicitConvertTo: null for a reference result type. val explicitConvertTo : convertType:System.Type -> target:obj -> 'TResult - /// Explicit conversion of to the inferred result type. + /// Explicit conversion of to the inferred result type. A null follows implicitConvert: null for a reference result type. 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 /// Member name to look up. /// Object to read from. /// - /// None if the binder cannot find the member. + /// None if is null or the binder cannot find the member. /// Some value if the member is present and converts to 'T /// (a present null is Some null for a reference 'T). /// Throws if the member is present but cannot convert to 'T. @@ -87,7 +87,7 @@ namespace FSharp.Interop.Dynamic /// Object to read from. /// /// true if InvokeGet succeeds, including when the value is null. - /// false if the binder cannot find the member. + /// false if is null or the binder cannot find the member. /// Does not convert the value, so this can be true while tryGet as a given type throws. /// A getter that throws something other than a binder miss is not reported as missing. /// diff --git a/README.md b/README.md index 9be5953..0fa006a 100644 --- a/README.md +++ b/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. diff --git a/Tests/Invocation.fs b/Tests/Invocation.fs index 4f1568f..dbd25d0 100644 --- a/Tests/Invocation.fs +++ b/Tests/Invocation.fs @@ -111,6 +111,24 @@ module Invocation = let toDecimal: int -> decimal = Dyn.implicitConvert toDecimal 50 |> should equal 50M + [] + 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 + Assert.Null viaImplicit + Assert.Null viaExplicit + Assert.Null viaExplicitTo + + [] + 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 + (fun () -> implicitFail () |> ignore) |> should throw typeof + (fun () -> explicitFail () |> ignore) |> should throw typeof + (fun () -> explicitToFail () |> ignore) |> should throw typeof + [] let ``getChain on a missing link raises a binder exception`` () = let target = ExpandoObject() diff --git a/Tests/TryGet.fs b/Tests/TryGet.fs index 052b4bb..52095cb 100644 --- a/Tests/TryGet.fs +++ b/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 \ No newline at end of file + missing |> should equal None + + [] + let ``tryGet on a null target is None`` () = + let missing: string option = (null: obj) |> Dyn.tryGet "X" + missing |> should equal None + + [] + let ``exists on a null target is false`` () = + (null: obj) |> Dyn.exists "X" |> should equal false \ No newline at end of file diff --git a/docfx/docs/caveats.md b/docfx/docs/caveats.md index a6123bc..060bf16 100644 --- a/docfx/docs/caveats.md +++ b/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 diff --git a/docfx/docs/tryget.md b/docfx/docs/tryget.md index 9543fa6..d5ec7aa 100644 --- a/docfx/docs/tryget.md +++ b/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.