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
ionide · May 24, 2024 · 610d0ab · 610d0ab
1 parent d3c13e7
commit 610d0ab
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 15 deletions.
diff --git 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
@@ -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
@@ -722,24 +722,28 @@ 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 =
         { defaultConfigDto with
             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,16 +952,16 @@ 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
       testScript "with root module with new line" "ModuleWithNewLine.fsx"
       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
@@ -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
@@ -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*)