Preserve type aliases in match | null refinement#19745
Open
T-Gro wants to merge 3 commits into
Open
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>
Contributor
❗ 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
commented
May 18, 2026
Member
Author
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.