evaluate arithmetic

[aaronjanse]

This PR adds basic parsing, evaluation, and hover-to-view-value support for arithmetic. This is a followup to #38.

A few notes:

• Because this only parses arithmetic, the entire file must be one arithmetic expression for evaluation to work. Otherwise, the evaluator fails silently, allowing the rest of rnix-lsp to operate like normal.
• Possible features and ideas #41 will probably be annoying while manually testing this PR, but that's okay

This PR is a superset of existing rnix-lsp functionality. It doesn't replace any rnix-lsp code; it merely adds a hover view for arithmetic. With this in mind, we should consider whether we want to:

1. Merge evaluation PRs into master, adding more evaluation features over time, possibly across releases
2. Merge evaluation PRs into a new branch like lsp-devel, eventually releasing fancy evaluator features all at once

I'm slightly in favor of option #1, since we'd get user feedback to guide future features. But I'll leave this decision to @Ma27, who knows much more about the rnix-lsp release and feedback process than I do.

---

Unlike #38, I want to make sure this code is production-ready. This PR is where we'll probably want to discuss the evaluator in detail, since it adds the structure that would be used for later evaluation features.

[Ma27]

From a conceptual PoV this looks good to me. Implementation-wise I'm not entirely done with a review yet, but it looks fine as well.

I guess I'll add this to my nvim-overlay next week and use it for a while to find potential issues :)

Perhaps @Mic92 or @zimbatm want to take a look as well?

Finally, thanks a lot for your work! 🎉

[Ma27]

Also, in case you haven't seen it, the tests aren't fully working :)

[aaronjanse]

Also, in case you haven't seen it, the tests aren't fully working :)

Oh huh, they pass on my laptop. What issues are you seeing?

[Mic92]

Also, in case you haven't seen it, the tests aren't fully working :)

Oh huh, they pass on my laptop. What issues are you seeing?

see CI: https://github.com/nix-community/rnix-lsp/runs/2981468861#step:4:4

[Ma27]

The current state seems quite OK for me. I'm not sure if I missed something, but does it still throw bogus errors if a Nix expressionw ith more than basic arithmetic is used?

I guess we'd wait here until 0.2.0 is out and then merge :)

[aaronjanse]

does it still throw bogus errors if a Nix expressionw ith more than basic arithmetic is used?

This should be fixed by 2b321da, which hides internal errors from the user.

[aaronjanse]

see CI: https://github.com/nix-community/rnix-lsp/runs/2981468861#step:4:4

Oh I see. I assume nixpkgs changed to use some feature that's not yet supported by rnix-lsp. I'll take a look tomorrow :-)

[aaronjanse]

Oh wait, I misread the error message. Either way I'll debug tomorrow :-)

[nrdxp]

Since this is the first step toward adding a more feature filled evaluator, it might be pertinent to split this work off into it's own workspace, or possibly even a separate repo such as rnix-evaluator. There are lot's of cool tool ideas I've had swimming around in my brain that could benefit from partial Nix evaluation.

Additionally, I wouldn't be surprised if someone got fed up with the C++ codebase at some point and started reimplementing all of Nix in Rust (which kinda sorta already happened) 😅

Of course, we could always do this at a later stage, but I've figured it might be simpler to start off this way rather than try to port everything to a separate crate later.

[Ma27]

Since this is the first step toward adding a more feature filled evaluator, it might be pertinent to split this work off into it's own workspace, or possibly even a separate repo such as rnix-evaluator. There are lot's of cool tool ideas I've had swimming around in my brain that could benefit from partial Nix evaluation.

@aaronjanse initially implemented this in its own repository: https://github.com/aaronjanse/nix-eval-lsp

Tbh, I'm rather a fan of monorepos (I personally find it tedious to work with more than one when developing on a single feature), but that doesn't mean we can't create cargo workspace and discuss a split later on. Actually, after skimming through the code, that shouldn't be too hard to do, I could also offer to do this after we've created a 0.2.0 & got this into a mergeable state.

However I should note that Aaron came up with most of this initially and has way more insights, so it makes sense if they also share their opinions here to make sure I'm not missing something :)

Additionally, I wouldn't be surprised if someone got fed up with the C++ codebase at some point and started reimplementing all of Nix in Rust (which kinda sorta already happened) sweat_smile

Isn't that nix-rust branch effectively dead? IIRC it was used at the end of 2019 on nixUnstable (if I'm not mistaken), but that got reverted later on.

See also NixOS/nix#4697, NixOS/nix#3450 for context.

[aaronjanse]

I'm also in favor of monorepos. I also like "just use rnix-lsp" as advice to new users, rather than users needing to make a choice. That's why I made the README to my nix-eval-lsp boring, to avoid splitting the rnix-lsp userbase.

I think this PR is close to being ready to merge. I need to make a few edits, such as the struct renaming suggests by @Ma27.

I also like how evaluation is a superset of existing functionality. If done properly, gradual implementation of evaluation features should always make tooling better, and never worse, than it was before :-)

[aaronjanse]

I've pushed some code to address feedback. Also, I think the CI error is due to nix-community/naersk#176

[Ma27]

First of all, thanks a lot! Gotta say again, that I'm very happy that you push this forward!

Apart from the nitpick below, I have on other note:

• on my neovim, I have tested it against the expression (1 + 1).
• On the opening bracket and the + it correctly shows correctly the value 2, on the 1 it shows the value 1 when hovering.
• However, on the closing bracket I also get a 1.
• When I change it to (1 + 1 ), I correctly get a 2 when hovering over the ).

If you think it's editor related, you can build my VIM config via https://gitlab.com/Ma27/nvim.nix, you only have to replace the rnix-lsp override with a derivation pointing to your branch[1] and then run :lua vim.lsp.buf.hover(), but think this should occur on each editor. Would you mind looking into this?

The rest looks OK to me, so after that, feel free to merge! :)

[1] EDIT: actually, I pushed the change to https://gitlab.com/Ma27/nvim.nix/-/tree/rnix-eval, so an editor to reproduce is just a single nix build away :)

[aaronjanse]

Oh good catch! For a while, the hover has felt weird, but I couldn't tell why. That was the issue.

I've reproduced and pushed a fix for the issue. The hover tooling wrongly used <= instead of < for the right bound.