style and clippy

[jordens]

CI does not run clippy and style/lints have apparently been a bit overlooked.
Since this is also true in macros (e.g. the box_*!()) this directly impacts downstream crates that care about clippy lints and style.
We should

• add clippy (all features) to the CI and
• address the clippy and style lints

[newAM]

Can you provide an example for reproducing this? This issue should be fixed, but I would like to double check before making a release.

[jordens]

@newAM Thanks a bunch for quickly adressing this.
An example is here:

https://github.com/quartiq/stabilizer/pull/807/files#diff-287c7b365cd67662db5fce6120a366c3e467d8d383e7624323f50b6ee2532249

https://github.com/quartiq/stabilizer/actions/runs/6847918046/job/18617087392#step:4:247

Note that making it camel case to satisfy the type naming leads to the complaint about the static value not being UPPER_CASE.
This pattern is exactly what the docstring suggests. Maybe we should make sure the docstrings runs as a doctest.

heapless/src/pool/boxed.rs

Lines 62 to 78 in d9b06bd

	//! ```
	//! use heapless::{box_pool, pool::boxed::BoxBlock};
	//!
	//! box_pool!(P: u128);
	//!
	//! const POOL_CAPACITY: usize = 8;
	//!
	//! let blocks: &'static mut [BoxBlock<u128>] = {
	//! const BLOCK: BoxBlock<u128> = BoxBlock::new(); // <=
	//! static mut BLOCKS: [BoxBlock<u128>; POOL_CAPACITY] = [BLOCK; POOL_CAPACITY];
	//! unsafe { &mut BLOCKS }
	//! };
	//!
	//! for block in blocks {
	//! P.manage(block);
	//! }
	//! ```

[newAM]

Thanks!

Maybe we should make sure the docstrings runs as a doctest.

As far as I know there's no (easy) way to run doctests as clippy lints (rust-lang/rust#56232)

[jordens]

Ah. That's unfortunate.

Maybe just not calling the pool P would also show up in the logs at least (P conveniently is both ALL_CAPS as well as CamelCase ;)
Similarly we should add tests for the pattern that is mentioned in the docs (with the const initializer).

[newAM]

Maybe just not calling the pool P would also show up in the logs at least (P conveniently is both ALL_CAPS as well as CamelCase ;)

Yeah that's an unfortunate miss in testing.

I added a test/fix for it here: #417. I feel like there should be a better way to express this though 🤔

Similarly we should add tests for the pattern that is mentioned in the docs (with the const initializer).

Yeah, we need coverage for that too as part of a normal test. I'm still thinking about the best way to handle this one.

[newAM]

I think for the const initializer that clippy lint should simply be suppressed in the example code (#418).

From the documentation for the declare_interior_mutable_const lint it says:

A "non-constant" const item is a legacy way to supply an initialized value to downstream static items (e.g., the std::sync::ONCE_INIT constant). In this case the use of const is legit, and this lint should be suppressed.

This is what we are doing here - using a "non-constant" const item to initialize a downstream static value.

What do you think?

[jordens]

Agreed

[jordens]

Those allows should then be applied to arc_pool!() and object_pool!() as well, no?

[newAM]

Yup! Thanks for pointing that out. I'm not familiar with all the code in heapless, I appreciate you pointing that out :)