diff --git a/Cargo.toml b/Cargo.toml index 7f7d98cf1010..39ec344fcd5f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -154,37 +154,99 @@ 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" +checked_conversions = "warn" +clear_with_drain = "warn" +comparison_chain = "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" +inconsistent_struct_constructor = "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_arrays = "warn" +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" +manual_midpoint = "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_cast_constness = "warn" 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" +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" +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" +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_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" # release inherited profile keeping debug information and symbols 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/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-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/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-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-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-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-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-buffer/src/bigint/mod.rs b/arrow-buffer/src/bigint/mod.rs index 81392c3c3a96..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 @@ -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-buffer/src/buffer/mutable.rs b/arrow-buffer/src/buffer/mutable.rs index b30a932b35a7..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 @@ -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/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/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), 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-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-flight/gen/src/main.rs b/arrow-flight/gen/src/main.rs index cd50214f1c85..08254ff2b2eb 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"); @@ -33,12 +29,7 @@ fn main() -> Result<(), Box> { .out_dir("src") .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) @@ -56,12 +47,7 @@ fn main() -> Result<(), Box> { .out_dir("src/sql") .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-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-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)); 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!( 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-ipc/src/writer.rs b/arrow-ipc/src/writer.rs index 6d0d9a7eee19..ae2c261324d3 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!( 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/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) }; diff --git a/arrow-select/src/take.rs b/arrow-select/src/take.rs index 0c550ea0b856..fccf354b9487 100644 --- a/arrow-select/src/take.rs +++ b/arrow-select/src/take.rs @@ -63,7 +63,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. /// @@ -132,7 +132,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. /// 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/arrow/benches/builder.rs b/arrow/benches/builder.rs index 21d4c3baaaa5..16d082bf3763 100644 --- a/arrow/benches/builder.rs +++ b/arrow/benches/builder.rs @@ -31,6 +31,10 @@ const BATCH_SIZE: usize = 8 << 10; const NUM_BATCHES: usize = 64; fn bench_primitive(c: &mut Criterion) { + #[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"); diff --git a/arrow/benches/cast_kernels.rs b/arrow/benches/cast_kernels.rs index 30c6b86137e9..9ffcde26437d 100644 --- a/arrow/benches/cast_kernels.rs +++ b/arrow/benches/cast_kernels.rs @@ -135,7 +135,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/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-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)), 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()); diff --git a/parquet/src/arrow/record_reader/definition_levels.rs b/parquet/src/arrow/record_reader/definition_levels.rs index b4182b166e04..c48c3269800a 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_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; } 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 { diff --git a/parquet/src/column/chunker/cdc.rs b/parquet/src/column/chunker/cdc.rs index 2627d8921b9a..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; @@ -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}" + ); + } } } diff --git a/parquet/src/file/metadata/thrift/mod.rs b/parquet/src/file/metadata/thrift/mod.rs index bd60de417725..eb5dc9e689cf 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. @@ -236,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 { @@ -252,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(()), }?; @@ -427,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, @@ -544,7 +536,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)?; @@ -781,13 +773,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 @@ -1459,7 +1448,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 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, 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 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,