Enable more clippy lints by emilk · Pull Request #10673 · apache/arrow-rs

emilk · 2026-08-13T09:25:18Z

Which issue does this PR close?

No issue in particular

Follow-up to Enable a few more clippy lints #10558
Follow-up to More workspace lints: fill out [workspace.lints.clippy] #10552
Follow-up to More workspace lints: finish [workspace.lints.rust] #10551
Follow-up to Introduce workspace lints #10533

Rationale for this change

More lints hand-picked from egui's Cargo.toml, to simplify the code, catch bugs, and write more efficient code.

What changes are included in this PR?

One commit per new lint (maybe easiest to review commit by commit!), except for ten lints with zero violations, which share one commit.
Let me know if you disagree with any of them.

I hope to add even more lints in later PRs.

Are these changes tested?

Covered by existing tests plus the clippy CI job.

Are there any user-facing changes?

Two additive ones: IntoIterator is now implemented for references to FixedSizeListArray, GenericListArray, GenericListViewArray, MapArray, BitChunks, UnalignedBitChunk and VariantArray, and the UnalignedBitChunkIterator alias now names Copied instead of Cloned.

None of these fire on the current code base, so this is purely a guard against future regressions: * `coerce_container_to_any` - catches `Box<Rc<dyn Any>>` upcasts that can never match * `decimal_bitwise_operands` - catches bitwise operators on decimal literals * `doc_broken_link` - catches malformed links in docs * `duration_suboptimal_units` - prefer the clearest `Duration` constructor * `empty_enums` - suggests a never type instead * `ip_constant` - prefer the named IP address constants * `manual_ilog2` - prefer `ilog2` over a hand-rolled version * `needless_type_cast` - catches casts to the same type * `self_only_used_in_recursion` - catches parameters that do nothing but get passed along * `unchecked_time_subtraction` - catches `Instant` subtraction that can panic

Hoists the code that every branch of an `if`/`else` shares out of the branches, which removes seven copy-pasted blocks.

Types with an `iter` method should also implement `IntoIterator` for their reference type, so that `for x in &collection` works. Adds the missing impls for `FixedSizeListArray`, `GenericListArray`, `GenericListViewArray`, `MapArray`, `BitChunks`, `UnalignedBitChunk` and `VariantArray`.

`&Option<T>` in an argument position forces the caller to have an owned `Option`; `Option<&T>` does not, and it is one less indirection to read through. All ten sites were private or `pub(crate)`, so no public API changed.

Makes the fallback of `unwrap_or`/`or`/`ok_or`/`map_or` lazy, so that the eleven sites that built an error, a `String` or a cursor only do that work when the fallback is actually taken.

`std::ptr::from_ref`/`from_mut` says what the code means and cannot silently change the pointee type, unlike a chain of `as` casts. The generated flatbuffers modules in `arrow-ipc/src/gen` are now exempt from `clippy::pedantic` as well as `clippy::all`: they say "do not modify" at the top, so no pedantic lint can ever be fixed there.

Removes 25 `continue` statements that were the last thing in their loop body, so they only restated what the loop already does. Two of them, in `arrow-ord::comparison`, become `break`: once a matching list element is found there is nothing left to look for.

Drops 25 type annotations that only repeated the type of the expression on the right hand side.

Building a one-element array only to iterate it allocates a temporary and hides how many items there are. Twelve `HashMap` cases become `HashMap::from([..])`; the rest become `std::iter::once`.

Removes trailing commas after the last macro argument, mostly in `panic!` and `assert_eq!` messages, where they look like a forgotten argument.

`to_string`/`to_vec`/`to_owned` on a type that is already owned goes through `Deref` and clones anyway, just less obviously. Four of the sites turned out to be clones nothing needed, so they are now gone entirely.

`copied` says the element is `Copy`, so the reader does not have to wonder whether the iterator is cloning something expensive. `UnalignedBitChunkIterator` changes from `Cloned<..>` to `Copied<..>` accordingly.

An `&mut` that is never used mutably asks callers for more than the function needs, which can stop them from sharing the value at all. Most of the sites are the stateless Avro encoders, which only read their array. The `#[cfg(not(feature = "encryption"))]` stubs keep their `&mut self` and get an `#[expect]` with a reason: their signatures mirror the encryption-enabled versions so callers need no `cfg` of their own.

A `_` arm that covers exactly one variant silently swallows every variant added later. Naming the variant makes the compiler point at the match when the enum grows. `get_encoder` keeps `Encoding::BIT_PACKED` explicit and gets an `#[expect(deprecated)]`: rejecting that encoding is the whole point of the arm.

`pointer::cast` cannot silently change constness the way `as` can, and it makes the target type explicit at the call site. This matters here: nearly every site feeds a `from_raw_parts` in unsafe code.

`for x in &collection` is the shorter spelling of `for x in collection.iter()`, and it makes the borrow visible at a glance.

emilk · 2026-08-13T11:52:53Z

                if list.is_valid(j) && (left.value(i) == list.value(j)) {
                    bit_util::set_bit(bool_slice, i);
-                    continue;
+                    break;


small perf win: we only need to mark bit i once.

emilk · 2026-08-13T11:53:00Z

                if list.is_valid(j) && (left.value(i) == list.value(j)) {
                    bit_util::set_bit(bool_slice, i);
-                    continue;
+                    break;


emilk force-pushed the emilk/more-clippy-lints branch from 50a1bf4 to 94c7560 Compare August 13, 2026 09:34

emilk commented Aug 13, 2026

View reviewed changes

emilk force-pushed the emilk/more-clippy-lints branch from 94c7560 to fc7228e Compare August 13, 2026 11:39

emilk changed the title ~~More workspace lints: fifteen more clippy lints~~ Enable more clippy lints Aug 13, 2026

emilk commented Aug 13, 2026

View reviewed changes

Comment thread parquet/src/arrow/async_reader/mod.rs

emilk commented Aug 13, 2026

View reviewed changes

Comment thread parquet/src/arrow/async_reader/mod.rs

emilk commented Aug 13, 2026

View reviewed changes

Comment thread arrow-data/src/equal/list_view.rs Outdated

emilk added 3 commits August 13, 2026 13:48

Enable clippy::branches_sharing_code

aad785f

Hoists the code that every branch of an `if`/`else` shares out of the branches, which removes seven copy-pasted blocks.

Enable clippy::ref_option

8ffc973

`&Option<T>` in an argument position forces the caller to have an owned `Option`; `Option<&T>` does not, and it is one less indirection to read through. All ten sites were private or `pub(crate)`, so no public API changed.

emilk added 12 commits August 13, 2026 13:48

Enable clippy::or_fun_call

ca5f09f

Makes the fallback of `unwrap_or`/`or`/`ok_or`/`map_or` lazy, so that the eleven sites that built an error, a `String` or a cursor only do that work when the fallback is actually taken.

Enable clippy::redundant_type_annotations

aeac23a

Drops 25 type annotations that only repeated the type of the expression on the right hand side.

Enable clippy::iter_on_single_items

f6a0352

Building a one-element array only to iterate it allocates a temporary and hides how many items there are. Twelve `HashMap` cases become `HashMap::from([..])`; the rest become `std::iter::once`.

Enable clippy::unnecessary_trailing_comma

4fbd559

Removes trailing commas after the last macro argument, mostly in `panic!` and `assert_eq!` messages, where they look like a forgotten argument.

Enable clippy::implicit_clone

e731fc4

`to_string`/`to_vec`/`to_owned` on a type that is already owned goes through `Deref` and clones anyway, just less obviously. Four of the sites turned out to be clones nothing needed, so they are now gone entirely.

Enable clippy::cloned_instead_of_copied

4ec247b

`copied` says the element is `Copy`, so the reader does not have to wonder whether the iterator is cloning something expensive. `UnalignedBitChunkIterator` changes from `Cloned<..>` to `Copied<..>` accordingly.

Enable clippy::ptr_as_ptr

af756a4

`pointer::cast` cannot silently change constness the way `as` can, and it makes the target type explicit at the call site. This matters here: nearly every site feeds a `from_raw_parts` in unsafe code.

Enable clippy::explicit_iter_loop

8469269

`for x in &collection` is the shorter spelling of `for x in collection.iter()`, and it makes the borrow visible at a glance.

emilk force-pushed the emilk/more-clippy-lints branch from 8542fd6 to 8469269 Compare August 13, 2026 11:49

emilk commented Aug 13, 2026

View reviewed changes

emilk marked this pull request as ready for review August 13, 2026 11:54

Navigation Menu

Uh oh!

Enable more clippy lints - #10673

Enable more clippy lints#10673
emilk wants to merge 16 commits into
apache:mainfrom
emilk:emilk/more-clippy-lints

emilk commented Aug 13, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emilk Aug 13, 2026

Uh oh!

emilk Aug 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

emilk commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

emilk Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

emilk Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

emilk commented Aug 13, 2026 •

edited

Loading