feat(compiler): suggest better suggestions for imports on unknown var…

[shellbear]

This improves the overall compiler unknown variables suggestions by listing all imported modules values in the scope so it can now recommends imports.

For example with the following code (described in the following GitHub issue):

import gleam/io

pub fn main() {
  let numbers = [1, 2, 3, 4, 5]
  debug(numbers) // Intended to use io.debug but mistakenly omitted 'io.'
}

The current version of the compiler provides the following error:

error: Unknown variable
  ┌─ /path/to/project/src/my_project.gleam:5:3
  │
5 │   debug(numbers)
  │   ^^^^^ Did you mean `True`?

With my changes the suggestions are a bit smarter:

error: Unknown variable
  ┌─ /Users/shellbear/code/gleamio/src/gleamio.gleam:5:3
  │
5 │   debug(numbers) // Intended to use io.debug but mistakenly omitted 'io.'
  │   ^^^^^ Did you mean `io.debug`?

The name `debug` is not in scope here.

It's my first contribution to the project so I would be really happy to receive any feedback or improvement. I didn't have a look at the full codebase, it was more a first attempt to implement this improvement and learn more about Gleam compiler.

Maybe some unit tests for these changes would be great.

Thanks ✌️

closes #2697

[lpil]

Nice! Thank you! The code looks nice and clean.

We'll need some tests though! To ensure the functionality works and that there are no regressions in future.

I have a feeling that while this works well for io.debug it'll not work so well for modules with longer names. The module. is going to have a big impact on the edit distance so they won't ever be selected as a suggestion. We can write some tests to verify this, but if it turns out to be the case we'll need to use a different approach, like the one I suggested here #2697 (comment).

Thanks again! Very nice work

[juntuu]

Suggested change

	.map(|value| format!("{}.{}", module_name, value).into())
	.collect::<Vec<EcoString>>()
	.map(move |value| format!("{}.{}", module_name, value).into())

Regarding the ci-lint error about flat_map, you can additionally avoid the intermediate .collect()s by moving the module_name reference into the closure. Its lifetime should last fine for the duration of this whole function (or otherwise the borrow checked would yell).

[lpil]

Could use that cool eco_format macro too

[shellbear]

Thanks for the feedback, appreciated. I will update this 🙂

[juntuu]

I think you missed the main part of my suggestion: removing the .collect::<Vec<EcoString>>(). Adding the move just enables that :)

[shellbear]

Updated it, Looks good to you? Thanks :)

[shellbear]

Nice! Thank you! The code looks nice and clean.

We'll need some tests though! To ensure the functionality works and that there are no regressions in future.

I have a feeling that while this works well for io.debug it'll not work so well for modules with longer names. The module. is going to have a big impact on the edit distance so they won't ever be selected as a suggestion. We can write some tests to verify this, but if it turns out to be the case we'll need to use a different approach, like the one I suggested here #2697 (comment).

Thanks again! Very nice work

Yes definitely, it works well for this small example but this implementation might quickly shows his limits with more complex examples. I will continue to look on how to improve this and find a better approach that handle more complex suggestions. If anyone else have some ideas or code solutions, I would be glad to take this as part of this PR.

Thanks 🙂

[lpil]

Closing due to inactivity. Please feel free to reopen! Thank you