Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 of 7 tasks
kerams opened this issue Nov 3, 2024 · 1 comment · May be fixed by #18057
Open
1 of 7 tasks

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

kerams opened this issue Nov 3, 2024 · 1 comment · May be fixed by #18057
Assignees
Labels
Area-Nullness Issues related to handling of Nullable Reference Types Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Milestone

Comments

@kerams
Copy link
Contributor

kerams commented Nov 3, 2024

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

  • Unexpected nullness warning (false positive in nullness checking, code uses --checknulls and langversion:preview).
  • Missing nullness warning in a case which can produce nulls (false negative, code uses --checknulls and langversion:preview).
  • Breaking change related to older null constructs in code not using the checknulls switch.
  • Breaking change related to generic code and explicit type constraints (null, not null).
  • Type inference issue (i.e. code worked without type annotations before, and applying the --checknulls enforces type annotations).
  • C#/F# interop issue related to nullness metadata.
  • Other (none of the categories above apply).

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

@kerams kerams added Area-Nullness Issues related to handling of Nullable Reference Types Bug Needs-Triage labels Nov 3, 2024
@github-actions github-actions bot added this to the Backlog milestone Nov 3, 2024
@T-Gro
Copy link
Member

T-Gro commented Nov 4, 2024

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:

  • Value types
  • Anon records
  • Tuples

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Nullness Issues related to handling of Nullable Reference Types Bug Impact-Medium (Internal MS Team use only) Describes an issue with moderate impact on existing code.
Projects
Status: New
3 participants