fix: autocomplete constants inside format strings

[norskeld]

trafficstars

Hi! This PR adds autocompletion for constants (including statics) inside format strings and closes #16608.

I'm not sure about adding the constants field to the CompletionContext. It kinda makes sense, since it's in line with the locals field, and this way everything looks a bit cleaner, but at the same time does it really need to be there?

Anyway, let me know if anything should/can be changed. :)

[lnicola]

This is arguably cosmetic, but I don't think shadowing is handled correctly:

const a: i32 = 10;

fn main() {
    let a = 10;
    format_args!("{a");
}

Here we complete the constant a, not the local variable.

[Veykril]

I think in that case we'd show both even? I don't think that matters, we don't really think about such cases elsewhere either for the time being

[Veykril]

I'm not sure about adding the constants field to the CompletionContext. It kinda makes sense, since it's in line with the locals field, and this way everything looks a bit cleaner, but at the same time does it really need to be there?

I think instead of adding that we should switch that completion part over to use process_all_names directly. We only collect locals eagerly in the context because of param completions which need to do some special stuff I think?

[lnicola]

No, just one, maybe they're de-duped at one point:

I don't even know this icon:

[Veykril]

I never know any of those VSCode icons as they are completely unreadable :D Also fun fact, stuff like this works too, that is any single segment value path that implements the required fmt trait works.

#[derive(Debug)]
struct C;
println!("{C:?}");

(not saying this PR needs to tackle that now, but an observation for the future / new issue to generalize this more)

[norskeld]

Here we complete the constant a, not the local variable.

I'm so used to naming constants using uppercase that didn't even think about a case like that! It'd nice to handle this correctly for sure.

I did some digging and I think the culprit here is hir_def::resolver::Resolver::names_in_scope which is used in process_all_names. It handles shadowing so that if there's a binding and a constant with the same Name (like in your example) in the resulting index map there'll be only the constant.

@Veykril @lnicola What should I do here?

I think instead of adding that we should switch that completion part over to use process_all_names directly. We only collect locals eagerly in the context because of param completions which need to do some special stuff I think?

Yeah, makes sense, I'll change that then.

I don't even know this icon

And statics use a different one for some reason... 😁

[lnicola]

What should I do here?

Ignore it?

I never know any of those VSCode icons as they are completely unreadable :D

Ah, I take it you're not a Visual C++ user. 😀

[norskeld]

Done. Rolled back changes made in CompletionContext and moved them to format_string, so PR is less invasive now.

[Veykril]

Thanks! @bors r+

[bors]

:pushpin: Commit 1d28aecd13a3a19c3a9fdce81e0c50f0b74caf6b has been approved by Veykril

It is now in the queue for this repository.

[bors]

:hourglass: Testing commit 1d28aecd13a3a19c3a9fdce81e0c50f0b74caf6b with merge 518cfe8cb7dc594c7b5e8f3a6448393951b066c6...

[bors]

:sunny: Test successful - checks-actions Approved by: Veykril Pushing 518cfe8cb7dc594c7b5e8f3a6448393951b066c6 to master...