Skip to content
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions modules/contributor/pages/code-style-guide.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -533,12 +533,16 @@ enum Error {

=== Using `unwrap`

Generally, it is not recommended to use `unwrap` (or any other method which consumes the error) in any fallible code path.
Instead, proper error handling like above should be used.
There are however cases, where it is fine to use `unwrap` or friends.
:unwrap_or: https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_or
:unwrap_or_default: https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_or_default
:unwrap_or_else: https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_or_else

One such an example is when compiling regular expressions inside const/static environments.
For such cases code must use `expect` instead of `unwrap` to provide additional context why a particular piece of code should never fail.
The `unwrap` function must not be used in any fallible code paths.
Instead, proper error handling like above should be used, unless there is a valid reason to use `expect` described below.
Using link:{unwrap_or}[`unwrap_or`], link:{unwrap_or_default}[`unwrap_or_default`] or link:{unwrap_or_else}[`unwrap_or_else`] is allowed because these functions will not panic.

The `expect` function can be used when external factors cannot influence whether a panic will happen. For example, when compiling regular expressions inside const/static environments.
For such cases code must use `expect` instead of `unwrap` to provide additional context for why a particular piece of code should never fail.

// Do we want to mention that this is enforced via clippy and that we actually enable that lint in our repos?

Expand Down Expand Up @@ -775,7 +779,7 @@ mod test {

=== Using `unwrap`

The usage of `unwrap` in unit tests is not recommended for the same reasons as mentioned above, but allowed.
The usage of `unwrap` in unit tests is also not allowed for the same reasons as mentioned above.

[TIP.code-rule,caption=Examples of correct code for this rule]
====
Expand All @@ -787,7 +791,15 @@ fn deserialize() {
let input: String = serde_yaml::from_str("my string").expect("input string must deserialize");
assert_eq(&input, "my string");
}
----

====

[WARNING.code-rule,caption=Examples of incorrect code for this rule]
====

[source,rust]
----
#[test]
fn serialize() {
let serialized = serde_yaml::to_string(&String::from("my string")).unwrap();
Expand Down
Loading