From 610d0ab7cdb65d4cf8a046b62696c6283d645d39 Mon Sep 17 00:00:00 2001 From: MrLuje Date: Fri, 24 May 2024 20:32:17 +0200 Subject: [PATCH] fix: wrong "open" completion position with namespace (#1300) * fix: wrong "open" completion position with namespace Currently, "open" are sometimes inserted before namespace which is invalid + fix "Completion.AutoOpen" tests * autoOpenTests cache server --- src/FsAutoComplete.Core/Commands.fs | 2 +- .../LspServers/AdaptiveFSharpLspServer.fs | 8 ++++++- .../CompletionTests.fs | 23 +++++++++++-------- .../CompletionAutoOpenTests/Namespace.fsx | 4 ++-- .../NamespaceWithNewLine.fsx | 4 ++-- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/FsAutoComplete.Core/Commands.fs b/src/FsAutoComplete.Core/Commands.fs index 2c7ecf15a..4c50e4ae4 100644 --- a/src/FsAutoComplete.Core/Commands.fs +++ b/src/FsAutoComplete.Core/Commands.fs @@ -699,7 +699,7 @@ module Commands = // this only happens when there are no other `open` // from insert position go up until first open OR namespace - ic.Pos.LinesToBeginning() + ic.Pos.IncLine().LinesToBeginning() |> Seq.tryFind (fun l -> let lineStr = getLine l // namespace MUST be top level -> no indentation diff --git a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs index 4512998a4..75c1d1431 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs @@ -761,10 +761,16 @@ type AdaptiveFSharpLspServer { (fcsPos |> fcsPosToLsp) with Character = 0 } + let displayText = + match config.ExternalAutocomplete, ci.Label.Split(" (open ") with + | true, [| label; _ |] -> label + | true, [| label |] -> label + | _, _ -> ci.Label + Some [| { TextEdit.NewText = text TextEdit.Range = { Start = insertPos; End = insertPos } } |], - $"{ci.Label} (open {ns})" + $"{displayText} (open {ns})" let d = Documentation.Markup(markdown comment) diff --git a/test/FsAutoComplete.Tests.Lsp/CompletionTests.fs b/test/FsAutoComplete.Tests.Lsp/CompletionTests.fs index a9a325d78..57487a7f7 100644 --- a/test/FsAutoComplete.Tests.Lsp/CompletionTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CompletionTests.fs @@ -722,12 +722,11 @@ let autocompleteTest state = [ testList "Autocomplete within project files" (makeAutocompleteTestList server) testList "Autocomplete within script files" (makeAutocompleteTestList scriptServer) ] -///TODO: these are broken in FCS 43.7.200 - something in the tokenization isn't searching the System namespace let autoOpenTests state = let dirPath = Path.Combine(__SOURCE_DIRECTORY__, "TestCases", "CompletionAutoOpenTests") - let serverFor (scriptPath: string) = + let autoOpenServer = async { // Auto Open requires unopened things in completions -> External let config = @@ -735,11 +734,16 @@ let autoOpenTests state = ExternalAutocomplete = Some true ResolveNamespaces = Some true } - let dirPath = Path.GetDirectoryName scriptPath - let scriptName = Path.GetFileName scriptPath let! (server, events) = serverInitialize dirPath config state do! waitForWorkspaceFinishedParsing events + return (server, events) + } + |> Async.Cache + let serverFor (scriptPath: string) = + async { + let! (server, events) = autoOpenServer + let scriptName = Path.GetFileName scriptPath let tdop: DidOpenTextDocumentParams = { TextDocument = loadDocument scriptPath } do! server.TextDocumentDidOpen tdop @@ -835,7 +839,8 @@ let autoOpenTests state = | Ok None -> failtest "Request none" | Ok(Some res) -> Expect.isFalse res.IsIncomplete "Result is incomplete" - let ci = res.Items |> Array.tryFind (fun c -> c.Label = word) + // with ExternalAutoComplete, completions are like "Regex (open System.Text.RegularExpressions)" + let ci = res.Items |> Array.tryFind (fun c -> c.Label.StartsWith word) if ci = None then failwithf @@ -934,7 +939,7 @@ let autoOpenTests state = yield! tests ] - let ptestScript name scriptName = + let _ptestScript name scriptName = testList name [ let scriptPath = Path.Combine(dirPath, scriptName) @@ -947,7 +952,7 @@ let autoOpenTests state = yield! tests ] - ptestList + testList "Completion.AutoOpen" [ // NOTE: Positions are ZERO-based!: { Line = 3; Character = 9 } -> Line 4, Column 10 in editor display @@ -955,8 +960,8 @@ let autoOpenTests state = testScript "with root module" "Module.fsx" testScript "with root module with open" "ModuleWithOpen.fsx" testScript "with root module with open and new line" "ModuleWithOpenAndNewLine.fsx" - ptestScript "with namespace with new line" "NamespaceWithNewLine.fsx" - ptestScript "with namespace" "Namespace.fsx" + testScript "with namespace with new line" "NamespaceWithNewLine.fsx" + testScript "with namespace" "Namespace.fsx" testScript "with namespace with open" "NamespaceWithOpen.fsx" testScript "with namespace with open and new line" "NamespaceWithOpenAndNewLine.fsx" testScript "with implicit top level module with new line" "ImplicitTopLevelModuleWithNewLine.fsx" diff --git a/test/FsAutoComplete.Tests.Lsp/TestCases/CompletionAutoOpenTests/Namespace.fsx b/test/FsAutoComplete.Tests.Lsp/TestCases/CompletionAutoOpenTests/Namespace.fsx index b50926d2f..8989ef9f7 100644 --- a/test/FsAutoComplete.Tests.Lsp/TestCases/CompletionAutoOpenTests/Namespace.fsx +++ b/test/FsAutoComplete.Tests.Lsp/TestCases/CompletionAutoOpenTests/Namespace.fsx @@ -12,8 +12,8 @@ module Nested1 = namespace OpenNamespace.OtherNamespace type T = { - Value: Regex(*-1,|-2*) + Value: Regex(*-1,|-4*) } with member _.Foo () = - Regex(*-5,|-4*) + Regex(*-5,|-6*) diff --git a/test/FsAutoComplete.Tests.Lsp/TestCases/CompletionAutoOpenTests/NamespaceWithNewLine.fsx b/test/FsAutoComplete.Tests.Lsp/TestCases/CompletionAutoOpenTests/NamespaceWithNewLine.fsx index 64beb27a4..231412d07 100644 --- a/test/FsAutoComplete.Tests.Lsp/TestCases/CompletionAutoOpenTests/NamespaceWithNewLine.fsx +++ b/test/FsAutoComplete.Tests.Lsp/TestCases/CompletionAutoOpenTests/NamespaceWithNewLine.fsx @@ -15,8 +15,8 @@ module Nested1 = namespace OpenNamespace.OtherNamespace type T = { - Value: Regex(*-2,|-2*) + Value: Regex(*-2,|-4*) } with member _.Foo () = - Regex(*-6,|-4*) + Regex(*-6,|-6*)