Skip to content

Plan 007: Make clippy and rustfmt real CI gates #22

Description

@duyetbot

Plan 007: Make clippy and rustfmt real CI gates

Executor instructions: Follow this plan step by step. Run every
verification command and confirm the expected result before moving to the
next step. If anything in the "STOP conditions" section occurs, stop and
report — do not improvise. When done, update the status row for this plan
in plans/README.md — unless a reviewer dispatched you and told you they
maintain the index.

Drift check (run first): git diff --stat 61ee3c7..HEAD -- .github/workflows/ci.yml src/
If source or CI changed since this plan was written, compare "Current state"
excerpts against the live code; on mismatch, STOP.

Status

  • Priority: P2
  • Effort: S/M
  • Risk: LOW (code changes are mechanical lint fixes)
  • Depends on: none (but run AFTER plans 002–006 land, or expect trivial conflicts in shared files)
  • Category: dx
  • Planned at: commit 61ee3c7, 2026-08-26

Why this matters

CI runs cargo fmt --check with continue-on-error: true (a no-op gate) and
never runs clippy at all. The tree currently carries 13 clippy warnings
(8× needless &PathBuf params, a derivable impl, a manual div_ceil, a
useless format!, a thread_local const candidate) — noise that hides real
warnings as the codebase grows, and formatting that drifts between
contributors because nothing enforces it.

Current state

  • .github/workflows/ci.yml lines 21–27:
      - uses: dtolnay/rust-toolchain@stable
        with:
          components: rustfmt, llvm-tools-preview

      - name: cargo fmt --check (optional)
        run: cargo fmt --check
        continue-on-error: true

No clippy invocation anywhere in ci.yml.

  • Baseline warnings (cargo clippy --locked --all-targets at 61ee3c7), all style-level:
    • src/commands.rs:889,1102,1218,1438,2650,2664,3035&PathBuf instead of &Path (clippy::ptr_arg)
    • src/commands.rs:2071 — match looks like matches! (clippy::manual_match)
    • src/help.rs:7:43 — thread_local const initializer
    • src/spawn.rs:37:1 — derivable impl (clippy::derivable_impls)
    • src/term.rs:79 — use div_ceil
    • src/tui/view.rs:72 — useless format!
    • src/upgrade.rs:113unwrap_or_default()

Repo conventions: conventional commits; steps named after their command.

Commands you will need

Purpose Command Expected on success
Lint cargo clippy --locked --all-targets -- -D warnings exit 0 after fixes
Format check cargo fmt --check exit 0
Full suite cargo test --locked --all-targets all pass

Scope

In scope:

  • .github/workflows/ci.yml
  • Source files ONLY to fix the listed baseline warnings: src/commands.rs,
    src/help.rs, src/spawn.rs, src/term.rs, src/tui/view.rs,
    src/upgrade.rs
  • Any file cargo fmt reformats

Out of scope:

  • Any behavioral change; any refactor beyond what each single warning demands.
  • Do NOT add new lints/pedantic groups — just -D warnings on defaults.
  • wasm job: leave --no-default-features clippy out of CI for now.

Git workflow

  • Branch: advisor/007-ci-lint-gates
  • Commit style: e.g. chore(dx): enforce clippy and rustfmt in CI; fix baseline warnings
  • Do NOT push or open a PR.

Steps

Step 1: Fix the baseline clippy warnings

Work through the list in "Current state". Mechanical guidance:

  • &PathBuf params → change signature AND callers' argument expressions stay
    identical (&PathBuf derefs to &Path automatically at call sites when the
    param is &Path; if the body needed PathBuf methods it clones already).
  • spawn.rs:37 derivable impl → replace hand-written Default with
    #[derive(Default)], keeping field values identical.
  • commands.rs:2071 → convert the match to matches!(...).
  • Apply div_ceil, drop useless format!, use unwrap_or_default(),
    make the help.rs thread_local initializer const { ... }.

After each file: cargo build --locked must stay green.

Verify: cargo clippy --locked --all-targets 2>&1 | grep -c "^warning:" || true → prints 0 (or only the final summary line without per-file warnings).

Step 2: Run cargo fmt

Run cargo fmt. Inspect the diff — it should be whitespace/ordering only.

Verify: git diff --stat shows no unexpected files; cargo fmt --check → exit 0.

Step 3: Wire the gates into ci.yml

Replace the fmt block:

      - name: cargo fmt --check
        run: cargo fmt --check

(no continue-on-error). Add before the llvm-cov step:

      - name: cargo clippy
        run: cargo clippy --locked --all-targets -- -D warnings

Verify: YAML parse: python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))" → exit 0; grep -A1 "cargo clippy" .github/workflows/ci.yml shows -D warnings and no continue-on-error on either step.

Step 4: Full suite

Verify: cargo test --locked --all-targets → exit 0.

Test plan

No new tests — this is tooling + mechanical fixes. The full suite is the
regression net; the lint commands themselves are the acceptance checks.

Done criteria

ALL must hold:

  • cargo clippy --locked --all-targets -- -D warnings exits 0.
  • cargo fmt --check exits 0.
  • ci.yml has a clippy step with -D warnings and a fmt step WITHOUT continue-on-error.
  • cargo test --locked --all-targets exits 0.
  • git status --porcelain lists only ci.yml + files from the warning list + formatted files.

STOP conditions

Stop and report if:

  • A clippy fix would require changing behavior (e.g. removing a match arm)
    rather than syntax.
  • Fixing spawn.rs:37 changes Default VALUES (the derive must produce identical output — verify against tests).
  • fmt reformats files outside src/ unexpectedly (e.g. build.rs heavily).

Maintenance notes

  • New warnings will now fail CI; contributors should run clippy locally first
    (worth one line in CONTRIBUTING later).
  • Reviewer: spot-check two of the &PathBuf&Path conversions for accidental
    clone introduction.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions