From b715dce082c2072d899d593628e7fad61ee883cf Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 14 May 2026 16:57:53 +0200 Subject: [PATCH 1/3] Add failing TDD tests for issue #19646 (alias dropped after | null pattern) After 'match x with | null -> ... | s -> ...', the type of s loses its alias (e.g. 'string' becomes 'System.String'). These tests lock in the desired behavior; they fail on main and will pass once the fix lands. Refs https://github.com/dotnet/fsharp/issues/19646 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Nullness/NullableReferenceTypesTests.fs | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs index c36c51b3a15..cfbfafc74ae 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs @@ -2373,3 +2373,78 @@ let main _ = 0 |> compile |> run |> verifyOutputContains [|"-1"|] + +// Regression for https://github.com/dotnet/fsharp/issues/19646 +// After `match x with | null -> … | s -> …` the binding `s` must keep +// the alias `string`, not be stripped to its BCL representation `String`. +[] +let ``Issue 19646 - string alias is preserved after null pattern`` () = + FSharp """module Test + +let test (x: string | null) : int = + match x with + | null -> 0 + | s -> s + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnosticMessageMatches "'string'" + +// Regression for https://github.com/dotnet/fsharp/issues/19646 +// Same scenario expressed with a user-defined transparent type alias. +[] +let ``Issue 19646 - user type alias is preserved after null pattern`` () = + FSharp """module Test + +type MyStr = string + +let test (x: MyStr | null) : int = + match x with + | null -> 0 + | s -> s + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnosticMessageMatches "'MyStr'" + +// Regression for https://github.com/dotnet/fsharp/issues/19646 +// The fix must not erase the alias on `Uri` (a non-string class type) either. +[] +let ``Issue 19646 - Uri alias is preserved after null pattern`` () = + FSharp """module Test + +open System + +type MyUri = Uri + +let test (x: MyUri | null) : int = + match x with + | null -> 0 + | s -> s + """ + |> asLibrary + |> typeCheckWithStrictNullness + |> shouldFail + |> withDiagnosticMessageMatches "'MyUri'" + +// Regression for https://github.com/dotnet/fsharp/issues/19646 +// Negative: the BCL name `String` MUST NOT appear in the error message +// when the source used the `string` alias. +[] +let ``Issue 19646 - error message must not show String when source used string`` () = + let result = + FSharp """module Test + +let test (x: string | null) : int = + match x with + | null -> 0 + | s -> s + """ + |> asLibrary + |> typeCheckWithStrictNullness + result |> shouldFail |> ignore + // After the fix, the diagnostic must mention `string`, not `String`. + result + |> withDiagnosticMessageMatches "'string'" From 8e727fcf79949eeee178ab1d72826ddd8706ba00 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 14 May 2026 17:21:29 +0200 Subject: [PATCH 2/3] Fix #19646: preserve type aliases in match | null refinement The removeNull helper in TcMatchClause was unconditionally calling stripTyEqns, which expanded transparent abbreviations like 'string' to 'System.String'. As a result, in: let test (x: string | null) = match x with | null -> null | s -> s.Replace("a", "b") the inferred type of 's' was displayed as 'String' instead of 'string'. Fix: try the cheap path first (replaceNullnessOfTy KnownWithoutNull), which preserves the abbreviation. If the resulting effective nullness is already WithoutNull (transparent aliases like 'string' / 'MyStr'), use that. Otherwise (abbreviations that encode nullness in their RHS, like 'type objnull = obj | null'), fall back to the full stripTyEqns path introduced by #18852 to keep that regression fixed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../.FSharp.Compiler.Service/11.0.100.md | 1 + .../Checking/Expressions/CheckExpressions.fs | 30 +++++++++++++++---- 2 files changed, 26 insertions(+), 5 deletions(-) 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 3e2c18a6ef0..cd70805c40d 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,5 +1,6 @@ ### Fixed +* Preserve type abbreviations (`string`, user-defined aliases) in the refined type of bindings introduced after a `| null` pattern in a `match` expression. ([Issue #19646](https://github.com/dotnet/fsharp/issues/19646)) * 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 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)) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 30d49e776c8..06c4b098324 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -10969,11 +10969,31 @@ and TcMatchClause cenv inputTy (resultTy: OverallTy) env isFirst tpenv synMatchC 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 + // We want to refine `t` to its "non-null" version for the remaining + // match clauses. Two cases: + // + // 1. Plain/transparent abbreviations such as `string` (an alias for + // `System.String`) or any user-defined `type MyStr = string` do + // NOT themselves introduce a `WithNull` annotation. For these + // just clearing the outer nullness via `replaceNullnessOfTy` + // yields a correctly non-null type AND preserves the alias in + // tooltips / error messages. See issue #19646. + // + // 2. Abbreviations whose RHS encodes nullness (e.g. + // `type objnull = obj | null`) hide a `WithNull` annotation + // inside the expansion. Just clearing the outer nullness is not + // enough because `addNullnessToTy KnownWithoutNull` over a type + // that already has `WithNull` returns the original `WithNull` + // (see `combineNullness` in TypedTreeBasics.fs). For these we + // must fully strip and re-mark the underlying type. See issue + // #18488 / PR #18852. + let nonNullOriginal = replaceNullnessOfTy KnownWithoutNull t + match (nullnessOfTy cenv.g nonNullOriginal).TryEvaluate() with + | ValueSome NullnessInfo.WithoutNull -> + nonNullOriginal + | _ -> + let stripped = stripTyEqns cenv.g t + replaceNullnessOfTy KnownWithoutNull stripped let rec isWild (p:Pattern) = match p with | TPat_wild _ -> true From 716d47d497ed100a44f031d9939e551d8f9e7d3b Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Thu, 14 May 2026 18:03:13 +0200 Subject: [PATCH 3/3] Collapse Issue 19646 tests into a Theory Per review feedback: parameterize the three alias preservation cases (string, user alias, Uri alias) into a single Theory with InlineData, and drop the redundant negative test (the case-sensitive substring match on 'string' already rules out 'String'). Refs #19646 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../.FSharp.Compiler.Service/11.0.100.md | 2 +- .../Checking/Expressions/CheckExpressions.fs | 23 ++---- .../Nullness/NullableReferenceTypesTests.fs | 77 +++---------------- 3 files changed, 18 insertions(+), 84 deletions(-) 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 cd70805c40d..288f14b9de6 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,6 @@ ### Fixed -* Preserve type abbreviations (`string`, user-defined aliases) in the refined type of bindings introduced after a `| null` pattern in a `match` expression. ([Issue #19646](https://github.com/dotnet/fsharp/issues/19646)) +* Preserve type abbreviations (`string`, user-defined aliases) in the refined type of bindings introduced after a `| null` pattern in a `match` expression. ([Issue #19646](https://github.com/dotnet/fsharp/issues/19646), [PR #19745](https://github.com/dotnet/fsharp/pull/19745)) * 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 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)) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 06c4b098324..14cd5266ae6 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -10969,24 +10969,11 @@ and TcMatchClause cenv inputTy (resultTy: OverallTy) env isFirst tpenv synMatchC let inputTypeForNextPatterns= let removeNull t = - // We want to refine `t` to its "non-null" version for the remaining - // match clauses. Two cases: - // - // 1. Plain/transparent abbreviations such as `string` (an alias for - // `System.String`) or any user-defined `type MyStr = string` do - // NOT themselves introduce a `WithNull` annotation. For these - // just clearing the outer nullness via `replaceNullnessOfTy` - // yields a correctly non-null type AND preserves the alias in - // tooltips / error messages. See issue #19646. - // - // 2. Abbreviations whose RHS encodes nullness (e.g. - // `type objnull = obj | null`) hide a `WithNull` annotation - // inside the expansion. Just clearing the outer nullness is not - // enough because `addNullnessToTy KnownWithoutNull` over a type - // that already has `WithNull` returns the original `WithNull` - // (see `combineNullness` in TypedTreeBasics.fs). For these we - // must fully strip and re-mark the underlying type. See issue - // #18488 / PR #18852. + // Remove nullness, preserving aliases when possible (#19646): + // `string | null` → `string` (not `System.String`) + // `MyStr | null` → `MyStr` + // Fall back to stripping when nullness is baked into the abbreviation + // RHS, e.g. `type objnull = obj | null` (#18488). let nonNullOriginal = replaceNullnessOfTy KnownWithoutNull t match (nullnessOfTy cenv.g nonNullOriginal).TryEvaluate() with | ValueSome NullnessInfo.WithoutNull -> diff --git a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs index cfbfafc74ae..f5687ffb349 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/Nullness/NullableReferenceTypesTests.fs @@ -2374,52 +2374,19 @@ let main _ = 0 |> run |> verifyOutputContains [|"-1"|] -// Regression for https://github.com/dotnet/fsharp/issues/19646 -// After `match x with | null -> … | s -> …` the binding `s` must keep -// the alias `string`, not be stripped to its BCL representation `String`. -[] -let ``Issue 19646 - string alias is preserved after null pattern`` () = - FSharp """module Test - -let test (x: string | null) : int = - match x with - | null -> 0 - | s -> s - """ - |> asLibrary - |> typeCheckWithStrictNullness - |> shouldFail - |> withDiagnosticMessageMatches "'string'" - -// Regression for https://github.com/dotnet/fsharp/issues/19646 -// Same scenario expressed with a user-defined transparent type alias. -[] -let ``Issue 19646 - user type alias is preserved after null pattern`` () = - FSharp """module Test - -type MyStr = string - -let test (x: MyStr | null) : int = - match x with - | null -> 0 - | s -> s - """ - |> asLibrary - |> typeCheckWithStrictNullness - |> shouldFail - |> withDiagnosticMessageMatches "'MyStr'" - -// Regression for https://github.com/dotnet/fsharp/issues/19646 -// The fix must not erase the alias on `Uri` (a non-string class type) either. -[] -let ``Issue 19646 - Uri alias is preserved after null pattern`` () = - FSharp """module Test - -open System +// https://github.com/dotnet/fsharp/issues/19646 +// After `| null -> … | s -> …`, `s` must keep its type alias, not the BCL type. +[] +[] +[] +[] +let ``Issue 19646 - type alias is preserved after null pattern`` + (typeDef: string, paramTypeName: string, expectedSubstring: string) = + FSharp $"""module Test -type MyUri = Uri +{typeDef} -let test (x: MyUri | null) : int = +let test (x: {paramTypeName} | null) : int = match x with | null -> 0 | s -> s @@ -2427,24 +2394,4 @@ let test (x: MyUri | null) : int = |> asLibrary |> typeCheckWithStrictNullness |> shouldFail - |> withDiagnosticMessageMatches "'MyUri'" - -// Regression for https://github.com/dotnet/fsharp/issues/19646 -// Negative: the BCL name `String` MUST NOT appear in the error message -// when the source used the `string` alias. -[] -let ``Issue 19646 - error message must not show String when source used string`` () = - let result = - FSharp """module Test - -let test (x: string | null) : int = - match x with - | null -> 0 - | s -> s - """ - |> asLibrary - |> typeCheckWithStrictNullness - result |> shouldFail |> ignore - // After the fix, the diagnostic must mention `string`, not `String`. - result - |> withDiagnosticMessageMatches "'string'" + |> withDiagnosticMessageMatches expectedSubstring