More workspace lints: fill out `[workspace.lints.clippy]` by emilk · Pull Request #10552 · apache/arrow-rs

emilk · 2026-08-04T19:30:20Z

Which issue does this PR close?

No issue in particular

Follow-up to Introduce workspace lints #10533

Rationale for this change

#10533 added [workspace.lints] with a minimal set of lints.

This fills out PR [workspace.lints.clippy] with more lints that I've hand-picked over the years.

Some stylistic changes, but also a lot of things that improve performance.

Let me know if you disagree with any of them and I'll revert them.

More coming in later PRs :)

What changes are included in this PR?

Best reviewed commit by commit!

Are these changes tested?

By CI

Are there any user-facing changes?

No public API changed.

Taken from egui's `Cargo.toml`. All of these are `allow` by default on the toolchain CI uses, already exist in the 1.88 MSRV (so no `unknown lint` warnings there), and have zero violations in the workspace, so no code changes are needed.

One violation: use `clone_from` instead of assigning a fresh `clone()`.

One violation: rewrite an `if`/`else if` chain over `==` and `<` as a `match` on `Ord::cmp`.

One violation: `(a + b) / 2` can overflow, so use `i64::midpoint`. The inputs are both non-negative here, so the result is unchanged.

Two violations, both in test and bench code: move a 64 KiB and a 20 KiB array off the stack and into a `Vec`.

Two violations, both from destructuring the tuple returned by `get_byte_array_buffers` only to put the two halves straight back into an array to iterate over. The helper is private, so it now returns `[Buffer; 2]` directly.

Two violations: `convert_geo_stats` and `convert_bounding_box` took an `Option` only to `map` over it. They now take and return plain values, and the single caller stopped wrapping its argument in `Some` just to have it unwrapped again.

Three violations, all deriving a `*mut` from a shared `as_ptr()`: - `trusted_len.rs` only used the pointer for `offset_from`, so it is now `*const T`. - `MutableBuffer::from(Vec<T>)` takes over the `Vec`'s allocation, so it uses `as_mut_ptr()`. - `Bytes::from(bytes::Bytes)` has no `as_mut_ptr` to use and never writes through the pointer, so it spells the constness change out with `cast_mut()`.

`check_len` now takes `Option<&[u8]>` by value instead of by reference. The two fields in `parquet_derive_test` keep their `&Option<&T>` types behind an `#[expect]`, since covering that type is the point of the struct.

Four safe functions had a `# Safety` section, which reads as if they had an `unsafe` contract: - `take` and `take_arrays` described a *panic*, so the section is now `# Panics`. - `ArrayData::build` and `with_skip_validation` describe undefined behavior reachable only once the caller has opted in through a separate `unsafe` API, so the text is kept under `# Undefined behavior`.

Four literals had more precision than their float type can hold, so the source no longer matched the value actually compiled in. Applied with `cargo clippy --fix`; every replacement has identical bits, so behavior is unchanged. For example `999_999_999f32` really was `1e9` all along.

Five `n <= T::MAX as usize` range checks become `T::try_from(n).is_ok()`. Applied with `cargo clippy --fix`. Equivalent, since `usize` is unsigned.

Four `as` casts that only changed pointer constness become `cast_const()` or `cast_mut()`, applied with `cargo clippy --fix`. The fifth violation was already fixed by the `as_ptr_cast_mut` commit.

Three private `async fn`s in `arrow-integration-testing` never awaited anything, so they are now plain functions and their call sites lost the `.await`. `FlightSqlServiceClient::close` keeps its `async` behind an `#[expect]`, since dropping it would break callers.

Six `let mut x = default; if cond { x = ... }` sequences become plain `let` bindings: two `and_then`, two tuple destructurings, one `if` expression, and one boolean expression. The last keeps its short-circuit, so `reps[base + i]` is still only indexed when the first check passed.

Three real violations become `fs::read_to_string`, which also drops two now-unused imports. The six in `arrow-csv`'s tests get an `#[expect]` on the test module: they read back a `tempfile::tempfile()`, which has no path, so `fs::read` is not an option there.

Ten struct literals reordered to match their declaration order, applied with `cargo clippy --fix`. Every reordered field is a shorthand move of an already-computed local, so evaluation order does not matter.

- Keep the benchmark's array on the stack, behind an `#[expect]`, so the benchmark measures what it always has. - Use `is_none_or` instead of a negated `is_some_and`. - Drop the now-superfluous `// read file contents to string` comments.

`clippy::collapsible_if` is deny-by-default in CI and has been failing on `main` since apache#10409; the nested `if let` is now a let-chain.

Jefffrey

generally looks good, just a minor comment

The original form reads better than the tuple-match; keep it behind an `#[expect]`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Jefffrey · 2026-08-05T15:26:19Z

thanks @emilk

github-actions Bot added parquet Changes to the parquet crate arrow Changes to the arrow crate arrow-ipc labels Aug 4, 2026