From 34d460acafa908ef2649f868152fe364776fc288 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 09:42:41 +0200 Subject: [PATCH 01/17] Build multi-project and multi-target Roslyn solutions in FSharp.Editor tests Test helpers so far put every synthetic file into one Roslyn project. CreateMultiProjectSolution creates one project per synthetic project with project references, the way VS wires project-to-project references; CreateMultiTargetSolution creates one project per target instance sharing the project path and the document paths, the way VS loads a multi-targeted project. Co-Authored-By: Claude Fable 5.1 --- .../Helpers/RoslynHelpers.fs | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 3089435309b..4183dcb27ee 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -203,7 +203,6 @@ type TestHostServices() = /// One Roslyn project instance of a multi-targeted F# project: its extra defines and the /// synthetic files left out of it, as VS does per target framework. -[] type TargetInstance = { Defines: string list @@ -384,9 +383,8 @@ type RoslynTestHelpers private () = let projects = syntheticProject.GetAllProjects() - |> Seq.distinctBy _.Name - |> Seq.map (fun project -> struct (project, ProjectId.CreateNewId())) - |> Seq.toArray + |> List.distinctBy _.Name + |> List.map (fun project -> project, ProjectId.CreateNewId()) let projectIds = dict [ for project, id in projects -> project.Name, id ] @@ -417,15 +415,10 @@ type RoslynTestHelpers private () = project.GetProjectOptions checker |> RoslynTestHelpers.SetProjectOptions id solution - struct (solution, checker) + solution, checker - /// /// One Roslyn project per target instance, all sharing the .fsproj path and the document file /// paths, like the per-target-framework projects VS creates for a multi-targeted project. - /// - /// The project every instance is made from; it must not depend on other projects. - /// One Roslyn project per entry: its extra defines and the synthetic files left out of it. - /// The solution and the id of each instance's project, in the order of . static member CreateMultiTargetSolution(syntheticProject: SyntheticProject, instances: TargetInstance list) = assert (syntheticProject.DependsOn = []) @@ -438,13 +431,13 @@ type RoslynTestHelpers private () = for instance in instances -> let excludedPaths = HashSet( - seq { + [ for fileId in instance.ExcludedFileIds do syntheticProject.GetFilePath fileId if (syntheticProject.Find fileId).HasSignatureFile then syntheticProject.GetSignatureFilePath fileId - }, + ], StringComparer.OrdinalIgnoreCase ) @@ -476,7 +469,7 @@ type RoslynTestHelpers private () = |] } - struct (id, projectInfo, instanceOptions) + id, projectInfo, instanceOptions ] let solution = @@ -485,7 +478,7 @@ type RoslynTestHelpers private () = for id, _, instanceOptions in instances do RoslynTestHelpers.SetProjectOptions id solution instanceOptions - struct (solution, [ for id, _, _ in instances -> id ]) + solution, [ for id, _, _ in instances -> id ] static member GetFsDocument(code, ?customProjectOption: string, ?customEditorOptions) = let customProjectOptions = From cd696e9a0514ee79d5c8196bde01821f69fe4bca Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 09:42:41 +0200 Subject: [PATCH 02/17] Resolve navigation targets through the workspace's current solution The document Roslyn hands to Go To Definition can come from a snapshot taken before every project of the solution finished loading. Deciding that a symbol is external because that snapshot has no document for its file sent F# to F# navigation into a generated signature, and the same lookup silently narrowed the scope of Find All References and Rename. Look the target up in the document's own solution first and in Workspace.CurrentSolution when it is missing, normalising the path and preferring the target-framework instance the origin project depends on. The branch of Go To Definition that already sits on the declaration moves into its own member. Co-Authored-By: Claude Fable 5.1 --- .../Common/CodeAnalysisExtensions.fs | 29 ++ .../LanguageService/SymbolHelpers.fs | 2 +- .../FSharp.Editor/LanguageService/Symbols.fs | 8 +- .../Navigation/FindUsagesService.fs | 9 +- .../Navigation/GoToDefinition.fs | 247 ++++++++---------- 5 files changed, 142 insertions(+), 153 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs index e0b29c8f9f1..1eaaaed3180 100644 --- a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs +++ b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs @@ -117,3 +117,32 @@ type Solution with | Some projectId -> self.TryGetDocumentIdFromFSharpRange(range, projectId) | None -> self.TryGetDocumentIdFromFSharpRange range |> Option.map self.GetDocument + +type Document with + + /// Runs a lookup against this document's solution and, when it finds nothing, against the + /// workspace's current solution: the document may come from a snapshot taken before every + /// project of the solution finished loading. + member document.TryFindInSolutions(find: Solution -> 'T voption) = + match find document.Project.Solution with + | ValueSome found -> ValueSome found + | ValueNone -> find document.Project.Solution.Workspace.CurrentSolution + + /// Every document with the file path, from whichever project includes it. + member document.GetSolutionDocumentsWithFilePath(filePath: string) = + let filePath = Path.GetFullPathSafe filePath + + document.TryFindInSolutions(fun solution -> + match solution.GetDocumentIdsWithFilePath filePath with + | ids when ids.IsEmpty -> ValueNone + | ids -> ValueSome [ for id in ids -> solution.GetDocument id ]) + |> ValueOption.defaultValue [] + + member document.TryGetSolutionDocumentFromPath(filePath: string) = + document.GetSolutionDocumentsWithFilePath filePath |> Seq.tryHeadV + + /// The document for the range's file, preferring this document's project or one it depends on. + member document.TryGetSolutionDocumentFromFSharpRange(range: range) = + document.TryFindInSolutions(fun solution -> + solution.TryGetDocumentFromFSharpRange(range, document.Project.Id) + |> ValueOption.ofOption) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/SymbolHelpers.fs b/vsintegration/src/FSharp.Editor/LanguageService/SymbolHelpers.fs index 36319820f80..becd935453a 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/SymbolHelpers.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/SymbolHelpers.fs @@ -124,7 +124,7 @@ module internal SymbolHelpers = let otherFile = getOtherFile currentDocument.FilePath let! otherFileCheckResults = - match currentDocument.Project.Solution.TryGetDocumentFromPath otherFile with + match currentDocument.TryGetSolutionDocumentFromPath otherFile with | ValueSome doc -> cancellableTask { let! _, checkFileResults = doc.GetFSharpParseAndCheckResultsAsync("findReferencedSymbolsAsync") diff --git a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs index 19e446f2d08..ecedeb3e536 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs @@ -64,11 +64,9 @@ type FSharpSymbolUse with Some(SymbolScope.Projects([ currentDocument.Project ], isSymbolLocalForProject)) else let projects = - currentDocument.Project.Solution.GetDocumentIdsWithFilePath(filePath) - |> Seq.map (fun x -> x.ProjectId) - |> Seq.distinct - |> Seq.map currentDocument.Project.Solution.GetProject - |> Seq.toList + currentDocument.GetSolutionDocumentsWithFilePath filePath + |> List.map _.Project + |> List.distinctBy _.Id match projects with | [] -> None diff --git a/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs b/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs index a81d2ec3fed..fbdf37397e3 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/FindUsagesService.fs @@ -59,18 +59,15 @@ module FSharpFindUsagesService = } // File can be included in more than one project, hence single `range` may results with multiple `Document`s. - let rangeToDocumentSpans (solution: Solution, range: range, symbolName: string) = + let rangeToDocumentSpans (document: Document, range: range, symbolName: string) = if range.Start = range.End then CancellableTask.singleton [||] else cancellableTask { - let documentIds = solution.GetDocumentIdsWithFilePath(range.FileName) - let! spans = seq { - for documentId in documentIds do + for doc in document.GetSolutionDocumentsWithFilePath range.FileName do cancellableTask { - let doc = solution.GetDocument(documentId) let! cancellationToken = CancellableTask.getCancellationToken () let! sourceText = doc.GetTextAsync(cancellationToken) @@ -119,7 +116,7 @@ module FSharpFindUsagesService = let! declarationSpans = match declarationRange with - | Some range -> rangeToDocumentSpans (document.Project.Solution, range, symbol.Ident.idText) + | Some range -> rangeToDocumentSpans (document, range, symbol.Ident.idText) | None -> CancellableTask.singleton [||] let declarationSpans = diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index 05da71ef97e..81ed086e6ba 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -174,29 +174,26 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = && symbol1.DeclaringEntity.CompiledName = symbol2.DeclaringEntity.CompiledName | _ -> false + /// The navigable item for the range in the document, when the range fits the document's text. + let navigableItemAt (document: Document) (range: range) = + cancellableTask { + let! cancellationToken = CancellableTask.getCancellationToken () + let! sourceText = document.GetTextAsync(cancellationToken) + + return + RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range) + |> ValueOption.map (fun textSpan -> FSharpGoToDefinitionNavigableItem(document, textSpan)) + } + /// Use an origin document to provide the solution & workspace used to /// find the corresponding textSpan and INavigableItem for the range let rangeToNavigableItem (range: range, document: Document) = cancellableTask { - let fileName = - try - System.IO.Path.GetFullPath range.FileName - with _ -> - range.FileName - - let refDocumentIds = document.Project.Solution.GetDocumentIdsWithFilePath fileName - - if not refDocumentIds.IsEmpty then - let refDocumentId = refDocumentIds.First() - let refDocument = document.Project.Solution.GetDocument refDocumentId - let! cancellationToken = Async.CancellationToken - let! refSourceText = refDocument.GetTextAsync(cancellationToken) |> Async.AwaitTask - - match RoslynHelpers.TryFSharpRangeToTextSpan(refSourceText, range) with - | ValueNone -> return None - | ValueSome refTextSpan -> return Some(FSharpGoToDefinitionNavigableItem(refDocument, refTextSpan)) - else - return None + match document.TryGetSolutionDocumentFromFSharpRange range with + | ValueNone -> return None + | ValueSome refDocument -> + let! navItem = navigableItemAt refDocument range + return ValueOption.toOption navItem } member _.TryGetExternalDeclarationAsync(targetSymbolUse: FSharpSymbolUse, metadataReferences: seq) = @@ -312,7 +309,7 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = if not (File.Exists fsfilePath) then return None else - let implDoc = originDocument.Project.Solution.TryGetDocumentFromPath fsfilePath + let implDoc = originDocument.TryGetSolutionDocumentFromPath fsfilePath match implDoc with | ValueNone -> return None @@ -336,14 +333,9 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = | ValueNone -> return None | ValueSome implTextSpan -> return Some(FSharpGoToDefinitionNavigableItem(implDoc, implTextSpan)) else - let targetDocument = - originDocument.Project.Solution.TryGetDocumentFromFSharpRange fsSymbolUse.Range - - match targetDocument with - | None -> return None - | Some targetDocument -> - let! navItem = rangeToNavigableItem (fsSymbolUse.Range, targetDocument) - return navItem + match originDocument.TryGetSolutionDocumentFromFSharpRange fsSymbolUse.Range with + | ValueNone -> return None + | ValueSome targetDocument -> return! rangeToNavigableItem (fsSymbolUse.Range, targetDocument) } /// if the symbol is defined in the given file, return its declaration location, otherwise use the targetSymbol to find the first @@ -373,6 +365,55 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = return implSymbol.Range } + /// The navigable item for the target symbol's declaration in the implementation document. + member private this.FindNavigableDeclarationIn(targetSymbolUse: FSharpSymbolUse, implDocument: Document) = + cancellableTask { + let! declarationRange = this.FindSymbolDeclarationInDocument(targetSymbolUse, implDocument) + + match declarationRange with + | None -> return ValueNone + | Some declarationRange -> return! navigableItemAt implDocument declarationRange + } + + /// The caret is already on the declaration: in a signature file the target is the implementation, + /// in an implementation file it is the signature. + member private this.FindCounterpartOfDeclarationAtCaret + ( + originDocument: Document, + targetSymbolUse: FSharpSymbolUse, + checkFileResults: FSharpCheckFileResults, + lexerSymbol: LexerSymbol, + fcsTextLineNumber: int, + textLineString: string + ) = + cancellableTask { + if isSignatureFile originDocument.FilePath then + let implFilePath = Path.ChangeExtension(originDocument.FilePath, "fs") + + if not (File.Exists implFilePath) then + return ValueNone + else + match originDocument.TryGetSolutionDocumentFromPath implFilePath with + | ValueNone -> return ValueNone + | ValueSome implDocument -> return! this.FindNavigableDeclarationIn(targetSymbolUse, implDocument) + else + let declarations = + checkFileResults.GetDeclarationLocation( + fcsTextLineNumber, + lexerSymbol.Ident.idRange.EndColumn, + textLineString, + lexerSymbol.FullIsland, + true + ) + + match declarations with + | FindDeclResult.DeclFound sigRange -> + match originDocument.TryGetSolutionDocumentFromFSharpRange sigRange with + | ValueNone -> return ValueNone + | ValueSome sigDocument -> return! navigableItemAt sigDocument sigRange + | _ -> return ValueNone + } + member internal this.FindDefinitionAtPosition(originDocument: Document, position: int) = cancellableTask { let userOpName = "FindDefinitionAtPosition" @@ -417,8 +458,9 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = match declarations with | FindDeclResult.ExternalDecl(assembly, targetExternalSym) -> let projectOpt = - originDocument.Project.Solution.Projects - |> Seq.tryFindV (fun p -> p.AssemblyName.Equals(assembly, StringComparison.OrdinalIgnoreCase)) + originDocument.TryFindInSolutions(fun solution -> + solution.Projects + |> Seq.tryFindV (fun p -> p.AssemblyName.Equals(assembly, StringComparison.OrdinalIgnoreCase))) match projectOpt with | ValueSome project -> @@ -456,122 +498,45 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = return ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), idRange) | FindDeclResult.DeclFound targetRange -> - // If the file is not associated with a document, it's considered external. - if not (originDocument.Project.Solution.ContainsDocumentWithFilePath(targetRange.FileName)) then + match originDocument.TryGetSolutionDocumentFromFSharpRange targetRange with + | ValueNone -> + // No document for the file anywhere in the workspace: the symbol comes from an assembly. let metadataReferences = originDocument.Project.MetadataReferences return ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), idRange) - else if - // if goto definition is called as we are already at the declaration location of a symbol in - // either a signature or an implementation file then we jump to its respective position in the document - lexerSymbol.Range = targetRange - then - // jump from signature to the corresponding implementation - if isSignatureFile originDocument.FilePath then - let implFilePath = Path.ChangeExtension(originDocument.FilePath, "fs") - - if not (File.Exists implFilePath) then - return ValueNone + | ValueSome _ when lexerSymbol.Range = targetRange -> + let! navItem = + this.FindCounterpartOfDeclarationAtCaret( + originDocument, + targetSymbolUse, + checkFileResults, + lexerSymbol, + fcsTextLineNumber, + textLineString + ) + + return + navItem + |> ValueOption.map (fun navItem -> FSharpGoToDefinitionResult.NavigableItem navItem, idRange) + | ValueSome targetDocument -> + // gotoDefn origin = signature, destination = signature; origin = implementation, destination = implementation + let! navItem = + if isSignatureFile targetRange.FileName && preferSignature then + navigableItemAt targetDocument targetRange else - let implDocument = - originDocument.Project.Solution.TryGetDocumentFromPath implFilePath - - match implDocument with - | ValueNone -> return ValueNone - | ValueSome implDocument -> - let! targetRange = this.FindSymbolDeclarationInDocument(targetSymbolUse, implDocument) - - match targetRange with - | None -> return ValueNone - | Some targetRange -> - let! implSourceText = implDocument.GetTextAsync(cancellationToken) - - let implTextSpan = - RoslynHelpers.TryFSharpRangeToTextSpan(implSourceText, targetRange) - - match implTextSpan with - | ValueNone -> return ValueNone - | ValueSome implTextSpan -> - let navItem = FSharpGoToDefinitionNavigableItem(implDocument, implTextSpan) - return ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), idRange) - - else // jump from implementation to the corresponding signature - let declarations = - checkFileResults.GetDeclarationLocation( - fcsTextLineNumber, - idRange.EndColumn, - textLineString, - lexerSymbol.FullIsland, - true - ) + // Bugfix: apparently the target document is not always a signature file + let implFilePath = + if isSignatureFile targetDocument.FilePath then + Path.ChangeExtension(targetDocument.FilePath, "fs") + else + targetDocument.FilePath - match declarations with - | FindDeclResult.DeclFound targetRange -> - let sigDocument = - originDocument.Project.Solution.TryGetDocumentFromPath targetRange.FileName - - match sigDocument with - | ValueNone -> return ValueNone - | ValueSome sigDocument -> - let! sigSourceText = sigDocument.GetTextAsync(cancellationToken) - - let sigTextSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sigSourceText, targetRange) - - match sigTextSpan with - | ValueNone -> return ValueNone - | ValueSome sigTextSpan -> - let navItem = FSharpGoToDefinitionNavigableItem(sigDocument, sigTextSpan) - return ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), idRange) - | _ -> return ValueNone - // when the target range is different follow the navigation convention of - // - gotoDefn origin = signature , gotoDefn destination = signature - // - gotoDefn origin = implementation, gotoDefn destination = implementation - else - let sigDocument = - originDocument.Project.Solution.TryGetDocumentFromPath targetRange.FileName + match originDocument.TryGetSolutionDocumentFromPath implFilePath with + | ValueNone -> CancellableTask.singleton ValueNone + | ValueSome implDocument -> this.FindNavigableDeclarationIn(targetSymbolUse, implDocument) - match sigDocument with - | ValueNone -> return ValueNone - | ValueSome sigDocument -> - let! sigSourceText = sigDocument.GetTextAsync(cancellationToken) - let sigTextSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sigSourceText, targetRange) - - match sigTextSpan with - | ValueNone -> return ValueNone - | ValueSome sigTextSpan -> - // if the gotodef call originated from a signature and the returned target is a signature, navigate there - if isSignatureFile targetRange.FileName && preferSignature then - let navItem = FSharpGoToDefinitionNavigableItem(sigDocument, sigTextSpan) - return ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), idRange) - else // we need to get an FSharpSymbol from the targetRange found in the signature - // that symbol will be used to find the destination in the corresponding implementation file - let implFilePath = - // Bugfix: apparently sigDocument not always is a signature file - if isSignatureFile sigDocument.FilePath then - Path.ChangeExtension(sigDocument.FilePath, "fs") - else - sigDocument.FilePath - - let implDocument = - originDocument.Project.Solution.TryGetDocumentFromPath implFilePath - - match implDocument with - | ValueNone -> return ValueNone - | ValueSome implDocument -> - let! targetRange = this.FindSymbolDeclarationInDocument(targetSymbolUse, implDocument) - - match targetRange with - | None -> return ValueNone - | Some targetRange -> - let! implSourceText = implDocument.GetTextAsync(cancellationToken) - - let implTextSpan = - RoslynHelpers.TryFSharpRangeToTextSpan(implSourceText, targetRange) - - match implTextSpan with - | ValueNone -> return ValueNone - | ValueSome implTextSpan -> - let navItem = FSharpGoToDefinitionNavigableItem(implDocument, implTextSpan) - return ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), idRange) + return + navItem + |> ValueOption.map (fun navItem -> FSharpGoToDefinitionResult.NavigableItem navItem, idRange) | _ -> return ValueNone } From d4370d379e79687747323c3a1d7589f2cf8d876d Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 09:42:41 +0200 Subject: [PATCH 03/17] Test Go To Definition against a solution snapshot that predates the target document Co-Authored-By: Claude Fable 5.1 --- .../GoToDefinitionServiceTests.fs | 112 +++++++++++++++++- 1 file changed, 111 insertions(+), 1 deletion(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs index fe10a42a125..68d8755ac89 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs @@ -2,6 +2,8 @@ namespace FSharp.Editor.Tests +open System +open System.Threading open Xunit open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.Text @@ -9,8 +11,8 @@ open Microsoft.VisualStudio.FSharp.Editor open FSharp.Compiler.EditorServices open FSharp.Compiler.Text open FSharp.Editor.Tests.Helpers +open FSharp.Test.ProjectGeneration open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks -open System.Threading module GoToDefinitionServiceTests = @@ -149,3 +151,111 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = let expected = Some(3, 3, 20, 34) GoToDefinitionTest(fileContents, caretMarker, expected) + + let private symbolUseAt (document: Document) (sourceText: SourceText) position = + maybe { + let textLine = sourceText.Lines.GetLineFromPosition position + let fcsTextLineNumber = Line.fromZ (sourceText.Lines.GetLinePosition position).Line + + let! lexerSymbol = + Tokenizer.getSymbolAtPosition ( + document.Id, + sourceText, + position, + document.FilePath, + [], + SymbolLookupKind.Greedy, + false, + false, + None, + CancellationToken.None + ) + + let _, checkFileResults = + document.GetFSharpParseAndCheckResultsAsync userOpName + |> CancellableTask.runSynchronouslyWithoutCancellation + + return! + checkFileResults.GetSymbolUseAtLocation( + fcsTextLineNumber, + lexerSymbol.Ident.idRange.EndColumn, + textLine.ToString(), + lexerSymbol.FullIsland + ) + } + + /// An app project referencing a library project. The app document comes from a snapshot that + /// predates the library's document, the way Roslyn hands out documents while a solution is + /// still loading, while the workspace's current solution already has it. + module internal StaleSnapshot = + + let library = SyntheticProject.Create("Library", sourceFile "Library" []) + + let app = + { SyntheticProject.Create( + "App", + { sourceFile "App" [ "Library" ] with + ExtraSource = "let mapped = List.map id [ 1 ]" + } + ) with + DependsOn = [ library ] + } + + let solution, _ = RoslynTestHelpers.CreateMultiProjectSolution app + let appPath = app.GetFilePath "App" + let libraryPath = library.GetFilePath "Library" + + let private documentId path = + solution.GetDocumentIdsWithFilePath path |> Seq.exactlyOne + + let appDocument = + solution.RemoveDocument(documentId libraryPath).GetDocument(documentId appPath) + + let appSourceText = appDocument.GetTextAsync(CancellationToken.None).Result + + /// The position of the last character of the text, inside the identifier it ends with. + let positionOf (text: string) = + appSourceText.ToString().IndexOf(text, StringComparison.Ordinal) + text.Length + - 1 + + let findDefinitionAt position = + GoToDefinition(FSharpMetadataAsSourceService()).FindDefinitionAtPosition(appDocument, position) + |> CancellableTask.runSynchronouslyWithoutCancellation + + [] + let ``goto definition finds the target document through the workspace when the origin snapshot predates it`` () = + let position = StaleSnapshot.positionOf "ModuleLibrary.f" + let document = StaleSnapshot.appDocument + + let range = + findDefinition (document, StaleSnapshot.appSourceText, position, [], None) + |> Option.defaultWith (fun () -> failwith "declaration not found") + + Assert.Equal(StaleSnapshot.libraryPath, range.FileName) + Assert.True(Option.isNone (document.Project.Solution.TryGetDocumentFromFSharpRange(range, document.Project.Id))) + + match document.TryGetSolutionDocumentFromFSharpRange range with + | ValueSome target -> Assert.Equal(StaleSnapshot.libraryPath, target.FilePath) + | ValueNone -> failwith "the workspace's current solution has the library document" + + match StaleSnapshot.findDefinitionAt position with + | ValueSome(FSharpGoToDefinitionResult.NavigableItem item, _) -> Assert.Equal(StaleSnapshot.libraryPath, item.Document.FilePath) + | result -> failwith $"expected a navigable item, got %A{result}" + + [] + let ``goto definition treats a symbol whose file is in no solution as external`` () = + match StaleSnapshot.findDefinitionAt (StaleSnapshot.positionOf "List.map") with + | ValueSome(FSharpGoToDefinitionResult.ExternalAssembly _, _) -> () + | result -> failwith $"expected an external assembly, got %A{result}" + + [] + let ``find references scope includes the declaring project the origin snapshot does not know`` () = + let position = StaleSnapshot.positionOf "ModuleLibrary.f" + + let symbolUse = + symbolUseAt StaleSnapshot.appDocument StaleSnapshot.appSourceText position + |> Option.defaultWith (fun () -> failwith "symbol not found") + + match symbolUse.GetSymbolScope StaleSnapshot.appDocument with + | Some(SymbolScope.Projects(projects, _)) -> Assert.Contains(StaleSnapshot.library.Name, projects |> List.map _.Name) + | scope -> failwith $"expected a project scope, got %A{scope}" From 0957b3f46ec663a28b37d5446da2f9cee1582281 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 6 Sep 2026 09:46:14 +0200 Subject: [PATCH 04/17] Add the release note for PR #20462 Co-Authored-By: Claude Fable 5.1 --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 2357fbcdff4..0249c82f900 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -28,6 +28,7 @@ * Fix syntax coloring being lost for a whole file when one symbol resolves into metadata that could not be read. ([Issue #20269](https://github.com/dotnet/fsharp/issues/20269), [PR #20274](https://github.com/dotnet/fsharp/pull/20274)) * Go To Definition into an F# project built with a path map (`DeterministicSourcePaths` or `PathMap`) opens its source instead of a generated signature: the IDE no longer applies `--pathmap` to the project options it checks with. ([PR #20470](https://github.com/dotnet/fsharp/pull/20470)) * Fix completion after a closed backticked identifier on the same line. ([PR #20785](https://github.com/dotnet/fsharp/issues/20785)) +* Go To Definition on a symbol declared in another project of the solution no longer opens a generated signature when the origin document comes from a solution snapshot that predates that project's documents: navigation, Find All References and Rename now look the target up in the workspace's current solution as well. ([PR #20462](https://github.com/dotnet/fsharp/pull/20462)) ### Changed From b7ae603d327ece7d0686186a805cafdaf10be9aa Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 16 Sep 2026 15:45:17 +0200 Subject: [PATCH 05/17] Document TryGetSolutionDocumentFromPath Co-Authored-By: Claude Sonnet 5 --- vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs | 1 + 1 file changed, 1 insertion(+) diff --git a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs index 1eaaaed3180..dd3b687b9a5 100644 --- a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs +++ b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs @@ -138,6 +138,7 @@ type Document with | ids -> ValueSome [ for id in ids -> solution.GetDocument id ]) |> ValueOption.defaultValue [] + /// The first document with the file path, from whichever project includes it. member document.TryGetSolutionDocumentFromPath(filePath: string) = document.GetSolutionDocumentsWithFilePath filePath |> Seq.tryHeadV From 6009f9df70ec013dfcaea52f915fe625df776b96 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 16 Sep 2026 15:51:29 +0200 Subject: [PATCH 06/17] Route the symbol scope's project list through Seq List.map followed by List.distinctBy allocates the mapped list and then the distinct one; a Seq pipeline materializes once, at Seq.toList. Co-Authored-By: Claude Sonnet 5 --- vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs index ecedeb3e536..fccb12bc21e 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs +++ b/vsintegration/src/FSharp.Editor/LanguageService/Symbols.fs @@ -65,8 +65,9 @@ type FSharpSymbolUse with else let projects = currentDocument.GetSolutionDocumentsWithFilePath filePath - |> List.map _.Project - |> List.distinctBy _.Id + |> Seq.map _.Project + |> Seq.distinctBy _.Id + |> Seq.toList match projects with | [] -> None From a8f6891bf8a539d9198ed46d5f341025113251f4 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 16 Sep 2026 16:00:04 +0200 Subject: [PATCH 07/17] Route the multi-project test helper's project list through Seq Co-Authored-By: Claude Sonnet 5 --- .../tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 4183dcb27ee..9e168aa278e 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -383,8 +383,9 @@ type RoslynTestHelpers private () = let projects = syntheticProject.GetAllProjects() - |> List.distinctBy _.Name - |> List.map (fun project -> project, ProjectId.CreateNewId()) + |> Seq.distinctBy _.Name + |> Seq.map (fun project -> struct (project, ProjectId.CreateNewId())) + |> Seq.toArray let projectIds = dict [ for project, id in projects -> project.Name, id ] From 15bde63487a487fa1de99d4107a1ead6b413402b Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 16 Sep 2026 16:03:05 +0200 Subject: [PATCH 08/17] Collapse the declaration lookup into match! Co-Authored-By: Claude Sonnet 5 --- vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index 81ed086e6ba..263f6479817 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -368,9 +368,7 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = /// The navigable item for the target symbol's declaration in the implementation document. member private this.FindNavigableDeclarationIn(targetSymbolUse: FSharpSymbolUse, implDocument: Document) = cancellableTask { - let! declarationRange = this.FindSymbolDeclarationInDocument(targetSymbolUse, implDocument) - - match declarationRange with + match! this.FindSymbolDeclarationInDocument(targetSymbolUse, implDocument) with | None -> return ValueNone | Some declarationRange -> return! navigableItemAt implDocument declarationRange } From bfbea5aff23ea1e46fb1b250daa49ae92b4b0fbb Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 16 Sep 2026 16:06:51 +0200 Subject: [PATCH 09/17] Make TargetInstance a struct record Co-Authored-By: Claude Fable 5.1 --- vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs | 1 + 1 file changed, 1 insertion(+) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 9e168aa278e..58eb21fedd4 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -203,6 +203,7 @@ type TestHostServices() = /// One Roslyn project instance of a multi-targeted F# project: its extra defines and the /// synthetic files left out of it, as VS does per target framework. +[] type TargetInstance = { Defines: string list From f11464592ace6365788447d02c64e058b5767a46 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 16 Sep 2026 16:11:47 +0200 Subject: [PATCH 10/17] Feed the excluded paths to the HashSet as a seq Co-Authored-By: Claude Fable 5.1 --- .../tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 58eb21fedd4..95932f27ea2 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -433,13 +433,13 @@ type RoslynTestHelpers private () = for instance in instances -> let excludedPaths = HashSet( - [ + seq { for fileId in instance.ExcludedFileIds do syntheticProject.GetFilePath fileId if (syntheticProject.Find fileId).HasSignatureFile then syntheticProject.GetSignatureFilePath fileId - ], + }, StringComparer.OrdinalIgnoreCase ) From 5c590bb1273c465091c16371f5058bb100ede543 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 16 Sep 2026 16:50:48 +0200 Subject: [PATCH 11/17] Make CreateMultiTargetSolution's instance tuples struct Co-Authored-By: Claude Sonnet 5 --- .../tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index 95932f27ea2..b1a1ebe33c5 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -471,7 +471,7 @@ type RoslynTestHelpers private () = |] } - id, projectInfo, instanceOptions + struct (id, projectInfo, instanceOptions) ] let solution = From e1f171bc2d0c7898fd386109889d282db56736ff Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 16 Sep 2026 17:39:33 +0200 Subject: [PATCH 12/17] Document CreateMultiTargetSolution and return its results as a struct tuple CreateMultiProjectSolution and CreateMultiTargetSolution now both return a struct tuple, so callers destructure with the struct pattern. Co-Authored-By: Claude Sonnet 5 --- .../FSharp.Editor.Tests/GoToDefinitionServiceTests.fs | 2 +- .../tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs index 68d8755ac89..a4c6d60b1c4 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs @@ -201,7 +201,7 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = DependsOn = [ library ] } - let solution, _ = RoslynTestHelpers.CreateMultiProjectSolution app + let struct (solution, _) = RoslynTestHelpers.CreateMultiProjectSolution app let appPath = app.GetFilePath "App" let libraryPath = library.GetFilePath "Library" diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index b1a1ebe33c5..3089435309b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -417,10 +417,15 @@ type RoslynTestHelpers private () = project.GetProjectOptions checker |> RoslynTestHelpers.SetProjectOptions id solution - solution, checker + struct (solution, checker) + /// /// One Roslyn project per target instance, all sharing the .fsproj path and the document file /// paths, like the per-target-framework projects VS creates for a multi-targeted project. + /// + /// The project every instance is made from; it must not depend on other projects. + /// One Roslyn project per entry: its extra defines and the synthetic files left out of it. + /// The solution and the id of each instance's project, in the order of . static member CreateMultiTargetSolution(syntheticProject: SyntheticProject, instances: TargetInstance list) = assert (syntheticProject.DependsOn = []) @@ -480,7 +485,7 @@ type RoslynTestHelpers private () = for id, _, instanceOptions in instances do RoslynTestHelpers.SetProjectOptions id solution instanceOptions - solution, [ for id, _, _ in instances -> id ] + struct (solution, [ for id, _, _ in instances -> id ]) static member GetFsDocument(code, ?customProjectOption: string, ?customEditorOptions) = let customProjectOptions = From 35b6f23327cd9e5f780f45e80e041af2b611c70b Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sat, 26 Sep 2026 14:30:55 +0200 Subject: [PATCH 13/17] Prefer the origin's own project when a path names several documents A sibling file looked up by path took whichever document came first, while the same lookup by range prefers the origin's project or one it depends on. The instances of a multi-targeted project hold the same file and read it under their own defines, so the path lookup now goes through that rule too. Co-Authored-By: Claude Opus 5 (1M context) --- .../FSharp.Editor/Common/CodeAnalysisExtensions.fs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs index dd3b687b9a5..ecd0fa48a90 100644 --- a/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs +++ b/vsintegration/src/FSharp.Editor/Common/CodeAnalysisExtensions.fs @@ -138,12 +138,15 @@ type Document with | ids -> ValueSome [ for id in ids -> solution.GetDocument id ]) |> ValueOption.defaultValue [] - /// The first document with the file path, from whichever project includes it. - member document.TryGetSolutionDocumentFromPath(filePath: string) = - document.GetSolutionDocumentsWithFilePath filePath |> Seq.tryHeadV - /// The document for the range's file, preferring this document's project or one it depends on. member document.TryGetSolutionDocumentFromFSharpRange(range: range) = document.TryFindInSolutions(fun solution -> solution.TryGetDocumentFromFSharpRange(range, document.Project.Id) |> ValueOption.ofOption) + + /// The document with the file path, preferring this document's project or one it depends on: the + /// instances of a multi-targeted project hold the same file, and the file is read under the defines of + /// whichever instance answers, so the origin's own instance is the one that answers about its own code. + member document.TryGetSolutionDocumentFromPath(filePath: string) = + Range.mkRange filePath Position.pos0 Position.pos0 + |> document.TryGetSolutionDocumentFromFSharpRange From b1ea8a1823a1ec33ba3a7cd094b4106900698721 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 1 Oct 2026 19:32:34 +0200 Subject: [PATCH 14/17] Rename only the uses the rename's own solution holds The symbol scope can now come from the workspace's current solution, so the rename search can find a document that the solution the rename started from does not have. Resolving it there gave null and the rename threw. Roslyn's rename session commits only the documents of that solution that changed against it, so a use anywhere else cannot be renamed by it: the rename leaves those out, and reads text and applies edits in its own solution, buffer edits included. The test project references Microsoft.CodeAnalysis.EditorFeatures because the rename types derive from it and Visual Studio, not the package, supplies it at run time. Co-Authored-By: Claude Opus 5.5 --- .../InlineRename/InlineRenameService.fs | 81 +++++-------------- .../FSharp.Editor.Tests.fsproj | 1 + .../GoToDefinitionServiceTests.fs | 46 ++++++++++- 3 files changed, 67 insertions(+), 61 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs b/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs index 04a8566be64..2639eea0a17 100644 --- a/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs +++ b/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs @@ -102,12 +102,6 @@ type internal InlineRenameInfo let symbolDisplayName = symbolUse.Symbol.DisplayName - let recordTypeName = - match symbolUse.Symbol with - | :? FSharpEntity as entity when entity.IsFSharpRecord || entity.IsOpaque -> - ValueSome(FSharpKeywords.NormalizeIdentifierBackticks symbolDisplayName) - | _ -> ValueNone - override _.CanRename = true override _.LocalizedErrorMessage = null override _.TriggerSpan = triggerSpan @@ -157,62 +151,31 @@ type internal InlineRenameInfo cancellableTask { let! symbolUsesByDocumentId = symbolUses + // Roslyn's rename session commits only documents of the solution it started from, and the search can + // find a document through the workspace's newer solution: such a use cannot be part of this rename. + let solution = document.Project.Solution + let! results = seq { for (KeyValue(documentId, symbolUses)) in symbolUsesByDocumentId do - - cancellableTask { - let! cancellationToken = CancellableTask.getCancellationToken () - let document = document.Project.Solution.GetDocument(documentId) - let! sourceText = document.GetTextAsync(cancellationToken) - - let! recordOptions = - match recordTypeName with - | ValueSome name -> - cancellableTask { - let! defines, langVersion = document.GetFsharpParsingOptionsAsync(nameof (InlineRenameInfo)) - return ValueSome(defines, langVersion, name) - } - | ValueNone -> CancellableTask.singleton ValueNone - - let tryGetEditSpan (textSpan: TextSpan) = - match recordOptions with - | ValueSome(defines, langVersion, name) when not textSpan.IsEmpty -> - // Record construction/update references can describe expressions, not type-name tokens. - match - Tokenizer.getSymbolAtPosition ( - document.Id, - sourceText, - textSpan.End - 1, - document.FilePath, - defines, - SymbolLookupKind.Greedy, - false, - false, - Some langVersion, - cancellationToken - ) - with - | Some token when FSharpKeywords.NormalizeIdentifierBackticks token.Ident.idText = name -> - match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, token.Range) with - | ValueSome tokenSpan when tokenSpan.End = textSpan.End && tokenSpan.Start >= textSpan.Start -> - ValueSome tokenSpan - | _ -> ValueNone - | _ -> ValueNone - | ValueSome _ -> ValueNone - | ValueNone -> ValueSome textSpan - - return - [| - for symbolUse in symbolUses do - match - Tokenizer.TryFSharpRangeToTextSpanForEditor(sourceText, symbolUse, symbolDisplayName) - |> ValueOption.bind tryGetEditSpan - with - | ValueSome textSpan -> yield FSharpInlineRenameLocation(document, textSpan) - | _ -> () - |] - } + match solution.GetDocument documentId with + | null -> () + | document -> + yield + cancellableTask { + let! cancellationToken = CancellableTask.getCancellationToken () + let! sourceText = document.GetTextAsync(cancellationToken) + + return + [| + for symbolUse in symbolUses do + match + Tokenizer.TryFSharpRangeToTextSpanForEditor(sourceText, symbolUse, symbolDisplayName) + with + | ValueSome textSpan -> yield FSharpInlineRenameLocation(document, textSpan) + | ValueNone -> () + |] + } } |> CancellableTask.whenAll diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index d5d1a09c79a..faf944d7caa 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -100,6 +100,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs index a4c6d60b1c4..d77b74ef973 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs @@ -186,7 +186,8 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = /// An app project referencing a library project. The app document comes from a snapshot that /// predates the library's document, the way Roslyn hands out documents while a solution is - /// still loading, while the workspace's current solution already has it. + /// still loading, while the workspace's current solution already has it. The app document also + /// holds an edit of its buffer that the workspace has not seen. module internal StaleSnapshot = let library = SyntheticProject.Create("Library", sourceFile "Library" []) @@ -204,12 +205,21 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = let struct (solution, _) = RoslynTestHelpers.CreateMultiProjectSolution app let appPath = app.GetFilePath "App" let libraryPath = library.GetFilePath "Library" + let unsavedEdit = "// not saved yet" let private documentId path = solution.GetDocumentIdsWithFilePath path |> Seq.exactlyOne let appDocument = - solution.RemoveDocument(documentId libraryPath).GetDocument(documentId appPath) + let appId = documentId appPath + + let savedText = + solution.GetDocument(appId).GetTextAsync(CancellationToken.None).Result + + solution + .RemoveDocument(documentId libraryPath) + .WithDocumentText(appId, SourceText.From($"{savedText}{Environment.NewLine}{unsavedEdit}")) + .GetDocument(appId) let appSourceText = appDocument.GetTextAsync(CancellationToken.None).Result @@ -259,3 +269,35 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = match symbolUse.GetSymbolScope StaleSnapshot.appDocument with | Some(SymbolScope.Projects(projects, _)) -> Assert.Contains(StaleSnapshot.library.Name, projects |> List.map _.Name) | scope -> failwith $"expected a project scope, got %A{scope}" + + [] + let ``rename from a snapshot that predates the declaring document edits the documents of that snapshot`` () = + let document = StaleSnapshot.appDocument + let snapshot = document.Project.Solution + + let renameInfo = + InlineRenameService().GetRenameInfoAsync(document, StaleSnapshot.positionOf "ModuleLibrary.f", CancellationToken.None).Result + + Assert.NotNull renameInfo + + let locations = + renameInfo.FindRenameLocationsAsync(false, false, CancellationToken.None).Result + + Assert.All(locations.Locations, (fun location -> Assert.Same(snapshot, location.Document.Project.Solution))) + + let newSolution = + locations.GetReplacementsAsync("g", CancellationToken.None).Result.NewSolution + + // What Roslyn's rename session commits: the documents changed against the solution it started from. + let changedDocuments = + newSolution.GetChanges(snapshot).GetProjectChanges() + |> Seq.collect _.GetChangedDocuments() + |> Seq.toList + + Assert.Equal([ document.Id ], changedDocuments) + + let renamedText = + newSolution.GetDocument(document.Id).GetTextAsync(CancellationToken.None).Result.ToString() + + Assert.Contains("ModuleLibrary.g", renamedText) + Assert.EndsWith(StaleSnapshot.unsavedEdit, renamedText) From d95d3e1b40c339b0f93ed0bdbc08a12b3112d6ad Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 1 Oct 2026 19:32:35 +0200 Subject: [PATCH 15/17] Test that Go To Definition reads the origin's own target instance From a signature in one instance of a multi-targeted project, the implementation is looked up in that instance: the other compiles the file under its own defines, where the declaration can be missing. This gives the multi-target half of the shared fixture a caller. Co-Authored-By: Claude Opus 5.5 --- .../GoToDefinitionServiceTests.fs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs index d77b74ef973..6e61de336aa 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs @@ -301,3 +301,45 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = Assert.Contains("ModuleLibrary.g", renamedText) Assert.EndsWith(StaleSnapshot.unsavedEdit, renamedText) + + [] + let ``goto definition from a signature reaches the implementation its own target instance compiles`` () = + let project = + SyntheticProject.Create( + "MultiTarget", + { sourceFile "A" [] with + Source = "#if NET\nlet onlyOnNet = 1\n#endif" + SignatureFile = SignatureFile.Custom "#if NET\nval onlyOnNet: int\n#endif" + } + ) + + let struct (solution, instances) = + RoslynTestHelpers.CreateMultiTargetSolution( + project, + [ + { Defines = []; ExcludedFileIds = [] } + { + Defines = [ "NET" ] + ExcludedFileIds = [] + } + ] + ) + + let netInstance = solution.GetProject(List.last instances) + + let signature = + netInstance.Documents + |> Seq.find (fun document -> document.FilePath = project.GetSignatureFilePath "A") + + let position = + signature.GetTextAsync(CancellationToken.None).Result.ToString().IndexOf("onlyOnNet", StringComparison.Ordinal) + + let result = + GoToDefinition(FSharpMetadataAsSourceService()).FindDefinitionAtPosition(signature, position) + |> CancellableTask.runSynchronouslyWithoutCancellation + + match result with + | ValueSome(FSharpGoToDefinitionResult.NavigableItem item, _) -> + Assert.Equal(project.GetFilePath "A", item.Document.FilePath) + Assert.Equal(netInstance.Id, item.Document.Project.Id) + | result -> failwith $"expected the implementation, got %A{result}" From 52cd981ef50135cad83ed66237391a62a049c4ec Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 11 Oct 2026 10:47:38 +0200 Subject: [PATCH 16/17] Find a rename's uses in the text the rename's own solution holds The reference search reads each document as the workspace holds it now, so the ranges it finds are ranges of that text. Rename applied them to the text of the solution it started from. Where that solution held another text of a document, an insertion before a use moved the edit onto the wrong characters. For such a document the uses are now found by checking the text the rename edits. A document whose text the workspace still holds keeps the ranges the search found. Co-Authored-By: Claude Opus 5.5 --- .../InlineRename/InlineRenameService.fs | 30 ++++++++++++ .../GoToDefinitionServiceTests.fs | 49 +++++++++++++------ 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs b/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs index 2639eea0a17..5c297d7fac9 100644 --- a/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs +++ b/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs @@ -102,6 +102,35 @@ type internal InlineRenameInfo let symbolDisplayName = symbolUse.Symbol.DisplayName + /// The search reads the text the workspace holds for a document, so the ranges it finds are ranges of that + /// text. Where the rename's solution holds another text, the uses are found in the text the rename edits. + let usesInTextOf (document: Document) (text: SourceText) (found: range[]) = + cancellableTask { + let! cancellationToken = CancellableTask.getCancellationToken () + + let! searchedThisText = + match document.Project.Solution.Workspace.CurrentSolution.GetDocument document.Id with + | null -> CancellableTask.singleton false + | searched -> + cancellableTask { + let! searchedText = searched.GetTextAsync cancellationToken + return text.ContentEquals searchedText + } + + if searchedThisText then + return found + else + let! _, checkFileResults = document.GetFSharpParseAndCheckResultsAsync(nameof (InlineRenameInfo)) + + return + checkFileResults.GetUsesOfSymbolInFile( + symbolUse.Symbol, + relatedSymbolKinds = RelatedSymbolUseKind.All, + cancellationToken = cancellationToken + ) + |> Array.map _.Range + } + override _.CanRename = true override _.LocalizedErrorMessage = null override _.TriggerSpan = triggerSpan @@ -165,6 +194,7 @@ type internal InlineRenameInfo cancellableTask { let! cancellationToken = CancellableTask.getCancellationToken () let! sourceText = document.GetTextAsync(cancellationToken) + let! symbolUses = usesInTextOf document sourceText symbolUses return [| diff --git a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs index 6e61de336aa..0640253448b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/GoToDefinitionServiceTests.fs @@ -196,7 +196,7 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = { SyntheticProject.Create( "App", { sourceFile "App" [ "Library" ] with - ExtraSource = "let mapped = List.map id [ 1 ]" + ExtraSource = "let mapped = List.map id [ 1 ]\nlet again = ModuleLibrary.f 2" } ) with DependsOn = [ library ] @@ -210,16 +210,17 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = let private documentId path = solution.GetDocumentIdsWithFilePath path |> Seq.exactlyOne - let appDocument = + /// The app document of the stale snapshot, holding the text `edit` makes of the saved one. + let appDocumentWith (edit: string -> string) = let appId = documentId appPath let savedText = - solution.GetDocument(appId).GetTextAsync(CancellationToken.None).Result + solution.GetDocument(appId).GetTextAsync(CancellationToken.None).Result.ToString() + + solution.RemoveDocument(documentId libraryPath).WithDocumentText(appId, SourceText.From(edit savedText)).GetDocument(appId) - solution - .RemoveDocument(documentId libraryPath) - .WithDocumentText(appId, SourceText.From($"{savedText}{Environment.NewLine}{unsavedEdit}")) - .GetDocument(appId) + let appDocument = + appDocumentWith (fun saved -> $"{saved}{Environment.NewLine}{unsavedEdit}") let appSourceText = appDocument.GetTextAsync(CancellationToken.None).Result @@ -270,20 +271,41 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = | Some(SymbolScope.Projects(projects, _)) -> Assert.Contains(StaleSnapshot.library.Name, projects |> List.map _.Name) | scope -> failwith $"expected a project scope, got %A{scope}" - [] - let ``rename from a snapshot that predates the declaring document edits the documents of that snapshot`` () = - let document = StaleSnapshot.appDocument + [] + [] + [] + [] + let ``rename from a snapshot that predates the declaring document edits the documents of that snapshot`` + (insertedBeforeUse: string, appended: string) + = + let useOfF = "ModuleLibrary.f" + + let document = + StaleSnapshot.appDocumentWith (fun saved -> saved.Replace($"= {useOfF}", $"= {insertedBeforeUse}{useOfF}") + appended) + let snapshot = document.Project.Solution + let originText = document.GetTextAsync(CancellationToken.None).Result.ToString() + + let position = + originText.LastIndexOf(useOfF, StringComparison.Ordinal) + useOfF.Length - 1 + let renameInfo = - InlineRenameService().GetRenameInfoAsync(document, StaleSnapshot.positionOf "ModuleLibrary.f", CancellationToken.None).Result + InlineRenameService().GetRenameInfoAsync(document, position, CancellationToken.None).Result Assert.NotNull renameInfo let locations = renameInfo.FindRenameLocationsAsync(false, false, CancellationToken.None).Result - Assert.All(locations.Locations, (fun location -> Assert.Same(snapshot, location.Document.Project.Solution))) + Assert.Equal(2, locations.Locations.Count) + + Assert.All( + locations.Locations, + (fun location -> + Assert.Same(snapshot, location.Document.Project.Solution) + Assert.Equal("f", location.Document.GetTextAsync(CancellationToken.None).Result.ToString location.TextSpan)) + ) let newSolution = locations.GetReplacementsAsync("g", CancellationToken.None).Result.NewSolution @@ -299,8 +321,7 @@ let f_IWSAM_flex_StaticProperty(x: #IStaticProperty<'T>) = let renamedText = newSolution.GetDocument(document.Id).GetTextAsync(CancellationToken.None).Result.ToString() - Assert.Contains("ModuleLibrary.g", renamedText) - Assert.EndsWith(StaleSnapshot.unsavedEdit, renamedText) + Assert.Equal(originText.Replace(useOfF, "ModuleLibrary.g"), renamedText) [] let ``goto definition from a signature reaches the implementation its own target instance compiles`` () = From 9ddcd112e46905d65b74668626e117a456cff5fc Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 29 Oct 2026 22:15:07 +0100 Subject: [PATCH 17/17] Bring the rebased branch to what it had before the rebase Replaying the branch onto main left some files short of the branch's own content; this restores it. Co-Authored-By: Claude Opus 5.5 --- .../InlineRename/InlineRenameService.fs | 43 +++++++++++++++++++ .../FSharp.Editor.Tests.fsproj | 1 - 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs b/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs index 5c297d7fac9..16a24805446 100644 --- a/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs +++ b/vsintegration/src/FSharp.Editor/InlineRename/InlineRenameService.fs @@ -102,6 +102,12 @@ type internal InlineRenameInfo let symbolDisplayName = symbolUse.Symbol.DisplayName + let recordTypeName = + match symbolUse.Symbol with + | :? FSharpEntity as entity when entity.IsFSharpRecord || entity.IsOpaque -> + ValueSome(FSharpKeywords.NormalizeIdentifierBackticks symbolDisplayName) + | _ -> ValueNone + /// The search reads the text the workspace holds for a document, so the ranges it finds are ranges of that /// text. Where the rename's solution holds another text, the uses are found in the text the rename edits. let usesInTextOf (document: Document) (text: SourceText) (found: range[]) = @@ -196,11 +202,48 @@ type internal InlineRenameInfo let! sourceText = document.GetTextAsync(cancellationToken) let! symbolUses = usesInTextOf document sourceText symbolUses + let! recordOptions = + match recordTypeName with + | ValueSome name -> + cancellableTask { + let! defines, langVersion = document.GetFsharpParsingOptionsAsync(nameof (InlineRenameInfo)) + return ValueSome(defines, langVersion, name) + } + | ValueNone -> CancellableTask.singleton ValueNone + + let tryGetEditSpan (textSpan: TextSpan) = + match recordOptions with + | ValueSome(defines, langVersion, name) when not textSpan.IsEmpty -> + // Record construction/update references can describe expressions, not type-name tokens. + match + Tokenizer.getSymbolAtPosition ( + document.Id, + sourceText, + textSpan.End - 1, + document.FilePath, + defines, + SymbolLookupKind.Greedy, + false, + false, + Some langVersion, + cancellationToken + ) + with + | Some token when FSharpKeywords.NormalizeIdentifierBackticks token.Ident.idText = name -> + match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, token.Range) with + | ValueSome tokenSpan when tokenSpan.End = textSpan.End && tokenSpan.Start >= textSpan.Start -> + ValueSome tokenSpan + | _ -> ValueNone + | _ -> ValueNone + | ValueSome _ -> ValueNone + | ValueNone -> ValueSome textSpan + return [| for symbolUse in symbolUses do match Tokenizer.TryFSharpRangeToTextSpanForEditor(sourceText, symbolUse, symbolDisplayName) + |> ValueOption.bind tryGetEditSpan with | ValueSome textSpan -> yield FSharpInlineRenameLocation(document, textSpan) | ValueNone -> () diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index faf944d7caa..d5d1a09c79a 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -100,7 +100,6 @@ -