Skip to content

Nullness FS3261: pinpoint receiver range, name the member & binding in dot-access warnings#19814

Open
T-Gro wants to merge 16 commits into
mainfrom
fix/nullness-range
Open

Nullness FS3261: pinpoint receiver range, name the member & binding in dot-access warnings#19814
T-Gro wants to merge 16 commits into
mainfrom
fix/nullness-range

Conversation

@T-Gro
Copy link
Copy Markdown
Member

@T-Gro T-Gro commented May 26, 2026

Fixes #19658

Architecture

New ContextInfo.MemberAccessOnNullable threads receiver range + member name + optional binding name through constraint solving. When FS3261 fires during dot-access, a dedicated ConstraintSolverNullnessWarningOnDotAccess emits a targeted diagnostic instead of the generic "types do not have compatible nullability".

Before → After

1. Simple method call on nullable receiver

let pad (x: string | null) = x.PadLeft(10)
  • Before: FS3261 on x.PadLeft(10)Nullness warning: The types 'string' and 'string | null' do not have compatible nullability.
  • After: FS3261 on xNullness warning: Possible dereference of a null value when accessing member 'PadLeft' on the nullable value 'x' of type 'string | null'.

2. Chained call — warning targets the actual nullable hop

let f (x: string | null) = x.Trim().Length
  • Before: Two FS3261 on x.Trim().Length and x.Trim()Nullness warning: The types 'string' and 'string | null' do not have compatible nullability.
  • After: Single FS3261 on xNullness warning: Possible dereference of a null value when accessing member 'Trim' on the nullable value 'x' of type 'string | null'.

3. Multi-line receiver gets "See also" pointing at call site

let process (data: string | null) =
    data
     .ToUpper()
  • Before: FS3261 on .ToUpper()Nullness warning: The types 'string' and 'string | null' do not have compatible nullability.
  • After: FS3261 on dataNullness warning: Possible dereference of a null value when accessing member 'ToUpper' on the nullable value 'data' of type 'string | null'. See also <line with .ToUpper()>

4. Property chain — warning pinpoints where nullability starts

// Given: type Foo with member Prop : Bar | null
let g (foo: Foo) = foo.Prop.ToString()
  • Before: FS3261 on foo.Prop.ToString()Nullness warning: The types 'Bar' and 'Bar | null' do not have compatible nullability.
  • After: FS3261 on foo.PropNullness warning: Possible dereference of a null value when accessing member 'ToString' on a nullable expression of type 'Bar | null'.

Entity.Typars cleanup (included because it fixes a parallel test race exposed by the new tests)

Entity.Typars changed from method Typars(m: range) to property. The range parameter was unused for F# types (always NotLazy — typars carry their own ranges from unpickle) and caused a parallel test race for IL-imported types (shared BCL entities via FrameworkImportsCache + LazyWithContext caching first caller's range). Now uses entity.Range deterministically. Removed redundant TyparsNoRange.

T-Gro and others added 16 commits May 15, 2026 14:50
…ning (#19658, sprint 1/3)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… sprint 2/3)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ness warning (#19658, sprint 3/3)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tests

- Extract withObjArgContext helper in ResolveOverloading and UnifyUniqueOverloading,
  replacing 3 inline match/rewrap duplicates
- Trim MemberAccessOnNullable doc comment to 1 line (named labels self-document)
- Remove redundant replaceNullnessOfTy (showNullnessAnnotations=false handles display)
- Tighten release notes: 'dotted method or property access' (not indexer/records)
- Add 5 new edge-case tests: chained access, mutable receiver, overloaded method,
  extension method, static call negative test
- Clean up existing 3 tests: remove EntryPoint boilerplate, minimal source code

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sumesType (#19658)

The MemberAccessOnNullable context is meant only for the receiver's outer
nullness check. It was leaking into recursive subsumption/equality checks on
tuple components, fun-type domains, and especially generic type arguments,
producing a dot-access warning with the wrong type (the inner type-arg) and
pinning unrelated deep nullness warnings to the receiver range.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#19658)

Defense-in-depth: avoid setting ContextInfo.MemberAccessOnNullable on csenv
when nullness checking is off, so future code that branches on this context
case without re-checking g.checkNullness can't accidentally fire.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…line (#19658)

Matches the convention of all sibling FS3261 handlers. When the receiver
and the member access are on different source lines (fluent chains), the
warning now hyperlinks the member range so IDEs surface both locations.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ames (#19658)

- tryGetBindingName recurses through Expr.Op(TOp.Coerce, ...) so binding
  names surface for interface-upcast receivers.
- vref.IsCompilerGenerated check prevents internal names (_arg1,
  matchValue, copyOfStruct, ...) from leaking into user-facing messages.
- Document why ConstraintSolverNullnessWarningOnDotAccess uses
  showNullnessAnnotations = Some false (avoid redundant '| null' phrasing).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…19658)

- Property setter and piped lambda receiver: verify new message fires.
- Indexer, F# record field, anonymous record, SRTP: pin current behavior
  of paths intentionally not routed through TcMethodApplication. Marked
  KNOWN GAP in test names. Widening coverage is tracked under #17409.
- Anonymous record gap is documented via the FS3260 'does not support a
  nullness qualification' error since '{| ... |} | null' is rejected by
  the type system; this still pins that the dot-access path cannot fire.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the anonymous (range * string * string option) option tuple with
a named ObjArgInfo record for readability, and extract the withObjArgContext
closure (duplicated in ResolveOverloadingCore, ResolveOverloading, and
UnifyUniqueOverloading) into a single top-level applyObjArgContext function.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Inside the else-branch recursion, shadow csenv with the stripped variant
so MemberAccessOnNullable context is dropped by default for deep
subsumption descents. Keep the original under csenvOuter for the two
sites that genuinely describe the outer ty1/ty2 pair: the same-tycon
outer-nullness check, and the FindUniqueFeasibleSupertype recursion
which still subsumes the original ty1.

This eliminates the fragility of having to remember to use csenvInner
on every new recursive branch — the safe behavior is now the default.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…el (#19658)

Merge seven template-identical Issue 19658 dot-access nullness tests
into a single [<Theory>][<InlineData>] driven by (source, range, member,
binding, type) tuples. The mechanical repetition added noise without
helping readability.

Rename the four 'KNOWN GAP' tests to neutral 'falls back to generic
nullness warning' / 'rejects nullable annotation' wording. These tests
pin the intentional scope boundary (paths NOT routed through
TcMethodApplication) and are not bugs. Update the comment block above
them to reflect this.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…#19658)

Internal API change: ResolveOverloading and ResolveOverloadingCore no
longer take an objArgInfo parameter. The public entry points
ResolveOverloadingForCall and UnifyUniqueOverloading still accept the
ObjArgInfo and set ContextInfo.MemberAccessOnNullable on the constructed
ConstraintSolverEnv. CanMemberSigsMatchUpToCheck call sites locally
strip the context for the non-obj-arg callbacks so deep arg/return
mismatches keep using the generic FS3261 message.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…es (#19658)

CODE-QUALITY fixup: the strip-MemberAccessOnNullable pattern was inlined at 5
sites (SolveTypeSubsumesType + 4 overload resolution sites in
ResolveOverloading). Extract a single helper near MakeConstraintSolverEnv and
call it at all five callsites.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use minimalStringOfTypeWithNullness instead of suppressing nullness
annotations. The type 'string' was misleading — the actual type is
'string | null'.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Entity.Typars was a method taking a range parameter that was passed to
LazyWithContext.Force as context for error recovery. For IL-imported
entities shared via FrameworkImportsCache, the first caller's range was
cached permanently, creating non-deterministic typar ranges in parallel
compilations.

Analysis of git history (back to the first open-source commit) shows the
range parameter was never meaningful:
- F# entities use NotLazy for entity_typars — Force ignores the context
- IL entities receive the import-time range via NewILTycon, stored in
  entity_range — the Force-time range was redundant

Change Entity.Typars from a method to a property that uses entity.Range
as the Force context. Remove the now-redundant TyparsNoRange member.
Clean up cascading unused range parameters in callers.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

❗ Release notes required


✅ Found changes and release notes in following paths:

Warning

No PR link found in some release notes, please consider adding it.

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/11.0.100.md No current pull request URL (#19814) found, please consider adding it

@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

Nullness issue - wrong place is reported with warning

1 participant