Rollup merge of #96033 - yaahc:expect-elaboration, r=scottmcm

Add section on common message styles for Result::expect

Based on a question from rust-lang/project-error-handling#50 (comment)

~~One thing I haven't decided on yet, should I duplicate this section on `Option::expect`, link to this section, or move it somewhere else and link to that location from both docs?~~: I ended up moving the section to `std::error` and referencing it from both `Result::expect` and `Option::expect`'s docs.

I think this section, when combined with the similar update I made on [`std::panic!`](https://doc.rust-lang.org/nightly/std/macro.panic.html#when-to-use-panic-vs-result) implies that we should possibly more aggressively encourage and support the "expect as precondition" style described in this section. The consensus among the libs team seems to be that panic should be used for bugs, not expected potential failure modes. The "expect as error message" style seems to align better with the panic for unrecoverable errors style where they're seen as normal errors where the only difference is a desire to kill the current execution unit (aka erlang style error handling). I'm wondering if we should be providing a panic hook similar to `human-panic` or more strongly recommending the "expect as precondition" style of expect message.

core/src/option.rs

@@ -708,6 +708,26 @@ impl<T> Option<T> {
     /// let x: Option<&str> = None;
     /// x.expect("fruits are healthy"); // panics with `fruits are healthy`
     /// ```
+    ///
+    /// # Recommended Message Style
+    ///
+    /// We recommend that `expect` messages are used to describe the reason you
+    /// _expect_ the `Option` should be `Some`.
+    ///
+    /// ```should_panic
+    /// # let slice: &[u8] = &[];
+    /// let item = slice.get(0)
+    ///     .expect("slice should not be empty");
+    /// ```
+    ///
+    /// **Hint**: If you're having trouble remembering how to phrase expect
+    /// error messages remember to focus on the word "should" as in "env
+    /// variable should be set by blah" or "the given binary should be available
+    /// and executable by the current user".
+    ///
+    /// For more detail on expect message styles and the reasoning behind our
+    /// recommendation please refer to the section on ["Common Message
+    /// Styles"](../../std/error/index.html#common-message-styles) in the [`std::error`](../../std/error/index.html) module docs.
     #[inline]
     #[track_caller]
     #[stable(feature = "rust1", since = "1.0.0")]

core/src/result.rs

@@ -1023,6 +1023,26 @@ impl<T, E> Result<T, E> {
     /// let x: Result<u32, &str> = Err("emergency failure");
     /// x.expect("Testing expect"); // panics with `Testing expect: emergency failure`
     /// ```
+    ///
+    /// # Recommended Message Style
+    ///
+    /// We recommend that `expect` messages are used to describe the reason you
+    /// _expect_ the `Result` should be `Ok`.
+    ///
+    /// ```should_panic
+    /// let path = std::env::var("IMPORTANT_PATH")
+    ///     .expect("env variable `IMPORTANT_PATH` should be set by `wrapper_script.sh`");
+    /// ```
+    ///
+    /// **Hint**: If you're having trouble remembering how to phrase expect
+    /// error messages remember to focus on the word "should" as in "env
+    /// variable should be set by blah" or "the given binary should be available
+    /// and executable by the current user".
+    ///
+    /// For more detail on expect message styles and the reasoning behind our recommendation please
+    /// refer to the section on ["Common Message
+    /// Styles"](../../std/error/index.html#common-message-styles) in the
+    /// [`std::error`](../../std/error/index.html) module docs.
     #[inline]
     #[track_caller]
     #[stable(feature = "result_expect", since = "1.4.0")]

std/src/error.rs

@@ -1,4 +1,140 @@
-//! Traits for working with Errors.
+//! Interfaces for working with Errors.
+//!
+//! # Error Handling In Rust
+//!
+//! The Rust language provides two complementary systems for constructing /
+//! representing, reporting, propagating, reacting to, and discarding errors.
+//! These responsibilities are collectively known as "error handling." The
+//! components of the first system, the panic runtime and interfaces, are most
+//! commonly used to represent bugs that have been detected in your program. The
+//! components of the second system, `Result`, the error traits, and user
+//! defined types, are used to represent anticipated runtime failure modes of
+//! your program.
+//!
+//! ## The Panic Interfaces
+//!
+//! The following are the primary interfaces of the panic system and the
+//! responsibilities they cover:
+//!
+//! * [`panic!`] and [`panic_any`] (Constructing, Propagated automatically)
+//! * [`PanicInfo`] (Reporting)
+//! * [`set_hook`], [`take_hook`], and [`#[panic_handler]`][panic-handler] (Reporting)
+//! * [`catch_unwind`] and [`resume_unwind`] (Discarding, Propagating)
+//!
+//! The following are the primary interfaces of the error system and the
+//! responsibilities they cover:
+//!
+//! * [`Result`] (Propagating, Reacting)
+//! * The [`Error`] trait (Reporting)
+//! * User defined types (Constructing / Representing)
+//! * [`match`] and [`downcast`] (Reacting)
+//! * The question mark operator ([`?`]) (Propagating)
+//! * The partially stable [`Try`] traits (Propagating, Constructing)
+//! * [`Termination`] (Reporting)
+//!
+//! ## Converting Errors into Panics
+//!
+//! The panic and error systems are not entirely distinct. Often times errors
+//! that are anticipated runtime failures in an API might instead represent bugs
+//! to a caller. For these situations the standard library provides APIs for
+//! constructing panics with an `Error` as it's source.
+//!
+//! * [`Result::unwrap`]
+//! * [`Result::expect`]
+//!
+//! These functions are equivalent, they either return the inner value if the
+//! `Result` is `Ok` or panic if the `Result` is `Err` printing the inner error
+//! as the source. The only difference between them is that with `expect` you
+//! provide a panic error message to be printed alongside the source, whereas
+//! `unwrap` has a default message indicating only that you unwraped an `Err`.
+//!
+//! Of the two, `expect` is generally preferred since its `msg` field allows you
+//! to convey your intent and assumptions which makes tracking down the source
+//! of a panic easier. `unwrap` on the other hand can still be a good fit in
+//! situations where you can trivially show that a piece of code will never
+//! panick, such as `"127.0.0.1".parse::<std::net::IpAddr>().unwrap()` or early
+//! prototyping.
+//!
+//! # 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` should be `Ok`. With this style we would
+//! prefer to write:
+//!
+//! ```should_panic
+//! let path = std::env::var("IMPORTANT_PATH")
+//!     .expect("env variable `IMPORTANT_PATH` should be set by `wrapper_script.sh`");
+//! ```
+//!
+//! The "expect as error message" style does not work as well with the
+//! default output of the std panic hooks, and often ends up repeating
+//! information that is already communicated by the source error being
+//! unwrapped:
+//!
+//! ```text
+//! thread 'main' panicked at 'env variable `IMPORTANT_PATH` is not set: NotPresent', src/main.rs:4:6
+//! ```
+//!
+//! In this example we end up mentioning that an env variable is not set,
+//! followed by our source message that says the env is not present, the
+//! only additional information we're communicating is the name of the
+//! environment variable being checked.
+//!
+//! 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.
+//! Also, by framing our expect in terms of what "SHOULD" have happened to
+//! prevent the source error, we end up introducing new information that is
+//! independent from our source error.
+//!
+//! ```text
+//! thread 'main' panicked at 'env variable `IMPORTANT_PATH` should be set by `wrapper_script.sh`: NotPresent', src/main.rs:4:6
+//! ```
+//!
+//! In this example we are communicating not only the name of the
+//! environment variable that should have been set, but also an explanation
+//! for why it should have been set, and we let the source error display as
+//! a clear contradiction to our expectation.
+//!
+//! **Hint**: If you're having trouble remembering how to phrase
+//! expect-as-precondition style error messages remember to focus on the word
+//! "should" as in "env variable should be set by blah" or "the given binary
+//! should be available and executable by the current user".
+//!
+//! [`panic_any`]: crate::panic::panic_any
+//! [`PanicInfo`]: crate::panic::PanicInfo
+//! [`catch_unwind`]: crate::panic::catch_unwind
+//! [`resume_unwind`]: crate::panic::resume_unwind
+//! [`downcast`]: crate::error::Error
+//! [`Termination`]: crate::process::Termination
+//! [`Try`]: crate::ops::Try
+//! [panic hook]: crate::panic::set_hook
+//! [`set_hook`]: crate::panic::set_hook
+//! [`take_hook`]: crate::panic::take_hook
+//! [panic-handler]: <https://doc.rust-lang.org/nomicon/panic-handler.html>
+//! [`match`]: ../../std/keyword.match.html
+//! [`?`]: ../../std/result/index.html#the-question-mark-operator-
 
 #![stable(feature = "rust1", since = "1.0.0")]