New suggestions engine

[msaraiva]

Closes #98.

This PR changes the logic to provide suggestions by splitting each individual type of suggestion into its own reducer. The main algorithm was simplified a lot and reduced to a single Enum.reduce_while/3 that traverses the list of reducers as you can see here:

elixir_sense/lib/elixir_sense/providers/suggestion.ex

Lines 82 to 95 in 013f490

	def find(hint, text_before, env, buffer_metadata) do
	acc = %{result: [], reducers: Keyword.keys(@reducers), context: %{}}
	%{result: result} =
	Enum.reduce_while(@reducers, acc, fn {key, fun}, acc ->
	if key in acc.reducers do
	fun.(hint, text_before, env, buffer_metadata, acc)
	else
	{:cont, acc}
	end
	end)
	result
	end

The new approach removes a lot of the conditionals that was necessary and allows each reducer to make decisions about how the final list of suggestions should look like based on the information they have about the context. Here's an example:

The list of suggestions provided inside a literal struct can be one of:

• The list of fields
• The list of fields + the of variables (for updates)

Currently, it works like this:

The list of keywords suggested do not apply here, so they just add noise. This is one of many cases where they show up and that's why I'm suggesting to remove them in elixir-lsp/elixir-ls#251. So let's remove them and see how it goes:

We still have a bunch of other suggestions that don't apply here either.

With this PR, there's now a way to easily restrict the list of suggestions based on the context. For our example, the Struct.add_fields/5 reducer can change the final list in 3 different ways:

1. If no field is found, don't do anything
2. If fields are found, add the list of fields to the list and inform that the list is final (no other suggestions will be appended)
3. If fields are found but it could be and update, add the list of fields and inform that, only variables should be still added

The code:

elixir_sense/lib/elixir_sense/providers/suggestion/reducers/struct.ex

Lines 22 to 31 in 013f490

	case find_struct_fields(hint, text_before, env, buffer_metadata) do
	{[], _} ->
	{:cont, acc}
	{fields, nil} ->
	{:halt, %{acc | result: fields}}
	{fields, :maybe_struct_update} ->
	{:cont, %{acc | result: fields, reducers: [:populate_common, :variables]}}
	end

The final result:

Personally, I'd even remove the __struct__ field since its use is discouraged.

The above improvement was already included in this PR. Another similar improvement is regarding the list of callbacks.

Before:

After:

The code:

elixir_sense/lib/elixir_sense/providers/suggestion/reducers/callbacks.ex

Lines 61 to 70 in 013f490

	cond do
	Regex.match?(~r/\s(def|defmacro)\s+[a-z|_]*$/, text_before) ->
	{:halt, %{acc | result: list}}
	match?({_f, _a}, scope) ->
	{:cont, acc}
	true ->
	{:cont, %{acc | result: acc.result ++ list}}
	end

In both cases, the logic was isolated inside each reducer and there was no need to change the main algorithm.

@lukaszsamson I don't think there's any need to review the whole thing since most of the changes was just about moving each piece of logic to its own reducer, however, if you have the time and can take a look at https://github.com/elixir-lsp/elixir_sense/blob/ms-new-suggestion-engine/lib/elixir_sense/providers/suggestion.ex, you'll find more info about the usage so you can selectively review whatever you find necessary.

Important note: I didn't remove the hint suggestion logic nor touched the Complete module. I wanted to minimize the changes in this PR so I'm currently just ignoring it.

[msaraiva]

One thing a forgot to mention is that the new approach not only makes it adding new suggestions a breeze (we just need to add a new reducer), but it also opens opportunities for optimizations. The last argument of a reducer, the context, can be used as a cache where a reducer can add information to it and the next reducers can reuse it. That's what Common.populate/5 does ;)

[lukaszsamson]

Shouldn't some_func/0 be inluded here? Consider code like

var = %{DateTime.now | seconds: 0}

[lukaszsamson]

IMO attributes and functions should be included here

[msaraiva]

Yeah, good catch. Although is more common to see the update syntax being used with variables, functions and attributes are valid suggestions. I'll update it.

[lukaszsamson]

Nice @msaraiva. I was thinking about doing a similar refactoring some day. I left some small comments in the code.

One particular improvement I would like to introduce later is providing more scope context so typespec suggestions show only inside @type, @spec etc.

[msaraiva]

@lukaszsamson regarding:

Personally, I'd even remove the __struct__ field since its use is discouraged.

do you think we should remove it?

[lukaszsamson]

Personally, I'd even remove the struct field since its use is discouraged.

I'd rather leave it. We don't filter other hidden APIs and this one is officially documented. I personally happen to use it now and then. Anyway, there was a plan to mark hidden stuff in metadata and deprioretize such suggestions in elixir-ls.

[axelson]

We might be able to handle __struct__ in ElixirLS by de-prioritizing any completions that start with __ (since they are generally internal).

Opened elixir-lsp/elixir-ls#259 to test removing the static keyword list. But since that list was previously always being added to completions I think it is safe to remove.

[msaraiva]

I also used the __struct__ field in the past since it was the only way to retrieve the module or pattern match against it. However, after the addition of the %mod{...} syntax, I never used it again.

Anyway, I have no problem leaving it and I have the same feeling regarding hidden functions like __info__ and similar.

If we had a configuration for that (which is probably not worth it), I would not include any of those __* stuff. I guess iex takes a similar approach regarding functions, although it includes the __struct__ field. 🤷‍♂️

Maybe just deprioritizing them is the best default.

[msaraiva]

@lukaszsamson I think we're good to go here. As soon as we start updating existing reducers and adding new ones, we'll get more insights about the direction we should take. I think the most important thing here was to split the implementation in individual reducers and centralize the main logic. From now on, any change we need will be much easier to implement.