From 408a84a30c8714faf36f2d2f7f955331496ebf19 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:15:33 +0200 Subject: [PATCH 01/20] Enable 46 clippy lints that have no violations 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. --- Cargo.toml | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index f31770395588..61dcfaa1c087 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -145,36 +145,82 @@ missing_crate_level_docs = "warn" [workspace.lints.clippy] bool_to_int_with_if = "warn" +clear_with_drain = "warn" dbg_macro = "warn" debug_assert_with_mut_call = "warn" +default_union_representation = "warn" +disallowed_script_idents = "warn" doc_comment_double_space_linebreaks = "warn" doc_include_without_cfg = "warn" +empty_enum_variants_with_brackets = "warn" +exit = "warn" +expl_impl_clone_on_copy = "warn" +explicit_deref_methods = "warn" +filter_map_next = "warn" flat_map_option = "warn" +float_cmp_const = "warn" +fn_params_excessive_bools = "warn" +fn_to_numeric_cast_any = "warn" format_push_string = "warn" +imprecise_flops = "warn" +index_refutable_slice = "warn" +inefficient_to_string = "warn" +infinite_loop = "warn" +into_iter_without_iter = "warn" invalid_upcast_comparisons = "warn" +iter_filter_is_ok = "warn" iter_filter_is_some = "warn" +iter_not_returning_iterator = "warn" +iter_on_empty_collections = "warn" iter_with_drain = "warn" +large_digit_groups = "warn" +large_futures = "warn" +large_include_file = "warn" +large_stack_frames = "warn" +large_types_passed_by_value = "warn" +linkedlist = "warn" literal_string_with_formatting_args = "warn" +macro_use_imports = "warn" manual_instant_elapsed = "warn" +manual_is_power_of_two = "warn" match_wild_err_arm = "warn" +mismatching_type_param_order = "warn" mut_mut = "warn" mutex_integer = "warn" +negative_feature_names = "warn" +non_zero_suggestions = "warn" +nonstandard_macro_braces = "warn" option_as_ref_cloned = "warn" +option_option = "warn" path_buf_push_overwrite = "warn" pathbuf_init_then_push = "warn" ptr_offset_by_literal = "warn" +pub_without_shorthand = "warn" rc_mutex = "warn" ref_binding_to_reference = "warn" +rest_pat_in_fully_bound_structs = "warn" +same_functions_in_if_condition = "warn" same_length_and_capacity = "warn" +set_contains_or_insert = "warn" should_panic_without_expect = "warn" single_char_pattern = "warn" stable_sort_primitive = "warn" +str_split_at_newline = "warn" +string_add_assign = "warn" +string_lit_chars_any = "warn" +suspicious_xor_used_as_pow = "warn" +todo = "warn" trailing_empty_array = "warn" trait_duplication_in_bounds = "warn" +transmute_ptr_to_ptr = "warn" +uninhabited_references = "warn" unnecessary_box_returns = "warn" +unnecessary_literal_bound = "warn" unnecessary_self_imports = "warn" unnecessary_struct_initialization = "warn" unused_peekable = "warn" +unused_rounding = "warn" +wildcard_dependencies = "warn" zero_sized_map_values = "warn" # release inherited profile keeping debug information and symbols From 7f2be22b62c47bfb7862944920e7fe04cbf51fdc Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:19:05 +0200 Subject: [PATCH 02/20] Enable `clippy::assigning_clones` lint One violation: use `clone_from` instead of assigning a fresh `clone()`. --- Cargo.toml | 1 + parquet/src/arrow/mod.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 61dcfaa1c087..690a3fda4124 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -144,6 +144,7 @@ broken_intra_doc_links = "warn" missing_crate_level_docs = "warn" [workspace.lints.clippy] +assigning_clones = "warn" bool_to_int_with_if = "warn" clear_with_drain = "warn" dbg_macro = "warn" diff --git a/parquet/src/arrow/mod.rs b/parquet/src/arrow/mod.rs index b89db361eda1..9414337739f3 100644 --- a/parquet/src/arrow/mod.rs +++ b/parquet/src/arrow/mod.rs @@ -410,7 +410,7 @@ impl ProjectionMask { /// ``` pub fn intersect(&mut self, other: &Self) { match (self.mask.as_ref(), other.mask.as_ref()) { - (None, _) => self.mask = other.mask.clone(), + (None, _) => self.mask.clone_from(&other.mask), (_, None) => {} (Some(a), Some(b)) => { debug_assert_eq!(a.len(), b.len()); From 68b57b06c70ee85388b88d607229ff7f75304d63 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:20:55 +0200 Subject: [PATCH 03/20] Enable `clippy::comparison_chain` lint One violation: rewrite an `if`/`else if` chain over `==` and `<` as a `match` on `Ord::cmp`. --- Cargo.toml | 1 + parquet/src/column/chunker/cdc.rs | 35 +++++++++++++++++-------------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 690a3fda4124..81bd9e58c022 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -147,6 +147,7 @@ missing_crate_level_docs = "warn" assigning_clones = "warn" bool_to_int_with_if = "warn" clear_with_drain = "warn" +comparison_chain = "warn" dbg_macro = "warn" debug_assert_with_mut_call = "warn" default_union_representation = "warn" diff --git a/parquet/src/column/chunker/cdc.rs b/parquet/src/column/chunker/cdc.rs index 2627d8921b9a..f0a43f7f6ad5 100644 --- a/parquet/src/column/chunker/cdc.rs +++ b/parquet/src/column/chunker/cdc.rs @@ -742,6 +742,7 @@ mod tests { #[cfg(all(test, feature = "arrow"))] mod arrow_tests { use std::borrow::Borrow; + use std::cmp::Ordering; use std::sync::Arc; use arrow::util::data_gen::create_random_batch; @@ -1261,22 +1262,24 @@ mod arrow_tests { for (left, right) in &diffs { let left_sum: i64 = left.iter().sum(); let right_sum: i64 = right.iter().sum(); - if left_sum == right_sum { - eq += 1; - } else if left_sum < right_sum { - larger += 1; - assert_eq!( - left_sum + edit_length, - right_sum, - "Larger diff mismatch: {left_sum} + {edit_length} != {right_sum}" - ); - } else { - smaller += 1; - assert_eq!( - left_sum, - right_sum + edit_length, - "Smaller diff mismatch: {left_sum} != {right_sum} + {edit_length}" - ); + match left_sum.cmp(&right_sum) { + Ordering::Equal => eq += 1, + Ordering::Less => { + larger += 1; + assert_eq!( + left_sum + edit_length, + right_sum, + "Larger diff mismatch: {left_sum} + {edit_length} != {right_sum}" + ); + } + Ordering::Greater => { + smaller += 1; + assert_eq!( + left_sum, + right_sum + edit_length, + "Smaller diff mismatch: {left_sum} != {right_sum} + {edit_length}" + ); + } } } From fb43fbbabfb2b2800bcdd9e47f8510e16afbd831 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:22:07 +0200 Subject: [PATCH 04/20] Enable `clippy::manual_midpoint` lint One violation: `(a + b) / 2` can overflow, so use `i64::midpoint`. The inputs are both non-negative here, so the result is unchanged. --- Cargo.toml | 1 + parquet/src/column/chunker/cdc.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 81bd9e58c022..0ce11664c9b5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -185,6 +185,7 @@ literal_string_with_formatting_args = "warn" macro_use_imports = "warn" manual_instant_elapsed = "warn" manual_is_power_of_two = "warn" +manual_midpoint = "warn" match_wild_err_arm = "warn" mismatching_type_param_order = "warn" mut_mut = "warn" diff --git a/parquet/src/column/chunker/cdc.rs b/parquet/src/column/chunker/cdc.rs index f0a43f7f6ad5..ca59913701dd 100644 --- a/parquet/src/column/chunker/cdc.rs +++ b/parquet/src/column/chunker/cdc.rs @@ -156,7 +156,7 @@ impl ContentDefinedChunker { )); } - let avg_chunk_size = (min_chunk_size + max_chunk_size) / 2; + let avg_chunk_size = min_chunk_size.midpoint(max_chunk_size); // Target size after subtracting the min-size skip window and dividing by the // number of hash tables (for central-limit-theorem normalization). let target_size = (avg_chunk_size - min_chunk_size) / NUM_GEARHASH_TABLES as i64; From 4f64f85aea9ddcb90c1660471a26040d1789a7b8 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:23:48 +0200 Subject: [PATCH 05/20] Enable `clippy::large_stack_arrays` lint Two violations, both in test and bench code: move a 64 KiB and a 20 KiB array off the stack and into a `Vec`. --- Cargo.toml | 1 + arrow/benches/builder.rs | 2 +- parquet/src/arrow/record_reader/mod.rs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0ce11664c9b5..07813909240f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -178,6 +178,7 @@ iter_with_drain = "warn" large_digit_groups = "warn" large_futures = "warn" large_include_file = "warn" +large_stack_arrays = "warn" large_stack_frames = "warn" large_types_passed_by_value = "warn" linkedlist = "warn" diff --git a/arrow/benches/builder.rs b/arrow/benches/builder.rs index 2374797961a1..7a1e8748a50c 100644 --- a/arrow/benches/builder.rs +++ b/arrow/benches/builder.rs @@ -35,7 +35,7 @@ const BATCH_SIZE: usize = 8 << 10; const NUM_BATCHES: usize = 64; fn bench_primitive(c: &mut Criterion) { - let data: [i64; BATCH_SIZE] = [100; BATCH_SIZE]; + let data = vec![100i64; BATCH_SIZE]; let mut group = c.benchmark_group("bench_primitive"); group.throughput(Throughput::Bytes( diff --git a/parquet/src/arrow/record_reader/mod.rs b/parquet/src/arrow/record_reader/mod.rs index cdb8084eb4e4..ba3c525b4ab1 100644 --- a/parquet/src/arrow/record_reader/mod.rs +++ b/parquet/src/arrow/record_reader/mod.rs @@ -835,7 +835,7 @@ mod tests { let mut record_reader = RecordReader::::new(desc.clone(), DEFAULT_BATCH_SIZE); { - let values = [100; 5000]; + let values = vec![100; 5000]; let def_levels = [1i16; 5000]; let mut rep_levels = [1i16; 5000]; for idx in 0..1000 { From 85629b4703813193f5390b61e354536fe99480d6 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:25:20 +0200 Subject: [PATCH 06/20] Enable `clippy::tuple_array_conversions` lint 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. --- Cargo.toml | 1 + arrow-ipc/src/writer.rs | 17 ++++++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 07813909240f..e3245f629316 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -217,6 +217,7 @@ todo = "warn" trailing_empty_array = "warn" trait_duplication_in_bounds = "warn" transmute_ptr_to_ptr = "warn" +tuple_array_conversions = "warn" uninhabited_references = "warn" unnecessary_box_returns = "warn" unnecessary_literal_bound = "warn" diff --git a/arrow-ipc/src/writer.rs b/arrow-ipc/src/writer.rs index ecd0f0f66724..4d26b7821ab6 100644 --- a/arrow-ipc/src/writer.rs +++ b/arrow-ipc/src/writer.rs @@ -2275,18 +2275,19 @@ fn reencode_offsets( /// In particular, this handles re-encoding the offsets if they don't start at `0`, /// slicing the values buffer as appropriate. This helps reduce the encoded /// size of sliced arrays, as values that have been sliced away are not encoded -fn get_byte_array_buffers(data: &ArrayData) -> (Buffer, Buffer) { +/// Returns the offsets and values buffers, in that order. +fn get_byte_array_buffers(data: &ArrayData) -> [Buffer; 2] { if data.is_empty() { // As per specification, offsets buffer has N+1 elements. // So an empty array should still be encoded with a single 0 offset. let mut offsets = MutableBuffer::new(size_of::()); offsets.extend_from_slice(O::usize_as(0).to_byte_slice()); - return (offsets.into(), MutableBuffer::new(0).into()); + return [offsets.into(), MutableBuffer::new(0).into()]; } let (offsets, original_start_offset, len) = reencode_offsets::(&data.buffers()[0], data); let values = data.buffers()[1].slice_with_length(original_start_offset, len); - (offsets, values) + [offsets, values] } /// Similar logic as [`get_byte_array_buffers()`] but slices the child array instead @@ -2409,8 +2410,7 @@ fn write_array_data( let data_type = array_data.data_type(); if matches!(data_type, DataType::Binary | DataType::Utf8) { - let (offsets, values) = get_byte_array_buffers::(array_data); - for buffer in [offsets, values] { + for buffer in get_byte_array_buffers::(array_data) { offset = encode_sink_buffer( buffer, meta, @@ -2451,8 +2451,7 @@ fn write_array_data( )?; } } else if matches!(data_type, DataType::LargeBinary | DataType::LargeUtf8) { - let (offsets, values) = get_byte_array_buffers::(array_data); - for buffer in [offsets, values] { + for buffer in get_byte_array_buffers::(array_data) { offset = encode_sink_buffer( buffer, meta, @@ -3089,7 +3088,7 @@ mod tests { #[test] fn test_empty_utf8_ipc_writes_nonempty_offsets_buffer() { let name = StringArray::from(Vec::::new()); - let (offsets, values) = get_byte_array_buffers::(&name.to_data()); + let [offsets, values] = get_byte_array_buffers::(&name.to_data()); assert_eq!(name.len(), 0); assert_eq!( @@ -3103,7 +3102,7 @@ mod tests { #[test] fn test_empty_large_utf8_ipc_writes_nonempty_offsets_buffer() { let name = LargeStringArray::from(Vec::::new()); - let (offsets, values) = get_byte_array_buffers::(&name.to_data()); + let [offsets, values] = get_byte_array_buffers::(&name.to_data()); assert_eq!(name.len(), 0); assert_eq!( From 87cf713cfaaaa5cf1b63ad47c1d884b5df51a4ec Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:27:31 +0200 Subject: [PATCH 07/20] Enable `clippy::single_option_map` lint 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. --- Cargo.toml | 1 + parquet/src/file/metadata/thrift/mod.rs | 57 +++++++++++-------------- 2 files changed, 25 insertions(+), 33 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e3245f629316..7858d123581e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -208,6 +208,7 @@ same_length_and_capacity = "warn" set_contains_or_insert = "warn" should_panic_without_expect = "warn" single_char_pattern = "warn" +single_option_map = "warn" stable_sort_primitive = "warn" str_split_at_newline = "warn" string_add_assign = "warn" diff --git a/parquet/src/file/metadata/thrift/mod.rs b/parquet/src/file/metadata/thrift/mod.rs index bd60de417725..685cc2b8e543 100644 --- a/parquet/src/file/metadata/thrift/mod.rs +++ b/parquet/src/file/metadata/thrift/mod.rs @@ -146,43 +146,34 @@ struct SizeStatistics { ); fn convert_geo_stats( - stats: Option, -) -> Option> { - stats.map(|st| { - let bbox = convert_bounding_box(st.bbox); - let geospatial_types: Option> = st.geospatial_types.filter(|v| !v.is_empty()); - Box::new(crate::geospatial::statistics::GeospatialStatistics::new( - bbox, - geospatial_types, - )) - }) + st: GeospatialStatistics, +) -> crate::geospatial::statistics::GeospatialStatistics { + let bbox = st.bbox.map(convert_bounding_box); + let geospatial_types: Option> = st.geospatial_types.filter(|v| !v.is_empty()); + crate::geospatial::statistics::GeospatialStatistics::new(bbox, geospatial_types) } -fn convert_bounding_box( - bbox: Option, -) -> Option { - bbox.map(|bb| { - let mut newbb = crate::geospatial::bounding_box::BoundingBox::new( - bb.xmin.into(), - bb.xmax.into(), - bb.ymin.into(), - bb.ymax.into(), - ); +fn convert_bounding_box(bb: BoundingBox) -> crate::geospatial::bounding_box::BoundingBox { + let mut newbb = crate::geospatial::bounding_box::BoundingBox::new( + bb.xmin.into(), + bb.xmax.into(), + bb.ymin.into(), + bb.ymax.into(), + ); - newbb = match (bb.zmin, bb.zmax) { - (Some(zmin), Some(zmax)) => newbb.with_zrange(zmin.into(), zmax.into()), - // If either None or mismatch, leave it as None and don't error - _ => newbb, - }; + newbb = match (bb.zmin, bb.zmax) { + (Some(zmin), Some(zmax)) => newbb.with_zrange(zmin.into(), zmax.into()), + // If either None or mismatch, leave it as None and don't error + _ => newbb, + }; - newbb = match (bb.mmin, bb.mmax) { - (Some(mmin), Some(mmax)) => newbb.with_mrange(mmin.into(), mmax.into()), - // If either None or mismatch, leave it as None and don't error - _ => newbb, - }; + newbb = match (bb.mmin, bb.mmax) { + (Some(mmin), Some(mmax)) => newbb.with_mrange(mmin.into(), mmax.into()), + // If either None or mismatch, leave it as None and don't error + _ => newbb, + }; - newbb - }) + newbb } /// Create a [`crate::file::statistics::Statistics`] from a thrift [`Statistics`] object. @@ -544,7 +535,7 @@ fn read_column_metadata<'a>( } 17 => { let val = GeospatialStatistics::read_thrift(&mut *prot)?; - column.geo_statistics = convert_geo_stats(Some(val)); + column.geo_statistics = Some(Box::new(convert_geo_stats(val))); } _ => { prot.skip(field_ident.field_type)?; From 0b3f04966516323bd262dd8fb544246f0c499de5 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:39:21 +0200 Subject: [PATCH 08/20] Enable `clippy::as_ptr_cast_mut` lint 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)` 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()`. --- Cargo.toml | 1 + arrow-array/src/trusted_len.rs | 2 +- arrow-buffer/src/buffer/mutable.rs | 6 +++--- arrow-buffer/src/bytes.rs | 4 +++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7858d123581e..55daad05233e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -144,6 +144,7 @@ broken_intra_doc_links = "warn" missing_crate_level_docs = "warn" [workspace.lints.clippy] +as_ptr_cast_mut = "warn" assigning_clones = "warn" bool_to_int_with_if = "warn" clear_with_drain = "warn" diff --git a/arrow-array/src/trusted_len.rs b/arrow-array/src/trusted_len.rs index b2e1948ccc76..8a3c3edec036 100644 --- a/arrow-array/src/trusted_len.rs +++ b/arrow-array/src/trusted_len.rs @@ -49,7 +49,7 @@ where dst = unsafe { dst.add(1) }; } assert_eq!( - unsafe { dst.offset_from(buffer.as_ptr() as *mut T) as usize }, + unsafe { dst.offset_from(buffer.as_ptr().cast::()) as usize }, upper, "Trusted iterator length was not accurately reported" ); diff --git a/arrow-buffer/src/buffer/mutable.rs b/arrow-buffer/src/buffer/mutable.rs index b30a932b35a7..579deaa94864 100644 --- a/arrow-buffer/src/buffer/mutable.rs +++ b/arrow-buffer/src/buffer/mutable.rs @@ -963,10 +963,10 @@ impl Extend for MutableBuffer { } impl From> for MutableBuffer { - fn from(value: Vec) -> Self { + fn from(mut value: Vec) -> Self { // Safety - // Vec::as_ptr guaranteed to not be null and ArrowNativeType are trivially transmutable - let data = unsafe { NonNull::new_unchecked(value.as_ptr() as _) }; + // Vec::as_mut_ptr guaranteed to not be null and ArrowNativeType are trivially transmutable + let data = unsafe { NonNull::new_unchecked(value.as_mut_ptr().cast()) }; let len = value.len() * mem::size_of::(); // Safety // Vec guaranteed to have a valid layout matching that of `Layout::array` diff --git a/arrow-buffer/src/bytes.rs b/arrow-buffer/src/bytes.rs index a80a347fa17a..d473f13dacad 100644 --- a/arrow-buffer/src/bytes.rs +++ b/arrow-buffer/src/bytes.rs @@ -231,7 +231,9 @@ impl From for Bytes { let len = value.len(); Self { len, - ptr: NonNull::new(value.as_ptr() as _).unwrap(), + // `bytes::Bytes` is shared and immutable, so the buffer is never written + // through this pointer; the cast only changes constness. + ptr: NonNull::new(value.as_ptr().cast_mut()).unwrap(), deallocation: Deallocation::Custom(std::sync::Arc::new(value), len), #[cfg(feature = "pool")] reservation: Mutex::new(None), From 3abe43cdd83f409bb9ce7021538b67005ecdfdd7 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:41:03 +0200 Subject: [PATCH 09/20] Enable `clippy::ref_option_ref` lint `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. --- Cargo.toml | 1 + parquet/src/file/metadata/thrift/mod.rs | 10 +++++----- parquet_derive_test/src/lib.rs | 4 ++++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 55daad05233e..4b1fd6aed358 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -203,6 +203,7 @@ ptr_offset_by_literal = "warn" pub_without_shorthand = "warn" rc_mutex = "warn" ref_binding_to_reference = "warn" +ref_option_ref = "warn" rest_pat_in_fully_bound_structs = "warn" same_functions_in_if_condition = "warn" same_length_and_capacity = "warn" diff --git a/parquet/src/file/metadata/thrift/mod.rs b/parquet/src/file/metadata/thrift/mod.rs index 685cc2b8e543..162c54b45f61 100644 --- a/parquet/src/file/metadata/thrift/mod.rs +++ b/parquet/src/file/metadata/thrift/mod.rs @@ -227,7 +227,7 @@ fn convert_stats( stats.max_value }; - fn check_len(min: &Option<&[u8]>, max: &Option<&[u8]>, len: usize) -> Result<()> { + fn check_len(min: Option<&[u8]>, max: Option<&[u8]>, len: usize) -> Result<()> { if let Some(min) = min && min.len() < len { @@ -243,10 +243,10 @@ fn convert_stats( let physical_type = column_descr.physical_type(); match physical_type { - Type::BOOLEAN => check_len(&min, &max, 1), - Type::INT32 | Type::FLOAT => check_len(&min, &max, 4), - Type::INT64 | Type::DOUBLE => check_len(&min, &max, 8), - Type::INT96 => check_len(&min, &max, 12), + Type::BOOLEAN => check_len(min, max, 1), + Type::INT32 | Type::FLOAT => check_len(min, max, 4), + Type::INT64 | Type::DOUBLE => check_len(min, max, 8), + Type::INT96 => check_len(min, max, 12), _ => Ok(()), }?; diff --git a/parquet_derive_test/src/lib.rs b/parquet_derive_test/src/lib.rs index c18b1c1c49be..106e0b4732e3 100644 --- a/parquet_derive_test/src/lib.rs +++ b/parquet_derive_test/src/lib.rs @@ -28,6 +28,10 @@ use parquet_derive::{ParquetRecordReader, ParquetRecordWriter}; use std::sync::Arc; #[derive(ParquetRecordWriter)] +#[expect( + clippy::ref_option_ref, + reason = "the point of this struct is to cover every field type the derive supports, including `&Option<&T>`" +)] struct ACompleteRecord<'a> { pub a_bool: bool, pub a_str: &'a str, From b6f6fb0cd5c8b5f99ffc2d91ad97e29c181e1192 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:42:56 +0200 Subject: [PATCH 10/20] Enable `clippy::unnecessary_safety_doc` lint 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`. --- Cargo.toml | 1 + arrow-data/src/data.rs | 2 +- arrow-ipc/src/reader.rs | 3 ++- arrow-select/src/take.rs | 4 ++-- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4b1fd6aed358..4d320ae9d5b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -224,6 +224,7 @@ tuple_array_conversions = "warn" uninhabited_references = "warn" unnecessary_box_returns = "warn" unnecessary_literal_bound = "warn" +unnecessary_safety_doc = "warn" unnecessary_self_imports = "warn" unnecessary_struct_initialization = "warn" unused_peekable = "warn" diff --git a/arrow-data/src/data.rs b/arrow-data/src/data.rs index 13e5f730e6e4..b1ce4f22b4ad 100644 --- a/arrow-data/src/data.rs +++ b/arrow-data/src/data.rs @@ -2222,7 +2222,7 @@ impl ArrayDataBuilder { /// Creates an `ArrayData`, consuming `self` /// - /// # Safety + /// # Undefined behavior /// /// By default the underlying buffers are checked to ensure they are valid /// Arrow data. However, if the [`Self::skip_validation`] flag has been set diff --git a/arrow-ipc/src/reader.rs b/arrow-ipc/src/reader.rs index 3cd11adda0ef..6e2a35690dd4 100644 --- a/arrow-ipc/src/reader.rs +++ b/arrow-ipc/src/reader.rs @@ -534,7 +534,8 @@ impl<'a> RecordBatchDecoder<'a> { /// - Offset bounds (e.g. list/string offsets pointing past the end of their value buffer) /// - UTF-8 validity of string columns (`Utf8` / `LargeUtf8`) /// - Null count consistency and buffer length checks - /// # Safety + /// + /// # Undefined behavior /// /// Relies on the caller only passing a flag with `true` value if they are /// certain that the data is valid. Invalid data that bypasses these checks diff --git a/arrow-select/src/take.rs b/arrow-select/src/take.rs index 772f21951ef8..4b027f9d2748 100644 --- a/arrow-select/src/take.rs +++ b/arrow-select/src/take.rs @@ -62,7 +62,7 @@ use num_traits::Zero; /// * An index cannot be casted to `usize` (typically 32 bit architectures) /// * An index is out of bounds and `options` is set to check bounds. /// -/// # Safety +/// # Panics /// /// When `options` is not set to check bounds, taking indexes after `len` will panic. /// @@ -131,7 +131,7 @@ pub fn take( /// * An index cannot be casted to `usize` (typically 32 bit architectures) /// * An index is out of bounds and `options` is set to check bounds. /// -/// # Safety +/// # Panics /// /// When `options` is not set to check bounds, taking indexes after `len` will panic. /// From 627e86dd8faea775ac2e1bd682002d9054161973 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:45:49 +0200 Subject: [PATCH 11/20] Enable `clippy::lossy_float_literal` lint 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. --- Cargo.toml | 1 + arrow-buffer/src/bigint/mod.rs | 2 +- arrow/benches/cast_kernels.rs | 2 +- parquet-variant/src/decoder.rs | 2 +- parquet-variant/tests/variant_interop.rs | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4d320ae9d5b2..1a69bf5db630 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -184,6 +184,7 @@ large_stack_frames = "warn" large_types_passed_by_value = "warn" linkedlist = "warn" literal_string_with_formatting_args = "warn" +lossy_float_literal = "warn" macro_use_imports = "warn" manual_instant_elapsed = "warn" manual_is_power_of_two = "warn" diff --git a/arrow-buffer/src/bigint/mod.rs b/arrow-buffer/src/bigint/mod.rs index 81392c3c3a96..504f18c39189 100644 --- a/arrow-buffer/src/bigint/mod.rs +++ b/arrow-buffer/src/bigint/mod.rs @@ -1812,7 +1812,7 @@ mod tests { assert_eq!(v.to_f64().unwrap(), 42.0); let v = i256::from_i128(-123456789012345678i128); - assert_eq!(v.to_f64().unwrap(), -123456789012345678.0); + assert_eq!(v.to_f64().unwrap(), -123_456_789_012_345_680.0); let v = i256::from_string("0").unwrap(); assert_eq!(v.to_f64().unwrap(), 0.0); diff --git a/arrow/benches/cast_kernels.rs b/arrow/benches/cast_kernels.rs index 85287a8c4042..5c7b5b7e92d6 100644 --- a/arrow/benches/cast_kernels.rs +++ b/arrow/benches/cast_kernels.rs @@ -137,7 +137,7 @@ fn build_string_float_array(size: usize, null_density: f32) -> ArrayRef { builder.append_null() } else { builder.append_value( - rng.random_range(-999_999_999f32..999_999_999f32) + rng.random_range(-1_000_000_000_f32..1_000_000_000_f32) .to_string(), ) } diff --git a/parquet-variant/src/decoder.rs b/parquet-variant/src/decoder.rs index d75f51e7cbfc..ea03f2e2a4d9 100644 --- a/parquet-variant/src/decoder.rs +++ b/parquet-variant/src/decoder.rs @@ -446,7 +446,7 @@ mod tests { test_float, [0x06, 0x2c, 0x93, 0x4e], decode_float, - 1234567890.1234 + 1_234_568_000.0 ); test_decoder_bounds!( diff --git a/parquet-variant/tests/variant_interop.rs b/parquet-variant/tests/variant_interop.rs index 4c7c10d51a15..26efcd0dc902 100644 --- a/parquet-variant/tests/variant_interop.rs +++ b/parquet-variant/tests/variant_interop.rs @@ -132,7 +132,7 @@ fn get_primitive_cases() -> Vec<(&'static str, Variant<'static, 'static>)> { "primitive_decimal16", Variant::Decimal16(VariantDecimal16::try_new(1234567891234567890, 2).unwrap()), ), - ("primitive_float", Variant::Float(1234567890.1234)), + ("primitive_float", Variant::Float(1_234_568_000.0)), ("primitive_double", Variant::Double(1234567890.1234)), ("primitive_int8", Variant::Int8(42)), ("primitive_int16", Variant::Int16(1234)), From 9217eac91d06e0b64226fa08f040936176e867b0 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:48:20 +0200 Subject: [PATCH 12/20] Enable `clippy::checked_conversions` lint 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. --- Cargo.toml | 1 + arrow-array/src/array/byte_view_array.rs | 2 +- arrow-avro/src/reader/record.rs | 5 ++++- arrow-string/src/concat_elements.rs | 2 +- parquet-variant/src/builder/metadata.rs | 2 +- parquet/src/file/metadata/thrift/mod.rs | 2 +- 6 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1a69bf5db630..1187ab4616b1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -147,6 +147,7 @@ missing_crate_level_docs = "warn" as_ptr_cast_mut = "warn" assigning_clones = "warn" bool_to_int_with_if = "warn" +checked_conversions = "warn" clear_with_drain = "warn" comparison_chain = "warn" dbg_macro = "warn" diff --git a/arrow-array/src/array/byte_view_array.rs b/arrow-array/src/array/byte_view_array.rs index 256c5996f256..d1b1721034d9 100644 --- a/arrow-array/src/array/byte_view_array.rs +++ b/arrow-array/src/array/byte_view_array.rs @@ -618,7 +618,7 @@ impl GenericByteViewArray { total_len: current_elements, }); } - debug_assert!(groups.len() <= i32::MAX as usize); + debug_assert!(i32::try_from(groups.len()).is_ok()); // Second pass: copy each group into an exactly-sized buffer. let mut views_buf = Vec::with_capacity(len); diff --git a/arrow-avro/src/reader/record.rs b/arrow-avro/src/reader/record.rs index 13382557ee6d..beb4102de19c 100644 --- a/arrow-avro/src/reader/record.rs +++ b/arrow-avro/src/reader/record.rs @@ -2330,7 +2330,10 @@ fn process_block_items( total: usize, on_item: &mut impl FnMut(&mut AvroCursor) -> Result<(), AvroError>, ) -> Result { - let Some(new_total) = total.checked_add(count).filter(|&t| t <= i32::MAX as usize) else { + let Some(new_total) = total + .checked_add(count) + .filter(|&t| i32::try_from(t).is_ok()) + else { return Err(AvroError::ParseError( "Capacity overflow when decoding array/map item blocks".to_string(), )); diff --git a/arrow-string/src/concat_elements.rs b/arrow-string/src/concat_elements.rs index 72898793b8bd..cbba7fbec6c6 100644 --- a/arrow-string/src/concat_elements.rs +++ b/arrow-string/src/concat_elements.rs @@ -318,7 +318,7 @@ where // in `concat_elements_view_array`, so offset cannot exceed it. // Not using `u32::try_from` on each insertion makes a ~5% difference // in benchmarking - debug_assert!(offset <= i32::MAX as usize); + debug_assert!(i32::try_from(offset).is_ok()); let view_offset: u32 = offset as u32; self.data.extend_from_slice(left); diff --git a/parquet-variant/src/builder/metadata.rs b/parquet-variant/src/builder/metadata.rs index 10b34ced03a6..ea1a6d46947c 100644 --- a/parquet-variant/src/builder/metadata.rs +++ b/parquet-variant/src/builder/metadata.rs @@ -178,7 +178,7 @@ impl WritableMetadataBuilder { /// If the number of field names exceeds the maximum allowed value for `u32`. fn num_field_names(&self) -> usize { let n = self.field_names.len(); - assert!(n <= u32::MAX as usize); + assert!(u32::try_from(n).is_ok()); n } diff --git a/parquet/src/file/metadata/thrift/mod.rs b/parquet/src/file/metadata/thrift/mod.rs index 162c54b45f61..ffab14606ecb 100644 --- a/parquet/src/file/metadata/thrift/mod.rs +++ b/parquet/src/file/metadata/thrift/mod.rs @@ -1450,7 +1450,7 @@ impl<'a> WriteThrift for FileMeta<'a> { fn write_thrift(&self, writer: &mut ThriftCompactOutputProtocol) -> Result<()> { writer.set_write_path_in_schema(self.write_path_in_schema); // only write ordinal if all values will fit in an i16 - writer.set_write_row_group_ordinal(self.row_groups.len() <= i16::MAX as usize); + writer.set_write_row_group_ordinal(i16::try_from(self.row_groups.len()).is_ok()); self.file_metadata .version From 454f504a4798d2fc49b682153a57817ad3d145a2 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:51:00 +0200 Subject: [PATCH 13/20] Enable `clippy::ptr_cast_constness` lint 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. --- Cargo.toml | 1 + arrow-array/src/ffi.rs | 2 +- arrow-buffer/src/buffer/mutable.rs | 2 +- arrow-schema/src/ffi.rs | 4 ++-- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1187ab4616b1..f198f9b059f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -201,6 +201,7 @@ option_as_ref_cloned = "warn" option_option = "warn" path_buf_push_overwrite = "warn" pathbuf_init_then_push = "warn" +ptr_cast_constness = "warn" ptr_offset_by_literal = "warn" pub_without_shorthand = "warn" rc_mutex = "warn" diff --git a/arrow-array/src/ffi.rs b/arrow-array/src/ffi.rs index e1761beb2bf4..970419c633b1 100644 --- a/arrow-array/src/ffi.rs +++ b/arrow-array/src/ffi.rs @@ -234,7 +234,7 @@ unsafe fn create_buffer( if array.num_buffers() == 0 { return None; } - NonNull::new(array.buffer(index) as _) + NonNull::new(array.buffer(index).cast_mut()) .map(|ptr| unsafe { Buffer::from_custom_allocation(ptr, len, owner) }) } diff --git a/arrow-buffer/src/buffer/mutable.rs b/arrow-buffer/src/buffer/mutable.rs index 579deaa94864..e83b0c47082d 100644 --- a/arrow-buffer/src/buffer/mutable.rs +++ b/arrow-buffer/src/buffer/mutable.rs @@ -381,7 +381,7 @@ impl MutableBuffer { let byte_count = to_copy * bytes_per_copy; unsafe { // Get to the start of the data before we started copying anything - let src = self.data.as_ptr().add(length_before) as *const u8; + let src = self.data.as_ptr().add(length_before).cast_const(); // Go to the current location to copy to (end of current data) let dst = self.data.as_ptr().add(self.len); // SAFETY: the pointers are not overlapping as there is `byte_count` or less between them diff --git a/arrow-schema/src/ffi.rs b/arrow-schema/src/ffi.rs index d8f779c36456..7b6332632e17 100644 --- a/arrow-schema/src/ffi.rs +++ b/arrow-schema/src/ffi.rs @@ -110,9 +110,9 @@ unsafe extern "C" fn release_schema(schema: *mut FFI_ArrowSchema) { let schema = unsafe { &mut *schema }; // take ownership back to release it. - drop(unsafe { CString::from_raw(schema.format as *mut c_char) }); + drop(unsafe { CString::from_raw(schema.format.cast_mut()) }); if !schema.name.is_null() { - drop(unsafe { CString::from_raw(schema.name as *mut c_char) }); + drop(unsafe { CString::from_raw(schema.name.cast_mut()) }); } if !schema.private_data.is_null() { let private_data = unsafe { Box::from_raw(schema.private_data as *mut SchemaPrivateData) }; From 632aaf8019cc7f6c5afddbf45691fce7e3615ca5 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 21:53:10 +0200 Subject: [PATCH 14/20] Enable `clippy::unused_async` lint 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. --- Cargo.toml | 1 + arrow-flight/src/sql/client.rs | 4 ++++ .../auth_basic_proto.rs | 24 +++++++++---------- .../integration_test.rs | 10 ++++---- 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f198f9b059f3..f1dd6edaeb46 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -230,6 +230,7 @@ unnecessary_literal_bound = "warn" unnecessary_safety_doc = "warn" unnecessary_self_imports = "warn" unnecessary_struct_initialization = "warn" +unused_async = "warn" unused_peekable = "warn" unused_rounding = "warn" wildcard_dependencies = "warn" diff --git a/arrow-flight/src/sql/client.rs b/arrow-flight/src/sql/client.rs index 28a4a7f972a8..8ef80da50b90 100644 --- a/arrow-flight/src/sql/client.rs +++ b/arrow-flight/src/sql/client.rs @@ -441,6 +441,10 @@ where } /// Explicitly shut down and clean up the client. + #[expect( + clippy::unused_async, + reason = "public API: dropping `async` would break callers that `.await` it" + )] pub async fn close(&mut self) -> Result<()> { // TODO: consume self instead of &mut self to explicitly prevent reuse? Ok(()) diff --git a/arrow-integration-testing/src/flight_server_scenarios/auth_basic_proto.rs b/arrow-integration-testing/src/flight_server_scenarios/auth_basic_proto.rs index 38582e6fef68..65466c171b3b 100644 --- a/arrow-integration-testing/src/flight_server_scenarios/auth_basic_proto.rs +++ b/arrow-integration-testing/src/flight_server_scenarios/auth_basic_proto.rs @@ -65,15 +65,15 @@ pub struct AuthBasicProtoScenarioImpl { } impl AuthBasicProtoScenarioImpl { - async fn check_auth(&self, metadata: &MetadataMap) -> Result { + fn check_auth(&self, metadata: &MetadataMap) -> Result { let token = metadata .get_bin("auth-token-bin") .and_then(|v| v.to_bytes().ok()) .and_then(|b| String::from_utf8(b.to_vec()).ok()); - self.is_valid(token).await + self.is_valid(token) } - async fn is_valid(&self, token: Option) -> Result { + fn is_valid(&self, token: Option) -> Result { match token { Some(t) if t == *self.username => Ok(GrpcServerCallContext { peer_identity: self.username.to_string(), @@ -107,7 +107,7 @@ impl FlightService for AuthBasicProtoScenarioImpl { &self, request: Request, ) -> Result, Status> { - self.check_auth(request.metadata()).await?; + self.check_auth(request.metadata())?; Err(Status::unimplemented("Not yet implemented")) } @@ -115,7 +115,7 @@ impl FlightService for AuthBasicProtoScenarioImpl { &self, request: Request, ) -> Result, Status> { - self.check_auth(request.metadata()).await?; + self.check_auth(request.metadata())?; Err(Status::unimplemented("Not yet implemented")) } @@ -170,7 +170,7 @@ impl FlightService for AuthBasicProtoScenarioImpl { &self, request: Request, ) -> Result, Status> { - self.check_auth(request.metadata()).await?; + self.check_auth(request.metadata())?; Err(Status::unimplemented("Not yet implemented")) } @@ -178,7 +178,7 @@ impl FlightService for AuthBasicProtoScenarioImpl { &self, request: Request, ) -> Result, Status> { - self.check_auth(request.metadata()).await?; + self.check_auth(request.metadata())?; Err(Status::unimplemented("Not yet implemented")) } @@ -186,7 +186,7 @@ impl FlightService for AuthBasicProtoScenarioImpl { &self, request: Request, ) -> Result, Status> { - self.check_auth(request.metadata()).await?; + self.check_auth(request.metadata())?; Err(Status::unimplemented("Not yet implemented")) } @@ -195,7 +195,7 @@ impl FlightService for AuthBasicProtoScenarioImpl { request: Request>, ) -> Result, Status> { let metadata = request.metadata(); - self.check_auth(metadata).await?; + self.check_auth(metadata)?; Err(Status::unimplemented("Not yet implemented")) } @@ -203,7 +203,7 @@ impl FlightService for AuthBasicProtoScenarioImpl { &self, request: Request, ) -> Result, Status> { - let flight_context = self.check_auth(request.metadata()).await?; + let flight_context = self.check_auth(request.metadata())?; // Respond with the authenticated username. let buf = flight_context.peer_identity().as_bytes().to_vec().into(); let result = arrow_flight::Result { body: buf }; @@ -215,7 +215,7 @@ impl FlightService for AuthBasicProtoScenarioImpl { &self, request: Request, ) -> Result, Status> { - self.check_auth(request.metadata()).await?; + self.check_auth(request.metadata())?; Err(Status::unimplemented("Not yet implemented")) } @@ -224,7 +224,7 @@ impl FlightService for AuthBasicProtoScenarioImpl { request: Request>, ) -> Result, Status> { let metadata = request.metadata(); - self.check_auth(metadata).await?; + self.check_auth(metadata)?; Err(Status::unimplemented("Not yet implemented")) } } diff --git a/arrow-integration-testing/src/flight_server_scenarios/integration_test.rs b/arrow-integration-testing/src/flight_server_scenarios/integration_test.rs index ae316886381a..42227b5c0956 100644 --- a/arrow-integration-testing/src/flight_server_scenarios/integration_test.rs +++ b/arrow-integration-testing/src/flight_server_scenarios/integration_test.rs @@ -318,7 +318,7 @@ async fn send_app_metadata( .map_err(|e| Status::internal(format!("Could not send PutResult: {e:?}"))) } -async fn record_batch_from_message( +fn record_batch_from_message( message: ipc::Message<'_>, data_body: &Buffer, schema_ref: SchemaRef, @@ -341,7 +341,7 @@ async fn record_batch_from_message( .map_err(|e| Status::internal(format!("Could not convert to RecordBatch: {e:?}"))) } -async fn dictionary_from_message( +fn dictionary_from_message( message: ipc::Message<'_>, data_body: &Buffer, schema_ref: SchemaRef, @@ -393,8 +393,7 @@ async fn save_uploaded_chunks( &Buffer::from(data.data_body.as_ref()), schema_ref.clone(), &dictionaries_by_id, - ) - .await?; + )?; chunks.push(batch); } @@ -404,8 +403,7 @@ async fn save_uploaded_chunks( &Buffer::from(data.data_body.as_ref()), schema_ref.clone(), &mut dictionaries_by_id, - ) - .await?; + )?; } t => { return Err(Status::internal(format!( From a37be42f45763cc29cc72eeb2f84a3e933500bd3 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 22:00:01 +0200 Subject: [PATCH 15/20] Enable `clippy::useless_let_if_seq` lint 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. --- Cargo.toml | 1 + arrow-ord/src/sort.rs | 10 ++-- .../arrow/record_reader/definition_levels.rs | 14 +---- parquet/src/file/metadata/thrift/mod.rs | 29 +++++------ parquet/src/file/serialized_reader.rs | 51 ++++++++++--------- parquet/src/record/reader.rs | 5 +- 6 files changed, 48 insertions(+), 62 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f1dd6edaeb46..61b9ce635c2c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -233,6 +233,7 @@ unnecessary_struct_initialization = "warn" unused_async = "warn" unused_peekable = "warn" unused_rounding = "warn" +useless_let_if_seq = "warn" wildcard_dependencies = "warn" zero_sized_map_values = "warn" diff --git a/arrow-ord/src/sort.rs b/arrow-ord/src/sort.rs index 0c6720e8f249..fea43dc1a74d 100644 --- a/arrow-ord/src/sort.rs +++ b/arrow-ord/src/sort.rs @@ -1095,11 +1095,11 @@ fn sift_down_worst_heap( } let right = left + 1; - let mut worst = left; - - if right < heap.len() && compare(heap[left], heap[right]) == Ordering::Less { - worst = right; - } + let worst = if right < heap.len() && compare(heap[left], heap[right]) == Ordering::Less { + right + } else { + left + }; if compare(heap[pos], heap[worst]) != Ordering::Less { break; diff --git a/parquet/src/arrow/record_reader/definition_levels.rs b/parquet/src/arrow/record_reader/definition_levels.rs index b4182b166e04..0096df0a0212 100644 --- a/parquet/src/arrow/record_reader/definition_levels.rs +++ b/parquet/src/arrow/record_reader/definition_levels.rs @@ -172,18 +172,8 @@ pub(crate) fn build_filtered_validity_bitmap( let mut include_mask: u64 = 0; let mut value_mask: u64 = 0; for (i, &d) in chunk.iter().enumerate() { - let mut include = true; - if let Some(threshold) = include_threshold - && d < threshold - { - include = false; - } - if include - && let Some((reps, max_rep)) = rep_filter - && reps[base + i] > max_rep - { - include = false; - } + let include = !include_threshold.is_some_and(|threshold| d < threshold) + && !rep_filter.is_some_and(|(reps, max_rep)| reps[base + i] > max_rep); include_mask |= (include as u64) << i; value_mask |= ((d >= value_level) as u64) << i; } diff --git a/parquet/src/file/metadata/thrift/mod.rs b/parquet/src/file/metadata/thrift/mod.rs index ffab14606ecb..656c661817cb 100644 --- a/parquet/src/file/metadata/thrift/mod.rs +++ b/parquet/src/file/metadata/thrift/mod.rs @@ -427,17 +427,15 @@ fn read_column_metadata<'a>( // mask for seen required fields in ColumnMetaData let mut seen_mask = 0u16; - let mut skip_pes = false; - let mut pes_mask = true; - let mut skip_col_stats = false; - let mut skip_size_stats = false; - - if let Some(opts) = options { - skip_pes = opts.skip_encoding_stats(col_index); - pes_mask = opts.encoding_stats_as_mask(); - skip_col_stats = opts.skip_column_stats(col_index); - skip_size_stats = opts.skip_size_stats(col_index); - } + let (skip_pes, pes_mask, skip_col_stats, skip_size_stats) = match options { + Some(opts) => ( + opts.skip_encoding_stats(col_index), + opts.encoding_stats_as_mask(), + opts.skip_column_stats(col_index), + opts.skip_size_stats(col_index), + ), + None => (false, true, false, false), + }; // struct ColumnMetaData { // 1: required Type type @@ -772,13 +770,10 @@ pub(crate) fn parquet_metadata_from_bytes( #[cfg(feature = "encryption")] let mut footer_signing_key_metadata: Option<&[u8]> = None; - // this will need to be set before parsing row groups - let mut schema_descr: Option> = None; - + // this will need to be set before parsing row groups. // see if we already have a schema. - if let Some(options) = options { - schema_descr = options.schema().cloned(); - } + let mut schema_descr: Option> = + options.and_then(|options| options.schema().cloned()); // struct FileMetaData { // 1: required i32 version diff --git a/parquet/src/file/serialized_reader.rs b/parquet/src/file/serialized_reader.rs index 7c9b33132e69..b52aac367bf9 100644 --- a/parquet/src/file/serialized_reader.rs +++ b/parquet/src/file/serialized_reader.rs @@ -411,31 +411,34 @@ pub(crate) fn decode_page( // // We always use 0 offset for other pages other than v2, `true` flag means // that compression will be applied if decompressor is defined - let mut offset: usize = 0; - let mut can_decompress = true; - - if let Some(ref header_v2) = page_header.data_page_header_v2 { - if header_v2.definition_levels_byte_length < 0 - || header_v2.repetition_levels_byte_length < 0 - || header_v2.definition_levels_byte_length + header_v2.repetition_levels_byte_length - > page_header.uncompressed_page_size - { - return Err(general_err!( - "DataPage v2 header contains implausible values \ - for definition_levels_byte_length ({}) \ - and repetition_levels_byte_length ({}) \ - given DataPage header provides uncompressed_page_size ({})", - header_v2.definition_levels_byte_length, - header_v2.repetition_levels_byte_length, - page_header.uncompressed_page_size - )); + let (offset, can_decompress): (usize, bool) = match page_header.data_page_header_v2 { + Some(ref header_v2) => { + if header_v2.definition_levels_byte_length < 0 + || header_v2.repetition_levels_byte_length < 0 + || header_v2.definition_levels_byte_length + header_v2.repetition_levels_byte_length + > page_header.uncompressed_page_size + { + return Err(general_err!( + "DataPage v2 header contains implausible values \ + for definition_levels_byte_length ({}) \ + and repetition_levels_byte_length ({}) \ + given DataPage header provides uncompressed_page_size ({})", + header_v2.definition_levels_byte_length, + header_v2.repetition_levels_byte_length, + page_header.uncompressed_page_size + )); + } + ( + usize::try_from( + header_v2.definition_levels_byte_length + + header_v2.repetition_levels_byte_length, + )?, + // When is_compressed flag is missing the page is considered compressed + header_v2.is_compressed.unwrap_or(true), + ) } - offset = usize::try_from( - header_v2.definition_levels_byte_length + header_v2.repetition_levels_byte_length, - )?; - // When is_compressed flag is missing the page is considered compressed - can_decompress = header_v2.is_compressed.unwrap_or(true); - } + None => (0, true), + }; let buffer = match decompressor { Some(decompressor) if can_decompress => { diff --git a/parquet/src/record/reader.rs b/parquet/src/record/reader.rs index bee49c70d81d..f1a262250ca9 100644 --- a/parquet/src/record/reader.rs +++ b/parquet/src/record/reader.rs @@ -785,10 +785,7 @@ impl Iterator for RowIter<'_> { type Item = Result; fn next(&mut self) -> Option> { - let mut row = None; - if let Some(ref mut iter) = self.row_iter { - row = iter.next(); - } + let mut row = self.row_iter.as_mut().and_then(|iter| iter.next()); while row.is_none() && self.current_row_group < self.num_row_groups { // We do not expect any failures when accessing a row group, and file reader From 47935b6804d1c0f2979f61ace6977a717df28e06 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 22:02:24 +0200 Subject: [PATCH 16/20] Enable `clippy::verbose_file_reads` lint 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. --- Cargo.toml | 1 + arrow-csv/src/writer.rs | 4 ++++ arrow-flight/gen/src/main.rs | 18 +++--------------- arrow-integration-test/src/lib.rs | 7 +------ 4 files changed, 9 insertions(+), 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 61b9ce635c2c..de330961c938 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -234,6 +234,7 @@ unused_async = "warn" unused_peekable = "warn" unused_rounding = "warn" useless_let_if_seq = "warn" +verbose_file_reads = "warn" wildcard_dependencies = "warn" zero_sized_map_values = "warn" diff --git a/arrow-csv/src/writer.rs b/arrow-csv/src/writer.rs index 72dcb640e1c9..5236775b68ee 100644 --- a/arrow-csv/src/writer.rs +++ b/arrow-csv/src/writer.rs @@ -666,6 +666,10 @@ impl WriterBuilder { } #[cfg(test)] +#[expect( + clippy::verbose_file_reads, + reason = "`tempfile::tempfile()` has no path, so the contents can only be read back through the handle" +)] mod tests { use super::*; diff --git a/arrow-flight/gen/src/main.rs b/arrow-flight/gen/src/main.rs index cd50214f1c85..5809b5ced822 100644 --- a/arrow-flight/gen/src/main.rs +++ b/arrow-flight/gen/src/main.rs @@ -17,11 +17,7 @@ //! Generates the Rust bindings for the Arrow Flight protobuf definitions. -use std::{ - fs::OpenOptions, - io::{Read, Write}, - path::Path, -}; +use std::{fs::OpenOptions, io::Write, path::Path}; fn main() -> Result<(), Box> { let proto_dir = Path::new("../format"); @@ -34,11 +30,7 @@ fn main() -> Result<(), Box> { .compile_with_config(prost_config(), &[proto_path], &[proto_dir])?; // read file contents to string - let mut file = OpenOptions::new() - .read(true) - .open("src/arrow.flight.protocol.rs")?; - let mut buffer = String::new(); - file.read_to_string(&mut buffer)?; + let buffer = std::fs::read_to_string("src/arrow.flight.protocol.rs")?; // append warning that file was auto-generated let mut file = OpenOptions::new() .write(true) @@ -57,11 +49,7 @@ fn main() -> Result<(), Box> { .compile_with_config(prost_config(), &[proto_path], &[proto_dir])?; // read file contents to string - let mut file = OpenOptions::new() - .read(true) - .open("src/sql/arrow.flight.protocol.sql.rs")?; - let mut buffer = String::new(); - file.read_to_string(&mut buffer)?; + let buffer = std::fs::read_to_string("src/sql/arrow.flight.protocol.sql.rs")?; // append warning that file was auto-generate let mut file = OpenOptions::new() .write(true) diff --git a/arrow-integration-test/src/lib.rs b/arrow-integration-test/src/lib.rs index ac027554b250..0d5477e955e3 100644 --- a/arrow-integration-test/src/lib.rs +++ b/arrow-integration-test/src/lib.rs @@ -1272,9 +1272,6 @@ impl ArrowJsonBatch { mod tests { use super::*; - use std::fs::File; - use std::io::Read; - #[test] fn test_schema_equality() { let json = r#" @@ -1596,9 +1593,7 @@ mod tests { ], ) .unwrap(); - let mut file = File::open("data/integration.json").unwrap(); - let mut json = String::new(); - file.read_to_string(&mut json).unwrap(); + let json = std::fs::read_to_string("data/integration.json").unwrap(); let arrow_json: ArrowJson = serde_json::from_str(&json).unwrap(); // test schemas assert!(arrow_json.schema.equals_schema(&schema)); From f217d18738ec9b0fb9715b886962ef054beeca36 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 22:04:53 +0200 Subject: [PATCH 17/20] Enable `clippy::inconsistent_struct_constructor` lint 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. --- Cargo.toml | 1 + arrow-array/src/array/byte_array.rs | 2 +- arrow-array/src/array/fixed_size_binary_array.rs | 2 +- arrow-array/src/builder/generic_bytes_builder.rs | 2 +- arrow-array/src/iterator.rs | 2 +- arrow-avro/src/codec.rs | 4 ++-- arrow-buffer/src/bigint/mod.rs | 4 ++-- arrow-buffer/src/buffer/run.rs | 4 ++-- parquet/src/file/page_index/column_index.rs | 2 +- 9 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index de330961c938..4e179b9d5e45 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -167,6 +167,7 @@ fn_params_excessive_bools = "warn" fn_to_numeric_cast_any = "warn" format_push_string = "warn" imprecise_flops = "warn" +inconsistent_struct_constructor = "warn" index_refutable_slice = "warn" inefficient_to_string = "warn" infinite_loop = "warn" diff --git a/arrow-array/src/array/byte_array.rs b/arrow-array/src/array/byte_array.rs index e58296817405..cd9d446f4d5d 100644 --- a/arrow-array/src/array/byte_array.rs +++ b/arrow-array/src/array/byte_array.rs @@ -574,9 +574,9 @@ impl From for GenericByteArray { // ArrayData is valid, and verified type above let value_offsets = unsafe { get_offsets_from_buffer(offset_buffer, offset, len) }; Self { + data_type, value_offsets, value_data, - data_type, nulls, } } diff --git a/arrow-array/src/array/fixed_size_binary_array.rs b/arrow-array/src/array/fixed_size_binary_array.rs index 98a812460bdc..254e00cd6a7c 100644 --- a/arrow-array/src/array/fixed_size_binary_array.rs +++ b/arrow-array/src/array/fixed_size_binary_array.rs @@ -694,9 +694,9 @@ impl From for FixedSizeBinaryArray { Self { data_type, + value_data, nulls, len, - value_data, value_size, } } diff --git a/arrow-array/src/builder/generic_bytes_builder.rs b/arrow-array/src/builder/generic_bytes_builder.rs index a2d044e80890..f449219c6be0 100644 --- a/arrow-array/src/builder/generic_bytes_builder.rs +++ b/arrow-array/src/builder/generic_bytes_builder.rs @@ -76,8 +76,8 @@ impl GenericByteBuilder { .unwrap_or_else(|| NullBufferBuilder::new_with_len(offsets_builder.len() - 1)); Self { - offsets_builder, value_builder, + offsets_builder, null_buffer_builder, } } diff --git a/arrow-array/src/iterator.rs b/arrow-array/src/iterator.rs index 9f72313b9a8b..156e296cf151 100644 --- a/arrow-array/src/iterator.rs +++ b/arrow-array/src/iterator.rs @@ -895,7 +895,7 @@ mod tests { let mut items = Vec::with_capacity(iter.len()); let cb = |acc, item| { - items.push(CallArgs { item, acc }); + items.push(CallArgs { acc, item }); item.map(|val| val + 100) }; diff --git a/arrow-avro/src/codec.rs b/arrow-avro/src/codec.rs index c4df569ba076..5618539c50f6 100644 --- a/arrow-avro/src/codec.rs +++ b/arrow-avro/src/codec.rs @@ -208,9 +208,9 @@ impl AvroDataType { resolution: Option, ) -> Self { Self { - codec, - metadata, nullability, + metadata, + codec, resolution, } } diff --git a/arrow-buffer/src/bigint/mod.rs b/arrow-buffer/src/bigint/mod.rs index 504f18c39189..d8328cb06f9e 100644 --- a/arrow-buffer/src/bigint/mod.rs +++ b/arrow-buffer/src/bigint/mod.rs @@ -768,7 +768,7 @@ impl i256 { .wrapping_add(rhs.high as u128) .wrapping_add(carry as u128) as i128; - let result = Self { high, low }; + let result = Self { low, high }; // Signed overflow occurs when: // - both operands have the same sign, and @@ -792,7 +792,7 @@ impl i256 { .wrapping_sub(rhs.high as u128) .wrapping_sub(borrow as u128) as i128; - let result = Self { high, low }; + let result = Self { low, high }; // Signed overflow occurs when: // - operands have opposite signs, and diff --git a/arrow-buffer/src/buffer/run.rs b/arrow-buffer/src/buffer/run.rs index 703ae913801d..c65e3b5cc642 100644 --- a/arrow-buffer/src/buffer/run.rs +++ b/arrow-buffer/src/buffer/run.rs @@ -132,8 +132,8 @@ where Self { run_ends, - logical_offset, logical_length, + logical_offset, } } @@ -151,8 +151,8 @@ where ) -> Self { Self { run_ends, - logical_offset, logical_length, + logical_offset, } } diff --git a/parquet/src/file/page_index/column_index.rs b/parquet/src/file/page_index/column_index.rs index 4b766cc97218..665b4bc454b0 100644 --- a/parquet/src/file/page_index/column_index.rs +++ b/parquet/src/file/page_index/column_index.rs @@ -405,9 +405,9 @@ impl ByteArrayColumnIndex { null_pages, boundary_order, null_counts, - nan_counts, repetition_level_histograms, definition_level_histograms, + nan_counts, }, min_bytes, min_offsets, From a68db448fd24f91342e82deec97846c84743d092 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 4 Aug 2026 22:06:59 +0200 Subject: [PATCH 18/20] Address review comments - 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. --- arrow-flight/gen/src/main.rs | 2 -- arrow/benches/builder.rs | 6 +++++- parquet/src/arrow/record_reader/definition_levels.rs | 4 ++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/arrow-flight/gen/src/main.rs b/arrow-flight/gen/src/main.rs index 5809b5ced822..08254ff2b2eb 100644 --- a/arrow-flight/gen/src/main.rs +++ b/arrow-flight/gen/src/main.rs @@ -29,7 +29,6 @@ fn main() -> Result<(), Box> { .out_dir("src") .compile_with_config(prost_config(), &[proto_path], &[proto_dir])?; - // read file contents to string let buffer = std::fs::read_to_string("src/arrow.flight.protocol.rs")?; // append warning that file was auto-generated let mut file = OpenOptions::new() @@ -48,7 +47,6 @@ fn main() -> Result<(), Box> { .out_dir("src/sql") .compile_with_config(prost_config(), &[proto_path], &[proto_dir])?; - // read file contents to string let buffer = std::fs::read_to_string("src/sql/arrow.flight.protocol.sql.rs")?; // append warning that file was auto-generate let mut file = OpenOptions::new() diff --git a/arrow/benches/builder.rs b/arrow/benches/builder.rs index 7a1e8748a50c..0f2532c77be5 100644 --- a/arrow/benches/builder.rs +++ b/arrow/benches/builder.rs @@ -35,7 +35,11 @@ const BATCH_SIZE: usize = 8 << 10; const NUM_BATCHES: usize = 64; fn bench_primitive(c: &mut Criterion) { - let data = vec![100i64; BATCH_SIZE]; + #[expect( + clippy::large_stack_arrays, + reason = "kept on the stack so the benchmark measures what it always has" + )] + let data: [i64; BATCH_SIZE] = [100; BATCH_SIZE]; let mut group = c.benchmark_group("bench_primitive"); group.throughput(Throughput::Bytes( diff --git a/parquet/src/arrow/record_reader/definition_levels.rs b/parquet/src/arrow/record_reader/definition_levels.rs index 0096df0a0212..c48c3269800a 100644 --- a/parquet/src/arrow/record_reader/definition_levels.rs +++ b/parquet/src/arrow/record_reader/definition_levels.rs @@ -172,8 +172,8 @@ pub(crate) fn build_filtered_validity_bitmap( let mut include_mask: u64 = 0; let mut value_mask: u64 = 0; for (i, &d) in chunk.iter().enumerate() { - let include = !include_threshold.is_some_and(|threshold| d < threshold) - && !rep_filter.is_some_and(|(reps, max_rep)| reps[base + i] > max_rep); + let include = include_threshold.is_none_or(|threshold| threshold <= d) + && rep_filter.is_none_or(|(reps, max_rep)| reps[base + i] <= max_rep); include_mask |= (include as u64) << i; value_mask |= ((d >= value_level) as u64) << i; } From e1e9790352c44a5b8100b6db6ffa4fcbe055c16d Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 5 Aug 2026 08:58:59 +0200 Subject: [PATCH 19/20] Collapse nested `if` in `interval_mul_f64` `clippy::collapsible_if` is deny-by-default in CI and has been failing on `main` since #10409; the nested `if let` is now a let-chain. --- arrow-arith/src/numeric.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arrow-arith/src/numeric.rs b/arrow-arith/src/numeric.rs index 533c6d330b0b..be6ae21ff51c 100644 --- a/arrow-arith/src/numeric.rs +++ b/arrow-arith/src/numeric.rs @@ -753,10 +753,10 @@ fn interval_mul_f64( const SECONDS_PER_DAY: f64 = SECONDS_IN_DAY as f64; // Keep integral factors exact instead of round-tripping i64 nanoseconds through f64. - if factor.fract() == 0. { - if let Some(factor) = ToPrimitive::to_i64(&factor) { - return IntervalMonthDayNanoType::mul_i64(interval, factor); - } + if factor.fract() == 0. + && let Some(factor) = ToPrimitive::to_i64(&factor) + { + return IntervalMonthDayNanoType::mul_i64(interval, factor); } // Based on DuckDB's INTERVAL * DOUBLE implementation, which is referenced from PostgreSQL's interval_mul: From 5fa9ba3a87368e18c0d1254a07d50087f5934e97 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Wed, 5 Aug 2026 16:11:53 +0200 Subject: [PATCH 20/20] Revert useless_let_if_seq change in read_column_metadata The original form reads better than the tuple-match; keep it behind an `#[expect]`. Co-Authored-By: Claude Opus 5 (1M context) --- parquet/src/file/metadata/thrift/mod.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/parquet/src/file/metadata/thrift/mod.rs b/parquet/src/file/metadata/thrift/mod.rs index 656c661817cb..eb5dc9e689cf 100644 --- a/parquet/src/file/metadata/thrift/mod.rs +++ b/parquet/src/file/metadata/thrift/mod.rs @@ -418,6 +418,7 @@ fn read_encoding_stats_as_mask<'a>( // Decode `ColumnMetaData`. Returns a mask of all required fields that were observed. // This mask can be passed to `validate_column_metadata`. +#[expect(clippy::useless_let_if_seq)] // the `let mut … if let …` below is more readable than the suggestion fn read_column_metadata<'a>( prot: &mut ThriftSliceInputProtocol<'a>, column: &mut ColumnChunkMetaData, @@ -427,15 +428,17 @@ fn read_column_metadata<'a>( // mask for seen required fields in ColumnMetaData let mut seen_mask = 0u16; - let (skip_pes, pes_mask, skip_col_stats, skip_size_stats) = match options { - Some(opts) => ( - opts.skip_encoding_stats(col_index), - opts.encoding_stats_as_mask(), - opts.skip_column_stats(col_index), - opts.skip_size_stats(col_index), - ), - None => (false, true, false, false), - }; + let mut skip_pes = false; + let mut pes_mask = true; + let mut skip_col_stats = false; + let mut skip_size_stats = false; + + if let Some(opts) = options { + skip_pes = opts.skip_encoding_stats(col_index); + pes_mask = opts.encoding_stats_as_mask(); + skip_col_stats = opts.skip_column_stats(col_index); + skip_size_stats = opts.skip_size_stats(col_index); + } // struct ColumnMetaData { // 1: required Type type