Skip to content

Introduce CallRelatedSymbolSink to avoid affect name resolution with related symbols#19361

Open
T-Gro wants to merge 3 commits intomainfrom
improve/rename-changes
Open

Introduce CallRelatedSymbolSink to avoid affect name resolution with related symbols#19361
T-Gro wants to merge 3 commits intomainfrom
improve/rename-changes

Conversation

@T-Gro
Copy link
Member

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

Follow-up to #19252 — refactors how "related" symbols (union case testers on properties, record types in copy-and-update expressions) are reported.

PR #19252 misused NotifyNameResolution with replace=true to inject these symbols as if they were actual name resolutions. This corrupted data for all sink consumers (colorization, symbol info, code lenses) since the items were not genuine resolutions.

This PR introduces:

  • A separate NotifyRelatedSymbolUse sink method, keeping the main resolution path clean
  • A [<Flags>] RelatedSymbolUseKind enum (UnionCaseTester, CopyAndUpdateRecord, All) in the public FCS API
  • Optional ?relatedSymbolKinds parameters on GetUsesOfSymbolInFile and GetSemanticClassification, letting consumers opt in to the specific related symbol kinds they need

VS passes All; consumers that do not pass it get no related symbols (backward compatible default).

T-Gro and others added 2 commits February 25, 2026 19:21
…bols

Address auduchinok's review comments on PR #19252: related symbols (union case
testers, copy-and-update record types) are now reported via a separate
NotifyRelatedSymbolUse sink instead of abusing NotifyNameResolution.

- Add [<Flags>] RelatedSymbolUseKind enum (None/UnionCaseTester/CopyAndUpdateRecord/All)
- Add NotifyRelatedSymbolUse to ITypecheckResultsSink interface
- Refactor RegisterUnionCaseTesterForProperty to use the new sink
- Refactor copy-and-update in TcRecdExpr to use CallRelatedSymbolSink
- Add ?relatedSymbolKinds parameter to GetUsesOfSymbolInFile (default: None)
- Wire up VS FAR to pass relatedSymbolKinds=All
- Write related symbols to ItemKeyStore for background FAR
- Update semantic classification tests: tester properties now classified as
  Property (not UnionCase) since they're no longer in capturedNameResolutions

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

github-actions bot commented Feb 26, 2026

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@T-Gro T-Gro requested a review from abonie February 26, 2026 13:43
@T-Gro T-Gro marked this pull request as ready for review February 26, 2026 13:43
@T-Gro T-Gro requested a review from a team as a code owner February 26, 2026 13:43
@T-Gro T-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Feb 26, 2026
Copy link
Member

@auduchinok auduchinok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

let results = checker.ParseAndCheckFileInProject(fileName, snapshot) |> Async.RunSynchronously
let checkResults = getTypeCheckResult results
checkResults.GetSemanticClassification(None)
checkResults.GetSemanticClassification(None, RelatedSymbolUseKind.All)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to add tests for other cases -- other values of RelatedSymbolUseKind?

@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants