From fb5a83dcba05b8cd4afe2282fee48a450f479cfe Mon Sep 17 00:00:00 2001 From: majocha Date: Tue, 28 Feb 2023 18:09:24 +0100 Subject: [PATCH 1/5] simplify code, cache everything --- .../src/FSharp.Editor/Common/Extensions.fs | 3 + .../Navigation/NavigateToSearchService.fs | 300 ++++++------------ 2 files changed, 93 insertions(+), 210 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Common/Extensions.fs b/vsintegration/src/FSharp.Editor/Common/Extensions.fs index fd9b80fff00..f641574e4ed 100644 --- a/vsintegration/src/FSharp.Editor/Common/Extensions.fs +++ b/vsintegration/src/FSharp.Editor/Common/Extensions.fs @@ -60,6 +60,9 @@ type Document with member this.IsFSharpScript = isScriptFile this.FilePath + member this.IsFSharpSignatureFile = + isSignatureFile this.FilePath + module private SourceText = open System.Runtime.CompilerServices diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs index c451a105e05..d640882b050 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -5,117 +5,45 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System open System.IO open System.Composition -open System.Collections.Generic open System.Collections.Immutable +open System.Collections.Concurrent open System.Threading.Tasks -open System.Runtime.Caching open System.Globalization open Microsoft.CodeAnalysis -open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Navigation open Microsoft.CodeAnalysis.ExternalAccess.FSharp.NavigateTo open Microsoft.VisualStudio.Text.PatternMatching open FSharp.Compiler.EditorServices open FSharp.Compiler.Syntax +open Microsoft.VisualStudio.LanguageServices -type internal NavigableItem(document: Document, sourceSpan: TextSpan, glyph: Glyph, logicalName: string, kind: string, additionalInfo: string) = - inherit FSharpNavigableItem(glyph, ImmutableArray.Create (TaggedText(TextTags.Text, logicalName)), document, sourceSpan) - - // This use of compiler logical names is leaking out into the IDE. We should only report display names here. - member _.LogicalName = logicalName - member _.Kind = kind - member _.AdditionalInfo = additionalInfo - -type internal NavigateToSearchResult(item: NavigableItem, matchKind: FSharpNavigateToMatchKind) = - inherit FSharpNavigateToSearchResult(item.AdditionalInfo, item.Kind, matchKind, item.LogicalName, item) - -module private Index = - [] - type private IndexEntry(str: string, offset: int, item: NavigableItem) = - member _.String = str - member _.Offset = offset - member _.Length = str.Length - offset - member _.Item = item - member x.StartsWith (s: string) = - if s.Length > x.Length then false - else CultureInfo.CurrentCulture.CompareInfo.IndexOf(str, s, offset, s.Length, CompareOptions.IgnoreCase) = offset - - let private indexEntryComparer = - { new IComparer with - member _.Compare(a, b) = - let res = CultureInfo.CurrentCulture.CompareInfo.Compare(a.String, a.Offset, b.String, b.Offset, CompareOptions.IgnoreCase) - if res = 0 then a.Offset.CompareTo(b.Offset) else res } - - type IIndexedNavigableItems = - abstract Find: searchValue: string -> FSharpNavigateToSearchResult [] - abstract AllItems: NavigableItem [] - - let private navigateToSearchResultComparer = - { new IEqualityComparer with - member _.Equals(x: FSharpNavigateToSearchResult, y: FSharpNavigateToSearchResult) = - match x, y with - | null, _ | _, null -> false - | _ -> x.NavigableItem.Document.Id = y.NavigableItem.Document.Id && - x.NavigableItem.SourceSpan = y.NavigableItem.SourceSpan - - member _.GetHashCode(x: FSharpNavigateToSearchResult) = - if isNull x then 0 - else 23 * (17 * 23 + x.NavigableItem.Document.Id.GetHashCode()) + x.NavigableItem.SourceSpan.GetHashCode() } +[)>] +type internal FSharpNavigateToSearchService + [] + ( + patternMatcherFactory: IPatternMatcherFactory, + workspace: VisualStudioWorkspace + ) = - let build (items: NavigableItem []) = - let entries = ResizeArray() + let cache = ConcurrentDictionary() - for item in items do - let name = - // This conversion back from logical names to display names should never be needed, because - // the FCS API should only report display names in the first place. - if PrettyNaming.IsLogicalOpName item.LogicalName then - PrettyNaming.ConvertValLogicalNameToDisplayNameCore item.LogicalName - else - item.LogicalName - for i = 0 to name.Length - 1 do - entries.Add(IndexEntry(name, i, item)) + do workspace.WorkspaceChanged.Add <| fun e -> if e.NewSolution.Id <> e.OldSolution.Id then cache.Clear() - entries.Sort(indexEntryComparer) - { new IIndexedNavigableItems with - member _.Find (searchValue) = - let result = HashSet(navigateToSearchResultComparer) - if entries.Count > 0 then - let entryToFind = IndexEntry(searchValue, 0, Unchecked.defaultof<_>) - - let initial = - let p = entries.BinarySearch(entryToFind, indexEntryComparer) - if p < 0 then ~~~p else p - - let handle index = - let entry = entries.[index] - let matchKind = - if entry.Offset = 0 then - if entry.Length = searchValue.Length then FSharpNavigateToMatchKind.Exact - else FSharpNavigateToMatchKind.Prefix - else FSharpNavigateToMatchKind.Substring - let item = entry.Item - result.Add (NavigateToSearchResult(item, matchKind) :> FSharpNavigateToSearchResult) |> ignore - - // in case if there are multiple matching items binary search might return not the first one. - // in this case we'll walk backwards searching for the applicable answers - let mutable pos = initial - while pos >= 0 && pos < entries.Count && entries.[pos].StartsWith searchValue do - handle pos - pos <- pos - 1 - - // value of 'initial' position was already handled on the previous step so here we'll bump it - let mutable pos = initial + 1 - while pos < entries.Count && entries.[pos].StartsWith searchValue do - handle pos - pos <- pos + 1 - Seq.toArray result - member _.AllItems = items } + let getNavigableItems (document: Document) = async { + let! currentVersion = document.GetTextVersionAsync() |> Async.AwaitTask + match cache.TryGetValue document.Id with + | true, (version, items) when version = currentVersion -> + return items + | _ -> + let! parseResults = document.GetFSharpParseResultsAsync(nameof(FSharpNavigateToSearchService)) + let items = parseResults.ParseTree |> NavigateTo.GetNavigableItems + cache[document.Id] <- currentVersion, items + return items + } -[] -module private Utils = + let kindsProvided = ImmutableHashSet.Create(FSharpNavigateToItemKind.Module, FSharpNavigateToItemKind.Class, FSharpNavigateToItemKind.Field, FSharpNavigateToItemKind.Property, FSharpNavigateToItemKind.Method, FSharpNavigateToItemKind.Enum, FSharpNavigateToItemKind.EnumItem) :> IImmutableSet let navigateToItemKindToRoslynKind = function | NavigableItemKind.Module -> FSharpNavigateToItemKind.Module @@ -144,86 +72,20 @@ module private Utils = | NavigableItemKind.UnionCase -> Glyph.EnumPublic let containerToString (container: NavigableContainer) (document: Document) = - let project = document.Project let typeAsString = match container.Type with - | NavigableContainerType.File -> "project " - | NavigableContainerType.Namespace -> "namespace " - | NavigableContainerType.Module -> "module " - | NavigableContainerType.Exception -> "exception " - | NavigableContainerType.Type -> "type " + | NavigableContainerType.File -> "project" + | NavigableContainerType.Namespace -> "namespace" + | NavigableContainerType.Module -> "module" + | NavigableContainerType.Exception -> "exception" + | NavigableContainerType.Type -> "type" let name = match container.Type with | NavigableContainerType.File -> - (Path.GetFileNameWithoutExtension project.Name) + ", " + (Path.GetFileName container.LogicalName) + $"{Path.GetFileNameWithoutExtension document.Project.Name}, {Path.GetFileName container.LogicalName}" | _ -> container.LogicalName - let combined = typeAsString + name - - if isSignatureFile document.FilePath then - "signature for: " + combined - else - combined - - type PerDocumentSavedData = { Hash: int; Items: Index.IIndexedNavigableItems } - -[)>] -type internal FSharpNavigateToSearchService - [] - ( - patternMatcherFactory: IPatternMatcherFactory - ) = - - let kindsProvided = ImmutableHashSet.Create(FSharpNavigateToItemKind.Module, FSharpNavigateToItemKind.Class, FSharpNavigateToItemKind.Field, FSharpNavigateToItemKind.Property, FSharpNavigateToItemKind.Method, FSharpNavigateToItemKind.Enum, FSharpNavigateToItemKind.EnumItem) :> IImmutableSet - - // Save the backing navigation data in a memory cache held in a sliding window - let itemsByDocumentId = new MemoryCache("FSharp.Editor.FSharpNavigateToSearchService") - - let GetNavigableItems(document: Document, kinds: IImmutableSet) = - async { - let! cancellationToken = Async.CancellationToken - let! parseResults = document.GetFSharpParseResultsAsync(nameof(FSharpNavigateToSearchService)) - let! sourceText = document.GetTextAsync(cancellationToken) |> Async.AwaitTask - let navItems parsedInput = - NavigateTo.GetNavigableItems parsedInput - |> Array.filter (fun i -> kinds.Contains(navigateToItemKindToRoslynKind i.Kind)) - - let items = parseResults.ParseTree |> navItems - let navigableItems = - [| - for item in items do - match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, item.Range) with - | None -> () - | Some sourceSpan -> - let glyph = navigateToItemKindToGlyph item.Kind - let kind = navigateToItemKindToRoslynKind item.Kind - let additionalInfo = containerToString item.Container document - let _name = - if isSignatureFile document.FilePath then - item.LogicalName + " (signature)" - else - item.LogicalName - yield NavigableItem(document, sourceSpan, glyph, item.LogicalName, kind, additionalInfo) - |] - return navigableItems - } - - let getCachedIndexedNavigableItems(document: Document, kinds: IImmutableSet) = - async { - let! cancellationToken = Async.CancellationToken - let! textVersion = document.GetTextVersionAsync(cancellationToken) |> Async.AwaitTask - let textVersionHash = hash textVersion - let key = document.Id.ToString() - match itemsByDocumentId.Get(key) with - | :? PerDocumentSavedData as data when data.Hash = textVersionHash -> return data.Items - | _ -> - let! items = GetNavigableItems(document, kinds) - let indexedItems = Index.build items - let data = { Hash= textVersionHash; Items = indexedItems } - let cacheItem = CacheItem(key, data) - let policy = CacheItemPolicy(SlidingExpiration=DefaultTuning.PerDocumentSavedDataSlidingWindow) - itemsByDocumentId.Set(cacheItem, policy) - return indexedItems } + if document.IsFSharpSignatureFile then $"signature for: {typeAsString} {name}" else $"{typeAsString} {name}" let patternMatchKindToNavigateToMatchKind = function | PatternMatchKind.Exact -> FSharpNavigateToMatchKind.Exact @@ -235,52 +97,70 @@ type internal FSharpNavigateToSearchService | PatternMatchKind.CamelCaseSubstring -> FSharpNavigateToMatchKind.CamelCaseSubstring | PatternMatchKind.CamelCaseNonContiguousSubstring -> FSharpNavigateToMatchKind.CamelCaseNonContiguousSubstring | PatternMatchKind.Fuzzy -> FSharpNavigateToMatchKind.Fuzzy - | _ -> FSharpNavigateToMatchKind.Fuzzy + | _ -> FSharpNavigateToMatchKind.None + + let createMatcherFor searchPattern = + let patternMatcher = + lazy patternMatcherFactory.CreatePatternMatcher(searchPattern, PatternMatcherCreationOptions( + cultureInfo = CultureInfo.CurrentUICulture, + flags = (PatternMatcherCreationFlags.AllowFuzzyMatching ||| PatternMatcherCreationFlags.AllowSimpleSubstringMatching) + )) + // PatternMatcher will not match operators and some backtick escaped identifiers. + // To handle them, do a simple substring match first. + fun (name: string) -> + match name.IndexOf(searchPattern, StringComparison.CurrentCultureIgnoreCase) with + | i when i > 0 -> + PatternMatch(PatternMatchKind.Substring, false, true) |> Some + | 0 when name.Length = searchPattern.Length -> + PatternMatch(PatternMatchKind.Exact, false, true) |> Some + | 0 -> + PatternMatch(PatternMatchKind.Prefix, false, true) |> Some + | _ -> + patternMatcher.Value.TryMatch name |> Option.ofNullable + + let processDocument (document: Document) (tryMatch: string -> PatternMatch option) (kinds: IImmutableSet) = + async { + let! sourceText = document.GetTextAsync Async.DefaultCancellationToken |> Async.AwaitTask + + let processItem item = asyncMaybe { + do! Option.guard (kinds.Contains(navigateToItemKindToRoslynKind item.Kind)) + let name = + // This conversion back from logical names to display names should never be needed, because + // the FCS API should only report display names in the first place. + if PrettyNaming.IsLogicalOpName item.LogicalName then + PrettyNaming.ConvertValLogicalNameToDisplayNameCore item.LogicalName + else + item.LogicalName + let! m = tryMatch name + let! sourceSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, item.Range) + let glyph = navigateToItemKindToGlyph item.Kind + let kind = navigateToItemKindToRoslynKind item.Kind + let additionalInfo = containerToString item.Container document + return FSharpNavigateToSearchResult(additionalInfo, kind, patternMatchKindToNavigateToMatchKind m.Kind, name, + FSharpNavigableItem(glyph, ImmutableArray.Create (TaggedText(TextTags.Text, name)), document, sourceSpan)) + } + + let! items = getNavigableItems document + let! processed = items |> Seq.map (fun item -> processItem item) |> Async.Parallel + return processed |> Array.choose id + } interface IFSharpNavigateToSearchService with member _.SearchProjectAsync(project, _priorityDocuments, searchPattern, kinds, cancellationToken) : Task> = - asyncMaybe { - let! items = - project.Documents - |> Seq.map (fun document -> getCachedIndexedNavigableItems(document, kinds)) + async { + if not project.IsFSharp then raise (ArgumentException("Project is not a F# project", nameof project)) + let tryMatch = createMatcherFor searchPattern + let! results = + seq { for document in project.Documents -> processDocument document tryMatch kinds } |> Async.Parallel - |> liftAsync - - let items = - if searchPattern.Length = 1 then - items - |> Array.map (fun items -> items.Find(searchPattern)) - |> Array.concat - |> Array.filter (fun x -> x.Name.Length = 1 && String.Equals(x.Name, searchPattern, StringComparison.InvariantCultureIgnoreCase)) - else - [| yield! items |> Array.map (fun items -> items.Find(searchPattern)) |> Array.concat - let patternMatcherOptions = PatternMatcherCreationOptions( - cultureInfo = CultureInfo.CurrentUICulture, - flags = PatternMatcherCreationFlags.AllowFuzzyMatching - ) - let patternMatcher = patternMatcherFactory.CreatePatternMatcher(searchPattern, patternMatcherOptions) - yield! items - |> Array.collect (fun item -> item.AllItems) - |> Array.Parallel.choose (fun x -> - patternMatcher.TryMatch(x.LogicalName) - |> Option.ofNullable - |> Option.map (fun pm -> - NavigateToSearchResult(x, patternMatchKindToNavigateToMatchKind pm.Kind) :> FSharpNavigateToSearchResult)) |] - - return items |> Array.distinctBy (fun x -> x.NavigableItem.Document.Id, x.NavigableItem.SourceSpan) - } - |> Async.map (Option.defaultValue [||]) - |> Async.map Seq.toImmutableArray - |> RoslynHelpers.StartAsyncAsTask(cancellationToken) - - member _.SearchDocumentAsync(document, searchPattern, kinds, cancellationToken) : Task> = - asyncMaybe { - let! items = getCachedIndexedNavigableItems(document, kinds) |> liftAsync - return items.Find(searchPattern) - } - |> Async.map (Option.defaultValue [||]) - |> Async.map Seq.toImmutableArray - |> RoslynHelpers.StartAsyncAsTask(cancellationToken) + return results |> Array.concat |> Array.toImmutableArray + } |> RoslynHelpers.StartAsyncAsTask cancellationToken + + member _.SearchDocumentAsync(document: Document, searchPattern, kinds, cancellationToken) : Task> = + async { + let! result = processDocument document (createMatcherFor searchPattern) kinds + return result |> Array.toImmutableArray + } |> RoslynHelpers.StartAsyncAsTask cancellationToken member _.KindsProvided = kindsProvided From c4aa105e681e8d957f6ac0c6f9006eb475dace22 Mon Sep 17 00:00:00 2001 From: majocha Date: Tue, 28 Feb 2023 18:20:30 +0100 Subject: [PATCH 2/5] format --- .../Navigation/NavigateToSearchService.fs | 166 +++++++++++------- 1 file changed, 107 insertions(+), 59 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs index d640882b050..4ad962cb9cc 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -13,15 +13,14 @@ open System.Globalization open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Navigation open Microsoft.CodeAnalysis.ExternalAccess.FSharp.NavigateTo +open Microsoft.VisualStudio.LanguageServices open Microsoft.VisualStudio.Text.PatternMatching open FSharp.Compiler.EditorServices open FSharp.Compiler.Syntax -open Microsoft.VisualStudio.LanguageServices [)>] -type internal FSharpNavigateToSearchService - [] +type internal FSharpNavigateToSearchService [] ( patternMatcherFactory: IPatternMatcherFactory, workspace: VisualStudioWorkspace @@ -29,23 +28,38 @@ type internal FSharpNavigateToSearchService let cache = ConcurrentDictionary() - do workspace.WorkspaceChanged.Add <| fun e -> if e.NewSolution.Id <> e.OldSolution.Id then cache.Clear() + do + workspace.WorkspaceChanged.Add + <| fun e -> + if e.NewSolution.Id <> e.OldSolution.Id then + cache.Clear() - let getNavigableItems (document: Document) = async { - let! currentVersion = document.GetTextVersionAsync() |> Async.AwaitTask - match cache.TryGetValue document.Id with - | true, (version, items) when version = currentVersion -> - return items - | _ -> - let! parseResults = document.GetFSharpParseResultsAsync(nameof(FSharpNavigateToSearchService)) - let items = parseResults.ParseTree |> NavigateTo.GetNavigableItems - cache[document.Id] <- currentVersion, items - return items - } + let getNavigableItems (document: Document) = + async { + let! currentVersion = document.GetTextVersionAsync() |> Async.AwaitTask - let kindsProvided = ImmutableHashSet.Create(FSharpNavigateToItemKind.Module, FSharpNavigateToItemKind.Class, FSharpNavigateToItemKind.Field, FSharpNavigateToItemKind.Property, FSharpNavigateToItemKind.Method, FSharpNavigateToItemKind.Enum, FSharpNavigateToItemKind.EnumItem) :> IImmutableSet + match cache.TryGetValue document.Id with + | true, (version, items) when version = currentVersion -> return items + | _ -> + let! parseResults = document.GetFSharpParseResultsAsync(nameof (FSharpNavigateToSearchService)) + let items = parseResults.ParseTree |> NavigateTo.GetNavigableItems + cache[document.Id] <- currentVersion, items + return items + } - let navigateToItemKindToRoslynKind = function + let kindsProvided = + ImmutableHashSet.Create( + FSharpNavigateToItemKind.Module, + FSharpNavigateToItemKind.Class, + FSharpNavigateToItemKind.Field, + FSharpNavigateToItemKind.Property, + FSharpNavigateToItemKind.Method, + FSharpNavigateToItemKind.Enum, + FSharpNavigateToItemKind.EnumItem + ) + + let navigateToItemKindToRoslynKind = + function | NavigableItemKind.Module -> FSharpNavigateToItemKind.Module | NavigableItemKind.ModuleAbbreviation -> FSharpNavigateToItemKind.Module | NavigableItemKind.Exception -> FSharpNavigateToItemKind.Class @@ -58,7 +72,8 @@ type internal FSharpNavigateToSearchService | NavigableItemKind.EnumCase -> FSharpNavigateToItemKind.EnumItem | NavigableItemKind.UnionCase -> FSharpNavigateToItemKind.EnumItem - let navigateToItemKindToGlyph = function + let navigateToItemKindToGlyph = + function | NavigableItemKind.Module -> Glyph.ModulePublic | NavigableItemKind.ModuleAbbreviation -> Glyph.ModulePublic | NavigableItemKind.Exception -> Glyph.ClassPublic @@ -79,15 +94,20 @@ type internal FSharpNavigateToSearchService | NavigableContainerType.Module -> "module" | NavigableContainerType.Exception -> "exception" | NavigableContainerType.Type -> "type" + let name = match container.Type with | NavigableContainerType.File -> $"{Path.GetFileNameWithoutExtension document.Project.Name}, {Path.GetFileName container.LogicalName}" | _ -> container.LogicalName - if document.IsFSharpSignatureFile then $"signature for: {typeAsString} {name}" else $"{typeAsString} {name}" + if document.IsFSharpSignatureFile then + $"signature for: {typeAsString} {name}" + else + $"{typeAsString} {name}" - let patternMatchKindToNavigateToMatchKind = function + let patternMatchKindToNavigateToMatchKind = + function | PatternMatchKind.Exact -> FSharpNavigateToMatchKind.Exact | PatternMatchKind.Prefix -> FSharpNavigateToMatchKind.Prefix | PatternMatchKind.Substring -> FSharpNavigateToMatchKind.Substring @@ -101,67 +121,95 @@ type internal FSharpNavigateToSearchService let createMatcherFor searchPattern = let patternMatcher = - lazy patternMatcherFactory.CreatePatternMatcher(searchPattern, PatternMatcherCreationOptions( - cultureInfo = CultureInfo.CurrentUICulture, - flags = (PatternMatcherCreationFlags.AllowFuzzyMatching ||| PatternMatcherCreationFlags.AllowSimpleSubstringMatching) - )) + lazy + patternMatcherFactory.CreatePatternMatcher( + searchPattern, + PatternMatcherCreationOptions( + cultureInfo = CultureInfo.CurrentUICulture, + flags = + (PatternMatcherCreationFlags.AllowFuzzyMatching + ||| PatternMatcherCreationFlags.AllowSimpleSubstringMatching) + ) + ) // PatternMatcher will not match operators and some backtick escaped identifiers. // To handle them, do a simple substring match first. fun (name: string) -> match name.IndexOf(searchPattern, StringComparison.CurrentCultureIgnoreCase) with - | i when i > 0 -> - PatternMatch(PatternMatchKind.Substring, false, true) |> Some - | 0 when name.Length = searchPattern.Length -> - PatternMatch(PatternMatchKind.Exact, false, true) |> Some - | 0 -> - PatternMatch(PatternMatchKind.Prefix, false, true) |> Some - | _ -> - patternMatcher.Value.TryMatch name |> Option.ofNullable + | i when i > 0 -> PatternMatch(PatternMatchKind.Substring, false, true) |> Some + | 0 when name.Length = searchPattern.Length -> PatternMatch(PatternMatchKind.Exact, false, true) |> Some + | 0 -> PatternMatch(PatternMatchKind.Prefix, false, true) |> Some + | _ -> patternMatcher.Value.TryMatch name |> Option.ofNullable let processDocument (document: Document) (tryMatch: string -> PatternMatch option) (kinds: IImmutableSet) = async { let! sourceText = document.GetTextAsync Async.DefaultCancellationToken |> Async.AwaitTask - let processItem item = asyncMaybe { - do! Option.guard (kinds.Contains(navigateToItemKindToRoslynKind item.Kind)) - let name = - // This conversion back from logical names to display names should never be needed, because - // the FCS API should only report display names in the first place. - if PrettyNaming.IsLogicalOpName item.LogicalName then - PrettyNaming.ConvertValLogicalNameToDisplayNameCore item.LogicalName - else - item.LogicalName - let! m = tryMatch name - let! sourceSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, item.Range) - let glyph = navigateToItemKindToGlyph item.Kind - let kind = navigateToItemKindToRoslynKind item.Kind - let additionalInfo = containerToString item.Container document - return FSharpNavigateToSearchResult(additionalInfo, kind, patternMatchKindToNavigateToMatchKind m.Kind, name, - FSharpNavigableItem(glyph, ImmutableArray.Create (TaggedText(TextTags.Text, name)), document, sourceSpan)) - } - - let! items = getNavigableItems document + let processItem item = + asyncMaybe { + do! Option.guard (kinds.Contains(navigateToItemKindToRoslynKind item.Kind)) + + let name = + // This conversion back from logical names to display names should never be needed, because + // the FCS API should only report display names in the first place. + if PrettyNaming.IsLogicalOpName item.LogicalName then + PrettyNaming.ConvertValLogicalNameToDisplayNameCore item.LogicalName + else + item.LogicalName + + let! m = tryMatch name + let! sourceSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, item.Range) + let glyph = navigateToItemKindToGlyph item.Kind + let kind = navigateToItemKindToRoslynKind item.Kind + let additionalInfo = containerToString item.Container document + + return + FSharpNavigateToSearchResult( + additionalInfo, + kind, + patternMatchKindToNavigateToMatchKind m.Kind, + name, + FSharpNavigableItem(glyph, ImmutableArray.Create(TaggedText(TextTags.Text, name)), document, sourceSpan) + ) + } + + let! items = getNavigableItems document let! processed = items |> Seq.map (fun item -> processItem item) |> Async.Parallel return processed |> Array.choose id } interface IFSharpNavigateToSearchService with - member _.SearchProjectAsync(project, _priorityDocuments, searchPattern, kinds, cancellationToken) : Task> = + member _.SearchProjectAsync + ( + project, + _priorityDocuments, + searchPattern, + kinds, + cancellationToken + ) : Task> = async { - if not project.IsFSharp then raise (ArgumentException("Project is not a F# project", nameof project)) let tryMatch = createMatcherFor searchPattern + let! results = seq { for document in project.Documents -> processDocument document tryMatch kinds } |> Async.Parallel - return results |> Array.concat |> Array.toImmutableArray - } |> RoslynHelpers.StartAsyncAsTask cancellationToken - member _.SearchDocumentAsync(document: Document, searchPattern, kinds, cancellationToken) : Task> = - async { + return results |> Array.concat |> Array.toImmutableArray + } + |> RoslynHelpers.StartAsyncAsTask cancellationToken + + member _.SearchDocumentAsync + ( + document: Document, + searchPattern, + kinds, + cancellationToken + ) : Task> = + async { let! result = processDocument document (createMatcherFor searchPattern) kinds return result |> Array.toImmutableArray - } |> RoslynHelpers.StartAsyncAsTask cancellationToken + } + |> RoslynHelpers.StartAsyncAsTask cancellationToken member _.KindsProvided = kindsProvided - member _.CanFilter = true \ No newline at end of file + member _.CanFilter = true From 6c8263d4259f58bd3950a4618f40bd7ab584e2fa Mon Sep 17 00:00:00 2001 From: majocha Date: Tue, 28 Feb 2023 18:57:21 +0100 Subject: [PATCH 3/5] enable skipped tests --- .../tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs index a1c16499dbc..44c282edd21 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/NavigateToSearchServiceTests.fs @@ -58,7 +58,7 @@ module HeyHo = let ``capitalized camel case`` () = assertResultsContain "CLN" "CamelCaseLongName" - [] + [] let ``lower camel case`` () = assertResultsContain "cln" "CamelCaseLongName" @@ -69,5 +69,5 @@ module HeyHo = let ``backticked identifier`` () = assertResultsContain "a few words" "a few words" - [] + [] let ``operator`` () = assertResultsContain "+>" "+>" From 58763ccc3a003f7b716d19ddc32b65db205f8e44 Mon Sep 17 00:00:00 2001 From: majocha Date: Wed, 1 Mar 2023 08:20:50 +0100 Subject: [PATCH 4/5] workspace import optional for simpler testing --- .../Navigation/NavigateToSearchService.fs | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs index 4ad962cb9cc..bc5f2d6e348 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -19,20 +19,21 @@ open Microsoft.VisualStudio.Text.PatternMatching open FSharp.Compiler.EditorServices open FSharp.Compiler.Syntax -[)>] +[); Shared>] type internal FSharpNavigateToSearchService [] ( patternMatcherFactory: IPatternMatcherFactory, - workspace: VisualStudioWorkspace + [] workspace: VisualStudioWorkspace ) = let cache = ConcurrentDictionary() do - workspace.WorkspaceChanged.Add - <| fun e -> - if e.NewSolution.Id <> e.OldSolution.Id then - cache.Clear() + if workspace <> null then + workspace.WorkspaceChanged.Add + <| fun e -> + if e.NewSolution.Id <> e.OldSolution.Id then + cache.Clear() let getNavigableItems (document: Document) = async { @@ -140,7 +141,7 @@ type internal FSharpNavigateToSearchService [] | 0 -> PatternMatch(PatternMatchKind.Prefix, false, true) |> Some | _ -> patternMatcher.Value.TryMatch name |> Option.ofNullable - let processDocument (document: Document) (tryMatch: string -> PatternMatch option) (kinds: IImmutableSet) = + let processDocument (tryMatch: string -> PatternMatch option) (kinds: IImmutableSet) (document: Document) = async { let! sourceText = document.GetTextAsync Async.DefaultCancellationToken |> Async.AwaitTask @@ -190,7 +191,8 @@ type internal FSharpNavigateToSearchService [] let tryMatch = createMatcherFor searchPattern let! results = - seq { for document in project.Documents -> processDocument document tryMatch kinds } + project.Documents + |> Seq.map (processDocument tryMatch kinds) |> Async.Parallel return results |> Array.concat |> Array.toImmutableArray @@ -205,7 +207,7 @@ type internal FSharpNavigateToSearchService [] cancellationToken ) : Task> = async { - let! result = processDocument document (createMatcherFor searchPattern) kinds + let! result = processDocument (createMatcherFor searchPattern) kinds document return result |> Array.toImmutableArray } |> RoslynHelpers.StartAsyncAsTask cancellationToken From 2e8f19cae594932f9e35f890292f68c21101fba6 Mon Sep 17 00:00:00 2001 From: majocha Date: Wed, 1 Mar 2023 18:32:49 +0100 Subject: [PATCH 5/5] simplify --- .../src/FSharp.Editor/Navigation/NavigateToSearchService.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs index bc5f2d6e348..88476828cfb 100644 --- a/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs +++ b/vsintegration/src/FSharp.Editor/Navigation/NavigateToSearchService.fs @@ -174,7 +174,7 @@ type internal FSharpNavigateToSearchService [] } let! items = getNavigableItems document - let! processed = items |> Seq.map (fun item -> processItem item) |> Async.Parallel + let! processed = items |> Seq.map processItem |> Async.Parallel return processed |> Array.choose id }