Introduce workspace lints by emilk · Pull Request #10533 · apache/arrow-rs

emilk · 2026-08-03T19:32:40Z

Which issue does this PR close?

No issue in particular

Related to Enable clippy lint clippy::allow_attributes #10458
Sibling PR: chore: apply workspace lints to all crates datafusion#24076

Rationale for this change

Lint configuration is currently repeated per crate in lib.rs. A [workspace.lints] table lets us configure lints once, for every crate, and makes rolling out new lints a one-line change.

Enabling more lints can also help keeping a higher code quality, reducing bugs.

What changes are included in this PR?

Modeled on egui's Cargo.toml. I started small. We can enable more lints in later PRs.

Are these changes tested?

Yes, by existing CI: cargo clippy --workspace --all-targets --all-features -- -D warnings and the docs job (RUSTDOCFLAGS=-Dwarnings) both pass. Two previously-dead doc examples now actually compile and run.

Are there any user-facing changes?

parquet_variant_compute::VariantArrayIter is now exported. It was already returned by the public VariantArray::iter, but was not nameable. No other API changes.

Add a `[workspace.lints]` table to the root `Cargo.toml` and have every workspace member inherit it with `[lints] workspace = true`, so that lint configuration lives in one place instead of being repeated per crate. This starts with a deliberately small set of lints that are `allow` by default and that have zero violations in the workspace today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Adds the `rust_2018_idioms`, `future_incompatible` and `nonstandard_style` lint groups, plus a few individual lints, to `[workspace.lints.rust]`. Three members of `rust_2018_idioms` have too many violations to fix here, so they are explicitly allowed for now, with a TODO. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Adds the `rustdoc::all` group to `[workspace.lints.rustdoc]` and fixes the violations, all of which were latent documentation bugs: * `rustdoc::unescaped_backticks`: 8 typos in doc comments. * `rustdoc::missing_crate_level_docs`: 8 binaries (and `parquet_derive_test`) had no crate level docs. * `rustdoc::private_doc_tests`: 4 doc examples that were never run: - The `parquet::compression` example was attached to a private `use` statement instead of to the module, so it was never compiled. It is now an inner attribute, and the example compiles. - `VariantArrayIter` is returned by the public `VariantArray::iter`, but was not exported, so its example was not run. It is now exported. - Two illustrative (non-compiling) examples are now marked `text`/`ignore`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Each of these has fewer than 10 violations, so they are fixed here rather than allowed: * `dbg_macro`: two stray `dbg!` calls, one of them in library code. * `flat_map_option`: 5x `flat_map` that should be `filter_map`. * `iter_filter_is_some`: 1 site, `#[expect]`ed - the test depends on the size hint of `filter`, which the suggested `flatten` does not have. * `manual_instant_elapsed`: 1x `Instant::now() - earlier`. * `mut_mut`: `SerializedPageWriter::page_encryptor_and_sink_mut` returned a `&mut &mut TrackedWrite<W>`; it now reborrows instead. * `pathbuf_init_then_push`: 5x `PathBuf` built by repeated `push`, now `join`. * `unnecessary_self_imports`: 1x `use ...::Message::{self}`. * `unused_peekable`: 1x `peekable()` that was never peeked. * `zero_sized_map_values`: a test used `HashMap<String, ()>` as a set. Two lints from the same batch were deliberately left out: * `mem_forget`: all 5 uses are deliberate ownership transfers into raw pointers, and would each need an `allow`. * `lossy_float_literal`: all 4 uses are readable test data (e.g. `1234567890.1234`), which the fix would obfuscate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

These are only reported with `--document-private-items`, which is what the docs CI job uses: * `parquet/src/arrow/arrow_reader/statistics.rs`: a doubled backtick inside the `make_stats_iterator!` macro body. * `parquet/src/column/writer/encoder.rs`: a missing opening backtick. * `parquet-variant/src/utils.rs`: the "Errors" list said "Trailing '`'", but the implementation treats a trailing *backslash* as an unclosed bracket. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

All of these have fewer than 10 violations each, so they are fixed rather than allowed. Three of them turned out to be real bugs rather than style issues: * `literal_string_with_formatting_args` found three error messages built with the single-argument arm of `general_err!`, which is `$fmt.to_owned()` and does *not* format. So `general_err!("expected {src_length} got {write_length}")` produced literal braces instead of the values. They now pass the values as explicit arguments. The same lint also caught two `expect` messages naming `DecodeResult::NeedsData{ranges}`, which is a tuple variant. * `trait_duplication_in_bounds` found `T: ArrowPrimitiveType + ArrowPrimitiveType`. * `should_panic_without_expect`: the six `#[should_panic]` tests now assert which panic they expect, so they cannot pass for the wrong reason. The rest are mechanical: `bool_to_int_with_if`, `doc_comment_double_space_linebreaks`, `doc_include_without_cfg`, `format_push_string`, `iter_with_drain`, `option_as_ref_cloned`, `ptr_offset_by_literal`, `ref_binding_to_reference`, `single_char_pattern`, `stable_sort_primitive`, `unnecessary_box_returns`, `unnecessary_struct_initialization`. Lints left out of this batch, and why: * `unused_async`: `arrow_flight::sql::client` has a public `async fn` with no await, so removing `async` would be a breaking change. * `tuple_array_conversions`: false positive on the idiomatic `for buffer in [offsets, values]`. * `suboptimal_flops`: `mul_add` changes floating point rounding. * `trivial_regex`: the arrow-csv tests use trivial regexes on purpose. * `ref_option_ref`, `single_option_map`: `parquet_derive_test` deliberately covers `&Option<&T>` fields. * `comparison_chain`, `assigning_clones`: left allowed, as in egui. * `future_not_send`, `large_stack_arrays`: structural / threshold based. * `cast_ptr_alignment`, `as_ptr_cast_mut`, `ptr_cast_constness`, `needless_bitwise_bool`, `mem_forget`, `lossy_float_literal`: need a case-by-case look at unsafe code, deliberate branchless code, or test data. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The bounds check in `merge_n` is behind `cfg(debug_assertions)`, so the panic message differs between debug ("Index out of bounds: 99 >= 1") and release, where the slice index panics instead ("index out of bounds: the len is 2 but the index is 99"). Expect the substring that both share. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`general_err!`, `nyi_err!`, `eof_err!` and `arrow_err!` all had a single-argument arm that did `$fmt.to_owned()` rather than formatting, so `general_err!("expected {a} got {b}")` emitted the braces literally. The previous commit worked around this at the three call sites that `clippy::literal_string_with_formatting_args` found, by passing the values as explicit arguments. Fix the cause instead: add a `$fmt:literal` arm that goes through `format!`, ahead of the existing `$fmt:expr` arm, which is still needed for callers that pass a non-literal. This restores the inline format arguments at the three call sites, and means the next such message cannot silently lose its values. Note that a literal containing braces that are *not* format arguments is now a compile error rather than silently wrong; the whole of `parquet` builds with `--all-features --all-targets`, so there are none today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Jefffrey

lots of nice cleanups here

# Conflicts: # arrow-buffer/Cargo.toml # arrow-cast/Cargo.toml # arrow-ord/Cargo.toml # arrow-select/Cargo.toml

alamb

looks great to me too -- thanks @emilk and @Jefffrey

alamb · 2026-08-04T16:50:17Z

 name = "decimal_arithmetic"
 harness = false
+
+[lints]


alamb · 2026-08-04T16:51:36Z


    #[test]
-    #[should_panic]
+    #[should_panic(expected = "values.len() >= predicate.filter.len()")]


this is a nice lint to insist on error messages

alamb · 2026-08-04T16:53:10Z

+    fn error_macros_format_inline_args() {
+        let expected = 1;
+        let actual = 2;
+        assert_eq!(


this seems like a nice fix

alamb · 2026-08-04T16:57:14Z

I merged up from main to resolve conflicts:

## Which issue does this PR close? - Part of apache#18467 - Broken out of apache#24066 - Sibling PR: apache/arrow-rs#10533 ## Rationale for this change The workspace already has a `[workspace.lints]` table, but it was missing from three crates ## What changes are included in this PR? Inheriting the workspace lints in all crates, and fixing the resulting violations ## Are these changes tested? Yes, by existing tests and CI ## 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 #10533 # Rationale for this change #10533 added `[workspace.lints]` with a minimal set of lints. This fills out `[workspace.lints.rust]` with more lints. # 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.

# Which issue does this PR close? No issue in particular - Follow-up to #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. --------- Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

emilk and others added 3 commits August 3, 2026 21:25

emilk mentioned this pull request Aug 3, 2026

Rerun Issues rerun-io/opensource#1

Open

5 tasks

emilk marked this pull request as ready for review August 3, 2026 19:37

emilk mentioned this pull request Aug 3, 2026

Rerun PRs rerun-io/opensource#2

Open

emilk and others added 2 commits August 3, 2026 21:47

emilk mentioned this pull request Aug 3, 2026

GeometryBounder::x() computes the wraparound width incorrectly, returning looser bounds than necessary #10534

Open

emilk and others added 3 commits August 3, 2026 22:17

Jefffrey added the development-process Related to development process of arrow-rs label Aug 4, 2026

Jefffrey approved these changes Aug 4, 2026

View reviewed changes

Merge remote-tracking branch 'apache/main' into emilk/workspace-lints

b4a3130

# Conflicts: # arrow-buffer/Cargo.toml # arrow-cast/Cargo.toml # arrow-ord/Cargo.toml # arrow-select/Cargo.toml

alamb approved these changes Aug 4, 2026

View reviewed changes

emilk mentioned this pull request Aug 4, 2026

chore: apply workspace lints to all crates apache/datafusion#24076

Merged

alamb merged commit f28db3e into apache:main Aug 4, 2026
42 checks passed

This was referenced Aug 4, 2026

More workspace lints: finish [workspace.lints.rust] #10551

Merged

More workspace lints: fill out [workspace.lints.clippy] #10552

Merged

emilk mentioned this pull request Aug 5, 2026

Enable a few more clippy lints #10558

Open

Navigation Menu

Uh oh!

Introduce workspace lints - #10533

Introduce workspace lints#10533
alamb merged 9 commits into
apache:mainfrom
emilk:emilk/workspace-lints

emilk commented Aug 3, 2026 •

edited

Loading

Uh oh!

Jefffrey left a comment

Uh oh!

alamb left a comment

Uh oh!

alamb Aug 4, 2026

Uh oh!

alamb Aug 4, 2026

Uh oh!

alamb Aug 4, 2026

Uh oh!

alamb commented Aug 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

emilk commented Aug 3, 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!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

alamb Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

alamb Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

alamb commented Aug 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

emilk commented Aug 3, 2026 •

edited

Loading