Improved hover support: Structs, qualified calls and types, more info for modules

[zachallaun]

This PR builds on #331, adding hover support for additional entities:

• Structs
• Qualified calls
• Qualified types

Improvements:

• Generally improved formatting for all hover content.
• Respond with range information to support editor highlighting.
• Include callback signatures and docs for modules that are behaviours.

Notes:

• Add vendored Future.Code.Typespec, which includes required fixes in @spec to make Dialyzer happy.
• Update :sourceror dep to the latest commit until a new release is cut. This latest version updates Sourceror's zippers to be a %Zipper{} struct and includes fixes/improvements to Sourceror.get_range/1.
• Lexical.Ast now uses the unescape: false option when parsing code. See ba4223d for more context.

[zachallaun]

Struct hover:

Qualified call hover:

[zachallaun]

@scohen would appreciate your input on this:

In d1edb31, I added some additional utilities to Lexical.RemoteControl.Modules that deal with extracting information from BEAM object code -- docs, types, specs, and callbacks. While I was looking at Code.Typespec to better understand some things, I noticed that all of the functions will also accept the beam object binary, which means they don't have to read it off disk (and we don't have to wait for just-compiled files to be written to disk).

Unfortunately, this introduced a Dialyzer error -- the @spec for these functions only accepts module() even though the guards are for is_atom(module) or is_binary(module). See here.

Options I can think of:

1. Revert the changes and just operate on modules
2. Vendor Code.Typespec into Future.Code.Typespec, which I think would cause the Dialyzer error to be ignored because lib/elixir/lib/future is in dialyzer.ignore-warnings
3. Add a bunch of @dialyzer {:nowarn_function, ...} attributes, but they'd have to go both in the Modules utility and where it's being used, which is gross.

[scohen]

@zachallaun this looks like a typespec bug in elixir, we should fix that too, then pull the fix into Future.Code, which we do already.

[scottming]

For me, it looks good overall. However, I'm not sure if we need the assert_hover abstraction. Can you explain the reason behind it?

[zachallaun]

For me, it looks good overall. However, I'm not sure if we need the assert_hover abstraction. Can you explain the reason behind it?

I think you're right. It was my attempt to clean up the tests a bit, but I think they'll be just as clear and more explicit just using variables (if a little bit longer).

[zachallaun]

@scohen @scottming This PR is ready for review. Please see the updated OP for additional context/notes.

While I was originally planning for this PR to also include hover support for unqualified calls, I chose to punt on that until the next one. Resolving unqualified calls is a lot more complex, since it requires import tracking, so I plan to address it separately.

[scottming]

Only a few minor comments were left. Overall, I feel that this improvement is excellent.

[scottming]

Another distracting element that I think can be removed is the block separator symbol ---. I find it too bright in the nvim editor, which causes distraction.

Of course, it would be good to have the option to customize the configuration and decide whether we need this separator.

Here's what it looks like when it's removed:

In vscode, the difference is minor.

before

after removing

[zachallaun]

@scottming I think that's fair; perhaps the --- can be reserved for only separating between arities. (It shows docs for arities >= the one currently used.)

This is more of a meta-thought, but I wish there were a way for all of us to easily test these kinds of features in all major editors. Maybe it's worth setting up an "editor test configuration" repo that contains config for all the major editors and scripts to launch the editor using that config. Needs more thought, but could be useful in the long term.

[scottming]

Very good, you're awesome, @zachallaun . Thank you.

Another improvement may be to convert the Examples section to an Elixir code block, but feel free to do that in another PR.

[zachallaun]

Another improvement may be to convert the Examples section to an Elixir code block, but feel free to do that in another PR.

@scottming This is a great idea! I wanted to see if I could sneak it in, but it's unfortunately a bit more complicated than it first seems and, I think, should be in its own PR. Basically, we need to handle all of these cases:

Indented code in an explicit code block shouldn't be re-wrapped:

```
    some_code
```

Multiple indented blocks separated by empty lines should be joined together into a single block:

## Examples

    some_code

    with_an_empty_line_between

Code indented relative to a parent element (like a list item) should be wrapped, but retain the parent's indentation:

## List of items

  - Something with a code example:

        some_code_4_spaces_in_from_Something

I think there's a somewhat simple way to do this that doesn't require a full Markdown parser by basically map-reducing over the lines, looking for lines that start with ``` to ignore until the block ends, and then tracking the indentation level of the previous line so that indented blocks can be identified by current_indent == previous_line_indent + 4.

But it's enough work that I don't want to include it here :)

[scohen]

This looks very good. I like the abstractions and what you've done with cursor support.

I have some comments along the lines of how positions are represented and if we can do better in tests with defining ranges and positions, but this is nearly ready.

[zachallaun]

Draft squash commit:

• Include the range of resolved entities when responding to hover requests

This allows editors to highlight the token that the hover content applies to. For instance, if you hovered Baz in the module Foo.Bar.Baz, the entire module will be highlighted, not just the hovered segment in the module.

• Improve formatting and docs when hovering modules

Behaviours (modules that have @callback attributes) will now show callback specs and below any module docs.

• Don't show return content when hovering modules without docs

This applies to modules that use @moduledoc false or modules that are missing a @moduledoc attribute and don't have other additional docs to show (e.g. behaviour callbacks).

• Add support for hovering structs

When hovering a module being used as a struct, e.g. %MyStruct{}, MyStruct.t types will now be shown before any additional module docs.

• Add support for hovering qualified functions, macros, and types

Qualified calls are those that begin with a module, e.g. MyModule.my_fun(). When hovering qualified functions or macros, the signatures of those calls will be shown followed by any specs and docs. When hovering qualified types, the type definition will be shown followed by any docs.

For functions with multiple arities, the arity currently being used will try to be inferred and those definitions/docs shown first, followed by those for any higher arities.

• [Internal] Add additional utilities to Lexical.Ast, including path_at/2 and contains_position?/2

• [Internal] Add Lexical.Test.CursorSupport.pop_range/2

• [Internal] Add Lexical.Test.RangeSupport.pop_range/1

• [Internal] Update Sourceror to 0.14

This version bump includes more complete and resilient APIs for getting ranges, as well as a new %Zipper{} struct that replaces the old {tree, meta} tuple that previously was used to represent zippers.