From 88f8da4d6f20379824c29f5da51fe635339a6255 Mon Sep 17 00:00:00 2001 From: nosami Date: Mon, 11 Dec 2017 13:17:22 +0000 Subject: [PATCH 1/2] Update GetDeclarationListSymbols to add getAllEntities --- src/fsharp/vs/service.fs | 13 +++++++------ src/fsharp/vs/service.fsi | 2 +- tests/service/EditorTests.fs | 6 +++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/fsharp/vs/service.fs b/src/fsharp/vs/service.fs index f96d560b76..0a541d2c78 100755 --- a/src/fsharp/vs/service.fs +++ b/src/fsharp/vs/service.fs @@ -924,11 +924,11 @@ type TypeCheckInfo FSharpDeclarationListInfo.Error msg) /// Get the symbols for auto-complete items at a location - member __.GetDeclarationListSymbols (ctok, parseResultsOpt, line, lineStr, partialName, hasTextChangedSinceLastTypecheck) = + member __.GetDeclarationListSymbols (ctok, parseResultsOpt, line, lineStr, partialName, getAllSymbols, hasTextChangedSinceLastTypecheck) = let isInterfaceFile = SourceFileImpl.IsInterfaceFile mainInputFileName ErrorScope.Protect Range.range0 (fun () -> - match GetDeclItemsForNamesAtPosition(ctok, parseResultsOpt, Some partialName.QualifyingIdents, Some partialName.PartialIdent, partialName.LastDotPos, line, lineStr, partialName.EndColumn + 1, ResolveTypeNamesToCtors, ResolveOverloads.Yes, (fun () -> []), hasTextChangedSinceLastTypecheck) with + match GetDeclItemsForNamesAtPosition(ctok, parseResultsOpt, Some partialName.QualifyingIdents, Some partialName.PartialIdent, partialName.LastDotPos, line, lineStr, partialName.EndColumn + 1, ResolveTypeNamesToCtors, ResolveOverloads.Yes, getAllSymbols, hasTextChangedSinceLastTypecheck) with | None -> List.Empty | Some (items, denv, _, m) -> let items = if isInterfaceFile then items |> List.filter (fun x -> IsValidSignatureFileItem x.Item) else items @@ -940,7 +940,7 @@ type TypeCheckInfo // - show types with fewer generic parameters first // - show types before over other related items - they usually have very useful XmlDocs let items = - items |> List.sortBy (fun d -> + items |> List.sortBy (fun d -> let n = match d.Item with | Item.Types (_,(TType_app(tcref,_) :: _)) -> 1 + tcref.TyparsNoRange.Length @@ -956,7 +956,8 @@ type TypeCheckInfo let items = items |> RemoveDuplicateCompletionItems g // Group by display name - let items = items |> List.groupBy (fun d -> d.Item.DisplayName) + let items = + items |> List.groupBy (fun d -> d.Item.DisplayName) // Filter out operators (and list) let items = @@ -1899,10 +1900,10 @@ type FSharpCheckFileResults(filename: string, errors: FSharpErrorInfo[], scopeOp reactorOp userOpName "GetDeclarations" FSharpDeclarationListInfo.Empty (fun ctok scope -> scope.GetDeclarations(ctok, parseResultsOpt, line, lineStr, partialName, getAllEntities, hasTextChangedSinceLastTypecheck)) - member info.GetDeclarationListSymbols(parseResultsOpt, line, lineStr, partialName, ?hasTextChangedSinceLastTypecheck, ?userOpName: string) = + member info.GetDeclarationListSymbols(parseResultsOpt, line, lineStr, partialName, getAllEntities, ?hasTextChangedSinceLastTypecheck, ?userOpName: string) = let userOpName = defaultArg userOpName "Unknown" let hasTextChangedSinceLastTypecheck = defaultArg hasTextChangedSinceLastTypecheck (fun _ -> false) - reactorOp userOpName "GetDeclarationListSymbols" List.empty (fun ctok scope -> scope.GetDeclarationListSymbols(ctok, parseResultsOpt, line, lineStr, partialName, hasTextChangedSinceLastTypecheck)) + reactorOp userOpName "GetDeclarationListSymbols" List.empty (fun ctok scope -> scope.GetDeclarationListSymbols(ctok, parseResultsOpt, line, lineStr, partialName, getAllEntities, hasTextChangedSinceLastTypecheck)) /// Resolve the names at the given location to give a data tip member info.GetStructuredToolTipText(line, colAtEndOfNames, lineStr, names, tokenTag, ?userOpName: string) = diff --git a/src/fsharp/vs/service.fsi b/src/fsharp/vs/service.fsi index 1e7005bff0..68a734c364 100755 --- a/src/fsharp/vs/service.fsi +++ b/src/fsharp/vs/service.fsi @@ -174,7 +174,7 @@ type internal FSharpCheckFileResults = /// and assume that we're going to repeat the operation later on. /// /// An optional string used for tracing compiler operations associated with this request. - member GetDeclarationListSymbols : ParsedFileResultsOpt:FSharpParseFileResults option * line: int * lineText:string * partialName: PartialLongName * ?hasTextChangedSinceLastTypecheck: (obj * range -> bool) * ?userOpName: string -> Async + member GetDeclarationListSymbols : ParsedFileResultsOpt:FSharpParseFileResults option * line: int * lineText:string * partialName: PartialLongName * getAllSymbols: (unit -> AssemblySymbol list) * ?hasTextChangedSinceLastTypecheck: (obj * range -> bool) * ?userOpName: string -> Async /// Compute a formatted tooltip for the given location diff --git a/tests/service/EditorTests.fs b/tests/service/EditorTests.fs index 2e050abe32..8448064fe7 100755 --- a/tests/service/EditorTests.fs +++ b/tests/service/EditorTests.fs @@ -357,7 +357,7 @@ type Test() = let file = "/home/user/Test.fsx" let parseResult, typeCheckResults = parseAndCheckScript(file, input) - let decls = typeCheckResults.GetDeclarationListSymbols(Some parseResult, 4, inputLines.[3], PartialLongName.Empty(20), fun _ -> false)|> Async.RunSynchronously + let decls = typeCheckResults.GetDeclarationListSymbols(Some parseResult, 4, inputLines.[3], PartialLongName.Empty(20), (fun () -> []), fun _ -> false)|> Async.RunSynchronously //decls |> List.map (fun d -> d.Head.Symbol.DisplayName) |> printfn "---> decls = %A" decls |> Seq.exists (fun d -> d.Head.Symbol.DisplayName = "abc") |> shouldEqual true @@ -374,7 +374,7 @@ type Test() = let file = "/home/user/Test.fsx" let parseResult, typeCheckResults = parseAndCheckScript(file, input) - let decls = typeCheckResults.GetDeclarationListSymbols(Some parseResult, 4, inputLines.[3], PartialLongName.Empty(21), fun _ -> false)|> Async.RunSynchronously + let decls = typeCheckResults.GetDeclarationListSymbols(Some parseResult, 4, inputLines.[3], PartialLongName.Empty(21), (fun () -> []), fun _ -> false)|> Async.RunSynchronously //decls |> List.map (fun d -> d.Head.Symbol.DisplayName) |> printfn "---> decls = %A" decls |> Seq.exists (fun d -> d.Head.Symbol.DisplayName = "abc") |> shouldEqual true @@ -391,7 +391,7 @@ type Test() = let file = "/home/user/Test.fsx" let parseResult, typeCheckResults = parseAndCheckScript(file, input) - let decls = typeCheckResults.GetDeclarationListSymbols(Some parseResult, 4, inputLines.[3], PartialLongName.Empty(14), fun _ -> false)|> Async.RunSynchronously + let decls = typeCheckResults.GetDeclarationListSymbols(Some parseResult, 4, inputLines.[3], PartialLongName.Empty(14), (fun () -> []), fun _ -> false)|> Async.RunSynchronously //decls |> List.map (fun d -> d.Head.Symbol.DisplayName) |> printfn "---> decls = %A" decls |> Seq.exists (fun d -> d.Head.Symbol.DisplayName = "abc") |> shouldEqual true From 899727603367f303bc35c3b3dffbaabace900408 Mon Sep 17 00:00:00 2001 From: nosami Date: Mon, 11 Dec 2017 13:26:05 +0000 Subject: [PATCH 2/2] Don't group types with the same display name as function overloads. Now that we can include all entities for top level completions, the grouping function can incorrectly group types with the same display name as function overloads. Use the compiled name instead as this includes the namespace. --- src/fsharp/vs/service.fs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/fsharp/vs/service.fs b/src/fsharp/vs/service.fs index 0a541d2c78..d70b18fc2e 100755 --- a/src/fsharp/vs/service.fs +++ b/src/fsharp/vs/service.fs @@ -955,23 +955,26 @@ type TypeCheckInfo // Remove all duplicates. We've put the types first, so this removes the DelegateCtor and DefaultStructCtor's. let items = items |> RemoveDuplicateCompletionItems g - // Group by display name + // Group by DisplayName, then namespace + // (We don't want types with the same display name to be grouped as overloads) let items = - items |> List.groupBy (fun d -> d.Item.DisplayName) + items |> List.groupBy (fun d -> d.Item.DisplayName, d.Unresolved) // Filter out operators (and list) let items = // Check whether this item looks like an operator. - let isOpItem(nm, item: CompletionItem list) = + let isOpItem(displayName, item: CompletionItem list) = match item |> List.map (fun x -> x.Item) with | [Item.Value _] - | [Item.MethodGroup(_,[_],_)] -> IsOperatorName nm - | [Item.UnionCase _] -> IsOperatorName nm + | [Item.MethodGroup(_,[_],_)] -> IsOperatorName displayName + | [Item.UnionCase _] -> IsOperatorName displayName | _ -> false - let isFSharpList nm = (nm = "[]") // list shows up as a Type and a UnionCase, only such entity with a symbolic name, but want to filter out of intellisense + let isFSharpList displayName = displayName = "[]" // list shows up as a Type and a UnionCase, only such entity with a symbolic name, but want to filter out of intellisense - items |> List.filter (fun (nm,items) -> not (isOpItem(nm,items)) && not(isFSharpList nm)) + items |> List.filter (fun ((displayName,_),items) -> + not (isOpItem(displayName,items)) + && not(isFSharpList displayName)) let items =