Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explore introducing semgrep to enforce stricter coding guidelines #2865

Open
tnull opened this issue Jan 31, 2024 · 3 comments
Open

Explore introducing semgrep to enforce stricter coding guidelines #2865

tnull opened this issue Jan 31, 2024 · 3 comments

Comments

@tnull
Copy link
Contributor

tnull commented Jan 31, 2024

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:

  • Require any non-lock()ing unwrap() to be accompanied by a // safety: comment.
  • Disallow usage of SystemTime::now / Instant::now / Instant::elapsed_since to maintain WASM compatibility.

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

(cc @tcharding)

@tcharding
Copy link
Contributor

@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
Copy link

dpc commented Jan 31, 2024

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
Copy link
Contributor

Simple now I read it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants