Show parameter hints for None parameters

[matklad]

Would be cool to know which None means what. Guess we need to just refactor the API, but having a hit would be a useful bandaid.

I think we literarly should just check for None, rather than use a more general rule, like "is this a const item". Though not sure here.

cc @SomeoneToIgnore

[lnicola]

Wait, we already have parameter hints, but they don't seem to work on that function most functions even though it does they do get resolved?

[matklad]

it workds with literal parameters:

[lnicola]

Ah. I guess it would be annoying if they were always shown? What does IntelliJ do? Displaying the hint for 92 but not for Some(92) doesn't strike me as particularly useful.

[SomeoneToIgnore]

What does IntelliJ do?

Nothing different from our case.

I think that having an inlay hints for all None cases might be a bit verbose.
Should we instead add this info to our hover hints?

[lnicola]

Nothing different from our case.

Um, so it always displays the parameter hints?

[SomeoneToIgnore]

Right, that should be formulated as

Nothing different from our case for type hints.

[SomeoneToIgnore]

For the context, here's the way to simplify the code proposed before: #1606 (comment)
(by using this way, presumably None type hints should be added automatically; although it needs some further investigation and tweaking)

I've applied the suggestion, but BindPat::pat gives me None for every inlay hint test I run, currently I'm trying to understand what am I doing wrong.

[SomeoneToIgnore]

I've sorted it out.

Old hints:

New hints:

How do you like the results?

[lnicola]

It does seem a bit busy when combined with field matching, but I could get used to it. Does it also work for function parameters?

[SomeoneToIgnore]

Not yet.

[SomeoneToIgnore]

Personally, I'm not sure we should add type hints to the function parameters.
Name hints is another story, here's how it looks like after my changes:

Now we're more or less on par with what intellij-rust displays for the functions/methods.

Imagine adding the type hints there (or, even worse, to a function with its parameters written in one line): IMO, this will be hard to comprehend.

So, should hovers be enough for displaying the function parameters' types after all?

[kjeremy]

So, should hovers be enough for displaying the function parameters' types after all?

I think so. We should definitely have name hints since that's more important for reading code. If you need type info you can get it through hover or signature help.

[bjorn3]

What about something like "if type is Foo and argument name is foo, don't show name inlay hint" and "if expr is something like foo or bar.foo and argument name is foo, don't show name inlay hint"? That omits the name inlay hint in the cases where the arg name is most obvious.

[SomeoneToIgnore]

In general, sounds like a good idea to implement, but maybe later, when we collect enough cases, along with the similar type hints ideas from #3022?

I feel like in the end we should come up with a set of toggles for those cases also, similar to what Rider has

[lnicola]

Personally, I'm not sure we should add type hints to the function parameters.

Oh, yeah, I was totally talking about name hints for function parameters.

[SomeoneToIgnore]

What about something like "if type is Foo and argument name is foo, don't show name inlay hint" and "if expr is something like foo or bar.foo and argument name is foo, don't show name inlay hint"? That omits the name inlay hint in the cases where the arg name is most obvious.

I've added a few heuristics here: #3279
But it's not over yet, so we'd better come up with a separate ticket to gather the ideas there.

[0x7CFE]

It does seem a bit busy when combined with field matching, but I could get used to it.

Agree. Consider the following example:

Here we see that inlay hint does not provide any new information... except for a ton of visual noise. Just by looking at match arms I already can infer types, more or less. So I'd rather say that None hint would work only in case of a standalone literal without any surrounding context.

Also it could get very noisy in case of several hints per line of code:

Maybe we should have some heuristics and limit not only the length of a hint, but the total length of all hints per line. And then either opt out hints completely, or reduce their complexity by displaying less nested types.

[SomeoneToIgnore]

So I'd rather say that None hint would work only in case of a standalone literal without any surrounding context.

Yes, I agree with you after using the new hints for a while, the PR fixing this is being merged to master right now.

For the other part of your proposal, seems valid, we need to investigate the possible solutions some time later.
I've added the text to the corresponding issue: #3138