LSP: Add support for Go to Definition

[jansul]

This PR adds support for the LSP feature Go to Definition. Only a few things are actually supported, namely finding the definition of HtmlAttribute, HtmlComponent and HtmlStyle nodes as demonstrated below:

Screencast.from.2023-04-15.16-37-04.webm

I've opened this as a draft, as I'm not sure what I've done is a very good approach or not, and also there are some unfinished bits:

• I need to check if the client supports LocationLink's before returning them (via textDocument.definition.linkSupport) and otherwise just return a Location's instead (also looks like I should be returning an array of them as per the spec?)

The main parts I would like feedback on are below:

---

Selecting the "name" part of a node

The LSP requires that you return both targetRange and targetSelectionRange as part of a LocationLink as described below:

/**
* The full target range of this link. If the target for example is a symbol
* then target range is the range enclosing this symbol not including
* leading/trailing whitespace but everything else like comments. This
* information is typically used to highlight the range in the editor.
*/
targetRange: Range;

/**
* The range that should be selected and revealed when this link is being
* followed, e.g the name of a function. Must be contained by the
* `targetRange`. See also `DocumentSymbol#range`
*/
targetSelectionRange: Range;

Currently I've implemented targetSelectionRange by dumping a bunch of selection methods onto the Definition class. These are used to figure out which part of each node is the "name" (which can be a bit complicated for some nodes)

These methods could probably be moved to the actual Ast nodes themselves a bit like we have .static? etc?

e.g.

class Ast::Node
    def name : Ast::Node
    end
    # or
    def name_location : Ast::Node::Location
    end
end

---

Finding other nodes in the "stack"

For figuring out the "definition" of a node, I needed to be able to look at other nodes in the stack as provided by server.nodes_at_cursor.

As an example, the stack when hovering over a HtmlStyle node might look like this:

Mint::Ast::Variable
 ↳ Mint::Ast::HtmlStyle
  ↳ Mint::Ast::HtmlElement
   ↳ Mint::Ast::Statement
    ↳ Mint::Ast::Block
     ↳ Mint::Ast::Function
      ↳ Mint::Ast::Component

To find the corresponding Style node, I also need to know which Component the HtmlStyle is in.

To do this, I added a bunch of methods via macro that allow you to look along the stack (next_<node_name> for finding an immediate parent node, and any_<node_name> for any ancestor node), returning Nil if they can't find anything

In code this looks something like this:

return unless variable =
                next_variable stack
return unless next_html_style stack
return unless component =
                any_component stack

I'm not sure if this is a very good or efficient approach, and am open to suggestions for a better one!

[gdotdesign]

• I need to check if the client supports LocationLink's before returning them (via textDocument.definition.linkSupport) and otherwise just return a Location's instead (also looks like I should be returning an array of them as per the spec?)

• Why do we use LocationLink instead of just Location?
• The return type can be many things Location | Location[] | LocationLink[] | null so returning a single location or link is OK

---

While I think the macro implementation is cool 😎 it can be written with just two methods:

def next_node(klass : N.class, stack : Array(Ast::Node)) : N | Nil forall N
  stack.shift?.try &.as?(N)
end

def any_node(klass : N.class, stack : Array(Ast::Node)) : N | Nil forall N
  while node = stack.shift?
    if node.is_a?(N)
      return node
    end
  end
end

And then used as:

next_node Ast::Variable, stack
next_node Ast::HtmlComponent, stack
any_node Ast::Component, stack

I'm not sure, but it might be refactored in a way, so we wouldn't need to create duplicates and have less memory footprint (maybe @Sija has some ideas? 😄 )

---

Otherwise I think it's OK 👍

[jansul]

• Why do we use LocationLink instead of just Location?

I haven't tried just using only Location yet, but I believe you get the cool underline when hovering (like a hyperlink) when using LocationLink so I believe it's worth supporting it.

Shouldn't be too hard to fall back to using Location if it's not supported by the client.

---

While I think the macro implementation is cool 😎 it can be written with just two methods:

Doh, it's easy to try and be too fancy with macros 🙈 I'll make this change!

---

What were your opinions on adding additional methods to Ast::Node? (for finding the "name" part of a node) Or should I just keep that seperate for now?

[gdotdesign]

What were your opinions on adding additional methods to Ast::Node? (for finding the "name" part of a node) Or should I just keep that seperate for now?

I would keep it separate for now, let's see it a common pattern emerges and then we can add it accordingly.

[jansul]

I think this is ready for review now 👍

I've made the changes above, as well as a few others:

• The tests are now closer in format to the type_checking ones. They allow multiple requests and responses, and potentially could be used to test the other LS features
• A few bug fixes, such as pointing to an incorrect location when a component had comments

I've tested it on the recently updated mint-docs-viewer repository and it seems to work reasonable well. Let me know if you find any other bugs. 😄

---

One of the changes is that It now ignores any components that are part of the "core" set, mostly because there is nowhere on disk where vscode etc can find them.

To be able to test this. I removed these lines from spec_helper.cr:

- module Mint
-   class Core
-     class_getter ast : Ast = Ast.new
-   end
- end

Let me know if you would like this back in. I could probably just add a minimal set of components to this for testing rather than all of the core files.

[Sija]

I've left some minor comments but it looks awesome, great job! 🚀