Skip to content

ci(rust): enforce -D warnings on clippy#1008

Open
drew wants to merge 3 commits intomainfrom
enforce-lint-checks
Open

ci(rust): enforce -D warnings on clippy#1008
drew wants to merge 3 commits intomainfrom
enforce-lint-checks

Conversation

@drew
Copy link
Copy Markdown
Collaborator

@drew drew commented Apr 28, 2026

Summary

Promote cargo clippy from warning-only to error-on-warning across the workspace and the e2e/rust crate so CI can gate on a clean lint run. Resolves every existing lint surfaced by the workspace all/pedantic/nursery config.

Related Issue

None — direct branch.

Changes

  • tasks/rust.toml: rust:lint now runs cargo clippy --workspace --all-targets -- -D warnings and additionally lints e2e/rust/Cargo.toml.
  • ~110 source files updated to fix clippy lints surfaced once -D warnings is in effect. Most fixes are mechanical:
    • Inlined format args ({var})
    • Collapsed nested if let / if into let-chains (&&)
    • Option::map_or_else / is_some_and / is_none_or rewrites
    • From::from for lossless casts; cast_signed/cast_unsigned/try_from for narrowing casts
    • let ... else { return } in place of match returns
    • Doc backticks around proper nouns and identifiers
    • r"..." instead of r#"..."# for raw strings without embedded quotes
    • pub(crate)pub inside private modules where redundant
    • [(...)].into_iter()std::iter::once(...)
  • A small number of narrow, function/field-scoped #[allow(...)] attributes were added with justification comments where the suggested fix would change a public/trait signature, an FFI/unsafe contract, a serde wire shape, or a tonic::Status return shape. No new entries were added to the workspace-level allow list in Cargo.toml.

Testing

  • mise run lint passes (workspace + e2e clippy with -D warnings, format check, license headers, markdown lint, etc.)
  • mise run rust:check passes
  • cargo test --workspace --no-fail-fast — all unit/integration tests pass except sandbox_create_keeps_sandbox_with_forwarding, which fails on main as well due to a local Docker process holding port 8080 (environmental, unrelated to this PR).

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (n/a — no behavior change)

@drew drew requested a review from a team as a code owner April 28, 2026 15:27
@drew drew self-assigned this Apr 28, 2026
@drew drew added the test:e2e Requires end-to-end coverage label Apr 28, 2026
@github-actions
Copy link
Copy Markdown

Label test:e2e applied for 8891f41. Open the existing run and click Re-run all jobs to execute with the label set. The E2E Gate check on this PR will flip green automatically once the run finishes.

@drew drew force-pushed the enforce-lint-checks branch from 8891f41 to 5945d08 Compare April 29, 2026 01:20
pimlock
pimlock previously approved these changes Apr 29, 2026
drew added 2 commits April 28, 2026 19:23
Promote cargo clippy from warning-only to error-on-warning across the
workspace and the e2e crate. Resolves all existing clippy lints
surfaced by the workspace pedantic/nursery config so CI can gate on a
clean lint run.

Most fixes are mechanical (inline format args, collapse if-let chains,
Option::map_or_else, From::from for lossless casts, doc backticks,
let-else, raw string hash trimming). A small number of narrow,
function-scoped #[allow(...)] attributes were added with comments where
clippy's suggestion would change a public/trait signature, FFI/unsafe
contracts, or a serde-deserialized protocol shape; no new entries were
added to the workspace-level allow list in Cargo.toml.
CI uses rustc 1.95 which catches lints not present in 1.93. Apply
suggested fixes:
- map(...).unwrap_or(false) -> is_ok_and(...)
- map(f).unwrap_or(default) -> map_or(default, f)
- Drop unnecessary trailing commas in macro args
- Replace manual is_some_and pattern with .is_ok_and(...)
- Replace sort_by(|a,b| a.cmp(b)) with sort_by_key
- Add module-level allow(result_large_err) on driver gRPC handlers
  that stream tonic::Status, mirroring openshell-server's grpc/* files.
@drew drew force-pushed the enforce-lint-checks branch from 3223cf9 to 1465762 Compare April 29, 2026 02:23
pimlock
pimlock previously approved these changes Apr 29, 2026
PR #1028 (preserve directory basename for filtered uploads) changed
the on-disk layout so that filtered directory uploads land under
`<dest>/<basename>/...` rather than `<dest>/...`, matching the
unfiltered upload semantics.

The accompanying e2e test was not updated and now fails because it
still expects `tracked.txt` directly under the download root. Update
the assertions to look under the preserved `repo/` basename.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants