Skip to content

chore(lint): ban unwrap by default — workspace clippy policy #13

Description

@zackees

Context

This template is greenfield from a lint standpoint:

  • Cargo.toml has no [workspace.lints] block.
  • Repo root has no clippy.toml.
  • Only three Rust source files (crates/template-cli/src/main.rs, crates/template-core/src/lib.rs, crates/template-py/src/lib.rs) and a code search for unwrap in the repo returns zero matches — i.e., there is no existing cleanup cost.

Because this is a template, every project spawned from it inherits the policy. Baking in the right defaults here pays back in every downstream repo. The right time to ban unwrap is before the first one is written, not after.

Proposal

Add the following to Cargo.toml:

```toml
[workspace.lints.clippy]
unwrap_used = "deny"
unwrap_in_result = "deny"
panic_in_result_fn = "deny"
expect_used = "warn"
```

Each member crate (template-core, template-cli, template-py) opts in with:

```toml
[lints]
workspace = true
```

Add a clippy.toml at workspace root so tests stay ergonomic:

```toml
allow-unwrap-in-tests = true
allow-expect-in-tests = true
allow-panic-in-tests = true

allow-unwrap-in-consts = true
allow-expect-in-consts = true
```

Why this policy and not a broader one

  • unwrap_used = \"deny\" — bans the blind landmine. Forces ? propagation or a documented expect.
  • unwrap_in_result = \"deny\" — catches .unwrap() inside a function that already returns Result, where ? is strictly better.
  • panic_in_result_fn = \"deny\" — same reasoning for explicit panic!().
  • expect_used = \"warn\"expect(\"hardcoded regex must compile\") is the correct idiom for documented invariants, so this stays advisory rather than denied. Reviewers can challenge case-by-case.
  • Not enabling clippy::restriction wholesale (Clippy's own docs warn against it — intentionally strict, sometimes contradictory).
  • Not enabling panic = \"deny\" workspace-wide — legitimate unreachable-assertion panic!() calls are part of the design surface.

Acceptance criteria

  • [workspace.lints.clippy] block added to Cargo.toml with the four-lint policy above.
  • Each of the three member crates declares [lints] workspace = true.
  • clippy.toml at workspace root carries the five allow-*-in-tests / allow-*-in-consts knobs.
  • ./ci.sh clippy passes locally with no source changes required (current code has zero unwrap references — verify before merging).
  • GitHub Actions ci.yml passes on the PR.

Decisions

  • Lint set: exactly the four lines specified. No bonus lints (no await_holding_lock, no indexing_slicing, etc.) — narrow scope, easier to land. Bonus lints can come in follow-ups if the maintainer wants them.
  • expect_used = \"warn\" left as warn, not promoted to deny. Caveat: if the template's clippy invocation (visible in ci.py) runs with -D warnings, warn becomes a hard failure in CI. If so, either (a) accept that and treat expect_used as effectively denied, or (b) drop -D warnings from the clippy invocation. The maintainer should decide; the policy itself is correct as written.
  • clippy.toml ships in the same PR. Without it, cargo test becomes painful because #[test] functions are no longer covered by the allow-list. Bundling it keeps the template usable on day one.
  • Priority: P3. Cosmetic / quality-of-life for downstream template consumers. Not blocking any feature.
  • No README update bundled. The lint policy is self-documenting via Cargo.toml; downstream readers will see it immediately when they crack open the file.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions