Preserve type aliases in match | null refinement#19745
Conversation
…ttern) 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 #19646 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
❗ Release notes required
|
T-Gro
added
AI-Auto-Resolve-Conflicts
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>
T-Gro
force-pushed
the
fix/nullness-dropping-alias
branch
from
a34dcfc to
716d47d
Compare
github-actions
Bot
added
the
AI-Tooling-Check-Bypassed
T-Gro
left a comment
T-Gro
left a comment
There was a problem hiding this comment.
Review (self-check)
Clean and well-reasoned fix.
Logic: The two-phase approach (optimistic alias-preserving path → validated via nullnessOfTy → fallback to strip) is correct. When nullness is only in the outer annotation (e.g. string | null), clearing it preserves the alias. When it's baked into the abbreviation RHS (e.g. type objnull = obj | null), the TryEvaluate() check correctly detects the residual nullness and falls back to the prior stripping behavior.
Edge cases covered:
- Direct BCL-alias types (
string→ preserved, not expanded toSystem.String) - User-defined aliases of BCL types (
type MyStr = string→ preserved) - User-defined aliases of reference types (
type MyUri = Uri→ preserved) - Abbreviations with baked-in nullness (
type objnull = obj | null) → correctly falls back (protected by #18488 regression tests)
Test: Using a Theory with InlineData to collapse cases is clean. The test design—intentionally returning s where int is expected to trigger a type-mismatch error that mentions the alias name—is a good indirect way to verify alias preservation.
No concerns on performance (this path runs once per match clause with a null pattern) or correctness.
|
Note The pull request was not created — a fallback review issue was created instead due to protected file changes: #19777 🤖 LabelOps — Conflict Resolution. Merged Auto-merged files:
|
github-project-automation
Bot
moved this from New
to In Progress
in F# Compiler and Tooling
2 participants
Fixes #19646
After
match x with | null -> … | s -> …, the bindingswas losing its type alias (e.g.stringbecameSystem.String). Now we try to clear nullness on the original type first, preserving the alias; only fall back to stripping abbreviations when nullness is embedded in the abbreviation RHS.