ANN: annotate wrong return type for empty functions

[Kobzol]

Fixes: #5166

[mchernyavsky]

Maybe it's better to annotate the function's closing bracket? (just like in Kotlin)

[Kobzol]

If we want to do this: #4833 then it would be consistent to mark the return type (although it might be missing in case of unit type, hmm).. Otherwise I'll change it to annotate the closing brace.

[Kobzol]

@Stzx I'm sorry, I don't understand, what do you mean?

[Kobzol]

@mchernyavsky What error type marking should be used? Only the return type, whole function, closing brace?

[mchernyavsky]

@mchernyavsky What error type marking should be used? Only the return type, whole function, closing brace?

I prefer the closing brace. @Undin @vlad20012 What do you think?

[vlad20012]

This is the reason why the PR is not merged yet? Oh...
I would not say that this is important. We can change it later. Let's make it somehow. cargo check already highlight a return type, so we won't make things worse if we do that too.
Btw #4833 can also be implemented by } brace highlighting

[Kobzol]

This is the reason why the PR is not merged yet? Oh...

Yup 😅

The closing brace is probably more general, since we cannot mark the return type if it's missing. Should I change it to the brace?

[vlad20012]

Should I change it to the brace?

Oh 😅
Okay, let's chenge

[Kobzol]

I changed it to the closing brace for empty functions. The ChangeReturnTypeFix is not currently offered for them. It would require non-trivial changes to it to make it work, and I don't think that you usually want to change the return type in situations when the function is empty. If you want the fix to work, let me know.

To solve #4833 with brace highlighting, it would have to only highlight the brace in the case of a trailing expr (and highlight return expressions for return statements, as it does now), which might be a bit confusing.

[vlad20012]

bors r+

[vlad20012]

Hmm, I think I found a false-positive case:

trait FooBar {}
impl FooBar for () {}

fn foobar() -> impl FooBar {

}

bors r-

[bors]

Canceled.

[Kobzol]

Oh, that's a very... interesting case :D Well, to support cases such as this one, I wonder if maybe the error annotation for empty functions should be moved into TypeInferenceWalker? Or should I just change the current implementation to use something like TypeInference::combineTypes to see if the result type is compatible with ()?

[vlad20012]

TypeInferenceWalker looks more robust solution, but impl Trait is poorly supported even there. Let's just ignore impl Traits at all for now

[vlad20012]

bors r+

[bors]

Build succeeded:

• check