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.

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;


Code added by the merge from main violated the newly enabled lint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Jefffrey · 2026-08-17T02:39:08Z

thanks @emilk

# Which issue does this PR close? No issue in particular - Follow-up to #10673 # Rationale for this change Minor stylistic improvements ## What changes are included in this PR? One commit per new lint (easiest to review commit by commit!): * [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args) * [`unnecessary_semicolon`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_semicolon) * [`elidable_lifetime_names`](https://rust-lang.github.io/rust-clippy/master/index.html#elidable_lifetime_names) * [`string_lit_as_bytes`](https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes) * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else) * [`return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) Let me know if you disagree with any of them. ## Are these changes tested? Covered by existing tests plus the clippy CI job. ## Are there any user-facing changes? No.

# Which issue does this PR close? N/A -- fixes CI on main # Rationale for this change CI clippy jobs on main are failing, e.g. https://github.com/apache/arrow-rs/actions/runs/32180217000/job/95851222817 ```text error: use the `?` operator instead of an `and_then` call --> parquet/src/arrow/arrow_reader/statistics.rs:1841:22 | 1841 | .map(|s| s.and_then(|s| s.distinct_count_opt())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` This is a logical merge conflict between two PRs that were each green on their own: - #10721 (merged 2026-08-18 09:09 PT, follow-up to #10673) enabled the [`clippy::return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) lint workspace-wide - #10652 (CI last ran 2026-08-11, before the lint was enabled; merged 2026-08-18 12:58 PT) added the `and_then` closure above # What changes are included in this PR? Rewrite the closure to use the `?` operator as clippy suggests (semantically identical, no behavior change): ```rust .map(|s| s?.distinct_count_opt()); ```

# Which issue does this PR close? No issue in particular - Follow-up to apache#10558 - Follow-up to apache#10552 - Follow-up to apache#10551 - Follow-up to apache#10533 # Rationale for this change More lints hand-picked from [egui's `Cargo.toml`](https://github.com/emilk/egui/blob/main/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`. --------- Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

# Which issue does this PR close? No issue in particular - Follow-up to apache#10673 # Rationale for this change Minor stylistic improvements ## What changes are included in this PR? One commit per new lint (easiest to review commit by commit!): * [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args) * [`unnecessary_semicolon`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_semicolon) * [`elidable_lifetime_names`](https://rust-lang.github.io/rust-clippy/master/index.html#elidable_lifetime_names) * [`string_lit_as_bytes`](https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes) * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else) * [`return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) Let me know if you disagree with any of them. ## Are these changes tested? Covered by existing tests plus the clippy CI job. ## Are there any user-facing changes? No.

# Which issue does this PR close? N/A -- fixes CI on main # Rationale for this change CI clippy jobs on main are failing, e.g. https://github.com/apache/arrow-rs/actions/runs/32180217000/job/95851222817 ```text error: use the `?` operator instead of an `and_then` call --> parquet/src/arrow/arrow_reader/statistics.rs:1841:22 | 1841 | .map(|s| s.and_then(|s| s.distinct_count_opt())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` This is a logical merge conflict between two PRs that were each green on their own: - apache#10721 (merged 2026-08-18 09:09 PT, follow-up to apache#10673) enabled the [`clippy::return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) lint workspace-wide - apache#10652 (CI last ran 2026-08-11, before the lint was enabled; merged 2026-08-18 12:58 PT) added the `and_then` closure above # What changes are included in this PR? Rewrite the closure to use the `?` operator as clippy suggests (semantically identical, no behavior change): ```rust .map(|s| s?.distinct_count_opt()); ```

# Which issue does this PR close? No issue in particular - Follow-up to apache#10558 - Follow-up to apache#10552 - Follow-up to apache#10551 - Follow-up to apache#10533 # Rationale for this change More lints hand-picked from [egui's `Cargo.toml`](https://github.com/emilk/egui/blob/main/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`. --------- Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

# Which issue does this PR close? No issue in particular - Follow-up to apache#10673 # Rationale for this change Minor stylistic improvements ## What changes are included in this PR? One commit per new lint (easiest to review commit by commit!): * [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args) * [`unnecessary_semicolon`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_semicolon) * [`elidable_lifetime_names`](https://rust-lang.github.io/rust-clippy/master/index.html#elidable_lifetime_names) * [`string_lit_as_bytes`](https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes) * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else) * [`return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) Let me know if you disagree with any of them. ## Are these changes tested? Covered by existing tests plus the clippy CI job. ## Are there any user-facing changes? No.

# Which issue does this PR close? N/A -- fixes CI on main # Rationale for this change CI clippy jobs on main are failing, e.g. https://github.com/apache/arrow-rs/actions/runs/32180217000/job/95851222817 ```text error: use the `?` operator instead of an `and_then` call --> parquet/src/arrow/arrow_reader/statistics.rs:1841:22 | 1841 | .map(|s| s.and_then(|s| s.distinct_count_opt())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` This is a logical merge conflict between two PRs that were each green on their own: - apache#10721 (merged 2026-08-18 09:09 PT, follow-up to apache#10673) enabled the [`clippy::return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) lint workspace-wide - apache#10652 (CI last ran 2026-08-11, before the lint was enabled; merged 2026-08-18 12:58 PT) added the `and_then` closure above # What changes are included in this PR? Rewrite the closure to use the `?` operator as clippy suggests (semantically identical, no behavior change): ```rust .map(|s| s?.distinct_count_opt()); ```

# Which issue does this PR close? No issue in particular - Follow-up to apache#10558 - Follow-up to apache#10552 - Follow-up to apache#10551 - Follow-up to apache#10533 # Rationale for this change More lints hand-picked from [egui's `Cargo.toml`](https://github.com/emilk/egui/blob/main/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`. --------- Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

# Which issue does this PR close? No issue in particular - Follow-up to apache#10673 # Rationale for this change Minor stylistic improvements ## What changes are included in this PR? One commit per new lint (easiest to review commit by commit!): * [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args) * [`unnecessary_semicolon`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_semicolon) * [`elidable_lifetime_names`](https://rust-lang.github.io/rust-clippy/master/index.html#elidable_lifetime_names) * [`string_lit_as_bytes`](https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes) * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else) * [`return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) Let me know if you disagree with any of them. ## Are these changes tested? Covered by existing tests plus the clippy CI job. ## Are there any user-facing changes? No.

# Which issue does this PR close? N/A -- fixes CI on main # Rationale for this change CI clippy jobs on main are failing, e.g. https://github.com/apache/arrow-rs/actions/runs/32180217000/job/95851222817 ```text error: use the `?` operator instead of an `and_then` call --> parquet/src/arrow/arrow_reader/statistics.rs:1841:22 | 1841 | .map(|s| s.and_then(|s| s.distinct_count_opt())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` This is a logical merge conflict between two PRs that were each green on their own: - apache#10721 (merged 2026-08-18 09:09 PT, follow-up to apache#10673) enabled the [`clippy::return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) lint workspace-wide - apache#10652 (CI last ran 2026-08-11, before the lint was enabled; merged 2026-08-18 12:58 PT) added the `and_then` closure above # What changes are included in this PR? Rewrite the closure to use the `?` operator as clippy suggests (semantically identical, no behavior change): ```rust .map(|s| s?.distinct_count_opt()); ```

# Which issue does this PR close? No issue in particular - Follow-up to #10673 - Follow-up to #10721 - Follow-up to #10742 # Rationale for this change #10742 left the `pedantic` lints we violate as `allow`, with a violation count each. These five are the ones worth paying for: two guard raw pointers, one guards bindings that claim to be unused, and two improve readability and test output. ## What changes are included in this PR? One commit per new lint (easiest to review commit by commit!): * [`redundant_else`](https://rust-lang.github.io/rust-clippy/master/index.html#redundant_else) * [`manual_assert_eq`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_assert_eq) * [`borrow_as_ptr`](https://rust-lang.github.io/rust-clippy/master/index.html#borrow_as_ptr) - the FFI stream code now builds its pointers with `&raw mut` instead of going through a `&mut` * [`used_underscore_binding`](https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding) - two conditionally-used parameters keep their names and get a feature-gated `expect` * [`cast_ptr_alignment`](https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment) - the five byte-pointer casts now say why they are sound Let me know if you disagree with any of them. ## Are these changes tested? Covered by existing tests plus the clippy CI job. ## Are there any user-facing changes? No. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

# Which issue does this PR close? No issue in particular - Follow-up to apache#10558 - Follow-up to apache#10552 - Follow-up to apache#10551 - Follow-up to apache#10533 # Rationale for this change More lints hand-picked from [egui's `Cargo.toml`](https://github.com/emilk/egui/blob/main/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`. --------- Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

# Which issue does this PR close? No issue in particular - Follow-up to apache#10673 # Rationale for this change Minor stylistic improvements ## What changes are included in this PR? One commit per new lint (easiest to review commit by commit!): * [`uninlined_format_args`](https://rust-lang.github.io/rust-clippy/master/index.html#uninlined_format_args) * [`unnecessary_semicolon`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_semicolon) * [`elidable_lifetime_names`](https://rust-lang.github.io/rust-clippy/master/index.html#elidable_lifetime_names) * [`string_lit_as_bytes`](https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes) * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else) * [`return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) Let me know if you disagree with any of them. ## Are these changes tested? Covered by existing tests plus the clippy CI job. ## Are there any user-facing changes? No.

# Which issue does this PR close? N/A -- fixes CI on main # Rationale for this change CI clippy jobs on main are failing, e.g. https://github.com/apache/arrow-rs/actions/runs/32180217000/job/95851222817 ```text error: use the `?` operator instead of an `and_then` call --> parquet/src/arrow/arrow_reader/statistics.rs:1841:22 | 1841 | .map(|s| s.and_then(|s| s.distinct_count_opt())); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ``` This is a logical merge conflict between two PRs that were each green on their own: - apache#10721 (merged 2026-08-18 09:09 PT, follow-up to apache#10673) enabled the [`clippy::return_and_then`](https://rust-lang.github.io/rust-clippy/master/index.html#return_and_then) lint workspace-wide - apache#10652 (CI last ran 2026-08-11, before the lint was enabled; merged 2026-08-18 12:58 PT) added the `and_then` closure above # What changes are included in this PR? Rewrite the closure to use the `?` operator as clippy suggests (semantically identical, no behavior change): ```rust .map(|s| s?.distinct_count_opt()); ```

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