Skip to content

Add CI check enforcing the unwrap/expect/panic clippy deny across all crates #45

Description

@christabel888

Context: Important correction: this repo (contracts) has no .github/workflows directory at all. The monorepo it was split from had contract-fuzzing.yml, gas-regression.yml, and a clippy.yml that ran working-directory: ./contracts -- but those lived at the monorepo root, not nested under contracts/, so git filter-repo --path contracts/ did not carry them over. They're now dead weight in Stellar-inights (pointing at a directory that no longer exists there) and simply absent here.

Where:

  • .github/workflows/clippy.yml (new in this repo)
  • Reference: Stellar-Insightss/Stellar-inights still has the old .github/workflows/clippy.yml you can adapt (drop the ./backend job, keep only the ./contracts job, fix the working directory since it's now the repo root)

What to do:

  • Recreate a clippy CI job that runs at the workspace root (no working-directory needed now) and enforces unwrap_used = deny, expect_used = deny, panic = deny -- these are already configured in [workspace.lints.clippy] in the root Cargo.toml, this issue is about enforcing that in CI, not configuring the lint.

Suggested approach:

  1. Pull the old .github/workflows/clippy.yml from Stellar-Insightss/Stellar-inights (it still exists there, just dead) as your starting point rather than writing a Soroban/Rust clippy workflow from scratch.
  2. Strip the ./backend-specific job (that workflow originally covered both backend and contracts in one file) and drop the working-directory: ./contracts since this repo's root is the contracts workspace now.
  3. Point it at rustup target add wasm32-unknown-unknown plus a cargo clippy --workspace --all-targets -- -D warnings (or equivalent) so the [workspace.lints.clippy] deny rules in the root Cargo.toml actually get enforced somewhere, not just locally.
  4. Test it on a throwaway branch with a deliberate unwrap() added to confirm the workflow actually fails before you consider this done.

Watch out for:

  • [workspace.lints.clippy] being configured in Cargo.toml does not mean CI enforces it -- a contributor who never runs cargo clippy locally can merge violations today. Confirm this workflow is what closes that gap, don't assume the Cargo.toml config alone is sufficient.
  • Match the Rust toolchain version this workspace actually needs (check for a rust-toolchain.toml or the edition in Cargo.toml) rather than defaulting to stable blindly.

Definition of done:

  • .github/workflows/clippy.yml exists and runs on PRs/pushes
  • CI fails if unwrap()/expect()/panic! is introduced in contract code
  • Old dead clippy.yml/contract-fuzzing.yml/gas-regression.yml in Stellar-inights are removed or fixed as a follow-up (flagged separately)

Activity

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

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions