Skip to content

Commit

Permalink
VS: fix unwanted navigation on hover (#17592)
Browse files Browse the repository at this point in the history
  • Loading branch information
majocha authored Aug 26, 2024
1 parent 0a715a5 commit 758f039
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 74 deletions.
2 changes: 2 additions & 0 deletions docs/release-notes/.VisualStudio/17.12.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
Original file line number Diff line number Diff line change
Expand Up @@ -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<_>
Expand Down
152 changes: 79 additions & 73 deletions vsintegration/src/FSharp.Editor/Navigation/GoToDefinition.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
(
Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 758f039

Please sign in to comment.