Use nim track for goto definition and find usages

[moigagoo]

Nim 2.4.0 adds a new command nim track --def|--usages|--defusages that replaces nimsuggest's def and use commands.

This PR adds an experimental implementation of goto def and find refs based on nim track.

[moigagoo]

I had to rewrite some things not directly related to nim track to make nimlangserver correctly find the right Nim binary to call. That also involves fixing Nimble so I'm using my fork in the CI until the Nimble PR is merged.

To test the new mode locally:

1. Install nimlangserver from feature/nim_track branch:

$ nimble install https://github.com/nim-lang/langserver@#feature/nim_track

2. Install the patched Nimble version:

$ nimble install https://github.com/moigagoo/nimble@#bugfix/nimble_dump_with_nim_head

3. In your langserver config, add useNimTrack = true. For example, here's how to do it in Helix:

[[language]]
name = "nim"
language-servers = ["nimlangserver"]

[language-server.nimlangserver]
command = "nimlangserver"
args = ["--lsp", "--stdio"]

[language-server.nimlangserver.config.nim]
useNimTrack = true

4. Since nim track is available only in pre-release Nim now, the project you want to test it with must have nim#head in its .nimble file and compile with nim#head.

5. After adding requires "nim#head" to .nimble file run nimble install -ly && nimble setup.

6. To test that your project compiles with nim#head, run ./nimbledeps/pkg2/nim-#headsomehashhere/bin/nim ic src/yourfile.nim

If everything is fine so far, launch your editor and try doing goto def and find refs (gd and gr in Helix). You can check your LSP logs to see the nim track calls (:log-open in Helix; tail -f ~./cache/helix/helix.log is a convenient way to see the logs update in real time).

The commands are slow at first because nim ic is invoked under the hood but it gets better quickly.

Happy testing!

[jmgomez]

This is what the AI says, verify and let me know so I can take a closer look. Thanks!

• High — large reference results can deadlock. trackapi.nim:47-53
  (

  langserver/trackapi.nim

  Lines 47 to 53 in 0d5f64a

  	try:
  	let exitCode = await process.waitForExit(timeout.milliseconds)
  	let stdoutBytes = process.stdoutStream.read().await
  	var stderrStr = ""
  	try:
  	stderrStr = process.stderrStream.read().await.toString
  	except CatchableError:

  )
  waits for nim track to exit before draining stdout or stderr. If either pipe fills, the
  compiler cannot exit and the request stalls until timeout. Both streams should be drained
  concurrently while the process runs.

• Medium — navigation stops working in unsaved buffers. routes/lsp.nim:162-163
  (

  langserver/routes/lsp.nim

  Lines 162 to 163 in 0d5f64a

  	if uri notin ls.openFiles or ls.openFiles[uri].changed:
  	return @[]

  )
  and routes/lsp.nim:463-464
  (

  langserver/routes/lsp.nim

  Lines 463 to 464 in 0d5f64a

  	if uri notin ls.openFiles or ls.openFiles[uri].changed:
  	return @[]

  )
  return no results whenever the document has changed. Current Nim supports an optional
  dirty-buffer path for these switches; alternatively, this case could fall back to
  nimsuggest.

• Medium — cancellation can leak nim track processes. trackapi.nim:61-63
  (

  langserver/trackapi.nim

  Lines 61 to 63 in 0d5f64a

  	except CatchableError as e:
  	debug "nim track exception", error = e.msg, name = e.name
  	result = @[]

  )
  catches CancelledError as CatchableError, returns an empty result, and never terminates the
  child. Cleanup should run on cancellation, and cancellation should be re-raised.

[moigagoo]

High — large reference results can deadlock. trackapi.nim:47-53
Medium — cancellation can leak nim track processes. trackapi.nim:61-63

Fixed.

Medium — navigation stops working in unsaved buffers. routes/lsp.nim:162-163

This is interesting. The thing is, I deliberately chose to run nim track only against saved files. Nimsuggest accepts an optional dirty param and therefore can work with unsaved files (the checks are run against a temporary file on the disk but the hint positioning is reported relative to the dirty file). AFAIK nim track doesn't have that, it just runs against a file on the disk, there is no dirty param.

So my options were to only serve saved files, fallback to nimsuggest, or invent something to work around that limitation. I chose the first option because it was the simplest solution. Using nimsuggest kind of defeats the whole purpose of nim track (which is to replace nimsuggest).

[jmgomez]

why lock is deleted?

[moigagoo]

Initially, I got confused between project dependencies and langserver dependencies and so tried setting up Nim#head as a dependency to langserver. This didn't work with lock files.

Later on this all became irrelevant since it's the project that must depend on Nim#head, not langserver. But I forgot to revert these commits.

Reverted.

[jmgomez]

I feel like bumping deps doesnt belong to this pr

[moigagoo]

Answered in a comment above: https://github.com/nim-lang/langserver/pull/434/changes/BASE..98e62df8720397ee8292b3c7a14787b5422843c0#r3748271486

[jmgomez]

what happens when it is enabled in a nim version that doesnt support track?

[moigagoo]

You'll get an error:

"nim track not supported (requires nim >= 2.3.1)"

The reasoning behind this decision is, this is a very experimental feature, it requires development version of Nim and some dedication. You can't accidentally turn it on, if it's on, you know what you did. So I thought I shouldn't invest too much into handling this, the simplest logic will do. If invalid command: track is in the stderr, that means you're not using Nim#devel.

[jmgomez]

AFAIK nim track doesn't have that, it just runs against a file on the disk, there is no dirty param.

Then a follow up must be done where its supported, otherwise it wont work 99% of the times when working with code, no? Or am I missing something else? AFAIR a file becomes dirty when there is unsaved modification. For the time being, we should fallback to ns if thats the case

[moigagoo]

Then a follow up must be done where its supported, otherwise it wont work 99% of the times when working with code, no? Or am I missing something else?

You are right, since most people don't use autosave this won't work for them.

AFAIR a file becomes dirty when there is unsaved modification.

When, exactly.

For the time being, we should fallback to ns if thats the case

Removed immediate return so that definition and references fall back to nimsuggest for unsaved files.