dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.93k stars 787 forks source link

Nullness issue - No warning when nullable generic return type used with anonymous record #17951

Open kerams opened 3 weeks ago

kerams commented 3 weeks ago

Issue description

type R = { x: int }
type RA = {| x: int |}

[<EntryPoint>]
let main _args =
    let a = System.Text.Json.JsonSerializer.Deserialize<{| x: int |}> "null"
    let _a = a.x

    let b = System.Text.Json.JsonSerializer.Deserialize<RA> "null"
    let _b = b.x

    let c = System.Text.Json.JsonSerializer.Deserialize<R> "null"
    let _c = c.x
    0

The return type of Deserialize is 'T | null, but a warning is only raised for the nominal record on c.x, yet a.x and b.x will throw too.

Additionally, there's a discrepancy in tooltips - a is val a: {| a: int |}, but on b, where the type is aliased, it is val b: RA | null.

Choose one or more from the following categories of impact

Operating System

Windows (Default)

What .NET runtime/SDK kind are you seeing the issue on

.NET SDK (.NET Core, .NET 5+)

.NET Runtime/SDK version

9.0.100-rc.2.24474.11

Reproducible code snippet and actual behavior

No response

Possible workarounds

No response

T-Gro commented 2 weeks ago

Thanks for the report @kerams .

F# tuples and F# anonymous records do not support nullable qualifiers by intention - since those are considered "F#-owned" building blocks and not something to come from the outside, the intention of the Nullable Reference Types features was not to sneak nulls into them.

It might make sense on generic usage if the API is marked as T | null, this is not being done at the moment.

As of now, APIs from C# returning T | null do not report any warning, but also are not allowed to put the nullness qualifier onto:

It should not be allowed to return to anon | null from purely F# code, but the diagnostics must get stricter at generic type instantiation.

The obvious workaround is to use named and not anonymous records.