Better error message for semicolon on last line of function #11482
Conversation
| if ends_with_stmt { | ||
| self.tcx.sess.span_err( | ||
| body.stmts.last().span, "not all control paths return a value, maybe \ | ||
| there is a semicolon on the last line of a this non-void function?"); |
There was a problem hiding this comment.
There is ExprSemi which represents an expression with a semicolon, so I think the compiler can actually answer this question itself.
|
I suggest putting the hint in a separate note, and putting the span right on the offending semicolon. Something like this: |
|
I agree with @lfairy about how to present this. We could probably try even harder and check if the type of the final statement unifies with the function return type. |
|
Thanks for the feedback! I've added a note about the semicolon and check if the type of the expression of the last statement matches the return type. But I'm not sure if this is the best way to check if the types match. |
| let span_semicolon = Span { | ||
| lo: span_stmt.hi, | ||
| hi: span_stmt.hi, | ||
| expn_info: None |
There was a problem hiding this comment.
This seems a little fishy setting it to None, I think this is used for macros, so are you sure this doesn't mess up the note positioning?
Something like
macro_rules! foo ( () => (fn foo() -> int { 3i; }) )There was a problem hiding this comment.
I think you are right. Can I pass the expn_info from the surrounding statement to the new span? I've pushed an updated commit with this change.
There was a problem hiding this comment.
The best way to check is to add a test :)
(I think this would be tested by a macro like macro_rules! test ( () => { fn foo() -> int { 1i; } } ).)
There was a problem hiding this comment.
I've added this macro to the test, the note points to the semicolon in the macro.
…st-stmt, r=alexcrichton This is a patch for #8005, thanks @lfairy for the hint. It seems like `block.expr` is None, if the last line of a function has a semi colon (= it ends with a statement). @kmcallister does this error message cover the intended use cases? I'm not sure about the message, the wording and the span could probably be improved.
…1995 Add colored help to be consistent with Cargo On rust-lang/cargo#12578, a new colored help message format was introduced. This PR introduces the same styling from that `cargo help` message to our `cargo clippy --help` message. More information is provided in the original PR, fixes rust-lang#11482. The perfect reviewing process would be that the reviewer installs this branch and checks for themselves, but here are some screenshots, there are some more screenshots in the original issue.  (Note that the actual text may change in the actual commit, that screenshot is just to test the colors). Also note that the `color-print` version **should always** be synced with Cargo's `color-print` version to avoid increasing build times in the rust-lang/rust repo. changelog:Add colors to the `cargo clippy --help` output 🎉.
This is a patch for #8005, thanks @lfairy for the hint.
It seems like
block.expris None, if the last line of a function has a semi colon (= it ends with a statement).@kmcallister does this error message cover the intended use cases?
I'm not sure about the message, the wording and the span could probably be improved.