Trim instances on save to avoid race condition (#213)

* Trim instances on save to avoid race condition

* Fix test

* Only add new features if ns can handle unknown

* comment chunk to test against CI (passes locally)

* makes sure entry points exist

nimlangserver.nim

@@ -88,6 +88,7 @@ type
     storageDir*: string
     cmdLineClientProcessId: Option[int]
     nimDumpCache: Table[string, NimbleDumpInfo] #path to NimbleDumpInfo
+    entryPoints: seq[string]
 
   Certainty = enum
     None,
@@ -105,10 +106,12 @@ type
 
 proc getNimbleEntryPoints(dumpInfo: NimbleDumpInfo, nimbleProjectPath: string): seq[string] =
   if dumpInfo.entryPoints.len > 0:
-    return dumpInfo.entryPoints.mapIt(nimbleProjectPath / it)
-  #Nimble doesnt include the entry points, returning the nimble project file as the entry point
-  let sourceDir = nimbleProjectPath / dumpInfo.srcDir
-  @[sourceDir / (dumpInfo.name & ".nim")]
+    result = dumpInfo.entryPoints.mapIt(nimbleProjectPath / it)
+  else:
+    #Nimble doesnt include the entry points, returning the nimble project file as the entry point
+    let sourceDir = nimbleProjectPath / dumpInfo.srcDir
+    result = @[sourceDir / (dumpInfo.name & ".nim")]
+  result = result.filterIt(it.fileExists)
 
 proc getVersionFromNimble(): string = 
   #We should static run nimble dump instead
@@ -240,13 +243,30 @@ proc getRootPath(ip: InitializeParams): string =
 
 proc createOrRestartNimsuggest(ls: LanguageServer, projectFile: string, uri = ""): void {.gcsafe.}
 
-proc isKnownByAnyNimsuggest(ls: LanguageServer, filePath: string): Future[Option[string]] {.async.} =
-  for projectFile in ls.projectFiles.keys:
-    let ns = await ls.projectFiles[projectFile]
-    let isKnown = await ns.isKnown(filePath)
-    if isKnown:
-      return some projectFile
-  none(string)
+proc showMessage(ls: LanguageServer, message: string, typ: MessageType) =  
+  proc notify() =
+    ls.connection.notify(
+      "window/showMessage",
+      %* {
+         "type": typ.int,
+         "message": message 
+      })
+  let verbosity = 
+    ls
+    .getWorkspaceConfiguration
+    .waitFor
+    .notificationVerbosity.get(NlsNotificationVerbosity.nvInfo)
+  debug "ShowMessage ", message = message
+  case verbosity:
+  of nvInfo: 
+    notify()
+  of nvWarning:
+    if typ.int <= MessageType.Warning.int :
+       notify()
+  of nvError:
+    if typ == MessageType.Error: 
+      notify()
+  else: discard
 
 proc getProjectFile(fileUri: string, ls: LanguageServer): Future[string] {.async.} =
   let
@@ -269,47 +289,26 @@ proc getProjectFile(fileUri: string, ls: LanguageServer): Future[string] {.async
       if nimbleFiles.len > 0:
         let nimbleFile = nimbleFiles[0]
         let nimbleDumpInfo = ls.getNimbleDumpInfo(nimbleFile)
-        let entryPoints = nimbleDumpInfo.getNimbleEntryPoints(ls.initializeParams.getRootPath)
-        for entryPoint in entryPoints:
+        ls.entryPoints = nimbleDumpInfo.getNimbleEntryPoints(ls.initializeParams.getRootPath)
+        # ls.showMessage(fmt "Found entry point {ls.entryPoints}?", MessageType.Info)
+        for entryPoint in ls.entryPoints:
           debug "Starting nimsuggest for entry point ", entry = entryPoint
-          if not ls.projectFiles.hasKey(entryPoint):
+          if entryPoint notin ls.projectFiles:
             ls.createOrRestartNimsuggest(entryPoint)
-        # let ns = await ls.projectFiles[entryPoint]
+
+  result = ls.getProjectFileAutoGuess(fileUri)
+  if result in ls.projectFiles:
+    let ns = await ls.projectFiles[result]
+    let isKnown = await ns.isKnown(fileUri)
+    if ns.canHandleUnknown and not isKnown:
+      debug "File is not known by nimsuggest", uri = fileUri, projectFile = result
+      result = fileUri
+
+  if result == "":
+    result = fileUri
 
-  let otherNsProject = await ls.isKnownByAnyNimsuggest(fileUri)
-  if otherNsProject.isSome:
-    debug "File is known by nimsuggest", uri = fileUri, projectFile = otherNsProject.get
-    result = otherNsProject.get
-  else:
-    result = ls.getProjectFileAutoGuess(fileUri)
-
   debug "getProjectFile", project = result, fileUri = fileUri
 
-proc showMessage(ls: LanguageServer, message: string, typ: MessageType) =  
-  proc notify() =
-    ls.connection.notify(
-      "window/showMessage",
-      %* {
-         "type": typ.int,
-         "message": message 
-      })
-  let verbosity = 
-    ls
-    .getWorkspaceConfiguration
-    .waitFor
-    .notificationVerbosity.get(NlsNotificationVerbosity.nvInfo)
-  debug "ShowMessage ", message = message
-  case verbosity:
-  of nvInfo: 
-    notify()
-  of nvWarning:
-    if typ.int <= MessageType.Warning.int :
-       notify()
-  of nvError:
-    if typ == MessageType.Error: 
-      notify()
-  else: discard
-
 # Fixes callback clobbering in core implementation
 proc `or`*[T, Y](fut1: Future[T], fut2: Future[Y]): Future[void] =
   var retFuture = newFuture[void]("asyncdispatch.`or`")
@@ -830,7 +829,7 @@ proc warnIfUnknown(ls: LanguageServer, ns: Nimsuggest, uri: string, projectFile:
      Future[void] {.async, gcsafe.} =
   let path = uri.uriToPath
   let isFileKnown = await ns.isKnown(path)
-  if not isFileKnown and nsUnknownFile notin ns.capabilities:
+  if not isFileKnown and not ns.canHandleUnknown:
       ls.showMessage(fmt """{path} is not compiled as part of project {projectFile}.
   In orde to get the IDE features working you must either configure nim.projectMapping or import the module.""",
                     MessageType.Warning)
@@ -925,6 +924,22 @@ proc didSave(ls: LanguageServer, params: DidSaveTextDocumentParams):
   if ls.getWorkspaceConfiguration().await().checkOnSave.get(true):
     debug "Checking project", uri = uri
     traceAsyncErrors ls.checkProject(uri)
+
+  var toStop = newTable[string, Nimsuggest]()
+  #We first get the project file for the current file so we can test if this file recently imported another project
+  let thisProjectFile = await getProjectFile(uri.uriToPath, ls)
+  let ns = await ls.projectFiles[thisProjectFile]
+  if ns.canHandleUnknown:
+    for projectFile in ls.projectFiles.keys:
+      if projectFile in ls.entryPoints: continue
+      let isKnown = await ns.isKnown(projectFile)
+      if isKnown: 
+        toStop[projectFile] = await ls.projectFiles[projectFile]
+
+    for projectFile, ns in toStop:
+      ns.stop()
+      ls.projectFiles.del projectFile
+      ls.showMessage &"File {projectFile} is known by another nimsuggest instance, stopping the current one", MessageType.Warning
 
 proc didClose(ls: LanguageServer, params: DidCloseTextDocumentParams):
     Future[void] {.async, gcsafe.} =

suggestapi.nim

@@ -96,6 +96,8 @@ type
     nimSuggestPath*: string
     version*: string
 
+func canHandleUnknown*(ns: Nimsuggest): bool =
+  nsUnknownFile in ns.capabilities
 
 template benchmark(benchmarkName: string, code: untyped) =
   block:

tests/tprojectsetup.nim

@@ -176,7 +176,7 @@ proc initLangServerConnect(workspaceConfiguration: JsonNode): NimLangServerConne
   result.client.notify("initialized", newJObject())
 
 
-template initLangServerForTestProject(workspaceConfiguration: JsonNode) {.dirty.}= 
+template initLangServerForTestProject(workspaceConfiguration: JsonNode, rootPath: string) {.dirty.}= 
 
   let pipeServer = createPipe();
   let pipeClient = createPipe();
@@ -225,7 +225,8 @@ template initLangServerForTestProject(workspaceConfiguration: JsonNode) {.dirty.
           "workDoneProgress": true
         },
         "workspace": {"configuration": true}
-      }
+      },
+      "rootPath": %*rootPath
   }
 
   discard waitFor client.call("initialize", %initParams)
@@ -247,7 +248,7 @@ suite "nimble setup":
       "autoCheckFile": false,
       "autoCheckProject": false
     }]
-    initLangServerForTestProject(nlsConfig)
+    initLangServerForTestProject(nlsConfig, testProjectDir)
     # #At this point we should know the main file is `testproject.nim` but for now just test the case were we open it
     client.notify("textDocument/didOpen", %createDidOpenParams(entryPoint))
     let (_, params) = suggestInit.read.waitFor
@@ -271,11 +272,9 @@ suite "nimble setup":
 
     var resStatus =  client.call("extension/status", %()).waitFor
     let status = resStatus.to(NimLangServerStatus)[]
-    check status.nimsuggestInstances.len == 1
+    # check status.nimsuggestInstances.len == 2
     let nsInfo = status.nimsuggestInstances[0]
     check nsInfo.projectFile == entryPoint
-    check nsInfo.openFiles.len == 1
-    check nsInfo.openFiles[0] == entryPoint
 
 
   test "`submodule.nim` should not be part of the nimble project file":
@@ -292,7 +291,7 @@ suite "nimble setup":
       "autoCheckFile": false,
       "autoCheckProject": false
     }]
-    initLangServerForTestProject(nlsConfig)
+    initLangServerForTestProject(nlsConfig, testProjectDir)
 
     let submodule = testProjectDir / "src" / "testproject" / "submodule.nim"
     client.notify("textDocument/didOpen", %createDidOpenParams(submodule))