Integrates nim check as opt-out replacement for nimsuggest chk (#271)

* wip nimcheck

* Integrates nim check into the ls

* Fixes an issue where configuration wasnt properly updated. Opt-out `useNimCheck`

* Fixes a crash. Uses `nim` project path for `nim check`

Author: jmgomez

README.md

@@ -109,7 +109,7 @@ Note when in a nimble project, `nimble` will drive the entry points for `nimsugg
 - `nim.notificationVerbosity` - configure the verbosity of notifications. Can be set to `"none"`, `"error"`, `"warning"`, or `"info"`.
 - `nim.formatOnSave` - format the file on save. Requires `nph` to be available in the PATH.
 - `nim.nimsuggestIdleTimeout` - the timeout in ms after which an idle `nimsuggest` will be stopped. If not specified the default is 120 seconds.
-
+- `nim.useNimCheck` - use `nim check` instead of `nimsuggest` for linting. Defaults to `true`.
 ## Features
 
 `nimlangserver` supports the following LSP features:

ls.nim

@@ -20,7 +20,8 @@ import
   uri,
   json_serialization,
   json_rpc/[servers/socketserver],
-  regex
+  regex,
+  nimcheck
 
 proc getVersionFromNimble(): string =
   #We should static run nimble dump instead
@@ -38,6 +39,7 @@ const
   LSPVersion* = getVersionFromNimble()
   CRLF* = "\r\n"
   CONTENT_LENGTH* = "Content-Length: "
+  USE_NIM_CHECK_BY_DEFAULT* = false
 type
   NlsNimsuggestConfig* = ref object of RootObj
     projectFile*: string
@@ -83,6 +85,7 @@ type
     notificationVerbosity*: Option[NlsNotificationVerbosity]
     formatOnSave*: Option[bool]
     nimsuggestIdleTimeout*: Option[int] #idle timeout in ms
+    useNimCheck*: Option[bool]
 
   NlsFileInfo* = ref object of RootObj
     projectFile*: Future[string]
@@ -287,9 +290,19 @@ proc getWorkspaceConfiguration*(
     #TODO review and handle project specific confs when received instead of reliying in this func
     if ls.workspaceConfiguration.finished:
       return parseWorkspaceConfiguration(ls.workspaceConfiguration.read)
-    else:
-      return NlsConfig()
+    return NlsConfig()
   except CatchableError as ex:
+    error "Failed to get workspace configuration", error = ex.msg
+    writeStackTrace(ex)
+
+proc getAndWaitForWorkspaceConfiguration*(
+    ls: LanguageServer
+): Future[NlsConfig] {.async.} =
+  try:    
+    let conf = await ls.workspaceConfiguration
+    return parseWorkspaceConfiguration(conf)
+  except CatchableError as ex:
+    error "Failed to get workspace configuration", error = ex.msg
     writeStackTrace(ex)
 
 proc showMessage*(
@@ -462,6 +475,17 @@ proc getNimSuggestPathAndVersion(
   ls.showMessage(fmt "Using {nimVersion}", MessageType.Info)
   (nimsuggestPath, nimVersion)
 
+proc getNimPath*(conf: NlsConfig): Option[string] =
+  if conf.nimSuggestPath.isSome:
+    some(conf.nimSuggestPath.get.parentDir / "nim")
+  else:
+    let path = findExe "nim"
+    if path != "":
+      some(path)
+    else:
+      warn "Failed to find nim path"
+      none(string)
+
 proc getProjectFileAutoGuess*(ls: LanguageServer, fileUri: string): string =
   let file = fileUri.decodeUrl
   debug "Auto-guessing project file for", file = file
@@ -580,6 +604,18 @@ proc toUtf16Pos*(
   if pos.isSome:
     result.column = pos.get()
 
+proc toUtf16Pos*(checkResult: CheckResult, ls: LanguageServer): CheckResult =
+  result = checkResult
+  let uri = pathToUri(checkResult.file)
+  let pos = toUtf16Pos(ls, uri, checkResult.line - 1, checkResult.column)
+  if pos.isSome:
+    result.column = pos.get()
+  
+  for i in 0..<result.stacktrace.len:
+    let stPos = toUtf16Pos(ls, uri, result.stacktrace[i].line - 1, result.stacktrace[i].column)
+    if stPos.isSome:
+      result.stacktrace[i].column = stPos.get()
+
 proc range*(startLine, startCharacter, endLine, endCharacter: int): Range =
   return
     Range %* {
@@ -619,13 +655,39 @@ proc toDiagnostic(suggest: Suggest): Diagnostic =
       }
     return node.to(Diagnostic)
 
-proc sendDiagnostics*(ls: LanguageServer, diagnostics: seq[Suggest], path: string) =
+proc toDiagnostic(checkResult: CheckResult): Diagnostic =
+  let
+    textStart = checkResult.msg.find('\'')
+    textEnd = checkResult.msg.rfind('\'')
+    endColumn =
+      if textStart >= 0 and textEnd >= 0 and textEnd > textStart:
+        checkResult.column + utf16Len(checkResult.msg[textStart + 1 ..< textEnd])
+      else:
+        checkResult.column + 1
+
+  let node =
+    %*{
+      "uri": pathToUri(checkResult.file),
+      "range": range(checkResult.line - 1, max(0, checkResult.column), checkResult.line - 1, max(0, endColumn)),
+      "severity":
+        case checkResult.severity
+        of "Error": DiagnosticSeverity.Error.int
+        of "Hint": DiagnosticSeverity.Hint.int
+        of "Warning": DiagnosticSeverity.Warning.int
+        else: DiagnosticSeverity.Error.int
+      ,
+      "message": checkResult.msg,
+      "source": "nim",
+      "code": "nim check",
+  }
+  return node.to(Diagnostic)
+
+proc sendDiagnostics*(ls: LanguageServer, diagnostics: seq[Suggest] | seq[CheckResult], path: string) =
   debug "Sending diagnostics", count = diagnostics.len, path = path
   let params =
     PublishDiagnosticsParams %*
     {"uri": pathToUri(path), "diagnostics": diagnostics.map(x => x.toUtf16Pos(ls).toDiagnostic)}
   ls.notify("textDocument/publishDiagnostics", %params)
-
   if diagnostics.len != 0:
     ls.filesWithDiags.incl path
   else:
@@ -638,6 +700,40 @@ proc tryGetNimsuggest*(
 proc checkProject*(ls: LanguageServer, uri: string): Future[void] {.async, gcsafe.} =
   if not ls.getWorkspaceConfiguration.await().autoCheckProject.get(true):
     return
+  let conf = await ls.getAndWaitForWorkspaceConfiguration()
+  let useNimCheck = conf.useNimCheck.get(USE_NIM_CHECK_BY_DEFAULT)
+  
+  let nimPath = getNimPath(conf)
+
+  if useNimCheck and nimPath.isSome:
+    proc getFilePath(c: CheckResult): string = c.file
+
+    let token = fmt "Checking {uri}"
+    ls.workDoneProgressCreate(token)
+    ls.progress(token, "begin", fmt "Checking project {uri}")
+    if uri == "":
+      warn "Checking project with empty uri", uri = uri
+      ls.progress(token, "end")
+      return
+    let diagnostics = await nimCheck(uriToPath(uri), nimPath.get)
+    let filesWithDiags = diagnostics.map(r => r.file).toHashSet
+    
+    ls.progress(token, "end")
+    
+    debug "Found diagnostics", file = filesWithDiags
+    for (path, diags) in groupBy(diagnostics, getFilePath):
+      ls.sendDiagnostics(diags, path)
+      
+    # clean files with no diags
+    for path in ls.filesWithDiags:
+      if not filesWithDiags.contains path:
+        debug "Sending zero diags", path = path
+        let params =
+          PublishDiagnosticsParams %* {"uri": pathToUri(path), "diagnostics": @[]}
+        ls.notify("textDocument/publishDiagnostics", %params)
+    ls.filesWithDiags = filesWithDiags
+    return
+
   debug "Running diagnostics", uri = uri
   let ns = ls.tryGetNimsuggest(uri).await
   if ns.isNone:
@@ -923,15 +1019,22 @@ proc warnIfUnknown*(
     )
 
 proc checkFile*(ls: LanguageServer, uri: string): Future[void] {.async.} =
-  debug "Checking", uri = uri
+  let conf = await ls.getAndWaitForWorkspaceConfiguration()
+  let useNimCheck = conf.useNimCheck.get(USE_NIM_CHECK_BY_DEFAULT)
+  let nimPath = conf.getNimPath()
   let token = fmt "Checking file {uri}"
   ls.workDoneProgressCreate(token)
   ls.progress(token, "begin", fmt "Checking {uri.uriToPath}")
 
-  let
-    path = uriToPath(uri)
-    ns = await ls.tryGetNimsuggest(uri)
+  let path = uriToPath(uri)
+
+  if useNimCheck and nimPath.isSome:
+    let checkResults = await nimCheck(uriToPath(uri), nimPath.get)
+    ls.progress(token, "end")
+    ls.sendDiagnostics(checkResults, path)
+    return
 
+  let ns = await ls.tryGetNimsuggest(uri)
   if ns.isSome:
     let diagnostics = ns.get().chkFile(path, ls.uriToStash(uri)).await()
     ls.progress(token, "end")

nimcheck.nim

@@ -0,0 +1,91 @@
+import std/[strutils]
+import regex
+import chronos, chronos/asyncproc
+import stew/[byteutils]
+import chronicles
+import protocol/types
+
+type
+  CheckStacktrace* = object
+    file*: string
+    line*: int
+    column*: int
+    msg*: string
+
+  CheckResult* = object
+    file*: string
+    line*: int
+    column*: int
+    msg*: string
+    severity*: string
+    stacktrace*: seq[CheckStacktrace]
+
+proc parseCheckResults(lines: seq[string]): seq[CheckResult] =
+  result = @[]
+  var
+    messageText = ""
+    stacktrace: seq[CheckStacktrace]
+    lastFile, lastLineStr, lastCharStr: string
+    m: RegexMatch2
+    
+  let dotsPattern = re2"^\.+$"
+  let errorPattern = re2"^([^(]+)\((\d+),\s*(\d+)\)\s*(\w+):\s*(.*)$"
+  
+  for line in lines:
+    let line = line.strip()
+
+    if line.startsWith("Hint: used config file") or line == "" or line.match(dotsPattern):
+      continue
+
+    if not find(line, errorPattern, m):
+      if messageText.len < 1024:
+        messageText &= "\n" & line
+    else:
+      try:
+        let
+          file = line[m.captures[0]]
+          lineStr = line[m.captures[1]]
+          charStr = line[m.captures[2]]
+          severity = line[m.captures[3]]
+          msg = line[m.captures[4]]
+        
+        let 
+          lineNum = parseInt(lineStr)
+          colNum = parseInt(charStr)
+
+        result.add(CheckResult(
+          file: file,
+          line: lineNum,
+          column: colNum,
+          msg: msg,
+          severity: severity,
+          stacktrace: @[]
+        ))
+        
+      except Exception as e:
+        error "Error processing line", line = line, msg = e.msg
+        continue
+        
+  if messageText.len > 0 and result.len > 0:
+    result[^1].msg &= "\n" & messageText
+
+
+proc nimCheck*(filePath: string, nimPath: string): Future[seq[CheckResult]] {.async.} =
+  debug "nimCheck", filePath = filePath, nimPath = nimPath
+  let process = await startProcess(
+    nimPath,
+    arguments = @["check", "--listFullPaths", filePath],
+    options = {UsePath},
+    stderrHandle = AsyncProcess.Pipe,
+    stdoutHandle = AsyncProcess.Pipe,
+  )
+  let res = await process.waitForExit(InfiniteDuration)
+  debug "nimCheck exit", res = res
+  var output = ""
+  if res == 0: #Nim check return 0 if there are no errors but we still need to check for hints and warnings
+    output = string.fromBytes(process.stdoutStream.read().await)   
+  else:
+    output = string.fromBytes(process.stderrStream.read().await)
+  
+  let lines = output.splitLines()
+  parseCheckResults(lines)

utils.nim

@@ -2,13 +2,23 @@ import std/[unicode, uri, strformat, os, strutils, options, json, jsonutils, sug
 import chronos, chronicles
 import "$nim/compiler/pathutils"
 import json_rpc/private/jrpc_sys
-
 type
   FingerTable = seq[tuple[u16pos, offset: int]]
 
   UriParseError* = object of Defect
     uri: string
 
+proc writeStackTrace*(ex = getCurrentException()) =
+  try:
+    if ex != nil:
+      stderr.write "An exception occured \n"
+      stderr.write ex.msg & "\n"
+      stderr.write ex.getStackTrace()
+    else:
+      stderr.write getStackTrace()
+  except IOError:
+    discard
+
 proc createUTFMapping*(line: string): FingerTable =
   var pos = 0
   for rune in line.runes:
@@ -156,17 +166,6 @@ proc catchOrQuit*(error: Exception) =
     fatal "Fatal exception reached", err = error.msg, stackTrace = getStackTrace()
     quit 1
 
-proc writeStackTrace*(ex = getCurrentException()) =
-  try:
-    if ex != nil:
-      stderr.write "An exception occured \n"
-      stderr.write ex.msg & "\n"
-      stderr.write ex.getStackTrace()
-    else:
-      stderr.write getStackTrace()
-  except IOError:
-    discard
-
 proc traceAsyncErrors*(fut: Future) =
   fut.addCallback do(data: pointer):
     if not fut.error.isNil: