rnix-lsp

rnix-lsp copied to clipboard

Related to https://github.com/nix-community/rnix-lsp/issues/31.

I created this PR to show what integration with an evaluator will look like. We'll also have to figure out how to review all this code (maybe split it across many PRs?), if we want to merge it at all.

This particular PR adds hover-to-see-value functionality. it's requires very little modification to main.rs. Using the evaluator for completions and goto-definition would similarly require very little modification to main.rs.

How it works

The evaluator works by associating rnix-parser text ranges with lazy Nix values. It uses the gc crate for garbage collection.

The heart of the evaluator is the following struct, with comments added:

#[derive(Clone, Trace, Finalize)]
pub struct Tree {
    // Associates an expression with its position in a text file.
    // We call `children()` and recurse to find the value at a 
    // given cursor position. This is also useful for `goto`.
    #[unsafe_ignore_trace]
    pub range: Option<TextRange>,
    // Tracks variable bindings in scope and also the file path.
    pub scope: Gc<Scope>,
    // Processed rnix-parser nodes, contains child `Tree`s
    pub source: TreeSource,
    // Lazily evaluated NixValue. GcCell lets a Tree cache
    // evaluations without needing a mutable reference to `self`.
    pub value: GcCell<Option<Gc<NixValue>>>,
    // Hash of the syntax used to define this Tree. If we assume
    // that the same syntax evaluates to the same `value` (after
    // variables are substituted with their value, etc), then we
    // can re-use cached evaluations across file edits.
    // NOTE: this doesn't detect changes to imported files.
    pub hash: GcCell<Option<String>>,
}

Files

• tests.rs has a few basic tests for the evaluator.
• value.rs defines NixValue, which holds Nix primitive types such as int/string/bool.
• scope.rs defines Scope, which handles scoping for Nix variables.
• parse.rs converts a rnix-parser node into a Tree, a struct with associates a range of text with a lazily-evaluated NixValue and scope.
• eval.rs converts a Tree's lazy value placeholder to a concrete NixValue.
• builtins.rs uses macros to define a massive NixBuiltin enum. This is where all Nix builtins are implemented.
• derivation.nix is a written-from-scratch implementation of nixpkgs/nix's derivation.nix, which defines a derivation function that calls derivationStrict. Note that the code looks similar due to needing to create sets with the same attribute names; the implementations themselves work in entirely different ways.

Code review

Perhaps it make sense to split these changes across a bunch of PRs? If so, I could start by submitting a PR with an arithmetic-only-evaluator then follow-up with PRs to slowly add back functionality until we can evaluate Nixpkgs again.

Oh for context, see the working proof of concept with all features enabled:

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/best-ide-for-writing-nix-expressions/13454/9

My rust foo is too low to be able to properly review this PR. Maybe @Mic92 or @Rizary can help?

Does maybe @tazjin also want to have a look at it?

To clarify, this PR is way too large to properly review its code. But if we're on board with the general idea (the PR description along with the general structure of the diff), I'd be happy to make smaller PRs that would be easier to review and discuss each step of the way

Oh, we also might want to consider exposing the functionality of this PR as a Rust crate, since it seems like it would be generally helpful for the Nix community to have a way to evaluate and inspect Nix code without shelling out

I think splitting this into multiple commits or suggesting where to start when reviewing might help :)

Oh definitely. For this PR, I recommend focusing on mostly the description. For actually reviewing code, I think it would make sense for me to make a PR that only handles arithmetic, would would make the code much easier to review. After that's merged, I could make PRs adding more features in small steps.

Also to clarify, I don't think the code itself is ready to be merged. But I wanted to make sure we're both on board with the structure before I spend significant time making the implementation do exactly what we want.

It uses the gc crate for garbage collection.

Perhaps I'm missing something obvious, but can you explain, why this is needed? I'm not against it, but I doubt that I fully understood the why behind this decision :)

No worries. The official Nix evaluator using a tracing garbage collector, and the Nix language is designed for garbage collection. I originally tried to implement this with borrowing and Rc, but that's unable to handle many scenarios. For example, consider how garbage collection needs to work for Nixpkgs's implementation of the fixed-point operator (lib/fixed-points.nix#L3-L19):

let
  fix = (f: let x = f x; in x);
  attrs = (self: { x = 1; y = self.x; });
in
  (fix attrs).x

I also found this helpful blog post about garbage collectors in Rust, written by the author of the gc crate (and much more).

Oh definitely. For this PR, I recommend focusing on mostly the description

The overall structure makes sense to me, however I'm not an expert on how libexpr of Nix is implemented internally, so I cannot say if there are potential flaws that I'm missing.

For actually reviewing code, I think it would make sense for me to make a PR that only handles arithmetic, would would make the code much easier to review. After that's merged, I could make PRs adding more features in small steps.

I think such an incremental progress where we (or at least I'd be up for that) could dogfood the changes by using them locally makes sense.

My next task here would be to finish incremental parsing (I think I found a small bug), but let me know if I can help!

@Ma27 do you want to adopt this project? You could also maintain this temporary until we find someone else.

@Mic92 Aaron and I already have commit access and we decided that they will become the "official" LSP maintainer and I'll become the "official" rnix-parser maintainer even though we'll work on both projects ;)

@Mic92 Aaron and I already have commit access and we decided that they will become the "official" LSP maintainer and I'll become the "official" rnix-parser maintainer even though we'll work on both projects ;)

I updated the repo description to reflect that.

Status update: I'm busy for the next few days, but next week I'll submit a PR that does basic arithmetic. I think that should be more helpful for discussion than this draft PR

Is there any blockage concerning this PR ? I'm very interested since I use Nix everyday, what is the current state of this PR ?