Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add section on common message styles for Result::expect #96033

Merged
merged 12 commits into from
May 26, 2022
3 changes: 3 additions & 0 deletions library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,6 +708,9 @@ impl<T> Option<T> {
/// let x: Option<&str> = None;
/// x.expect("fruits are healthy"); // panics with `fruits are healthy`
/// ```
///
/// **Note**: Please refer to the documentation on [`Result::expect`] for further information
/// on common message styles.
yaahc marked this conversation as resolved.
Show resolved Hide resolved
#[inline]
#[track_caller]
#[stable(feature = "rust1", since = "1.0.0")]
Expand Down
56 changes: 56 additions & 0 deletions library/core/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,62 @@ impl<T, E> Result<T, E> {
/// let x: Result<u32, &str> = Err("emergency failure");
/// x.expect("Testing expect"); // panics with `Testing expect: emergency failure`
/// ```
///
/// # Common Message Styles
///
/// There are two common styles for how people word `expect` messages. Using the message to
/// present information to users encountering a panic ("expect as error message") or using the
/// message to present information to developers debugging the panic ("expect as
/// precondition").
///
/// In the former case the expect message is used to describe the error that has occurred which
/// is considered a bug. Consider the following example:
///
/// ```should_panic
/// // Read environment variable, panic if it is not present
/// let path = std::env::var("IMPORTANT_PATH").unwrap();
/// ```
///
/// In the "expect as error message" style we would use expect to describe that the environment
/// variable was not set when it should have been:
///
/// ```should_panic
/// let path = std::env::var("IMPORTANT_PATH")
/// .expect("env variable `IMPORTANT_PATH` is not set");
/// ```
///
/// In the "expect as precondition" style, we would instead describe the reason we _expect_ the
/// `Result` will always be `Ok`. With this style we would prefer to write:
///
/// ```should_panic
/// let path = std::env::var("IMPORTANT_PATH")
/// .expect("env variable `IMPORTANT_PATH` is always be set by `wrapper_script.sh`");
yaahc marked this conversation as resolved.
Show resolved Hide resolved
/// ```
///
/// The "expect as error message" style has the advantage of giving a more user friendly error
/// message, and is more consistent with the default output of the [panic hook] provided by
/// `std`.
///
/// ```text
/// thread 'main' panicked at 'env variable `IMPORTANT_PATH` is not set: NotPresent', src/main.rs:4:6
/// ```
///
/// The "expect as precondition" style instead focuses on source code readability, making it
/// easier to understand what must have gone wrong in situations where panics are being used to
/// represent bugs exclusively. But this extra information often looks confusing when presented
/// directly to users with the default `std` panic hook's report format:
///
/// ```text
/// thread 'main' panicked at 'env variable `IMPORTANT_PATH` is always be set by `wrapper_script.sh`: NotPresent', src/main.rs:4:6
yaahc marked this conversation as resolved.
Show resolved Hide resolved
/// ```
///
/// This style works best when paired with a custom [panic hook] like the one provided by the
/// CLI working group library, [`human-panic`], which dumps the panic messages to a crash
/// report file while showing users a more friendly "Oops, something went wrong!" message with
/// a suggestion to send the crash report file back to the developers.
///
/// [panic hook]: https://doc.rust-lang.org/stable/std/panic/fn.set_hook.html
/// [`human-panic`]: https://docs.rs/human-panic
yaahc marked this conversation as resolved.
Show resolved Hide resolved
#[inline]
#[track_caller]
#[stable(feature = "result_expect", since = "1.4.0")]
Expand Down