Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Improve error message on Err early returns that forgot the return keyword #86094

Closed
sdroege opened this issue Jun 7, 2021 · 4 comments · Fixed by #115196
Closed

Improve error message on Err early returns that forgot the return keyword #86094

sdroege opened this issue Jun 7, 2021 · 4 comments · Fixed by #115196
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sdroege
Copy link
Contributor

sdroege commented Jun 7, 2021

Given the following code: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=d5eabab7099a911f6bb9e93e9b7a0886

struct MyError;

fn foo(x: bool) -> Result<(), MyError> {
    if x {
        Err(MyError);
    }
    
    Ok(())
}

The current output is:

error[E0282]: type annotations needed
 --> src/main.rs:5:9
  |
5 |         Err(MyError);
  |         ^^^ cannot infer type for type parameter `T` declared on the enum `Result`

Ideally the output should look like:

error[E0282]: type annotations needed
 --> src/main.rs:5:9
  |
5 |         Err(MyError);
  |         ^^^ cannot infer type for type parameter `T` declared on the enum `Result`
help: try using `return` here:
  |
5 |         return Err(MyError);
  |         ^^^
@sdroege sdroege added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 7, 2021
@sdroege
Copy link
Contributor Author

sdroege commented Jun 7, 2021

For the slightly modified case without the ; the compiler already gives a useful hint

error[E0308]: mismatched types
 --> src/main.rs:5:9
  |
4 | /     if x {
5 | |         Err(MyError)
  | |         ^^^^^^^^^^^^ expected `()`, found enum `Result`
6 | |     }
  | |_____- expected this to be `()`
  |
  = note: expected unit type `()`
                  found enum `Result<_, MyError>`
help: you might have meant to return this value
  |
5 |         return Err(MyError);
  |         ^^^^^^             ^

@m-ou-se
Copy link
Member

m-ou-se commented Jun 7, 2021

For the slightly modified case without the ; the compiler already gives a useful hint

That suggestion is a special case of the mismatched types lint. The version with ; will need special casing on a very different lint, unfortunately.

When a type-mismatch occurs, this checks if the type matches the return type of the surrounding function:

if let hir::FnRetTy::Return(ty) = fn_decl.output {
let ty = <dyn AstConv<'_>>::ast_ty_to_ty(self, ty);
let bound_vars = self.tcx.late_bound_vars(fn_id);
let ty = self.tcx.erase_late_bound_regions(Binder::bind_with_vars(ty, bound_vars));
let ty = self.normalize_associated_types_in(expr.span, ty);
if self.can_coerce(found, ty) {
err.multipart_suggestion(
"you might have meant to return this value",
vec![
(expr.span.shrink_to_lo(), "return ".to_string()),
(expr.span.shrink_to_hi(), ";".to_string()),
],
Applicability::MaybeIncorrect,
);
}
}

For the 'type annotations needed' lint, I don't think we can apply the same logic.

We might also want a similar suggestion for unused_must_use, in cases where the type is fully known.

@JohnTitor JohnTitor added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jun 7, 2021
@inquisitivecrystal
Copy link
Contributor

@rustbot label +D-confusing +D-newcomer-roadblock

@rustbot rustbot added D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. labels Jun 8, 2021
@estebank
Copy link
Contributor

Current output:

error[E0282]: type annotations needed
 --> f71.rs:5:9
  |
5 |         Err(MyError);
  |         ^^^ cannot infer type of the type parameter `T` declared on the enum `Result`
  |
help: consider specifying the generic arguments
  |
5 |         Err::<T, MyError>(MyError);
  |            ++++++++++++++

We still don't realize that you probably wanted to return this expression.

@estebank estebank added the D-papercut Diagnostics: An error or lint that needs small tweaks. label Aug 14, 2023
@chenyukang chenyukang self-assigned this Aug 19, 2023
@bors bors closed this as completed in 1de910f Oct 16, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 16, 2023
Rollup merge of rust-lang#115196 - chenyukang:yukang-fix-86094, r=estebank

Suggest adding `return` if the for semi which can coerce to the fn return type

Fixes rust-lang#86094
r? `@estebank`
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants