Merge pull request #17606 from dotnet/merges/main-to-release/dev17.12

Merge main to release/dev17.12

Author: vzarytovskii

docs/release-notes/.VisualStudio/17.12.md

@@ -5,6 +5,8 @@
 ### Added
 
 ### Changed
+* Fix unwanted navigation on hover [PR #17592](https://github.com/dotnet/fsharp/pull/17592))
+
 
 ### Breaking Changes
 * Enable FSharp 9.0 Language Version ([Issue #17497](https://github.com/dotnet/fsharp/issues/17438)), [PR](https://github.com/dotnet/fsharp/pull/17500)))

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

@@ -99,7 +99,10 @@ module internal MetadataAsSource =
             ErrorHandler.ThrowOnFailure(windowFrame.SetProperty(int __VSFPROPID5.VSFPROPID_OverrideToolTip, name))
             |> ignore
 
-        windowFrame.Show() |> ignore
+        // This is commented out as a temporary fix for https://github.com/dotnet/fsharp/issues/17230
+        // With the new IFindDefinitionService interface we don't need to manipulate VS text windows directly.
+
+        // windowFrame.Show() |> ignore
 
         let textContainer = textBuffer.AsTextContainer()
         let mutable workspace = Unchecked.defaultof<_>

vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs

@@ -199,6 +199,76 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) =
                 return None
         }
 
+    member _.TryGetExternalDeclarationAsync(targetSymbolUse: FSharpSymbolUse, metadataReferences: seq<MetadataReference>) =
+        let textOpt =
+            match targetSymbolUse.Symbol with
+            | :? FSharpEntity as symbol -> symbol.TryGetMetadataText() |> Option.map (fun text -> text, symbol.DisplayName)
+            | :? FSharpMemberOrFunctionOrValue as symbol ->
+                symbol.ApparentEnclosingEntity.TryGetMetadataText()
+                |> Option.map (fun text -> text, symbol.ApparentEnclosingEntity.DisplayName)
+            | :? FSharpField as symbol ->
+                match symbol.DeclaringEntity with
+                | Some entity ->
+                    let text = entity.TryGetMetadataText()
+
+                    match text with
+                    | Some text -> Some(text, entity.DisplayName)
+                    | None -> None
+                | None -> None
+            | :? FSharpUnionCase as symbol ->
+                symbol.DeclaringEntity.TryGetMetadataText()
+                |> Option.map (fun text -> text, symbol.DisplayName)
+            | _ -> None
+
+        match textOpt with
+        | None -> CancellableTask.singleton None
+        | Some(text, fileName) ->
+            foregroundCancellableTask {
+                let! cancellationToken = CancellableTask.getCancellationToken ()
+                do! ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken)
+
+                let tmpProjInfo, tmpDocInfo =
+                    MetadataAsSource.generateTemporaryDocument (
+                        AssemblyIdentity(targetSymbolUse.Symbol.Assembly.QualifiedName),
+                        fileName,
+                        metadataReferences
+                    )
+
+                let tmpShownDocOpt =
+                    metadataAsSource.ShowDocument(tmpProjInfo, tmpDocInfo.FilePath, SourceText.From(text.ToString()))
+
+                match tmpShownDocOpt with
+                | ValueNone -> return None
+                | ValueSome tmpShownDoc ->
+                    let! _, checkResults = tmpShownDoc.GetFSharpParseAndCheckResultsAsync("NavigateToExternalDeclaration")
+
+                    let r =
+                        // This tries to find the best possible location of the target symbol's location in the metadata source.
+                        // We really should rely on symbol equality within FCS instead of doing it here,
+                        //     but the generated metadata as source isn't perfect for symbol equality.
+                        let symbols = checkResults.GetAllUsesOfAllSymbolsInFile(cancellationToken)
+
+                        symbols
+                        |> Seq.tryFindV (tryFindExternalSymbolUse targetSymbolUse)
+                        |> ValueOption.map (fun x -> x.Range)
+
+                    let! span =
+                        cancellableTask {
+                            let! cancellationToken = CancellableTask.getCancellationToken ()
+
+                            match r with
+                            | ValueNone -> return TextSpan.empty
+                            | ValueSome r ->
+                                let! text = tmpShownDoc.GetTextAsync(cancellationToken)
+
+                                match RoslynHelpers.TryFSharpRangeToTextSpan(text, r) with
+                                | ValueSome span -> return span
+                                | _ -> return TextSpan.empty
+                        }
+
+                    return Some(FSharpGoToDefinitionNavigableItem(tmpShownDoc, span) :> FSharpNavigableItem)
+            }
+
     /// Helper function that is used to determine the navigation strategy to apply, can be tuned towards signatures or implementation files.
     member private _.FindSymbolHelper(originDocument: Document, originRange: range, sourceText: SourceText, preferSignature: bool) =
         let originTextSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, originRange)
@@ -566,75 +636,13 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) =
         }
 
     member this.NavigateToExternalDeclarationAsync(targetSymbolUse: FSharpSymbolUse, metadataReferences: seq<MetadataReference>) =
-        let textOpt =
-            match targetSymbolUse.Symbol with
-            | :? FSharpEntity as symbol -> symbol.TryGetMetadataText() |> Option.map (fun text -> text, symbol.DisplayName)
-            | :? FSharpMemberOrFunctionOrValue as symbol ->
-                symbol.ApparentEnclosingEntity.TryGetMetadataText()
-                |> Option.map (fun text -> text, symbol.ApparentEnclosingEntity.DisplayName)
-            | :? FSharpField as symbol ->
-                match symbol.DeclaringEntity with
-                | Some entity ->
-                    let text = entity.TryGetMetadataText()
-
-                    match text with
-                    | Some text -> Some(text, entity.DisplayName)
-                    | None -> None
-                | None -> None
-            | :? FSharpUnionCase as symbol ->
-                symbol.DeclaringEntity.TryGetMetadataText()
-                |> Option.map (fun text -> text, symbol.DisplayName)
-            | _ -> None
-
-        match textOpt with
-        | None -> CancellableTask.singleton false
-        | Some(text, fileName) ->
-            foregroundCancellableTask {
-                let! cancellationToken = CancellableTask.getCancellationToken ()
-                do! ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken)
-
-                let tmpProjInfo, tmpDocInfo =
-                    MetadataAsSource.generateTemporaryDocument (
-                        AssemblyIdentity(targetSymbolUse.Symbol.Assembly.QualifiedName),
-                        fileName,
-                        metadataReferences
-                    )
-
-                let tmpShownDocOpt =
-                    metadataAsSource.ShowDocument(tmpProjInfo, tmpDocInfo.FilePath, SourceText.From(text.ToString()))
-
-                match tmpShownDocOpt with
-                | ValueNone -> return false
-                | ValueSome tmpShownDoc ->
-                    let! _, checkResults = tmpShownDoc.GetFSharpParseAndCheckResultsAsync("NavigateToExternalDeclaration")
-
-                    let r =
-                        // This tries to find the best possible location of the target symbol's location in the metadata source.
-                        // We really should rely on symbol equality within FCS instead of doing it here,
-                        //     but the generated metadata as source isn't perfect for symbol equality.
-                        let symbols = checkResults.GetAllUsesOfAllSymbolsInFile(cancellationToken)
-
-                        symbols
-                        |> Seq.tryFindV (tryFindExternalSymbolUse targetSymbolUse)
-                        |> ValueOption.map (fun x -> x.Range)
-
-                    let! span =
-                        cancellableTask {
-                            let! cancellationToken = CancellableTask.getCancellationToken ()
-
-                            match r with
-                            | ValueNone -> return TextSpan.empty
-                            | ValueSome r ->
-                                let! text = tmpShownDoc.GetTextAsync(cancellationToken)
-
-                                match RoslynHelpers.TryFSharpRangeToTextSpan(text, r) with
-                                | ValueSome span -> return span
-                                | _ -> return TextSpan.empty
-                        }
+        foregroundCancellableTask {
+            let! cancellationToken = CancellableTask.getCancellationToken ()
 
-                    let navItem = FSharpGoToDefinitionNavigableItem(tmpShownDoc, span)
-                    return this.NavigateToItem(navItem, cancellationToken)
-            }
+            match! this.TryGetExternalDeclarationAsync(targetSymbolUse, metadataReferences) with
+            | Some navItem -> return this.NavigateToItem(navItem, cancellationToken)
+            | None -> return false
+        }
 
     member this.NavigateToExternalDeclaration
         (
@@ -780,11 +788,9 @@ type internal FSharpNavigation(metadataAsSource: FSharpMetadataAsSourceService,
             match result with
             | ValueSome(FSharpGoToDefinitionResult.NavigableItem(navItem), _) -> return ImmutableArray.create navItem
             | ValueSome(FSharpGoToDefinitionResult.ExternalAssembly(targetSymbolUse, metadataReferences), _) ->
-                do!
-                    gtd.NavigateToExternalDeclarationAsync(targetSymbolUse, metadataReferences)
-                    |> CancellableTask.ignore
-
-                return ImmutableArray.empty
+                match! gtd.TryGetExternalDeclarationAsync(targetSymbolUse, metadataReferences) with
+                | Some navItem -> return ImmutableArray.Create navItem
+                | _ -> return ImmutableArray.empty
             | _ -> return ImmutableArray.empty
         }