Rust Clippy Triage

Clippy triage for the Rocky engine

The CI rule

.github/workflows/engine-ci.yml runs:

cargo clippy --all-targets --all-features -- -D warnings

That means any clippy warning in any target (lib, bin, tests, examples, benches), across all feature combinations, fails CI. Baseline lint policy lives in a [workspace.lints.clippy] table in engine/Cargo.toml — correctness = "deny" plus a few warn-level lints (needless_pass_by_value, redundant_closure_for_method_calls, cloned_instead_of_copied, large_enum_variant). engine/clippy.toml holds only lints that need configuration DATA rather than a level — today a single disallowed-methods entry routing test code away from a bare Cli::try_parse_from (it overflows the 2 MB test-thread stack and aborts the test binary; parse through try_parse_with_big_stack in main.rs). Put lint levels in the Cargo.toml table, not there. The policy is "correctness is a hard error; zero warnings overall."

When CI goes red — triage order

When clippy fires, work through this in order. Don't jump to step 4.

1. Read the lint name and the message

Clippy output starts with the lint name, e.g. warning: this could be a match`` followed by [#[warn(clippy::single_match)]]. That name is load-bearing — it tells you which lint group, what the fix usually looks like, and what docs to read. The full docs are at https://rust-lang.github.io/rust-clippy/master/#/<lint_name>.

2. Does the fix make the code better?

Most clippy lints are genuinely useful. If the suggested fix makes the code clearer or shorter, just take it — that's the default path for the vast majority of triage.

Common "just fix it" lints in Rocky-shaped code:

• clippy::redundant_clone — remove the .clone()
• clippy::needless_borrow — remove the &
• clippy::needless_collect — drop the .collect::<Vec<_>>() before the iterator consumer
• clippy::let_and_return — return directly
• clippy::single_match → if let
• clippy::or_fun_call → .unwrap_or_else(|| …) for side-effecting defaults
• clippy::unnecessary_wraps — return T instead of Option<T> / Result<T, _>

3. Is the fix wrong for your case?

Sometimes clippy is wrong for a specific call site. Classic examples:

• clippy::needless_pass_by_value on a function that's part of an #[async_trait] trait impl — the ownership is load-bearing for the trait signature.
• clippy::too_many_arguments on a function whose arguments are all required config. The right response is usually a newtype (see rust-style), not #[allow].
• clippy::large_enum_variant on a SQL AST enum — splitting the large variant behind Box would pessimise hot-path matching. Box the variant or allow, with a reason.
• clippy::await_holding_lock when the lock is a tokio::sync::Mutex (it's cancel-safe and designed for this). Clippy can't always tell std::sync::Mutex from tokio::sync::Mutex by type name — check which one is actually held.

In those cases, allow at the tightest scope with a reason = "…":

// DO — allow at the function, with a reason clippy-watchers can evaluate
#[allow(
    clippy::too_many_arguments,
    reason = "all args are required config and a struct-wrapper hurts call-site readability"
)]
pub fn build_plan(
    catalog: &str,
    schema: &str,
    table: &str,
    strategy: MaterializationStrategy,
    watermark: Option<Instant>,
    // ...
) -> Plan { ... }

• Scope the #[allow] to the function or block, never the crate or the module, unless the lint is a categorical mismatch (see step 4).
• Always include reason = "..." — an unexplained #[allow] is a future maintenance trap.
• Never #[allow(clippy::all)] or #[allow(warnings)].