diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 4a6110cf514..670fb7c2c9c 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,6 +1,7 @@ ### Fixed * Fix attributes on return type of unparenthesized tuple methods being silently dropped from IL. ([Issue #462](https://github.com/dotnet/fsharp/issues/462), [PR #19714](https://github.com/dotnet/fsharp/pull/19714)) +* 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 attributes not resolved from opened namespaces in `namespace rec` / `module rec` scopes. ([Issue #7931](https://github.com/dotnet/fsharp/issues/7931), [PR #19502](https://github.com/dotnet/fsharp/pull/19502)) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 30d49e776c8..a2abd7ec83d 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/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) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fsi b/src/Compiler/Checking/Expressions/CheckExpressions.fsi index d8f801c7c5c..d99bfa88e18 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fsi +++ b/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 + [] val (|BinOpExpr|_|): SynExpr -> (Ident * SynExpr * SynExpr) voption diff --git a/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs b/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs index f8eeb4da5c1..86bf635d58a 100644 --- a/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs +++ b/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 diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs index bd3d90a8074..a7ff6c2e89b 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableRegressionTests.fs @@ -198,3 +198,65 @@ 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")] + +[] +let ``Issue 19644 - match-null narrowing inside list comprehension`` () = + FSharp """ +module M + +let lengths (xs: (string | null)[]) = [ + for x in xs do + match x with + | null -> () + | s -> yield s.Length +] +""" + |> withVersionAndCheckNulls ("preview", true) + |> compile + |> shouldSucceed + +[] +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 + +[] +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 + +[] +let ``Issue 19644 - match-null narrowing inside list comprehension (no for)`` () = + FSharp """ +module M + +let v (s: string | null) = [ + match s with + | null -> () + | x -> yield x.Length +] +""" + |> withVersionAndCheckNulls ("preview", true) + |> compile + |> shouldSucceed