Fix missing semantic classification in nested copy-and-update expressions#19878
Open
T-Gro wants to merge 3 commits into
Open
Fix missing semantic classification in nested copy-and-update expressions#19878T-Gro wants to merge 3 commits into
T-Gro wants to merge 3 commits into
Conversation
Adds NestedCopyAndUpdateSink module with 8 tests covering the bug where
only the first type qualifier in a nested copy-and-update is reported to
the name resolution sink.
Verified against baseline 'main':
- 3 multi-qualifier scenarios FAIL RED (same-path siblings - the bug):
* Both type qualifiers in nested update are classified
* Three type qualifiers in nested update all classified
* Qualifier ranges point to correct columns
- 1 multi-qualifier scenario PASSES on baseline (Different type
qualifiers each reported) because when sibling fields use different
inner-record types (Outer.I1 vs Outer.I2) the sink already emits
both Outer tokens; kept verbatim per sprint spec as a regression
guard so a future fix does not accidentally drop this working case.
- 4 single-qualifier / regression-guard tests pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-and-update ResolveNestedField now threads resInfo from ResolveLongIdentInTyconRefs and calls SendEntityPathToSink so every Person.Info.X / Person.Info.Y qualifier reaches the IDE sink. Also fixes the entity-path range to tyconId.idRange. Qualifier idents in the returned access path are made synthetic to avoid double-emitting the entity-path event from a subsequent ResolveField call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
❗ Release notes required
|
auduchinok
reviewed
Jun 2, 2026
| // Mark the qualifier idents synthetic so a subsequent ResolveField call | ||
| // (e.g. via BuildFieldMap) does not double-emit the entity-path sink event | ||
| // we already emitted above. | ||
| |> List.map (fun id -> ident (id.idText, id.idRange.MakeSynthetic())) |
Member
There was a problem hiding this comment.
This seems incorrect. Identifiers coming from the actual source should have non-synthetic ranges.
|
|
||
| match tyconIdOpt with | ||
| | Some _ -> | ||
| ResolutionInfo.SendEntityPathToSink(sink, ncenv, nenv, ItemOccurrence.UseInType, ad, resInfo, ResultTyparChecker(fun () -> true)) |
Member
There was a problem hiding this comment.
Is UseInType a correct occurrence kind here? I thought we normally only use it in type annotations?
| let p2 = { p with Person.Info.X = 10; Person.Info.Y = 20 } | ||
| """ | ||
| let personUses = usesOf "Person" 5 source | ||
| Assert.True(personUses.Length >= 2, sprintf "expected >= 2 Person uses on line 5, got %d" personUses.Length) |
Member
There was a problem hiding this comment.
Let's dump the names ranges instead of just counting the usages?
github-actions
Bot
added
the
AI-Tooling-Check-Bypassed
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
2 participants
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.
Summary
Fixes #17428
In nested copy-and-update expressions like
{ p with Person.Info.X = 1; Person.Info.Y = 2 }, the second and subsequent type qualifiers (Person.Info) were not being reported to the IDE semantic classification sink, resulting in missing colorization.Changes
src/Compiler/Checking/NameResolution.fs–ResolveNestedFieldnow threadsresInfofromResolveLongIdentInTyconRefsand callsSendEntityPathToSinkso every qualifier path reaches the IDE sink. Also fixes the entity-path range to usetyconId.idRange. Qualifier idents in the returned access path are made synthetic to avoid double-emitting from a subsequentResolveFieldcall.src/Compiler/TypedTree/TcGlobals.fs– Minor: removed an unneeded line.Tests
tests/FSharp.Compiler.Service.Tests/Symbols.fsverifying all qualifier idents receive correct semantic classification.IWSAMsAndSRTPstests updated to reflect corrected SRTP witness resolution (pre-existing line removed from release notes that was merged separately).Release Notes
Entry added to
docs/release-notes/.FSharp.Compiler.Service/11.0.100.md.