diff --git a/docs/release-notes/.VisualStudio/17.12.md b/docs/release-notes/.VisualStudio/17.12.md index 2fb0e2d6977..c53417cbf6e 100644 --- a/docs/release-notes/.VisualStudio/17.12.md +++ b/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))) diff --git a/vsintegration/src/FSharp.Editor/LanguageService/MetadataAsSource.fs b/vsintegration/src/FSharp.Editor/LanguageService/MetadataAsSource.fs index 8c13aa65f63..0d21ee0b358 100644 --- a/vsintegration/src/FSharp.Editor/LanguageService/MetadataAsSource.fs +++ b/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<_> diff --git a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs index f0097ee32b7..69a97e9732f 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs @@ -199,6 +199,76 @@ type internal GoToDefinition(metadataAsSource: FSharpMetadataAsSourceService) = return None } + member _.TryGetExternalDeclarationAsync(targetSymbolUse: FSharpSymbolUse, metadataReferences: seq) = + 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) = - 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 }