From 930bd2a661abc11df16c46aea4928db80caf920e Mon Sep 17 00:00:00 2001 From: Ed Morley <501702+edmorley@users.noreply.github.com> Date: Thu, 9 Dec 2021 08:53:17 +0000 Subject: [PATCH] Add `must_use` attributes and enable `clippy::must-use-candidate` (#232) If a semicolon discards the result of a function or method tagged with `#[must_use]`, the compiler will emit a lint message warning that the return value is expected to be used. This lint suggests adding `#[must_use]` to public functions that: - return something that's not already marked as `must_use` - have no mutable *reference* args (having a mutable owned arg is fine, since as the method took ownership, the caller can only access the result via the returned self, so it therefore still must be used) - don't mutate statics Docs: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute https://rust-lang.github.io/rust-clippy/master/index.html#must_use_candidate For more on best practices of when to use `must_use`, see: https://github.com/rust-lang/rust/issues/48926#issuecomment-769286972 Fixes #57. GUS-W-10222390. --- CHANGELOG.md | 2 ++ libcnb-data/src/build_plan.rs | 4 ++++ libcnb-data/src/launch.rs | 2 ++ libcnb-data/src/lib.rs | 3 --- libcnb/src/build.rs | 4 ++++ libcnb/src/detect.rs | 5 +++++ libcnb/src/env.rs | 5 +++++ libcnb/src/generic.rs | 1 + libcnb/src/layer/public_interface.rs | 3 +++ libcnb/src/layer_env.rs | 3 +++ libcnb/src/lib.rs | 3 --- 11 files changed, 29 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b39a6524..0eebee9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## [Unreleased] +- Add `must_use` attributes to a number of pure public methods. + ## [0.4.0] 2021/12/08 - Add `PartialEq` and `Eq` implementations for `Process`. diff --git a/libcnb-data/src/build_plan.rs b/libcnb-data/src/build_plan.rs index 3afe7b5f..ef185855 100644 --- a/libcnb-data/src/build_plan.rs +++ b/libcnb-data/src/build_plan.rs @@ -13,6 +13,7 @@ pub struct BuildPlan { } impl BuildPlan { + #[must_use] pub fn new() -> Self { Self { provides: vec![], @@ -35,6 +36,7 @@ pub struct BuildPlanBuilder { } impl BuildPlanBuilder { + #[must_use] pub fn new() -> Self { Self { acc: VecDeque::new(), @@ -53,6 +55,7 @@ impl BuildPlanBuilder { self } + #[must_use] pub fn or(mut self) -> Self { self.acc .push_back((self.current_provides, self.current_requires)); @@ -62,6 +65,7 @@ impl BuildPlanBuilder { self } + #[must_use] pub fn build(self) -> BuildPlan { let mut xyz = self.or(); diff --git a/libcnb-data/src/launch.rs b/libcnb-data/src/launch.rs index 5bb0a0f4..4acecca9 100644 --- a/libcnb-data/src/launch.rs +++ b/libcnb-data/src/launch.rs @@ -29,6 +29,7 @@ pub struct Launch { /// assert!(toml::to_string(&launch_toml).is_ok()); /// ``` impl Launch { + #[must_use] pub fn new() -> Self { Self { bom: bom::Bom::new(), @@ -38,6 +39,7 @@ impl Launch { } } + #[must_use] pub fn process(mut self, process: Process) -> Self { self.processes.push(process); self diff --git a/libcnb-data/src/lib.rs b/libcnb-data/src/lib.rs index f174536b..07cb2dc0 100644 --- a/libcnb-data/src/lib.rs +++ b/libcnb-data/src/lib.rs @@ -7,9 +7,6 @@ #![warn(clippy::pedantic)] // This lint is too noisy and enforces a style that reduces readability in many cases. #![allow(clippy::module_name_repetitions)] -// Re-disable pedantic lints that are currently failing, until they are triaged and fixed/wontfixed. -// https://github.com/Malax/libcnb.rs/issues/57 -#![allow(clippy::must_use_candidate)] pub mod bom; pub mod build; diff --git a/libcnb/src/build.rs b/libcnb/src/build.rs index 605558a7..f9aa18cc 100644 --- a/libcnb/src/build.rs +++ b/libcnb/src/build.rs @@ -146,6 +146,7 @@ pub struct BuildResultBuilder { } impl BuildResultBuilder { + #[must_use] pub fn new() -> Self { Self { launch: None, @@ -166,6 +167,7 @@ impl BuildResultBuilder { Ok(self.build_unwrapped()) } + #[must_use] pub fn build_unwrapped(self) -> BuildResult { BuildResult(InnerBuildResult::Pass { launch: self.launch, @@ -173,11 +175,13 @@ impl BuildResultBuilder { }) } + #[must_use] pub fn launch(mut self, launch: Launch) -> Self { self.launch = Some(launch); self } + #[must_use] pub fn store(mut self, store: Store) -> Self { self.store = Some(store); self diff --git a/libcnb/src/detect.rs b/libcnb/src/detect.rs index b2ee7a3c..a41912de 100644 --- a/libcnb/src/detect.rs +++ b/libcnb/src/detect.rs @@ -46,10 +46,12 @@ pub(crate) enum InnerDetectResult { pub struct DetectResultBuilder; impl DetectResultBuilder { + #[must_use] pub fn pass() -> PassDetectResultBuilder { PassDetectResultBuilder { build_plan: None } } + #[must_use] pub fn fail() -> FailDetectResultBuilder { FailDetectResultBuilder {} } @@ -73,12 +75,14 @@ impl PassDetectResultBuilder { Ok(self.build_unwrapped()) } + #[must_use] pub fn build_unwrapped(self) -> DetectResult { DetectResult(InnerDetectResult::Pass { build_plan: self.build_plan, }) } + #[must_use] pub fn build_plan(mut self, build_plan: BuildPlan) -> Self { self.build_plan = Some(build_plan); self @@ -102,6 +106,7 @@ impl FailDetectResultBuilder { } #[allow(clippy::unused_self)] + #[must_use] pub fn build_unwrapped(self) -> DetectResult { DetectResult(InnerDetectResult::Fail) } diff --git a/libcnb/src/env.rs b/libcnb/src/env.rs index cbcb5136..62a391e5 100644 --- a/libcnb/src/env.rs +++ b/libcnb/src/env.rs @@ -38,11 +38,13 @@ impl Env { /// variables afterwards will not be reflected in the returned value. /// /// See [`std::env::vars_os`] + #[must_use] pub fn from_current() -> Self { env::vars_os().into() } /// Creates an empty `Env` struct. + #[must_use] pub fn new() -> Self { Self { inner: HashMap::new(), @@ -57,15 +59,18 @@ impl Env { } /// Returns a cloned value corresponding to the given key. + #[must_use] pub fn get(&self, key: impl AsRef) -> Option { self.inner.get(key.as_ref()).cloned() } /// Returns true if the environment contains a value for the specified key. + #[must_use] pub fn contains_key(&self, key: impl AsRef) -> bool { self.inner.contains_key(key.as_ref()) } + #[must_use] pub fn iter(&self) -> std::collections::hash_map::Iter<'_, OsString, OsString> { self.inner.iter() } diff --git a/libcnb/src/generic.rs b/libcnb/src/generic.rs index 1dee9450..e04b1a26 100644 --- a/libcnb/src/generic.rs +++ b/libcnb/src/generic.rs @@ -24,6 +24,7 @@ pub struct GenericPlatform { } impl GenericPlatform { + #[must_use] pub fn new(env: Env) -> Self { Self { env } } diff --git a/libcnb/src/layer/public_interface.rs b/libcnb/src/layer/public_interface.rs index 49987f87..7f903ea2 100644 --- a/libcnb/src/layer/public_interface.rs +++ b/libcnb/src/layer/public_interface.rs @@ -175,6 +175,7 @@ pub struct LayerResultBuilder { } impl LayerResultBuilder { + #[must_use] pub fn new(metadata: M) -> Self { Self { metadata, @@ -182,6 +183,7 @@ impl LayerResultBuilder { } } + #[must_use] pub fn env(mut self, layer_env: LayerEnv) -> Self { self.env = Some(layer_env); self @@ -199,6 +201,7 @@ impl LayerResultBuilder { Ok(self.build_unwrapped()) } + #[must_use] pub fn build_unwrapped(self) -> LayerResult { LayerResult { metadata: self.metadata, diff --git a/libcnb/src/layer_env.rs b/libcnb/src/layer_env.rs index 198d618b..4bc7ce76 100644 --- a/libcnb/src/layer_env.rs +++ b/libcnb/src/layer_env.rs @@ -113,6 +113,7 @@ impl LayerEnv { /// let modified_env = layer_env.apply(TargetLifecycle::Build, &env); /// assert_eq!(env, modified_env); /// ``` + #[must_use] pub fn new() -> Self { Self { all: LayerEnvDelta::new(), @@ -143,6 +144,7 @@ impl LayerEnv { /// assert_eq!(modified_env.get("VAR").unwrap(), "foobar"); /// assert_eq!(modified_env.get("VAR2").unwrap(), "previous-value"); /// ``` + #[must_use] pub fn apply(&self, target: TargetLifecycle, env: &Env) -> Env { let deltas = match target { TargetLifecycle::All => vec![&self.all], @@ -243,6 +245,7 @@ impl LayerEnv { /// ), /// ); /// ``` + #[must_use] pub fn chainable_insert( mut self, target: TargetLifecycle, diff --git a/libcnb/src/lib.rs b/libcnb/src/lib.rs index a5aaf3c8..9d6c1e69 100644 --- a/libcnb/src/lib.rs +++ b/libcnb/src/lib.rs @@ -11,9 +11,6 @@ #![allow(clippy::module_name_repetitions)] // This lint triggers when both layer_dir and layers_dir are present which are quite common. #![allow(clippy::similar_names)] -// Re-disable pedantic lints that are currently failing, until they are triaged and fixed/wontfixed. -// https://github.com/Malax/libcnb.rs/issues/57 -#![allow(clippy::must_use_candidate)] pub mod build; pub mod detect;