Skip to content

Commit

Permalink
Add must_use attributes and enable clippy::must-use-candidate (#232)
Browse files Browse the repository at this point in the history
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:
rust-lang/rust#48926 (comment)

Fixes #57.
GUS-W-10222390.
  • Loading branch information
edmorley authored Dec 9, 2021
1 parent ac8f17f commit 930bd2a
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 6 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
4 changes: 4 additions & 0 deletions libcnb-data/src/build_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub struct BuildPlan {
}

impl BuildPlan {
#[must_use]
pub fn new() -> Self {
Self {
provides: vec![],
Expand All @@ -35,6 +36,7 @@ pub struct BuildPlanBuilder {
}

impl BuildPlanBuilder {
#[must_use]
pub fn new() -> Self {
Self {
acc: VecDeque::new(),
Expand All @@ -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));
Expand All @@ -62,6 +65,7 @@ impl BuildPlanBuilder {
self
}

#[must_use]
pub fn build(self) -> BuildPlan {
let mut xyz = self.or();

Expand Down
2 changes: 2 additions & 0 deletions libcnb-data/src/launch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -38,6 +39,7 @@ impl Launch {
}
}

#[must_use]
pub fn process(mut self, process: Process) -> Self {
self.processes.push(process);
self
Expand Down
3 changes: 0 additions & 3 deletions libcnb-data/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions libcnb/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ pub struct BuildResultBuilder {
}

impl BuildResultBuilder {
#[must_use]
pub fn new() -> Self {
Self {
launch: None,
Expand All @@ -166,18 +167,21 @@ impl BuildResultBuilder {
Ok(self.build_unwrapped())
}

#[must_use]
pub fn build_unwrapped(self) -> BuildResult {
BuildResult(InnerBuildResult::Pass {
launch: self.launch,
store: self.store,
})
}

#[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
Expand Down
5 changes: 5 additions & 0 deletions libcnb/src/detect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}
}
Expand All @@ -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
Expand All @@ -102,6 +106,7 @@ impl FailDetectResultBuilder {
}

#[allow(clippy::unused_self)]
#[must_use]
pub fn build_unwrapped(self) -> DetectResult {
DetectResult(InnerDetectResult::Fail)
}
Expand Down
5 changes: 5 additions & 0 deletions libcnb/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -57,15 +59,18 @@ impl Env {
}

/// Returns a cloned value corresponding to the given key.
#[must_use]
pub fn get(&self, key: impl AsRef<OsStr>) -> Option<OsString> {
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<OsStr>) -> bool {
self.inner.contains_key(key.as_ref())
}

#[must_use]
pub fn iter(&self) -> std::collections::hash_map::Iter<'_, OsString, OsString> {
self.inner.iter()
}
Expand Down
1 change: 1 addition & 0 deletions libcnb/src/generic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ pub struct GenericPlatform {
}

impl GenericPlatform {
#[must_use]
pub fn new(env: Env) -> Self {
Self { env }
}
Expand Down
3 changes: 3 additions & 0 deletions libcnb/src/layer/public_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,15 @@ pub struct LayerResultBuilder<M> {
}

impl<M> LayerResultBuilder<M> {
#[must_use]
pub fn new(metadata: M) -> Self {
Self {
metadata,
env: None,
}
}

#[must_use]
pub fn env(mut self, layer_env: LayerEnv) -> Self {
self.env = Some(layer_env);
self
Expand All @@ -199,6 +201,7 @@ impl<M> LayerResultBuilder<M> {
Ok(self.build_unwrapped())
}

#[must_use]
pub fn build_unwrapped(self) -> LayerResult<M> {
LayerResult {
metadata: self.metadata,
Expand Down
3 changes: 3 additions & 0 deletions libcnb/src/layer_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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],
Expand Down Expand Up @@ -243,6 +245,7 @@ impl LayerEnv {
/// ),
/// );
/// ```
#[must_use]
pub fn chainable_insert(
mut self,
target: TargetLifecycle,
Expand Down
3 changes: 0 additions & 3 deletions libcnb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 930bd2a

Please sign in to comment.