fsharp
fsharp copied to clipboard
Better diagnostics for mixing different nullness pattern matching ways
There are 2 ways to do null-related pattern matching now:
let len1 (str: string | null) =
match str with
| Null -> -1
| NonNull s -> s.Length
let len2 (str: string | null) =
match str with
| null -> -1
| s -> s.Length
Both produce no warnings, all good. What I don't like is the behavior when we start mixing those two approaches:
let len3 (str: string | null) =
match str with
| null -> -1
| NonNull s -> s.Length
warning FS3262: Value known to be without null passed to a function meant for nullables: You can remove this |Null|NonNull| pattern usage.
let len4 (str: string | null) =
match str with
| Null -> -1
| s -> s.Length
warning FS3261: Nullness warning: The types 'string' and 'string | null' do not have compatible nullability.
As a user I just want consistent diags here in the spirit of "hey, pick one way or another, don't mix things up" - ideally with exact pointers what to change but that's an extra.
I think such errors can happen really often because of fatfingering or copypasting - and casing differences are not even very noticeable, so there is accessibility in the game as well.
Originally posted by @psfinaki in https://github.com/dotnet/fsharp/pull/15181#discussion_r1677921389
Related to https://github.com/dotnet/fsharp/issues/17409
Might be implicitely solved by contextual information to nullness diag (... "possible derefence"..)
Idea: Make the active pattern |Null|NonNull| to an module which needs explicit opening or explicit prefixing.
Idea: RQA for the active pattern:
[<RequireQualifiedAccess>]
module Nullable =
let inline (|Null|NonNull|) (x: 'T) : Choice<unit,'T> =
match x with null -> Null | v -> NonNull v
What is the main reason for those patterns to exist?
I find matching on literal null is most natural thing to do.
I'd favour explicit opening but no RQA attribute as it makes it really inconvenient to use for those that prefer it.
The active pattern can eliminated nullness from a type (i.e. what you match as values will be known to be non-nullable) even in deeply nested scenarios. Imagine a string | null field within record, being wrapped in voption, within another record.
The the classical null literal this only eliminated nullness on first level values, but not from nested members of types.
From it's signature, NonNull will give you a value known to be not null.