Make Janet LSP evaluation error tolerant about janet-lsp HOT 8 CLOSED

Well for starters, it seems like the LSP currently highlights the LAST error in the file, for whatever reason. 😜 That might not be the most helpful thing to highlight, especially when, as you say, there's a cascading issue that originates further up the file somewhere. Doing things in this error-tolerant way that I'm proposing would show red on each top-level form in the file that has an issue, at which point the programmer can go about figuring it out in whatever order they want to.

But another issue is, the way eval-buffer is written currently, evaluation errors anywhere in a file cause problems with on-hover documentation for ALL symbols defined in that same file.

More detail below—mostly just to capture current-me's understanding for the benefit of future-me 🙂

Essentially, the LSP server is running its own separate Janet runtime and updating the symbol definitions in that fiber's environment table to match whatever is defined in the source code file you're working on. Inside eval.janet, the LSP distinguishes between evaluations that may have side effects and those known not to (that's what flycheck-evaluator does). eval-buffer uses flycheck-evaluator to expand macros and compile code, but not actually run it. On-hover documentation is just wrapping a lookup to the LSP fiber's environment table—a lookup almost identical to the one that (doc) runs when you evaluate it in the REPL—meaning that for on-hover docs to work, that symbol has to have been added to the fiber's environment table to be looked up from.

But that means any errors encountered when evaluating a file will cause the environment table in the LSP's fiber to not update at all, even with symbols in the same file that would otherwise compile correctly.

So if eval-buffer encountered any error in a file—doesn't matter if it's a 500 line file and the error is in the very last function definition—on-hover documentation won't be updated for any of the symbols in the current file. This is fine and you might not even notice if you first open a file with no errors and then introduce one, because the symbol definition from the previous successful evaluation will still be there to get documentation info from. But, if you open up a file that has an error from the start (like, in the current case where having a non-root install of Janet is messing up module imports) then at no point will the current file's symbols have been loaded into the LSP's environment table, and on-hover docs for locally-defined symbols just won't work.

Going to (more) error-tolerant evaluation will at least mean that any symbols defined up until the first error in the file will behave correctly, which I think would be beneficial.

from janet-lsp.

On first glance, this seems like a non-trivial endeavor.

Sounds nice if practical though!

from janet-lsp.

Well so, it's probably way more complicated than it seems on first blush (because why wouldn't it be, right 😜) but here's how I'm thinking of experimenting at least:

Part 1: Accumulate subsets of the file to evaluate broken out by S-Exp ("chunks"), sorted in descending order from the full file down to only the first expression

Part 2: Evaluate each chunk in order, returning early at the first success without errors

Part 3: For any errors, extract the error, deduplicate, and then return the list of unique errors to the client

In the happy path 99% case, there will be either exactly zero (0) or exactly one (1) compilation errors for any given file at any given time (assuming most programmers will open up a file that is compilation-error-free, code until they create an error, fix it, and then repeat the process). In the first case, the whole file will evaluate without any errors and we'll only have to check the whole file that one time, which is the same as what we're doing now. In the second case, where there is an error, it'll evaluate more than once, but only as many times as necessary to evaluate up to the expression PRIOR TO the spot with the error, at which point we can stop.

In the case where there's more than one error in the same file, we'll keep evaluating until we've found a subset of the file without errors and stop eval'ing there. But since that's hopefully not the state that most programmers spend most of their time in, it seems ok with me, initially speaking, to have the rarer case be more processing intensive.

One Weakness: This naive implementation would not be able to identify multiple errors within the same top-level S-Expression. But I don't know if that would be easily doable without essentially writing a custom error-tolerant compiler, which is definitely beyond my ability.

Another Weakness: On-hover documentation would only work for symbols defined in a file prior to the first-encountered error in the file. But, as of today on-hover documentation doesn't work AT ALL for symbols defined in the file if there's any eval errors in that file.

This wouldn't be a perfect solution, but, even being able to identify errors on a per-top-level-expression basis would be a pretty powerful upgrade, and because lisp syntax is so regular it seems (knock on wood) like this could be a fairly straight-forward thing to implement. Questions are whether this way of doing it is performant enough and whether there's any other hidden gotchas with anything else that I just haven't thought of.

EDIT: ...this all makes some assumptions about evaluation order that would need to be validated. I know as it's working right now, eval-buffer complains about the last identified error in the file (except that it checks imports ahead of everything else...? I think?). From a quick and dirty test, it seems like the Janet compiler reports the first error encountered in the file, not the last one. Experimentation needed here.

from janet-lsp.

Thinking about it further, it probably makes sense to only calculate the line numbers that would comprise each "chunk" instead of allocating a bunch of memory to hold redundant duplicated bytes. Then, each time eval-buffer needs to run, it can slice out a new substring based on those line numbers right before running.

This might also mean that the first error that returns, since it reports line numbers, can be used to "forward" the evaluation on to the next smallest "chunk" rather than having to iterate through every chunk in order, searching for one that doesn't error. In that case (EDIT: and assuming evaluation order works the way I think it works), the number of evaluations should generally be [the number of top-level S-exps with errors]+1.

from janet-lsp.

If you're saying this ("99% case"):

In the happy path 99% case, there will be either exactly zero (0) or exactly one (1) compilation errors for any given file at any given time (assuming most programmers will open up a file that is compilation-error-free, code until they create an error, fix it, and then repeat the process).

Isn't stopping at the first error providing a significant amount of benefit?

If the situation was one of executing tests for a test suite, I think an option to continue with subsequent tests makes sense. That can give the entity running a test suite a broader perspective about which failed test to examine first.

For typical source code though, to me it seems likely one would be interested in fixing the earliest error first -- particularly when the evaluation order is from top-to-bottom as it is in Janet. It seems possible that some types of later errors might vanish if earlier errors are fixed (doesn't this seem to happen with fixing compilation errors in C and other similar languages?).

Perhaps I'm missing something though (^^;

from janet-lsp.

it seems like the LSP currently highlights the LAST error in the file, for whatever reason

Ohhhhh. Doh!

from janet-lsp.