Fix nullness narrowing in match inside comprehensions

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -1,5 +1,6 @@
 ### Fixed
 
+* Fix false-positive nullness warning (FS3261) when pattern matching narrows nullness inside seq/list/array comprehensions. ([Issue #19644](https://github.com/dotnet/fsharp/issues/19644), [PR #19743](https://github.com/dotnet/fsharp/pull/19743))
 * Fix internal error FS0073 "Undefined or unsolved type variable" in IlxGen when nested inline SRTP functions with multiple overloads leave unsolved typars in the non-witness codegen path. ([Issue #19709](https://github.com/dotnet/fsharp/issues/19709), [PR #19710](https://github.com/dotnet/fsharp/pull/19710))
 * Fix NRE when calling virtual Object methods on value types through inline SRTP functions. ([Issue #8098](https://github.com/dotnet/fsharp/issues/8098), [PR #19511](https://github.com/dotnet/fsharp/pull/19511))
 * Fix DU case names matching IWSAM member names no longer cause duplicate property entries. (Issue [#14321](https://github.com/dotnet/fsharp/issues/14321), [PR #19341](https://github.com/dotnet/fsharp/pull/19341))

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -4125,6 +4125,41 @@ let formatAvailableNames (names: string array) =
     let result = truncated |> String.concat ", "
     if names.Length > 5 then result + ", ..." else result
 
+/// Given the current 'inputTy' of a match clause, the elaborated pattern, and the optional 'when' clause,
+/// returns the (possibly narrowed) inputTy to use for subsequent clauses.
+/// E.g. `match x with | null -> ... | y -> ...` narrows `inputTy` of the y-clause to non-null.
+let EliminateNullnessFromInputType (g: TcGlobals) (inputTy: TType) (pat: Pattern) (whenExprOpt: Expr option) : TType =
+    let removeNull t =
+        // Strip type equations (including abbreviations) and set nullness to non-null.
+        // For type abbreviations like `type objnull = obj | null`, we need to expand
+        // the abbreviation and apply non-null to the underlying type.
+        let stripped = stripTyEqns g t
+        replaceNullnessOfTy KnownWithoutNull stripped
+    let rec isWild (p: Pattern) =
+        match p with
+        | TPat_wild _ -> true
+        | TPat_as (p,_,_) -> isWild p
+        | TPat_disjs(patterns,_) -> patterns |> List.exists isWild
+        | TPat_conjs(patterns,_) -> patterns |> List.forall isWild
+        | TPat_tuple (_,pats,_,_) -> pats |> List.forall isWild
+        | _ -> false
+
+    let rec eliminateNull (ty: TType) (p: Pattern) =
+        match p with
+        | TPat_null _ -> removeNull ty
+        | TPat_as (p,_,_) -> eliminateNull ty p
+        | TPat_disjs(patterns,_) -> (ty,patterns) ||> List.fold eliminateNull
+        | TPat_tuple (_,pats,_,_) ->
+            match stripTyparEqns ty with
+            // In a tuple, if 1 elem is matched for null and the rest are wild => subsequent clauses can strip nullness
+            | TType_tuple(ti,tys) when tys.Length = pats.Length && (pats |> List.count (isWild >> not)) = 1 ->
+                TType_tuple(ti, List.map2 eliminateNull tys pats)
+            | _ -> ty
+        | _ -> ty
+    match whenExprOpt with
+    | None -> eliminateNull inputTy pat
+    | _ -> inputTy
+
 //-------------------------------------------------------------------------
 // Checking types and type constraints
 //-------------------------------------------------------------------------
@@ -10967,37 +11002,7 @@ and TcMatchClause cenv inputTy (resultTy: OverallTy) env isFirst tpenv synMatchC
 
     let target = TTarget(vspecs, resultExpr, None)
 
-    let inputTypeForNextPatterns=
-        let removeNull t =
-            // Strip type equations (including abbreviations) and set nullness to non-null.
-            // For type abbreviations like `type objnull = obj | null`, we need to expand
-            // the abbreviation and apply non-null to the underlying type.
-            let stripped = stripTyEqns cenv.g t
-            replaceNullnessOfTy KnownWithoutNull stripped
-        let rec isWild (p:Pattern) =
-            match p with
-            | TPat_wild _ -> true
-            | TPat_as (p,_,_) -> isWild p
-            | TPat_disjs(patterns,_) -> patterns |> List.exists isWild
-            | TPat_conjs(patterns,_) -> patterns |> List.forall isWild
-            | TPat_tuple (_,pats,_,_) -> pats |> List.forall isWild
-            | _ -> false
-
-        let rec eliminateNull (ty:TType) (p:Pattern) =
-            match p with
-            | TPat_null _ -> removeNull ty
-            | TPat_as (p,_,_) -> eliminateNull ty p
-            | TPat_disjs(patterns,_) -> (ty,patterns) ||> List.fold eliminateNull
-            | TPat_tuple (_,pats,_,_) ->
-                match stripTyparEqns ty with
-                // In a tuple, if 1 elem is matched for null and the rest are wild => subsequent clauses can strip nullness
-                | TType_tuple(ti,tys) when tys.Length = pats.Length && (pats |> List.count (isWild >> not)) = 1 ->
-                    TType_tuple(ti, List.map2 eliminateNull tys pats)
-                | _ -> ty
-            | _ -> ty
-        match whenExprOpt with
-        | None -> eliminateNull inputTy pat
-        | _ -> inputTy
+    let inputTypeForNextPatterns = EliminateNullnessFromInputType cenv.g inputTy pat whenExprOpt
 
     MatchClause(pat, whenExprOpt, target, patm), (tpenv,inputTypeForNextPatterns)
 

src/Compiler/Checking/Expressions/CheckExpressions.fsi

@@ -721,6 +721,11 @@ val TcMatchPattern:
     tcTrueMatchClause: TcTrueMatchClause ->
         Pattern * Expr option * Val list * TcEnv * UnscopedTyparEnv
 
+/// Given the current 'inputTy' of a match clause, the elaborated pattern, and the optional 'when' clause,
+/// returns the (possibly narrowed) inputTy to use for subsequent clauses.
+/// E.g. `match x with | null -> ... | y -> ...` narrows `inputTy` of the y-clause to non-null.
+val EliminateNullnessFromInputType: g: TcGlobals -> inputTy: TType -> pat: Pattern -> whenExprOpt: Expr option -> TType
+
 [<return: Struct>]
 val (|BinOpExpr|_|): SynExpr -> (Ident * SynExpr * SynExpr) voption
 

src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs

@@ -272,9 +272,9 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
         | SynExpr.Match(spMatch, expr, clauses, _m, _trivia) ->
             let inputExpr, inputTy, tpenv = TcExprOfUnknownType cenv env tpenv expr
 
-            let tclauses, tpenv =
-                (tpenv, clauses)
-                ||> List.mapFold (fun tpenv (SynMatchClause(pat, cond, innerComp, _, sp, _trivia) as clause) ->
+            let tclauses, (tpenv, _finalInputTy) =
+                ((tpenv, inputTy), clauses)
+                ||> List.mapFold (fun (tpenv, inputTy) (SynMatchClause(pat, cond, innerComp, _, sp, _trivia) as clause) ->
                     let isTrueMatchClause =
                         if clause.IsTrueMatchClause then
                             TcTrueMatchClause.Yes
@@ -290,7 +290,9 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT
                         | DebugPointAtTarget.No -> envinner
 
                     let innerExpr, tpenv = tcSequenceExprBody envinner genOuterTy tpenv innerComp
-                    MatchClause(patR, condR, TTarget(vspecs, innerExpr, None), patR.Range), tpenv)
+
+                    let nextInputTy = EliminateNullnessFromInputType cenv.g inputTy patR condR
+                    MatchClause(patR, condR, TTarget(vspecs, innerExpr, None), patR.Range), (tpenv, nextInputTy))
 
             let inputExprTy = tyOfExpr cenv.g inputExpr
             let inputExprMark = inputExpr.Range

tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs

@@ -198,3 +198,67 @@ type C7 =
             |> shouldFail
             |> withDiagnostics 
                 [(Error 444, Line 7, Col 17, Line 7, Col 23, "The type of a field using the 'DefaultValue' attribute must admit default initialization, i.e. have 'null' as a proper value or be a struct type whose fields all admit default initialization. You can use 'DefaultValue(false)' to disable this check")]
+
+[<Fact>]
+let ``Issue 19644 - match-null narrowing inside list comprehension`` () =
+    FSharp """
+module M
+open System.Collections.Immutable
+
+let sets (builders: (ImmutableHashSet<int>.Builder | null)[]) = [
+    for builder in builders do
+        match builder with
+        | null -> ()
+        | b -> yield b.ToImmutable()
+]
+"""
+    |> withVersionAndCheckNulls ("preview", true)
+    |> compile
+    |> shouldSucceed
+
+[<Fact>]
+let ``Issue 19644 - match-null narrowing inside seq expression`` () =
+    FSharp """
+module M
+let v (xs: (string | null) seq) =
+    seq {
+        for x in xs do
+            match x with
+            | null -> ()
+            | y -> yield y.Length
+    }
+"""
+    |> withVersionAndCheckNulls ("preview", true)
+    |> compile
+    |> shouldSucceed
+
+[<Fact>]
+let ``Issue 19644 - match-null narrowing inside array comprehension`` () =
+    FSharp """
+module M
+let v (xs: (string | null)[]) = [|
+    for x in xs do
+        match x with
+        | null -> ()
+        | y -> yield y.Length
+|]
+"""
+    |> withVersionAndCheckNulls ("preview", true)
+    |> compile
+    |> shouldSucceed
+
+[<Fact>]
+let ``Issue 19644 - match-null narrowing inside list comprehension (no for)`` () =
+    FSharp """
+module M
+open System.Collections.Immutable
+
+let v (b: ImmutableHashSet<int>.Builder | null) = [
+    match b with
+    | null -> ()
+    | x -> yield x.ToImmutable()
+]
+"""
+    |> withVersionAndCheckNulls ("preview", true)
+    |> compile
+    |> shouldSucceed