oxalica / nil

NIx Language server, an incremental analysis assistant for writing in Nix.
Apache License 2.0
1.28k stars 39 forks source link

When an attribute is reused (nested syntax), the header line breadcrumb path is only correct for the first occurrence #107

Closed sir4ur0n closed 10 months ago

sir4ur0n commented 11 months ago

The title is probably unclear, I wasn't sure how to describe it.

Let's use an example:

{
  a = "a";
  b = {
    c.d = true;
    c.e = "boom!";
  };
}

By "header line breadcrumb" I refer to this: image

I think that putting the cursor on the second c or on e should display the breadcrumb b > c > e.

Note that if we inverse c.d and c.e, the problem now appears for c.d, so I feel like the breadcrumb is only correct for the first occurrence of a nested attribute.

oxalica commented 10 months ago

The problem here is that Nix allows an attribute to be defined in multiple places but semantically merge them, and we currently print the symbol tree semantically (with attributes merged). Consider this case:

{
  a.b = 1;
  d = 2;
  a.c = 3;
}

Then the current symbol tree is:

|- a (covers ???)
|  |- b (covers "b = 1;")
|  \- c (covers "c = 3;")
\- d (covers "d = 2;")

If the covering range (range in LSP spec) of a is the first line a.b = 1;, then the client will not show properly when the cursor is on a.c, because c is outside the range of its parent. If its covering all 3 lines inside, then d will not be shown properly either, because its unexpectedly inside another node.

I think the only way is to split the tree to be syntactical, only possibly merge adjacent ones.

|- a (covers "a.b = 1";)
|  \- b (covers "b = 1;")
|- d (covers "d = 2;")
|- a (covers "a.c = 3";)
   \- c (covers "c = 3;")

This will give the correct breadcrumb, but make the symbol tree more complex and hard to find everything under a in one shot. As a reference, likewise, impl blocks in Rust can be placed multiple times but merged semantically, rust-analyzer shows them syntactically (unmerged) in the symbol tree.