Find All References and Rename Symbol Bug Fixes by T-Gro · Pull Request #19252 · dotnet/fsharp

T-Gro · 2026-02-03T08:30:09Z

Find All References and Rename Symbol Bug Fixes

This PR addresses multiple longstanding bugs in the Find All References (FAR) and Rename Symbol features.

F# Compiler Service Layer

Fixes FindBackgroundReferencesInFile for TransparentCompiler not returning Partial/Active Pattern Values #19173, Finding references / renaming doesn't work for active patterns in signature files #14969 - Active patterns in signature files. Report each case as Item.ActivePatternResult.
Fixes Find all references of a DU case should include case testers #16621 - DU case testers. Register get_IsCase properties via name resolution.
Fixes Find all references of records should include copy-and-update and construction #15290 - Record copy-and-update expressions.
Fixes Finding references of additional constructors in VS #14902 - Constructor references. Register .ctor as Item.Value alongside Item.CtorGroup.
Fixes Go to definition and Find References not working for C# extension method AsMemory() in this repo #16993 - C# extension methods. Use type-prefixed ItemKeyStore key format.
Fixes Get all symbols: all symbols in SynPat.Or patterns considered bindings #5546 - SynPat.Or non-left-most variables now correctly classified as uses.
Fixes Symbols are not always found in SAFE bookstore project #5545 - DU types inside modules.
Fixes Symbols API: GetAllUsesOfAllSymbolsInFile contains generated handler value for events #4136 - Skip synthetic (zero-width) ranges.
Fixes Find References doesn't work if #line directives are used #9928 - Apply #line directive remapping.

Visual Studio Editor Layer

Fixes Renaming property renames get and set keywords use braking the code #18270 - Filter out get/set tokens in Tokenizer.FixedSpan.
Fixes Roslyn crashes F# rename when F# project contains cshtml file #16394 - Skip non-F# documents (.cshtml etc).
Fixes [VS] Find-all references on symbol from referenced DLL optimization #10227 - Only search projects referencing the specific assembly (perf improvement).

Notes

No breaking changes for FCS consumers
[VS] Find-all references on symbol from referenced DLL optimization #10227 fix improves FAR performance for external symbols

This fixes issues #14969 and #19173 where active pattern cases were not being found in signature files by Find All References. Root cause: When a value is published via MakeAndPublishVal, only Item.Value was registered with CallNameResolutionSink. For active patterns in implementation files, the individual cases are registered during TcLetBinding. However, signature files don't go through that path, so active pattern cases were missing. Fix: In MakeAndPublishVal, when processing a signature file (inSig=true), also check if the value is an active pattern using TryGetActivePatternInfo. If so, register each case with Item.ActivePatternResult at its range. Changes: - src/Compiler/Checking/Expressions/CheckExpressions.fs: Add active pattern case registration for signature files - tests/FSharp.Compiler.ComponentTests/FSharpChecker/FindReferences.fs: Update tests to expect correct behavior and add new test for case distinction in signature files

When renaming a property with explicit get/set accessors, the rename operation now correctly preserves the get and set keywords instead of replacing them with the new property name. Changes: - Added Tokenizer.tryFixupSpan to detect and filter out property accessor keywords (get/set) from rename operations - Updated InlineRenameService.FindRenameLocationsAsync to use tryFixupSpan and skip locations where it returns ValueNone - Added test documenting compiler service behavior for property references - Added VS layer test for property rename with get/set accessors The fix works by detecting when the span text after fixup is exactly 'get' or 'set' and excluding such spans from the rename locations.

The previous fix only applied tryFixupSpan filtering to InlineRenameService. This commit extends the fix to FindUsagesService.onSymbolFound so that 'Find All References' also filters out property accessor keywords (get/set). Changes: - FindUsagesService.onSymbolFound: Use Tokenizer.tryFixupSpan instead of fixupSpan to filter out property accessor keywords from references - Update test comment to accurately describe where filtering occurs

- Fix fixupSpan to not split operators on '.' character - Fix operator-to-operator rename by skipping backtick normalization for operators - Add test for operators with '.' (like '-.-') Fixes #14057 Fixes #17221

- Add 2 tests in SingleLineInterfaceSyntax module to verify Find All References works correctly for single-line interface syntax - Test interface member references (definition, implementation, usage) - Test interface type references (definition, implementation, cast) - All 42 FindReferences tests pass Note: The VS crash mentioned in #15399 was identified as a VS-side bug that also affects C# when renaming the last symbol in a file (not an F# issue).

When using #line directives (common in generated code like parser/lexer files), Find All References now correctly returns ranges at the remapped file/line positions from the directive, matching C# behavior. The fix applies range.ApplyLineDirectives() when returning ranges from ItemKeyStore.FindAll, converting original file positions to the positions specified by #line directives. Added test to verify references are found at correct remapped locations.

Fix SynPat.Or pattern binding classification (#5546): - In disjunctive 'or' patterns like '| x | x', the second occurrence of 'x' was incorrectly marked as ItemOccurrence.Binding. It should be marked as ItemOccurrence.Use since only the left-most path introduces the binding. - Changed CheckPatterns.fs to emit ItemOccurrence.Use for non-left-most paths. Filter synthetic event handler symbols (#4136): - Events with [<CLIEvent>] generate synthetic 'handler' values that were incorrectly appearing in GetAllUsesOfAllSymbolsInFile results. - Added filtering in IncrementalBuild.fs and TransparentCompiler.fs to skip items with synthetic ranges when building the ItemKeyStore. Tests: - Added test for SynPat.Or pattern to verify second binding is classified as Use - Added test for event handler to verify synthetic 'handler' symbols are filtered - All 45 FindReferences tests pass

- Add isFSharpSourceFile function to Pervasive.fs to check for F# file extensions - Filter Project.FindFSharpReferencesAsync to only process F# source files - Fixes crash when F# projects contain non-F# files (e.g., .cshtml) Issue #15721 (disposable type rename timing) was investigated and determined to be a VS IDE layer caching issue, not a compiler service bug.

…fix (#5546) The fix for #5546 changes Or pattern secondary bindings from Binding to Use. This updates the ProjectAnalysisTests baseline to match the new behavior.

…ers (#15290, #16621) ## Record copy-and-update (#15290) When a record copy-and-update expression like `{ m with V = "m" }` is processed, the record type is now registered as a reference. This ensures "Find All References" on the record type includes copy-and-update usages. Implementation: Added CallNameResolutionSink call in CheckExpressions.fs after BuildFieldMap determines the record type for copy-and-update expressions. ## DU case testers (#16621) When accessing a union case tester property like `.IsB` on a union type, a reference to the underlying union case (`B`) is now registered. This ensures "Find All References" on a union case includes usages via its tester. Implementation: Modified CallNameResolutionSink, CallMethodGroupNameResolutionSink, and CallNameResolutionSinkReplacing in NameResolution.fs to detect union case testers and register the corresponding union case reference with a slightly shifted range to avoid duplicate filtering. ## Tests Added 4 new tests in FindReferences.fs: - Record copy-and-update detection (2 tests) - DU case tester detection (2 tests) All 49 FindReferences tests pass.

Additional constructors can now be found via Find All References. When a constructor usage is resolved (e.g., MyClass()), we now also register it as Item.Value(vref) in addition to Item.CtorGroup. This allows symbol lookup from constructor definitions to find usages. Changes: - src/Compiler/Checking/Expressions/CheckExpressions.fs: Register Item.Value for F# constructor usages in ForNewConstructors - tests: Added tests for additional constructor references - tests: Updated symbol count baseline (79→80)

When searching for references to symbols from external DLLs (e.g., System.String), the code previously searched ALL projects in the solution. This commit optimizes the search by filtering to only projects that actually reference the specific DLL. Changes: - Added getProjectsReferencingAssembly helper function in SymbolHelpers.fs - Updated findSymbolUses to use assembly file path for filtering when scope is None - Added 2 new tests in ExternalDllOptimization module to verify the optimization The optimization checks the assembly file path of external symbols and matches it against project metadata references, significantly reducing unnecessary searches in large solutions with many projects that don't reference the specific DLL.

For C# extension methods, ItemKeyStore was using ApparentEnclosingType (the type being extended, e.g., Array) instead of DeclaringTyconRef (the static class declaring the extension, e.g., Enumerable). This caused inconsistent keys between usages of the same extension method, preventing Find All References from finding all uses. The fix uses IsILExtensionMethod to detect C# extension methods and writes the DeclaringTyconRef to the key instead of ApparentEnclosingType. Added 2 tests in CSharpExtensionMethods module: - Find references for same overload finds all usages - Extension method has correct symbol information

Added issue references (#NNNNN) to source code for traceability: - #19173, #14969: Active patterns in signature files (CheckExpressions.fs) - #18270: Property rename get/set keyword filtering (Tokenizer.fs) - #17221: Operator handling in fixupSpan (Tokenizer.fs) - #16394: Non-F# file filtering (Pervasive.fs) - #16621: Union case tester references (NameResolution.fs) - #10227: DLL optimization for Find All References (SymbolHelpers.fs) #15399 already has documentation in test file (FindReferences.fs). All 55 FindReferences tests pass.

- Add #15290 comment to CheckExpressions.fs for record copy-and-update fix - Add #14057 to Tokenizer.fs comment (alongside #17221 for operator fixes) - Update docs/TASKLIST.md to reflect 14 fixed issues All 14 fixed issues now have searchable source code comments: #19173, #14969, #18270, #17221, #14057, #16394, #9928, #5546, #4136, #16993, #15290, #16621, #14902, #10227 Open issues (4): - #15399: VS layer interface rename crash (test coverage added) - #15721: VS layer disposable timing issue - #5545: Not investigated (2018 SAFE bookstore bug, low impact) - #4760: Feature request (rename in strings, deferred)

…ixed The 2018 issue #5545 reported that DU types like Wishlist/Msg and Database/DatabaseType were not always found by Find All References. Investigation shows the issue has been fixed by other changes in this PR. Added 2 tests in SAFEBookstoreSymbols module to verify: - DU type definitions are found - Type annotations in parameters are found - Qualified usages (TypeName.Case) are found All 57 FindReferences tests pass (55 existing + 2 new).

Extract the union case tester registration logic that was duplicated 3 times in CallNameResolutionSink, CallMethodGroupNameResolutionSink, and CallNameResolutionSinkReplacing into a single private helper function. This reduces ~62 lines of duplicate code to ~39 lines of DRY code.

Updates all Find All References and Rename code paths to use tryFixupSpan instead of fixupSpan. The tryFixupSpan function filters out property accessor keywords (get/set) which should not be included in rename operations. Changes: - InlineRenameService.fs: Use tryFixupSpan in GetReferenceEditSpan and trigger span initialization - FindUsagesService.fs: Use tryFixupSpan in rangeToDocumentSpans - RenameParamToMatchSignature.fs: Use tryFixupSpan to filter keywords - ClassificationService.fs: Added comment documenting intentional use of fixupSpan for syntax coloring - Tokenizer.fs: Enhanced tryFixupSpan documentation with usage pattern Usage pattern: - tryFixupSpan: For Find All References and Rename (filters get/set) - fixupSpan: Only for semantic classification/syntax coloring

- Add 6 helper functions: findRefsInSource, testFindRefsInSource, expectRanges, expectLines, expectLinesInclude, expectMinRefs - Refactor 12+ tests to use new helpers - Reduce file from 1417 to 1168 lines (17.5% reduction, 249 lines saved) - Remove duplicate sortBy/map/Array.ofSeq assertion patterns - All 57 FindReferences tests pass

Documents 15 fixed issues: - #19173, #14969: Active patterns in signature files - #17221, #14057: Operators with . and operator-to-operator rename - #18270: Property get/set keyword rename - #16394: cshtml file crash - #9928: #line directive remapping - #5546, #5545: SynPat.Or and DU types in modules - #4136: Synthetic event handlers filtering - #16993: C# extension methods - #16621: DU case tester properties - #15290: Record copy-and-update expressions - #14902: Constructor references - #10227: DLL optimization

Updated test baselines in ProjectAnalysisTests and EditorTests to include the new constructor reference symbols. The #14902 fix registers F# constructor usages as Item.Value references, which adds additional symbol entries to GetAllUsesOfAllSymbols results.

Previously, registerUnionCaseTesterIfApplicable was called from 3 generic sink functions (CallNameResolutionSink, CallMethodGroupNameResolutionSink, CallNameResolutionSinkReplacing), pattern-matching Item.Property on EVERY name resolution (variables, methods, types, everything). Now: - Renamed to RegisterUnionCaseTesterForProperty (public, takes PropInfo list) - Called directly from 2 property-specific handlers in CheckExpressions.fs: 1. TcItemThen for static properties 2. TcLookupItemThen for instance properties - Removed from 3 generic sink functions Benefits: - Zero overhead for non-property name resolutions - Logic colocated with property-specific handling - Clearer architectural layering (property logic in property handlers) - No redundant pattern matching on every sink call This addresses the architectural inefficiency identified in code review where union case tester logic was scattered across generic sinks instead of being targeted at property-specific code paths.

vsintegration/src/FSharp.Editor/LanguageService/Tokenizer.fs

T-Gro · 2026-02-12T18:39:03Z

/run fantomas

github-actions · 2026-02-12T18:42:12Z

🔧 CLI Command Report

Command: /run fantomas
Outcome: success

✅ Patch applied:
- Files changed: 1
- Lines changed: 15

TheAngryByrd · 2026-02-12T22:34:25Z

🥳

src/Compiler/Checking/Expressions/CheckExpressions.fs

src/Compiler/Checking/NameResolution.fs

auduchinok · 2026-02-23T11:28:41Z

src/Compiler/Checking/Expressions/CheckExpressions.fs

+                let shiftedRange = Range.mkRange mObjTy.FileName (Position.mkPos mObjTy.StartLine (mObjTy.StartColumn + 1)) mObjTy.End
+                CallNameResolutionSink tcSink (shiftedRange, env.NameEnv, Item.Value vref, minst, ItemOccurrence.Use, env.AccessRights)


@T-Gro Why do we need an additional symbol and the range arithmetic?

A proper symbol was already reported there and had a correct range. Was the intention here to fix some specific case?

This was to match secondary constructors, but it is also what caused the duplicate .ctor symbol regressions for FAR (find all references) you noticed.

#19358 attempts to solve this differently.

auduchinok · 2026-02-23T11:34:53Z

src/Compiler/Checking/NameResolution.fs

+                let shiftedStart = Position.mkPos m.StartLine (m.StartColumn + 1)
+                let shiftedRange = Range.withStart shiftedStart m
+                currentSink.NotifyNameResolution(shiftedRange.End, ucItem, emptyTyparInst, occurrenceType, nenv, ad, shiftedRange, false)


@T-Gro I believe this is a wrong approach. The symbol at this range is the tester, but its union case is reported here instead. I understand it helps to implement finding usages of related symbols, but it should not come at a cost of reporting wrong data.

Let's find another approach? You can define an additional sink, like it's done for method groups and do something like sink.NotifyRelatedSymbol and then get symbols from this sink via a separate FCS API or with a custom flag.

But wouldnt we rather want the results be included in existing API calls instead of coming up with a new API (where we would have to make sure to call it alongside existing calls) ?

The sink.NotifyRelatedSymbol is a good idea and semantically A LOT better then NameResolution misuse, I just want to find the right spot to integrate related symbols into existing results, to provide a transparent fix for the renaming/find all references scenarios.

But wouldnt we rather want the results be included in existing API calls instead of coming up with a new API (where we would have to make sure to call it alongside existing calls) ?

In Rider we remove all the extra symbols, and usually it was relatively easy to check because the symbols API wasn't misused on purpose like this. I think it'd be better to keep the related symbols separately (another sink? synthetic ranges?) and have a flag in FCS to allow returning/filtering these symbols.

Flag for existing APIs (or maybe a bitwise flagging mechanism, as there might be more dimensions of what to ask for) sounds good, let me try it 👍

auduchinok · 2026-02-23T11:49:12Z

src/Compiler/Checking/Expressions/CheckExpressions.fs

+                // so that "Find All References" on the record type includes copy-and-update usages
+                if hasOrigExpr then
+                    let item = Item.Types(tcref.DisplayName, [gtyp])
+                    CallNameResolutionSink cenv.tcSink (mWholeExpr, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)


This is also not the result of name resolution. Please add a separate reporting path, as suggested before.

commit cd1f270 Author: Eugene Auduchinok <eugene.auduchinok@jetbrains.com> Date: Mon Feb 16 17:44:59 2026 +0100 Fantomas commit 4edb3dc Author: Eugene Auduchinok <eugene.auduchinok@gmail.com> Date: Mon Feb 16 17:12:08 2026 +0100 Release notes commit b737ebc Author: Eugene Auduchinok <eugene.auduchinok@jetbrains.com> Date: Mon Feb 16 16:54:21 2026 +0100 FCS: capture additional types during analysis commit e82d6f6 Author: Tomas Grosup <Tomas.Grosup@gmail.com> Date: Mon Feb 16 12:56:35 2026 +0100 Create SKILL.md (dotnet#19282) commit 01bad01 Author: Tomas Grosup <Tomas.Grosup@gmail.com> Date: Thu Feb 12 21:49:32 2026 +0100 Find All References and Rename Symbol Bug Fixes (dotnet#19252) FCS: add more predefined types checks

…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>

T-Gro added 30 commits January 27, 2026 13:11

Fix operator rename issues (#14057, #17221)

0fbd0a4

- Fix fixupSpan to not split operators on '.' character - Fix operator-to-operator rename by skipping backtick normalization for operators - Add test for operators with '.' (like '-.-') Fixes #14057 Fixes #17221

Fix baseline: Update Project15 test for Or pattern secondary binding …

d0cebb8

…fix (#5546) The fix for #5546 changes Or pattern secondary bindings from Binding to Use. This updates the ProjectAnalysisTests baseline to match the new behavior.

Clean up: Remove TASKLIST.md and simplify #14902 comments

8bab75e

Polish: Remove architecture doc and placeholder PR links

448bab6

Remove verbose comments, keep issue refs

6c488ab

Extract testFindAllRefs helpers to reduce test duplication

c5663a0

Extract ProjectFiltering module to eliminate duplicate code

eee3339

Extract union case tester constants to PrettyNaming module

4a7d221

Centralize #18270 filtering in RoslynHelpers conversion layer

82ecf78

refactor .md agentic instructions

887c986

T-Gro marked this pull request as ready for review February 7, 2026 19:22

T-Gro requested a review from a team as a code owner February 7, 2026 19:22

T-Gro added 3 commits February 7, 2026 20:22

Merge branch 'main' into rename-fixes

ef317b8

Merge branch 'main' into rename-fixes

041f2c2

Merge branch 'main' into rename-fixes

d6355c1

abonie reviewed Feb 11, 2026

View reviewed changes

vsintegration/src/FSharp.Editor/LanguageService/Tokenizer.fs Outdated Show resolved Hide resolved

abonie approved these changes Feb 12, 2026

View reviewed changes

github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Feb 12, 2026

T-Gro added 2 commits February 12, 2026 15:51

fix get/set tokenizer logic

3074106

Merge branch 'main' into rename-fixes

b091c43

T-Gro enabled auto-merge (squash) February 12, 2026 17:00

Apply patch from /run fantomas

42aaad8

T-Gro merged commit 01bad01 into main Feb 12, 2026
46 checks passed

github-project-automation bot moved this from In Progress to Done in F# Compiler and Tooling Feb 12, 2026

dotnet-maestro bot mentioned this pull request Feb 13, 2026

[main] Source code updates from dotnet/fsharp dotnet/dotnet#4806

Merged

kerams reviewed Feb 14, 2026

View reviewed changes

src/Compiler/Checking/Expressions/CheckExpressions.fs Show resolved Hide resolved

kerams reviewed Feb 14, 2026

View reviewed changes

src/Compiler/Checking/NameResolution.fs Show resolved Hide resolved

dotnet-maestro bot mentioned this pull request Feb 16, 2026

[release/10.0.3xx] Source code updates from dotnet/fsharp dotnet/dotnet#4902

Merged

T-Gro deleted the rename-fixes branch February 17, 2026 12:04

T-Gro mentioned this pull request Feb 17, 2026

Bugfix :: Coloring for case testers and with #19311

Merged

auduchinok mentioned this pull request Feb 20, 2026

Regression: symbols include corrupted duplicate constructor symbol for attribute #19336

Open

auduchinok reviewed Feb 23, 2026

View reviewed changes

auduchinok mentioned this pull request Feb 24, 2026

Bugfix :: Sequence points #19278

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Find All References and Rename Symbol Bug Fixes#19252

Find All References and Rename Symbol Bug Fixes#19252
T-Gro merged 45 commits intomainfrom
rename-fixes

T-Gro commented Feb 3, 2026 •

edited

Loading

Uh oh!

Uh oh!

T-Gro commented Feb 12, 2026

Uh oh!

github-actions bot commented Feb 12, 2026

Uh oh!

Uh oh!

TheAngryByrd commented Feb 12, 2026

Uh oh!

Uh oh!

Uh oh!

auduchinok Feb 23, 2026 •

edited

Loading

Uh oh!

T-Gro Feb 25, 2026

Uh oh!

auduchinok Feb 23, 2026 •

edited

Loading

Uh oh!

T-Gro Feb 25, 2026

Uh oh!

auduchinok Feb 25, 2026

Uh oh!

T-Gro Feb 25, 2026

Uh oh!

auduchinok Feb 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

		let shiftedRange = Range.mkRange mObjTy.FileName (Position.mkPos mObjTy.StartLine (mObjTy.StartColumn + 1)) mObjTy.End
		CallNameResolutionSink tcSink (shiftedRange, env.NameEnv, Item.Value vref, minst, ItemOccurrence.Use, env.AccessRights)

Conversation

T-Gro commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!