lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.11k stars 345 forks source link

Explore introducing `semgrep` to enforce stricter coding guidelines #2865

Open tnull opened 5 months ago

tnull commented 5 months ago

Recent bugs and discussions highlighted that we may want to enforce some stricter (automated) code checks.

In particular, we may want to introduce semgrep to:

Similar approaches are currently applied by other projects in the rust-bitcoin ecosystem, related usages are for example:

(cc @tcharding)

tcharding commented 5 months ago

@dpc should be credited with finding semgrep not me :)

Require any non-lock()ing unwrap() to be accompanied by a // safety: comment.

Could disallow any non-lock()ing unwraps altogether and use expect. Not sure how you could whitelist unit tests though, not doing so might be annoying. (In rust-bitcoin we have an (unwritten) no-unwrap outside of unit tests policy.

dpc commented 5 months ago

Not sure how you could whitelist unit tests though, not doing so might be annoying.

Ban unwrap, except in functions that have #[test]? Not perfect but better than nothing.

tcharding commented 5 months ago

Simple now I read it.