Skip to content

Commit 5b53280

Browse files
docs(code-style-guide): Add section about unwraps (#741)
* docs(code-style-guide): Add section about unwraps * chore: Apply suggestions Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com> * chore(code-style-guide): Use link definitions * chore(code-style-guide): Add bad unwrap example for tests * chore: Apply suggestions from code review Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com> * chore: Apply suggestion Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com> --------- Co-authored-by: Nick <10092581+NickLarsenNZ@users.noreply.github.com>
1 parent 1e80ef8 commit 5b53280

File tree

1 file changed

+71
-0
lines changed

1 file changed

+71
-0
lines changed

modules/contributor/pages/code-style-guide.adoc

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,45 @@ enum Error {
531531
. `unable to read config file from ...` to indicate that the file could not be loaded (for example because the file doesn't exist).
532532
. `unable to parse value ...` to indicate that parsing a user provided value failed (for example because it didn't conform to the expected syntax).
533533

534+
=== Using `unwrap`
535+
536+
:unwrap_or: https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_or
537+
:unwrap_or_default: https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_or_default
538+
:unwrap_or_else: https://doc.rust-lang.org/std/result/enum.Result.html#method.unwrap_or_else
539+
540+
The `unwrap` function must not be used in any code.
541+
Instead, proper error handling like above should be used, unless there is a valid reason to use `expect` described below.
542+
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.
543+
544+
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.
545+
For such cases code must use `expect` instead of `unwrap` to provide additional context for why a particular piece of code should never fail.
546+
547+
// Do we want to mention that this is enforced via clippy and that we actually enable that lint in our repos?
548+
549+
[TIP.code-rule,caption=Examples of correct code for this rule]
550+
====
551+
552+
[source,rust]
553+
----
554+
static VERSION_REGEX: LazyLock<Regex> = LazyLock::new(|| {
555+
Regex::new(r".*").expect("valid regular expression")
556+
});
557+
----
558+
559+
====
560+
561+
[WARNING.code-rule,caption=Examples of incorrect code for this rule]
562+
====
563+
564+
[source,rust]
565+
----
566+
static VERSION_REGEX: LazyLock<Regex> = LazyLock::new(|| {
567+
Regex::new(r".*").unwrap()
568+
});
569+
----
570+
571+
====
572+
534573
== String formatting
535574

536575
=== Named versus unnamed format string identifiers
@@ -737,3 +776,35 @@ mod test {
737776
----
738777
739778
====
779+
780+
=== Using `unwrap`
781+
782+
The usage of `unwrap` in unit tests is also not allowed for the same reasons as mentioned above.
783+
784+
[TIP.code-rule,caption=Examples of correct code for this rule]
785+
====
786+
787+
[source,rust]
788+
----
789+
#[test]
790+
fn deserialize() {
791+
let input: String = serde_yaml::from_str("my string").expect("constant input string must deserialize");
792+
assert_eq(&input, "my string");
793+
}
794+
----
795+
796+
====
797+
798+
[WARNING.code-rule,caption=Examples of incorrect code for this rule]
799+
====
800+
801+
[source,rust]
802+
----
803+
#[test]
804+
fn serialize() {
805+
let serialized = serde_yaml::to_string(&String::from("my string")).unwrap();
806+
println!("{serialized}");
807+
}
808+
----
809+
810+
====

0 commit comments

Comments
 (0)