Nullness issue - Option.toObj infers input to be an option of a nullable type

kerams commented 1 month ago

Issue description

When trying to pass a string option value to a null-agnostic method taking string using Option.toObj, the input type gets inferred as string | null option.

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).
[X] 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.1.24452.12

Reproducible code snippet and actual behavior

// assembly A, null-agnostic
let work (x: string) = ()

// assembly B, null-aware
let whatever x =
    work (Option.toObj x)

The type of whatever is inferred to be string | null option -> unit, but string option -> unit seems more reasonable.

Possible workarounds

let whatever (x: string option) =
    work (Option.toObj x)

vzarytovskii commented 1 month ago

Thanks, yeah, that should infer non-nullable. We are likely not make it the fix before sdk freeze (next Monday), but following release.

T-Gro commented 1 month ago

Yes, that does make more sense. I believe that in this case, the inference for the "work" agnostic function took precedence. The call will still work and will be open to a bigger variety of inputs for "whatever", however quickly becomes a negative once there are more lines working with "x" further below.

In the general case (ignoring ofObj), the inference should favour values without null, but fslib functions were special cased in a few places.

abonie commented 1 month ago

@kerams the title says ofObj, but you meant toObj, correct?

dotnet / fsharp