Enable initial support for completions

[Schottkyc137]

With this PR I want to lay the groundwork / discuss possible implementations for completions.

This PR enables the following two basic completions:

library |

gives the user the choice of all currently selected libraries and

use ieee.st|

gives the user the choice of all primary units starting with st (such as std_logic_1164, std_logic_arith, ...)

While the current implementation works, it is fundamentally based on pattern matching. A token-stream is generated until the position of the cursor. Based on the last couple of input parameters, suggestions for the following parameter is given.
This approach works quite well for simple examples but it has the caveat that logic needs to be duplicated. The parser already expects certain values and the logic embedded in the parser could potentially be reused. However, I did not find a way to use what's already implemented in a meaningful way without rewriting a lot of code. If there is another approach I'm happy to discuss / implement that. For now, I think that this approach might be a good start.

Edit: Also contains initial support for "regions" to make completions more focused. I.e. in declarative regions other completions should be triggered compared to sequential regions or others. Nested regions are also supported
Example:

|

entity my_ent is
end my_ent;

architecture arch of my_ent is

begin

end arch;

Should trigger completions for library, use, entity, ...
while

entity my_ent is
end my_ent;

architecture arch of my_ent is
    |
begin

end arch;

should trigger completions for declarative items, i.e. signal, constant, e.t.c.

[kraigher]

I think it is a good design decision to keep the completion code separate from the parsing code. They will have different goals and it will be complex to try to share code and it will not provide a benefit.

As you write the user benefits from the completion to be aware of the context where it is triggered.
But also having global completion without context is very valuable.
Currently the VSCode client has completion snippets but there is nothing in the server.
It is thus a very good area to contribute to the server as it will benefit all clients.

The challenging part is to make the completion more and more context aware.
Ideally it would want to know which variables are in scope and which record elements exists for selected names.

[kraigher]

I took a first look at the code. The code looks nice overall. The only thing I did not like was the hardcoded positions in the test case. This is something I have really wanted to avoid as it maskes reviewing, changing and debugging test cases very tiresome.

A more high level comment is if it is premature to enable completions of keywords such as "loop" etc.
In VSCode the "loop" keyword will complete to a complete snippet with begin/end where the user is asked to "fill-in" the blanks.
You can see the snippets here: https://github.com/VHDL-LS/rust_hdl_vscode/blob/master/snippets/vhdl.json.
Just having the keyword appear on its own is probably not as useful.

I think where the server can really improve the completion is that it actually knows which libraries, design units and functions are defined.

[Schottkyc137]

The challenging part is to make the completion more and more context aware. Ideally it would want to know which variables are in scope and which record elements exists for selected names.

I guess a good starting point here is to somehow store some more analysis results to have them for later analysis. At the moment, if I'm not wrong, a Scope is created and filled with symbols, but then discarded once it's no longer needed for analysis (correct me if I'm wrong). Ideally, this should be stored somewhere.

I took a first look at the code. The code looks nice overall. The only thing I did not like was the hardcoded positions in the test case. This is something I have really wanted to avoid as it maskes reviewing, changing and debugging test cases very tiresome.

I will change that, thanks for the feedback!

A more high level comment is if it is premature to enable completions of keywords such as "loop" etc. In VSCode the "loop" keyword will complete to a complete snippet with begin/end where the user is asked to "fill-in" the blanks. You can see the snippets here: https://github.com/VHDL-LS/rust_hdl_vscode/blob/master/snippets/vhdl.json. Just having the keyword appear on its own is probably not as useful.

I can remove the bare word completions for now. In future, however, it might be beneficial to remove the snippets from the VSCode extension and put them into the server. The server should be able to provide the same capabilities and the completions can be a bit more context aware. At the moment, for example, the entarch snippets gets recommended anywhere but ideally you would only want to see it at the top level. Similarly, ifgen should only be recommended in an architecture body. But this requires some more work.

[kraigher]

I guess a good starting point here is to somehow store some more analysis results to have them for later analysis. At the moment, if I'm not wrong, a Scope is created and filled with symbols, but then discarded once it's no longer needed for analysis (correct me if I'm wrong). Ideally, this should be stored somewhere.

Yes the Scope is discarded after the analysis of the design unit is completed. Actually as soon as any nested scope is closed that information is discarded. There is a Scope created for every nesting of declarative region as it is a tree structure that follows the structure of the AST. The Scope keeps track of both definitions within declarative regions as well as the visibility from use clauses. It also acts as a cache for fast lookups. So the current Scope struct is not a good candidate for permanent storage of information for later lookups.

What is really needed for completion is a list of symbols annotated by the interval in the source file where they are visible. Such data could maybe be generated in parallel with the scope. Alternatively an AST traversal could be done when a completion was triggered. The AST would be traversed until the point of completion and all visible symbols recorded.

For performance reasons it is probably better to do extra work at the time of completion rather than creating a completion specific data-structure during every analysis. Re-analysis will happen every time the user types but completion will happen much more seldom. Thus I would initially prefer to do a search of the AST to re-create the scope at the completion point.

Another issue with completing names in scope is that the source file must not be "broken". When the user is typing something and trigger completion it is often results in an illegal VHDL file because they only partially typed a function signature. It is then important that the parser can recover and skip the broken part to finish the analysis of the file. Otherwise there will be no symbols since VHDL LS stopped processing the file.

I can remove the bare word completions for now. In future, however, it might be beneficial to remove the snippets from the VSCode extension and put them into the server. The server should be able to provide the same capabilities and the completions can be a bit more context aware.

Yes the goal should be to remove the snippets from the client and ship them with the server. Then all clients will behave the same. Until that point it makes no sense to add bare word completions.

[kraigher]

I have no further comment. I tried it locally in vscode and it offers me use-clause completions. Is this ready to be merged for your side?

[Schottkyc137]

I have no further comment. I tried it locally in vscode and it offers me use-clause completions. Is this ready to be merged for your side?

I'll add 2-3 unit tests, just for regression (I noticed I had nothing for the actual completion). But then it should be good to go

[Schottkyc137]

Okay I added the tests. I had to be a bit creative (can't use LibraryBuilder::code() because LibraryBuilder::get_analyzed_root() would error out on incorrect code) so let me know what you think about these, but if you're content then this is good to go from my side