log_error(), log_warning(), log_info() and log_header() should use .expect() instead of .unwrap()
#712
Labels
Comments
edmorley
added a commit
that referenced
this issue
Nov 2, 2023
…`panic!` (#711) The `clippy::unwrap_used` lint prevents usage of `.unwrap()` outside of tests: https://rust-lang.github.io/rust-clippy/master/index.html#/unwrap_used The `clippy::panic_in_result_fn` lint prevents usages of `assert!` or `panic!` in functions that already return a `Result`: https://rust-lang.github.io/rust-clippy/master/index.html#/panic_in_result_fn These lints catch cases like #709, #710 and #712. The lints have not been enabled for the integration test modules (since using `unwrap()` is fine there), or to the example buildpacks, since it's a lot of boilerplate to add to an example. Very soon we will be able to switch to the upcoming `[lints]` table feature, which will mean the duplication of lint definitions can be avoided entirely: https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#lints GUS-W-14413286.
Merged
|
Note: The |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently there are lots of usages of
unwrap()in production code, in the logging helpers in:https://github.com/heroku/libcnb.rs/blob/2698a713edf19034e382aa702e12acc1516da8fa/libherokubuildpack/src/log.rs
Whilst the nature of these failure modes means that we can't use a Result (and so panicing is appropriate) - we should use
.expect()instead of an.unwrap()so at least the panic message is more descriptive.The text was updated successfully, but these errors were encountered: