Skip to content

Explore introducing semgrep to enforce stricter coding guidelines #2865

Open
@tnull

Description

@tnull

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)

Activity

tcharding

tcharding commented on Jan 31, 2024

@tcharding
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

dpc commented on Jan 31, 2024

@dpc

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

tcharding commented on Jan 31, 2024

@tcharding
Contributor

Simple now I read it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @dpc@tnull@tcharding

        Issue actions

          Explore introducing `semgrep` to enforce stricter coding guidelines · Issue #2865 · lightningdevkit/rust-lightning