Skip to content

Commit

Permalink
Enable lints to prevent unwanted usages of unwrap(), assert! and …
Browse files Browse the repository at this point in the history
…`panic!`

The `clippy::unwrap_used` lint prevent 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 prevent 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 and #710.

The lints have not been enabled for the integration test modules (since it's
pointless, as they only apply to non-test usages), 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
  • Loading branch information
edmorley committed Nov 2, 2023
1 parent 2698a71 commit bebd998
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 8 deletions.
1 change: 1 addition & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-unwrap-in-tests = true
4 changes: 3 additions & 1 deletion libcnb-cargo/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![doc = include_str!("../README.md")]
#![warn(clippy::pedantic)]
#![warn(unused_crate_dependencies)]
#![warn(clippy::pedantic)]
#![warn(clippy::panic_in_result_fn)]
#![warn(clippy::unwrap_used)]
// This lint is too noisy and enforces a style that reduces readability in many cases.
#![allow(clippy::module_name_repetitions)]

Expand Down
4 changes: 3 additions & 1 deletion libcnb-common/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![doc = include_str!("../README.md")]
#![warn(clippy::pedantic)]
#![warn(unused_crate_dependencies)]
#![warn(clippy::pedantic)]
#![warn(clippy::panic_in_result_fn)]
#![warn(clippy::unwrap_used)]
// This lint is too noisy and enforces a style that reduces readability in many cases.
#![allow(clippy::module_name_repetitions)]

Expand Down
4 changes: 3 additions & 1 deletion libcnb-data/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![doc = include_str!("../README.md")]
#![warn(clippy::pedantic)]
#![warn(unused_crate_dependencies)]
#![warn(clippy::pedantic)]
#![warn(clippy::panic_in_result_fn)]
#![warn(clippy::unwrap_used)]
// This lint is too noisy and enforces a style that reduces readability in many cases.
#![allow(clippy::module_name_repetitions)]

Expand Down
4 changes: 3 additions & 1 deletion libcnb-package/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![doc = include_str!("../README.md")]
#![warn(clippy::pedantic)]
#![warn(unused_crate_dependencies)]
#![warn(clippy::pedantic)]
#![warn(clippy::panic_in_result_fn)]
#![warn(clippy::unwrap_used)]
// This lint is too noisy and enforces a style that reduces readability in many cases.
#![allow(clippy::module_name_repetitions)]

Expand Down
4 changes: 3 additions & 1 deletion libcnb-proc-macros/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![doc = include_str!("../README.md")]
#![warn(clippy::pedantic)]
#![warn(unused_crate_dependencies)]
#![warn(clippy::pedantic)]
#![warn(clippy::panic_in_result_fn)]
#![warn(clippy::unwrap_used)]

use proc_macro::TokenStream;
use quote::quote;
Expand Down
11 changes: 10 additions & 1 deletion libcnb-test/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ use std::collections::BTreeMap;
use std::fs;
use std::path::{Path, PathBuf};

/// Packages the current crate as a buildpack into a temporary directory.
/// Packages the current crate as a buildpack into the provided directory.
// TODO: Convert the `assert!` usages to an additional `PackageBuildpackError` variant instead:
// https://github.com/heroku/libcnb.rs/issues/709
#[allow(clippy::panic_in_result_fn)]
pub(crate) fn package_crate_buildpack(
cargo_profile: CargoProfile,
target_triple: impl AsRef<str>,
Expand All @@ -38,6 +41,9 @@ pub(crate) fn package_crate_buildpack(
)
}

// TODO: Convert the `assert!` usages to an additional `PackageBuildpackError` variant instead:
// https://github.com/heroku/libcnb.rs/issues/709
#[allow(clippy::panic_in_result_fn)]
pub(crate) fn package_buildpack(
buildpack_id: &BuildpackId,
cargo_profile: CargoProfile,
Expand Down Expand Up @@ -87,6 +93,9 @@ pub(crate) fn package_buildpack(
for node in &build_order {
let buildpack_destination_dir = buildpack_dir_resolver(&node.buildpack_id);

// TODO: Convert the `unwrap()` to an additional `PackageBuildpackError` variant instead:
// https://github.com/heroku/libcnb.rs/issues/710
#[allow(clippy::unwrap_used)]
fs::create_dir_all(&buildpack_destination_dir).unwrap();

libcnb_package::package::package_buildpack(
Expand Down
2 changes: 2 additions & 0 deletions libcnb-test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// Enable lints that are disabled by default.
#![warn(unused_crate_dependencies)]
#![warn(clippy::pedantic)]
#![warn(clippy::panic_in_result_fn)]
#![warn(clippy::unwrap_used)]
// This lint is too noisy and enforces a style that reduces readability in many cases.
#![allow(clippy::module_name_repetitions)]

Expand Down
4 changes: 3 additions & 1 deletion libcnb/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#![doc = include_str!("../README.md")]
#![warn(clippy::pedantic)]
#![warn(unused_crate_dependencies)]
#![warn(clippy::pedantic)]
#![warn(clippy::panic_in_result_fn)]
#![warn(clippy::unwrap_used)]
// Most of libcnb's public API returns user-provided errors, making error docs redundant.
#![allow(clippy::missing_errors_doc)]
// This lint is too noisy and enforces a style that reduces readability in many cases.
Expand Down
4 changes: 3 additions & 1 deletion libherokubuildpack/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#![doc = include_str!("../README.md")]
// Enable lints that are disabled by default.
#![warn(clippy::pedantic)]
#![warn(unused_crate_dependencies)]
#![warn(clippy::pedantic)]
#![warn(clippy::panic_in_result_fn)]
#![warn(clippy::unwrap_used)]
// In most cases adding error docs provides little value.
#![allow(clippy::missing_errors_doc)]
// This lint is too noisy and enforces a style that reduces readability in many cases.
Expand Down

0 comments on commit bebd998

Please sign in to comment.