From 7bfc5941e407616769270d0cc5adf807df18395e Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:50:21 +0200 Subject: [PATCH 01/16] feat: enable a batch of non-default clippy lints workspace-wide All of these are already violation-free across the workspace, so this is pure future-proofing: they only fire on newly written code. They are a subset of the lint set used by https://github.com/emilk/egui, picked because they trigger no (or almost no) violations in DataFusion today. The single existing violation is a deliberate `mem::forget` in an FFI test helper, now marked with `#[expect]`. Part of https://github.com/apache/datafusion/issues/18467 Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 51 +++++++++++++++++++++++++++++++ datafusion/ffi/src/tests/utils.rs | 1 + 2 files changed, 52 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 03b90480fe164..2bfe382921f80 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -210,20 +210,71 @@ uuid = "1.23" zstd = { version = "0.13", default-features = false } # Keep this list sorted alphabetically. +# See https://github.com/apache/datafusion/issues/18467 for the ongoing effort of +# picking useful non-default lints. [workspace.lints.clippy] # https://github.com/apache/datafusion/issues/18881 allow_attributes = "warn" +as_ptr_cast_mut = "warn" assigning_clones = "warn" +char_lit_as_u8 = "warn" +clear_with_drain = "warn" +coerce_container_to_any = "warn" +decimal_bitwise_operands = "warn" +default_union_representation = "warn" +empty_enum_variants_with_brackets = "warn" +empty_line_after_outer_attr = "warn" +exit = "warn" +fn_to_numeric_cast_any = "warn" +if_let_mutex = "warn" +imprecise_flops = "warn" +index_refutable_slice = "warn" inefficient_to_string = "warn" +infinite_loop = "warn" +invalid_upcast_comparisons = "warn" +ip_constant = "warn" +iter_filter_is_ok = "warn" +iter_filter_is_some = "warn" +iter_on_empty_collections = "warn" # Detects large stack-allocated futures that may cause stack overflow crashes (see threshold in clippy.toml) large_futures = "warn" +large_include_file = "warn" +macro_use_imports = "warn" +manual_ilog2 = "warn" +manual_is_power_of_two = "warn" +manual_ok_or = "warn" +match_wild_err_arm = "warn" +mem_forget = "warn" +mismatching_type_param_order = "warn" +missing_enforced_import_renames = "warn" +mut_mut = "warn" +mutex_integer = "warn" # https://github.com/apache/datafusion/issues/18503 needless_pass_by_value = "warn" +negative_feature_names = "warn" +non_zero_suggestions = "warn" +nonstandard_macro_braces = "warn" or_fun_call = "warn" +path_buf_push_overwrite = "warn" +precedence_bits = "warn" +pub_underscore_fields = "warn" +rc_mutex = "warn" +same_length_and_capacity = "warn" +string_add_assign = "warn" +suspicious_command_arg_space = "warn" +suspicious_xor_used_as_pow = "warn" +trailing_empty_array = "warn" +transmute_ptr_to_ptr = "warn" +uninhabited_references = "warn" uninlined_format_args = "warn" unnecessary_lazy_evaluations = "warn" +unnecessary_self_imports = "warn" unused_async = "warn" +unused_rounding = "warn" used_underscore_binding = "warn" +useless_transmute = "warn" +wildcard_dependencies = "warn" +zero_sized_map_values = "warn" [workspace.lints.rust] unexpected_cfgs = { level = "warn", check-cfg = [ diff --git a/datafusion/ffi/src/tests/utils.rs b/datafusion/ffi/src/tests/utils.rs index b6b50cbce875c..b975a87394efe 100644 --- a/datafusion/ffi/src/tests/utils.rs +++ b/datafusion/ffi/src/tests/utils.rs @@ -83,6 +83,7 @@ pub fn get_module() -> Result { assert_eq!((module.version)(), expected_version); // Leak the library to keep it loaded for the duration of the test + #[expect(clippy::mem_forget)] std::mem::forget(lib); Ok(module) From f585f00a4d0ba64a0348816b4a0fa2ae5d85c83a Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:52:09 +0200 Subject: [PATCH 02/16] feat: enable clippy::manual_instant_elapsed Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/physical-plan/src/analyze.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 2bfe382921f80..4264f087a1d8b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -241,6 +241,7 @@ large_futures = "warn" large_include_file = "warn" macro_use_imports = "warn" manual_ilog2 = "warn" +manual_instant_elapsed = "warn" manual_is_power_of_two = "warn" manual_ok_or = "warn" match_wild_err_arm = "warn" diff --git a/datafusion/physical-plan/src/analyze.rs b/datafusion/physical-plan/src/analyze.rs index 31e0a27410ff9..85fffc4a1d901 100644 --- a/datafusion/physical-plan/src/analyze.rs +++ b/datafusion/physical-plan/src/analyze.rs @@ -284,7 +284,7 @@ impl ExecutionPlan for AnalyzeExec { } drop(input_stream); - let duration = Instant::now() - start; + let duration = start.elapsed(); create_output_batch( verbose, show_statistics, From 5ebd57df50321e5e02f441c4a20e8112ad4b67c6 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:53:47 +0200 Subject: [PATCH 03/16] feat: enable clippy::pathbuf_init_then_push Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/datasource-parquet/src/metadata.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4264f087a1d8b..dcebb7bc3cd4c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -257,6 +257,7 @@ non_zero_suggestions = "warn" nonstandard_macro_braces = "warn" or_fun_call = "warn" path_buf_push_overwrite = "warn" +pathbuf_init_then_push = "warn" precedence_bits = "warn" pub_underscore_fields = "warn" rc_mutex = "warn" diff --git a/datafusion/datasource-parquet/src/metadata.rs b/datafusion/datasource-parquet/src/metadata.rs index 3294ee00f10e7..74446398a0dc5 100644 --- a/datafusion/datasource-parquet/src/metadata.rs +++ b/datafusion/datasource-parquet/src/metadata.rs @@ -1557,8 +1557,8 @@ mod tests { #[test] fn test_distinct_count_from_real_parquet_file() { // Path to test file created by DuckDB with distinct_count statistics - let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); - path.push("src/test_data/ndv_test.parquet"); + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("src/test_data/ndv_test.parquet"); let file = File::open(&path).expect("Failed to open test parquet file"); let reader = From 5c2e60b6094db3fe70dd3ed1d2aa203c72f403d9 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:55:50 +0200 Subject: [PATCH 04/16] feat: enable clippy::ref_option_ref Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/functions-nested/src/range.rs | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dcebb7bc3cd4c..f49c87cc91b8c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -261,6 +261,7 @@ pathbuf_init_then_push = "warn" precedence_bits = "warn" pub_underscore_fields = "warn" rc_mutex = "warn" +ref_option_ref = "warn" same_length_and_capacity = "warn" string_add_assign = "warn" suspicious_command_arg_space = "warn" diff --git a/datafusion/functions-nested/src/range.rs b/datafusion/functions-nested/src/range.rs index 65d9244ecdd4c..0a02a8b7bbd72 100644 --- a/datafusion/functions-nested/src/range.rs +++ b/datafusion/functions-nested/src/range.rs @@ -434,8 +434,8 @@ impl Range { let stop = cast_to_ns(stop)?; let stop = as_timestamp_nanosecond_array(&stop)?; - let start_tz = parse_tz(&start.timezone())?; - let stop_tz = parse_tz(&stop.timezone())?; + let start_tz = parse_tz(start.timezone())?; + let stop_tz = parse_tz(stop.timezone())?; // values are timestamps let values_builder = start @@ -609,8 +609,8 @@ fn generate_range_values( Ok(()) } -fn parse_tz(tz: &Option<&str>) -> Result { - let tz = tz.unwrap_or_else(|| "+00"); +fn parse_tz(tz: Option<&str>) -> Result { + let tz = tz.unwrap_or("+00"); Tz::from_str(tz) .map_err(|op| exec_datafusion_err!("failed to parse timezone {tz}: {:?}", op)) From 2435d5332702c2eb99d847bb78ebc85a4829baaa Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:58:17 +0200 Subject: [PATCH 05/16] feat: enable clippy::debug_assert_with_mut_call `HashTable::find_entry` takes `&mut self`; `find` does the same lookup through `&self`. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/common/src/utils/proxy.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index f49c87cc91b8c..078549f2387da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -220,6 +220,7 @@ assigning_clones = "warn" char_lit_as_u8 = "warn" clear_with_drain = "warn" coerce_container_to_any = "warn" +debug_assert_with_mut_call = "warn" decimal_bitwise_operands = "warn" default_union_representation = "warn" empty_enum_variants_with_brackets = "warn" diff --git a/datafusion/common/src/utils/proxy.rs b/datafusion/common/src/utils/proxy.rs index 846c928515d60..7661cfa491878 100644 --- a/datafusion/common/src/utils/proxy.rs +++ b/datafusion/common/src/utils/proxy.rs @@ -166,7 +166,7 @@ where if cfg!(debug_assertions) { // In debug mode, check that the element is not already present debug_assert!( - self.find_entry(hash, |y| y == &x).is_err(), + self.find(hash, |y| y == &x).is_none(), "attempted to insert duplicate element into HashTableAllocExt::insert_accounted" ); } From bc5dd6dc38a3518542de72ec9256bb82aa65c0e5 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 21:59:59 +0200 Subject: [PATCH 06/16] feat: enable clippy::str_split_at_newline Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/core/tests/physical_optimizer/pushdown_utils.rs | 2 +- datafusion/core/tests/physical_optimizer/test_utils.rs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 078549f2387da..dfd5d8b81957a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -264,6 +264,7 @@ pub_underscore_fields = "warn" rc_mutex = "warn" ref_option_ref = "warn" same_length_and_capacity = "warn" +str_split_at_newline = "warn" string_add_assign = "warn" suspicious_command_arg_space = "warn" suspicious_xor_used_as_pow = "warn" diff --git a/datafusion/core/tests/physical_optimizer/pushdown_utils.rs b/datafusion/core/tests/physical_optimizer/pushdown_utils.rs index 2ffd1899b3c1d..27a1428a28597 100644 --- a/datafusion/core/tests/physical_optimizer/pushdown_utils.rs +++ b/datafusion/core/tests/physical_optimizer/pushdown_utils.rs @@ -417,7 +417,7 @@ pub fn format_execution_plan(plan: &Arc) -> Vec { } fn format_lines(s: &str) -> Vec { - s.trim().split('\n').map(|s| s.to_string()).collect() + s.trim().lines().map(|s| s.to_string()).collect() } pub fn format_plan_for_test(plan: &Arc) -> String { diff --git a/datafusion/core/tests/physical_optimizer/test_utils.rs b/datafusion/core/tests/physical_optimizer/test_utils.rs index 74230b24e2ab5..097251878949a 100644 --- a/datafusion/core/tests/physical_optimizer/test_utils.rs +++ b/datafusion/core/tests/physical_optimizer/test_utils.rs @@ -801,7 +801,7 @@ pub fn format_execution_plan(plan: &Arc) -> Vec { } fn format_lines(s: &str) -> Vec { - s.trim().split('\n').map(|s| s.to_string()).collect() + s.trim().lines().map(|s| s.to_string()).collect() } /// Create a simple ProjectionExec with column indices (simplified version) From 76494143da6f4873c214f6bd0d2f1c3529bdb06c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:02:28 +0200 Subject: [PATCH 07/16] feat: enable clippy::flat_map_option Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/common/src/cse.rs | 2 +- datafusion/expr/src/logical_plan/builder.rs | 2 +- datafusion/physical-expr/src/equivalence/properties/mod.rs | 2 +- datafusion/physical-plan/src/aggregates/mod.rs | 2 +- 5 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index dfd5d8b81957a..ae8888465cb23 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -226,6 +226,7 @@ default_union_representation = "warn" empty_enum_variants_with_brackets = "warn" empty_line_after_outer_attr = "warn" exit = "warn" +flat_map_option = "warn" fn_to_numeric_cast_any = "warn" if_let_mutex = "warn" imprecise_flops = "warn" diff --git a/datafusion/common/src/cse.rs b/datafusion/common/src/cse.rs index 93169d6a02ff1..64dad4f5c038d 100644 --- a/datafusion/common/src/cse.rs +++ b/datafusion/common/src/cse.rs @@ -808,7 +808,7 @@ mod test { ) -> HashSet { id_array .iter_mut() - .flat_map(|(_, id_option)| { + .filter_map(|(_, id_option)| { id_option.as_mut().map(|node_id| { let hash = node_id.hash; node_id.hash = 0; diff --git a/datafusion/expr/src/logical_plan/builder.rs b/datafusion/expr/src/logical_plan/builder.rs index 1f32d9c6da445..6b6ea9c1c72d4 100644 --- a/datafusion/expr/src/logical_plan/builder.rs +++ b/datafusion/expr/src/logical_plan/builder.rs @@ -2177,7 +2177,7 @@ pub fn wrap_projection_for_join_if_necessary( // Expr contains Arc with interior mutability but is intentionally used as hash key let join_key_items = alias_join_keys .iter() - .flat_map(|expr| expr.try_as_col().is_none().then_some(expr)) + .filter(|expr| expr.try_as_col().is_none()) .cloned() .collect::>(); projection.extend(join_key_items); diff --git a/datafusion/physical-expr/src/equivalence/properties/mod.rs b/datafusion/physical-expr/src/equivalence/properties/mod.rs index 22b3382f50638..54269e07f9309 100644 --- a/datafusion/physical-expr/src/equivalence/properties/mod.rs +++ b/datafusion/physical-expr/src/equivalence/properties/mod.rs @@ -1157,7 +1157,7 @@ impl EquivalenceProperties { let indices = mapping .iter() .flat_map(|(_, targets)| { - targets.iter().flat_map(|(target, _)| { + targets.iter().filter_map(|(target, _)| { target.downcast_ref::().map(|c| c.index()) }) }) diff --git a/datafusion/physical-plan/src/aggregates/mod.rs b/datafusion/physical-plan/src/aggregates/mod.rs index 33860d3f51c0b..5aed51c2e2ebc 100644 --- a/datafusion/physical-plan/src/aggregates/mod.rs +++ b/datafusion/physical-plan/src/aggregates/mod.rs @@ -1379,7 +1379,7 @@ impl AggregateExec { group_expr_mapping .iter() .flat_map(|(_, target_cols)| { - target_cols.iter().flat_map(|(expr, _)| { + target_cols.iter().filter_map(|(expr, _)| { expr.downcast_ref::().map(|c| c.index()) }) }) From a7cc2220eca59645b13fcf1ba86d11043ebd4825 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:05:35 +0200 Subject: [PATCH 08/16] feat: enable clippy::verbose_file_reads Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/core/benches/parquet_query_sql.rs | 6 +--- datafusion/datasource-arrow/src/source.rs | 30 ++++++-------------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ae8888465cb23..04e3aa1a1a6e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -279,6 +279,7 @@ unused_async = "warn" unused_rounding = "warn" used_underscore_binding = "warn" useless_transmute = "warn" +verbose_file_reads = "warn" wildcard_dependencies = "warn" zero_sized_map_values = "warn" diff --git a/datafusion/core/benches/parquet_query_sql.rs b/datafusion/core/benches/parquet_query_sql.rs index 2e7794bfd19b4..9ed261839212f 100644 --- a/datafusion/core/benches/parquet_query_sql.rs +++ b/datafusion/core/benches/parquet_query_sql.rs @@ -32,8 +32,6 @@ use parquet::file::properties::{WriterProperties, WriterVersion}; use rand::distr::Alphanumeric; use rand::distr::uniform::SampleUniform; use rand::prelude::*; -use std::fs::File; -use std::io::Read; use std::ops::Range; use std::path::Path; use std::sync::Arc; @@ -211,9 +209,7 @@ fn criterion_benchmark(c: &mut Criterion) { .unwrap(); // We read the queries from a file so they can be changed without recompiling the benchmark - let mut queries_file = File::open("benches/parquet_query_sql.sql").unwrap(); - let mut queries = String::new(); - queries_file.read_to_string(&mut queries).unwrap(); + let queries = std::fs::read_to_string("benches/parquet_query_sql.sql").unwrap(); for query in queries.split(';') { let query = query.trim(); diff --git a/datafusion/datasource-arrow/src/source.rs b/datafusion/datasource-arrow/src/source.rs index 27533052ce03f..b3a8092c05328 100644 --- a/datafusion/datasource-arrow/src/source.rs +++ b/datafusion/datasource-arrow/src/source.rs @@ -444,7 +444,7 @@ impl From for Arc { #[cfg(test)] mod tests { - use std::{fs::File, io::Read}; + use std::fs::File; use arrow::datatypes::{DataType, Field, Schema}; use arrow_ipc::reader::{FileReader, StreamReader}; @@ -460,11 +460,8 @@ mod tests { for filename in ["example.arrow", "example_stream.arrow"] { let path = format!("tests/data/{filename}"); let path_str = path.as_str(); - let mut file = File::open(path_str)?; - let file_size = file.metadata()?.len(); - - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; + let buffer = std::fs::read(path_str)?; + let file_size = buffer.len() as u64; let bytes = Bytes::from(buffer); let object_store = Arc::new(InMemory::new()); @@ -504,11 +501,8 @@ mod tests { let filename = "example.arrow"; let path = format!("tests/data/{filename}"); let path_str = path.as_str(); - let mut file = File::open(path_str)?; - let file_size = file.metadata()?.len(); - - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; + let buffer = std::fs::read(path_str)?; + let file_size = buffer.len() as u64; let bytes = Bytes::from(buffer); let object_store = Arc::new(InMemory::new()); @@ -545,11 +539,8 @@ mod tests { let filename = "example_stream.arrow"; let path = format!("tests/data/{filename}"); let path_str = path.as_str(); - let mut file = File::open(path_str)?; - let file_size = file.metadata()?.len(); - - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; + let buffer = std::fs::read(path_str)?; + let file_size = buffer.len() as u64; let bytes = Bytes::from(buffer); let object_store = Arc::new(InMemory::new()); @@ -610,11 +601,8 @@ mod tests { let filename = "example_stream.arrow"; let path = format!("tests/data/{filename}"); let path_str = path.as_str(); - let mut file = File::open(path_str)?; - let file_size = file.metadata()?.len(); - - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; + let buffer = std::fs::read(path_str)?; + let file_size = buffer.len() as u64; let bytes = Bytes::from(buffer); let object_store = Arc::new(InMemory::new()); From d87454f9b108dc45ff468d0cee91c9e24ed787d4 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:08:44 +0200 Subject: [PATCH 09/16] feat: enable clippy::doc_include_without_cfg Gating the `include_str!` docs on `cfg(doc)` means touching the included markdown no longer forces a rebuild of the crate for non-doc builds. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion-cli/src/lib.rs | 2 +- datafusion/core/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 04e3aa1a1a6e6..9e6dbe617b0a4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -223,6 +223,7 @@ coerce_container_to_any = "warn" debug_assert_with_mut_call = "warn" decimal_bitwise_operands = "warn" default_union_representation = "warn" +doc_include_without_cfg = "warn" empty_enum_variants_with_brackets = "warn" empty_line_after_outer_attr = "warn" exit = "warn" diff --git a/datafusion-cli/src/lib.rs b/datafusion-cli/src/lib.rs index f0b0bc23fd73d..51202818d3c01 100644 --- a/datafusion-cli/src/lib.rs +++ b/datafusion-cli/src/lib.rs @@ -20,7 +20,7 @@ html_favicon_url = "https://raw.githubusercontent.com/apache/datafusion/19fe44cf2f30cbdd63d4a4f52c74055163c6cc38/docs/logos/standalone_logo/logo_original.svg" )] #![cfg_attr(docsrs, feature(doc_cfg))] -#![doc = include_str!("../README.md")] +#![cfg_attr(doc, doc = include_str!("../README.md"))] pub const DATAFUSION_CLI_VERSION: &str = env!("CARGO_PKG_VERSION"); pub mod catalog; diff --git a/datafusion/core/src/lib.rs b/datafusion/core/src/lib.rs index 3170f4be7f683..28ea4b4490c3e 100644 --- a/datafusion/core/src/lib.rs +++ b/datafusion/core/src/lib.rs @@ -761,7 +761,7 @@ //! [`RecordBatch`]: arrow::array::RecordBatch //! [`RecordBatchReader`]: arrow::record_batch::RecordBatchReader //! [`Array`]: arrow::array::Array -#![doc = include_str!("optimizer_rule_reference.md")] +#![cfg_attr(doc, doc = include_str!("optimizer_rule_reference.md"))] extern crate core; #[cfg(feature = "sql")] From 2a6f2963dd9632eb88c492e8219cf1470df6cbe8 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:15:48 +0200 Subject: [PATCH 10/16] feat: enable clippy::unnecessary_safety_comment `SAFETY:` comments should sit next to the `unsafe` block they justify. The existing ones either documented safe code (reworded) or were attached to the enclosing `if` rather than the `unsafe` block (moved). Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 + datafusion/datasource-json/src/source.rs | 2 +- datafusion/functions-aggregate/src/percentile_cont.rs | 2 +- datafusion/functions/src/string/repeat.rs | 2 +- datafusion/functions/src/unicode/character_length.rs | 2 +- datafusion/optimizer/src/extract_equijoin_predicate.rs | 2 +- datafusion/physical-expr-common/src/binary_map.rs | 2 +- datafusion/physical-plan/src/sorts/cursor.rs | 6 +++--- datafusion/spark/src/function/string/length.rs | 2 +- 9 files changed, 11 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9e6dbe617b0a4..0a59b0f54b729 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -275,6 +275,7 @@ transmute_ptr_to_ptr = "warn" uninhabited_references = "warn" uninlined_format_args = "warn" unnecessary_lazy_evaluations = "warn" +unnecessary_safety_comment = "warn" unnecessary_self_imports = "warn" unused_async = "warn" unused_rounding = "warn" diff --git a/datafusion/datasource-json/src/source.rs b/datafusion/datasource-json/src/source.rs index 8632d6b942bc1..ee39858e873e1 100644 --- a/datafusion/datasource-json/src/source.rs +++ b/datafusion/datasource-json/src/source.rs @@ -65,7 +65,7 @@ const JSON_CONVERTER_BUFFER_SIZE: usize = 2 * 1024 * 1024; /// A stream wrapper that holds SpawnedTask handles to keep them alive /// until the stream is fully consumed or dropped. /// -/// This ensures cancel-safety: when the stream is dropped, the tasks +/// This makes the stream cancel-safe: when the stream is dropped, the tasks /// are properly aborted via SpawnedTask's Drop implementation. struct JsonArrayStream { inner: ReceiverStream>, diff --git a/datafusion/functions-aggregate/src/percentile_cont.rs b/datafusion/functions-aggregate/src/percentile_cont.rs index 3a98900bbb446..8052247586df7 100644 --- a/datafusion/functions-aggregate/src/percentile_cont.rs +++ b/datafusion/functions-aggregate/src/percentile_cont.rs @@ -63,7 +63,7 @@ use crate::utils::validate_percentile_expr; /// Precision multiplier for linear interpolation calculations. /// -/// This value of 1,000,000 was chosen to balance precision with overflow safety: +/// This value of 1,000,000 was chosen to balance precision against overflow risk: /// - Provides 6 decimal places of precision for the fractional component /// - Small enough to avoid overflow when multiplied with typical numeric values /// - Sufficient precision for most statistical applications diff --git a/datafusion/functions/src/string/repeat.rs b/datafusion/functions/src/string/repeat.rs index a53f1e2e4fc42..09ecfd00168eb 100644 --- a/datafusion/functions/src/string/repeat.rs +++ b/datafusion/functions/src/string/repeat.rs @@ -306,7 +306,7 @@ where // Doubling strategy: copy what we have so far until we reach the target while buffer.len() < src.len() * count { let copy_len = buffer.len().min(src.len() * count - buffer.len()); - // SAFETY: we're copying valid UTF-8 bytes that we already verified + // We are copying valid UTF-8 bytes that we already verified buffer.extend_from_within(..copy_len); } } diff --git a/datafusion/functions/src/unicode/character_length.rs b/datafusion/functions/src/unicode/character_length.rs index 9f0d952a02636..2b99d942f6c0a 100644 --- a/datafusion/functions/src/unicode/character_length.rs +++ b/datafusion/functions/src/unicode/character_length.rs @@ -158,10 +158,10 @@ where } else { let values: Vec<_> = (0..array.len()) .map(|i| { - // Safety: we are iterating with array.len() so the index is always valid if array.is_null(i) { T::default_value() } else { + // Safety: we are iterating with array.len() so the index is always valid let value = unsafe { array.value_unchecked(i) }; if value.is_empty() { T::default_value() diff --git a/datafusion/optimizer/src/extract_equijoin_predicate.rs b/datafusion/optimizer/src/extract_equijoin_predicate.rs index 0a50761e8a9f7..58f9a4cd42a2d 100644 --- a/datafusion/optimizer/src/extract_equijoin_predicate.rs +++ b/datafusion/optimizer/src/extract_equijoin_predicate.rs @@ -95,7 +95,7 @@ impl OptimizerRule for ExtractEquijoinPredicate { && equijoin_predicates.is_empty() && non_equijoin_expr.is_some() { - // SAFETY: checked in the outer `if` + // Checked in the outer `if` let expr = non_equijoin_expr.clone().unwrap(); let (equijoin_predicates, non_equijoin_expr) = split_is_not_distinct_from_and_other_join_predicate( diff --git a/datafusion/physical-expr-common/src/binary_map.rs b/datafusion/physical-expr-common/src/binary_map.rs index 44ca35c7f8708..48568665d27a4 100644 --- a/datafusion/physical-expr-common/src/binary_map.rs +++ b/datafusion/physical-expr-common/src/binary_map.rs @@ -557,7 +557,7 @@ fn single_null_buffer(num_values: usize, null_index: usize) -> NullBuffer { null_builder.append_n_non_nulls(null_index); null_builder.append_null(); null_builder.append_n_non_nulls(num_values - null_index - 1); - // SAFETY: inner builder must be constructed + // The appends above guarantee the inner builder has been constructed. null_builder.finish().unwrap() } diff --git a/datafusion/physical-plan/src/sorts/cursor.rs b/datafusion/physical-plan/src/sorts/cursor.rs index d71eaad663410..f23c242a36285 100644 --- a/datafusion/physical-plan/src/sorts/cursor.rs +++ b/datafusion/physical-plan/src/sorts/cursor.rs @@ -410,10 +410,10 @@ impl CursorValues for StringViewArray { #[inline(always)] fn compare(l: &Self, l_idx: usize, r: &Self, r_idx: usize) -> Ordering { - // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. - // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). - // And the bound is checked in is_finished, it is safe to call get_unchecked if l.data_buffers().is_empty() && r.data_buffers().is_empty() { + // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. + // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). + // And the bound is checked in is_finished, it is safe to call get_unchecked let l_view = unsafe { l.views().get_unchecked(l_idx) }; let r_view = unsafe { r.views().get_unchecked(r_idx) }; return StringViewArray::inline_key_fast(*l_view) diff --git a/datafusion/spark/src/function/string/length.rs b/datafusion/spark/src/function/string/length.rs index 8c5539a0577d8..b3683dac5b83e 100644 --- a/datafusion/spark/src/function/string/length.rs +++ b/datafusion/spark/src/function/string/length.rs @@ -154,10 +154,10 @@ where } else { let values: Vec<_> = (0..array.len()) .map(|i| { - // Safety: we are iterating with array.len() so the index is always valid if array.is_null(i) { i32::default() } else { + // Safety: we are iterating with array.len() so the index is always valid let value = unsafe { array.value_unchecked(i) }; if value.is_empty() { i32::default() From e476f1a0993eac3481267650bcf88923841c24a2 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:22:17 +0200 Subject: [PATCH 11/16] feat: add a [workspace.lints.rust] set Enables the rustc lint groups and individual lints from https://github.com/emilk/egui/blob/main/Cargo.toml that DataFusion violates at most 10 times today: * `future_incompatible`, `nonstandard_style`, `rust_2018_idioms` (groups) * `rust_2021_prelude_collisions`, `semicolon_in_expressions_from_macros`, `unsafe_op_in_unsafe_fn`, `unused_extern_crates`, `unused_import_braces`, `unused_lifetimes` `elided_lifetimes_in_paths` is part of `rust_2018_idioms` but has ~800 violations, so it is explicitly allowed for now. `trivial_numeric_casts` (31 violations) and `unsafe_code` are left out entirely. The violations fixed here are vestigial `extern crate` items, redundant import braces, and two `rstest` helpers whose lifetime is unused after macro expansion. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 13 +++++++++++++ datafusion-cli/tests/cli_integration.rs | 4 ++++ datafusion/common/src/rounding.rs | 6 ------ datafusion/core/src/lib.rs | 3 --- .../tests/physical_optimizer/enforce_sorting.rs | 4 ++-- datafusion/expr/src/lib.rs | 2 -- datafusion/functions/benches/atan2.rs | 2 -- datafusion/functions/benches/get_field.rs | 2 -- datafusion/functions/benches/nanvl.rs | 2 -- datafusion/functions/benches/power.rs | 2 -- datafusion/macros/src/user_doc.rs | 1 - datafusion/wasmtest/src/lib.rs | 2 -- 12 files changed, 19 insertions(+), 24 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0a59b0f54b729..6d41ecc354ab7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -285,12 +285,25 @@ verbose_file_reads = "warn" wildcard_dependencies = "warn" zero_sized_map_values = "warn" +# Keep this list sorted alphabetically. [workspace.lints.rust] +# Part of the `rust_2018_idioms` group, but ~800 violations today: +# https://github.com/apache/datafusion/issues/18467 +elided_lifetimes_in_paths = "allow" +future_incompatible = { level = "warn", priority = -1 } +nonstandard_style = { level = "warn", priority = -1 } +rust_2018_idioms = { level = "warn", priority = -1 } +rust_2021_prelude_collisions = "warn" +semicolon_in_expressions_from_macros = "warn" unexpected_cfgs = { level = "warn", check-cfg = [ 'cfg(datafusion_coop, values("tokio", "tokio_fallback", "per_stream"))', "cfg(coverage)", "cfg(coverage_nightly)", ] } +unsafe_op_in_unsafe_fn = "warn" +unused_extern_crates = "warn" +unused_import_braces = "warn" +unused_lifetimes = "warn" unused_qualifications = "deny" # -------------------- diff --git a/datafusion-cli/tests/cli_integration.rs b/datafusion-cli/tests/cli_integration.rs index 4dc244445a2eb..9b33c1a243711 100644 --- a/datafusion-cli/tests/cli_integration.rs +++ b/datafusion-cli/tests/cli_integration.rs @@ -428,6 +428,8 @@ fn test_cli_format<'a>(#[case] format: &'a str) { #[case("top2", ["--top-memory-consumers", "2"])] #[case("top3_default", [])] #[test] +// `'a` is used by the signature below, but not by the per-case functions `rstest` generates. +#[expect(unused_lifetimes)] fn test_cli_top_memory_consumers<'a>( #[case] snapshot_name: &str, #[case] top_memory_consumers: impl IntoIterator, @@ -446,6 +448,8 @@ fn test_cli_top_memory_consumers<'a>( #[case("no_track", ["--top-memory-consumers", "0"])] #[case("top2", ["--top-memory-consumers", "2"])] #[test] +// `'a` is used by the signature below, but not by the per-case functions `rstest` generates. +#[expect(unused_lifetimes)] fn test_cli_top_memory_consumers_with_mem_pool_type<'a>( #[case] snapshot_name: &str, #[case] top_memory_consumers: impl IntoIterator, diff --git a/datafusion/common/src/rounding.rs b/datafusion/common/src/rounding.rs index 1796143d7cf1a..7da3bd0c6ac07 100644 --- a/datafusion/common/src/rounding.rs +++ b/datafusion/common/src/rounding.rs @@ -37,12 +37,6 @@ const FE_UPWARD: i32 = 0x0800; #[cfg(all(target_arch = "x86_64", not(target_os = "windows")))] const FE_DOWNWARD: i32 = 0x0400; -#[cfg(all( - any(target_arch = "x86_64", target_arch = "aarch64"), - not(target_os = "windows") -))] -extern crate libc; - #[cfg(all( any(target_arch = "x86_64", target_arch = "aarch64"), not(target_os = "windows") diff --git a/datafusion/core/src/lib.rs b/datafusion/core/src/lib.rs index 28ea4b4490c3e..a3d8bff5ccada 100644 --- a/datafusion/core/src/lib.rs +++ b/datafusion/core/src/lib.rs @@ -763,9 +763,6 @@ //! [`Array`]: arrow::array::Array #![cfg_attr(doc, doc = include_str!("optimizer_rule_reference.md"))] -extern crate core; -#[cfg(feature = "sql")] -extern crate sqlparser; /// DataFusion crate version pub const DATAFUSION_VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/datafusion/core/tests/physical_optimizer/enforce_sorting.rs b/datafusion/core/tests/physical_optimizer/enforce_sorting.rs index d94253a84aa5f..dd67e4895b1ac 100644 --- a/datafusion/core/tests/physical_optimizer/enforce_sorting.rs +++ b/datafusion/core/tests/physical_optimizer/enforce_sorting.rs @@ -29,7 +29,7 @@ use crate::physical_optimizer::test_utils::{ spr_repartition_exec, stream_exec_ordered, union_exec, }; -use arrow::compute::{SortOptions}; +use arrow::compute::SortOptions; use arrow::datatypes::{DataType, SchemaRef}; use datafusion_common::config::{ConfigOptions, CsvOptions}; use datafusion_common::tree_node::{TreeNode, TransformedResult}; @@ -61,7 +61,7 @@ use datafusion_physical_optimizer::output_requirements::OutputRequirementExec; use datafusion_physical_optimizer::PhysicalOptimizerRule; use datafusion::prelude::*; use arrow::array::{record_batch, Array, ArrayRef, Int32Array, RecordBatch}; -use arrow::datatypes::{Field}; +use arrow::datatypes::Field; use arrow_schema::Schema; use datafusion_execution::TaskContext; use datafusion_catalog::streaming::StreamingTable; diff --git a/datafusion/expr/src/lib.rs b/datafusion/expr/src/lib.rs index 1033952642a2b..b8d30346025c1 100644 --- a/datafusion/expr/src/lib.rs +++ b/datafusion/expr/src/lib.rs @@ -35,8 +35,6 @@ //! //! The [expr_fn] module contains functions for creating expressions. -extern crate core; - mod higher_order_function; mod literal; mod operation; diff --git a/datafusion/functions/benches/atan2.rs b/datafusion/functions/benches/atan2.rs index f1c9756a0cc08..d1f2b3d1332ad 100644 --- a/datafusion/functions/benches/atan2.rs +++ b/datafusion/functions/benches/atan2.rs @@ -15,8 +15,6 @@ // specific language governing permissions and limitations // under the License. -extern crate criterion; - use arrow::datatypes::{DataType, Field, Float32Type, Float64Type}; use arrow::util::bench_util::create_primitive_array; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/benches/get_field.rs b/datafusion/functions/benches/get_field.rs index 8a5fd0a1e2fa9..a274bfb385d64 100644 --- a/datafusion/functions/benches/get_field.rs +++ b/datafusion/functions/benches/get_field.rs @@ -15,8 +15,6 @@ // specific language governing permissions and limitations // under the License. -extern crate criterion; - use arrow::array::{ArrayRef, Int32Builder, MapBuilder, StringBuilder}; use arrow::datatypes::{DataType, Field}; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/benches/nanvl.rs b/datafusion/functions/benches/nanvl.rs index d3d2c7ebff998..830a1ea3888ca 100644 --- a/datafusion/functions/benches/nanvl.rs +++ b/datafusion/functions/benches/nanvl.rs @@ -15,8 +15,6 @@ // specific language governing permissions and limitations // under the License. -extern crate criterion; - use arrow::array::{ArrayRef, Float32Array, Float64Array}; use arrow::datatypes::{DataType, Field}; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/benches/power.rs b/datafusion/functions/benches/power.rs index 5336e42ebe59b..d40f330d76c23 100644 --- a/datafusion/functions/benches/power.rs +++ b/datafusion/functions/benches/power.rs @@ -23,8 +23,6 @@ //! through a Float64 round-trip, which is measurably slower than the //! decimal kernel for the cases the kernel can handle. -extern crate criterion; - use arrow::array::{Decimal128Array, Int64Array}; use arrow::datatypes::{DataType, Field, FieldRef}; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/macros/src/user_doc.rs b/datafusion/macros/src/user_doc.rs index ce9e7d55ef103..bbfe7877fc211 100644 --- a/datafusion/macros/src/user_doc.rs +++ b/datafusion/macros/src/user_doc.rs @@ -21,7 +21,6 @@ )] #![cfg_attr(docsrs, feature(doc_cfg))] -extern crate proc_macro; use datafusion_doc::scalar_doc_sections::doc_sections_const; use proc_macro::TokenStream; use quote::quote; diff --git a/datafusion/wasmtest/src/lib.rs b/datafusion/wasmtest/src/lib.rs index f545ccf19306a..6289a0c3956fb 100644 --- a/datafusion/wasmtest/src/lib.rs +++ b/datafusion/wasmtest/src/lib.rs @@ -22,8 +22,6 @@ )] #![cfg_attr(docsrs, feature(doc_cfg))] -extern crate wasm_bindgen; - use datafusion_common::ScalarValue; use datafusion_expr::lit; use datafusion_expr::simplify::SimplifyContext; From 73cc2b1ddd2570b7e92e26a3d0de2b70f5b3ce7e Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:28:00 +0200 Subject: [PATCH 12/16] feat: add a [workspace.lints.rustdoc] set Enables the `rustdoc::all` group. Two of its lints have more than 10 violations today and are explicitly allowed for now: `missing_crate_level_docs` (18) and `unescaped_backticks` (28). Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 6d41ecc354ab7..48eea0e020c9c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -306,6 +306,15 @@ unused_import_braces = "warn" unused_lifetimes = "warn" unused_qualifications = "deny" +# Keep this list sorted alphabetically. +[workspace.lints.rustdoc] +all = { level = "warn", priority = -1 } +broken_intra_doc_links = "warn" +# Part of the `all` group, but has too many violations today to enable: +# https://github.com/apache/datafusion/issues/18467 +missing_crate_level_docs = "allow" +unescaped_backticks = "allow" + # -------------------- # Compilation Profiles # -------------------- From caa4d6e4455946a98e5fbf68afdd73ec3306d3ef Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:37:33 +0200 Subject: [PATCH 13/16] fix: keep `extern crate libc` in rounding.rs Removing it made `libc` an unused dependency (`cargo machete` CI failure): the crate has no path references, it is only linked so the `fesetround`/`fegetround` symbols declared in this module resolve. Co-Authored-By: Claude Opus 5 (1M context) --- datafusion/common/src/rounding.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/datafusion/common/src/rounding.rs b/datafusion/common/src/rounding.rs index 7da3bd0c6ac07..4b8ad8c41f7e4 100644 --- a/datafusion/common/src/rounding.rs +++ b/datafusion/common/src/rounding.rs @@ -37,6 +37,15 @@ const FE_UPWARD: i32 = 0x0800; #[cfg(all(target_arch = "x86_64", not(target_os = "windows")))] const FE_DOWNWARD: i32 = 0x0400; +// Links `libc`, which provides the `fesetround`/`fegetround` symbols declared below. +// There is no path reference to the crate, so `unused_extern_crates` cannot see the use. +#[expect(unused_extern_crates)] +#[cfg(all( + any(target_arch = "x86_64", target_arch = "aarch64"), + not(target_os = "windows") +))] +extern crate libc; + #[cfg(all( any(target_arch = "x86_64", target_arch = "aarch64"), not(target_os = "windows") From d3e857aee77bf66a6a4a388c702b12639a34877b Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 3 Aug 2026 22:45:20 +0200 Subject: [PATCH 14/16] chore: self-review fixups * Keep the `LogicalPlanType::` prefix on the `CustomScan` match arm so it matches its sibling arms; drop the variant import instead. * Restore a `SAFETY:` comment on the second `unsafe` block in `cursor.rs`. * Drop `rustdoc::broken_intra_doc_links`, already covered by `rustdoc::all`. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 1 - datafusion/physical-plan/src/sorts/cursor.rs | 2 ++ datafusion/proto/src/logical_plan/mod.rs | 5 ++--- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 48eea0e020c9c..f1a80d7c19427 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -309,7 +309,6 @@ unused_qualifications = "deny" # Keep this list sorted alphabetically. [workspace.lints.rustdoc] all = { level = "warn", priority = -1 } -broken_intra_doc_links = "warn" # Part of the `all` group, but has too many violations today to enable: # https://github.com/apache/datafusion/issues/18467 missing_crate_level_docs = "allow" diff --git a/datafusion/physical-plan/src/sorts/cursor.rs b/datafusion/physical-plan/src/sorts/cursor.rs index f23c242a36285..f595ec6fcb06b 100644 --- a/datafusion/physical-plan/src/sorts/cursor.rs +++ b/datafusion/physical-plan/src/sorts/cursor.rs @@ -420,6 +420,8 @@ impl CursorValues for StringViewArray { .cmp(&StringViewArray::inline_key_fast(*r_view)); } + // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. + // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). unsafe { GenericByteViewArray::compare_unchecked(l, l_idx, r, r_idx) } } } diff --git a/datafusion/proto/src/logical_plan/mod.rs b/datafusion/proto/src/logical_plan/mod.rs index 653ae9ab05355..a900f86b2476e 100644 --- a/datafusion/proto/src/logical_plan/mod.rs +++ b/datafusion/proto/src/logical_plan/mod.rs @@ -20,7 +20,6 @@ use std::fmt::Debug; use std::sync::Arc; use crate::convert::{FromProto, TryFromProto}; -use crate::protobuf::logical_plan_node::LogicalPlanType::CustomScan; use crate::protobuf::{ ColumnUnnestListItem, ColumnUnnestListRecursion, CteWorkTableScanNode, CustomTableScanNode, DmlNode, SortExprNodeCollection, dml_node, @@ -680,7 +679,7 @@ impl AsLogicalPlan for LogicalPlanNode { )? .build() } - CustomScan(scan) => { + LogicalPlanType::CustomScan(scan) => { let schema: Schema = convert_required!(scan.schema)?; let schema = Arc::new(schema); let mut projection = None; @@ -1506,7 +1505,7 @@ impl AsLogicalPlan for LogicalPlanNode { extension_codec .try_encode_table_provider(table_name, provider, &mut bytes) .map_err(|e| context!("Error serializing custom table", e))?; - let scan = CustomScan(CustomTableScanNode { + let scan = LogicalPlanType::CustomScan(CustomTableScanNode { table_name: Some(protobuf::TableReference::from_proto( table_name.clone(), )), From 47680f9db95aa125f85856c7f1ab3fb29d400c32 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 6 Aug 2026 12:15:53 +0200 Subject: [PATCH 15/16] chore: address review feedback: narrow this PR to non-default clippy lints * Drop lints that clippy already enables by default (`char_lit_as_u8`, `empty_line_after_outer_attr`, `if_let_mutex`, `manual_ok_or`, `missing_enforced_import_renames`, `suspicious_command_arg_space`, `useless_transmute`). * Drop `unnecessary_safety_comment`: it conflicts with the existing convention of using `// SAFETY:` comments on safe-but-delicate internal methods. * Move the `[workspace.lints.rust]` and `[workspace.lints.rustdoc]` sets out to follow-up PRs to keep this one reviewable. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 29 ------------------- datafusion-cli/tests/cli_integration.rs | 4 --- datafusion/common/src/rounding.rs | 3 -- datafusion/core/src/lib.rs | 3 ++ .../physical_optimizer/enforce_sorting.rs | 4 +-- datafusion/datasource-json/src/source.rs | 2 +- datafusion/expr/src/lib.rs | 2 ++ .../src/percentile_cont.rs | 2 +- datafusion/functions/benches/atan2.rs | 2 ++ datafusion/functions/benches/get_field.rs | 2 ++ datafusion/functions/benches/nanvl.rs | 2 ++ datafusion/functions/benches/power.rs | 2 ++ datafusion/functions/src/string/repeat.rs | 2 +- .../functions/src/unicode/character_length.rs | 2 +- datafusion/macros/src/user_doc.rs | 1 + .../src/extract_equijoin_predicate.rs | 2 +- .../physical-expr-common/src/binary_map.rs | 2 +- datafusion/physical-plan/src/sorts/cursor.rs | 8 ++--- .../spark/src/function/string/length.rs | 2 +- datafusion/wasmtest/src/lib.rs | 2 ++ 20 files changed, 28 insertions(+), 50 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f1a80d7c19427..76fccb648feca 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -217,7 +217,6 @@ zstd = { version = "0.13", default-features = false } allow_attributes = "warn" as_ptr_cast_mut = "warn" assigning_clones = "warn" -char_lit_as_u8 = "warn" clear_with_drain = "warn" coerce_container_to_any = "warn" debug_assert_with_mut_call = "warn" @@ -225,11 +224,9 @@ decimal_bitwise_operands = "warn" default_union_representation = "warn" doc_include_without_cfg = "warn" empty_enum_variants_with_brackets = "warn" -empty_line_after_outer_attr = "warn" exit = "warn" flat_map_option = "warn" fn_to_numeric_cast_any = "warn" -if_let_mutex = "warn" imprecise_flops = "warn" index_refutable_slice = "warn" inefficient_to_string = "warn" @@ -246,11 +243,9 @@ macro_use_imports = "warn" manual_ilog2 = "warn" manual_instant_elapsed = "warn" manual_is_power_of_two = "warn" -manual_ok_or = "warn" match_wild_err_arm = "warn" mem_forget = "warn" mismatching_type_param_order = "warn" -missing_enforced_import_renames = "warn" mut_mut = "warn" mutex_integer = "warn" # https://github.com/apache/datafusion/issues/18503 @@ -268,52 +263,28 @@ ref_option_ref = "warn" same_length_and_capacity = "warn" str_split_at_newline = "warn" string_add_assign = "warn" -suspicious_command_arg_space = "warn" suspicious_xor_used_as_pow = "warn" trailing_empty_array = "warn" transmute_ptr_to_ptr = "warn" uninhabited_references = "warn" uninlined_format_args = "warn" unnecessary_lazy_evaluations = "warn" -unnecessary_safety_comment = "warn" unnecessary_self_imports = "warn" unused_async = "warn" unused_rounding = "warn" used_underscore_binding = "warn" -useless_transmute = "warn" verbose_file_reads = "warn" wildcard_dependencies = "warn" zero_sized_map_values = "warn" -# Keep this list sorted alphabetically. [workspace.lints.rust] -# Part of the `rust_2018_idioms` group, but ~800 violations today: -# https://github.com/apache/datafusion/issues/18467 -elided_lifetimes_in_paths = "allow" -future_incompatible = { level = "warn", priority = -1 } -nonstandard_style = { level = "warn", priority = -1 } -rust_2018_idioms = { level = "warn", priority = -1 } -rust_2021_prelude_collisions = "warn" -semicolon_in_expressions_from_macros = "warn" unexpected_cfgs = { level = "warn", check-cfg = [ 'cfg(datafusion_coop, values("tokio", "tokio_fallback", "per_stream"))', "cfg(coverage)", "cfg(coverage_nightly)", ] } -unsafe_op_in_unsafe_fn = "warn" -unused_extern_crates = "warn" -unused_import_braces = "warn" -unused_lifetimes = "warn" unused_qualifications = "deny" -# Keep this list sorted alphabetically. -[workspace.lints.rustdoc] -all = { level = "warn", priority = -1 } -# Part of the `all` group, but has too many violations today to enable: -# https://github.com/apache/datafusion/issues/18467 -missing_crate_level_docs = "allow" -unescaped_backticks = "allow" - # -------------------- # Compilation Profiles # -------------------- diff --git a/datafusion-cli/tests/cli_integration.rs b/datafusion-cli/tests/cli_integration.rs index 9b33c1a243711..4dc244445a2eb 100644 --- a/datafusion-cli/tests/cli_integration.rs +++ b/datafusion-cli/tests/cli_integration.rs @@ -428,8 +428,6 @@ fn test_cli_format<'a>(#[case] format: &'a str) { #[case("top2", ["--top-memory-consumers", "2"])] #[case("top3_default", [])] #[test] -// `'a` is used by the signature below, but not by the per-case functions `rstest` generates. -#[expect(unused_lifetimes)] fn test_cli_top_memory_consumers<'a>( #[case] snapshot_name: &str, #[case] top_memory_consumers: impl IntoIterator, @@ -448,8 +446,6 @@ fn test_cli_top_memory_consumers<'a>( #[case("no_track", ["--top-memory-consumers", "0"])] #[case("top2", ["--top-memory-consumers", "2"])] #[test] -// `'a` is used by the signature below, but not by the per-case functions `rstest` generates. -#[expect(unused_lifetimes)] fn test_cli_top_memory_consumers_with_mem_pool_type<'a>( #[case] snapshot_name: &str, #[case] top_memory_consumers: impl IntoIterator, diff --git a/datafusion/common/src/rounding.rs b/datafusion/common/src/rounding.rs index 4b8ad8c41f7e4..1796143d7cf1a 100644 --- a/datafusion/common/src/rounding.rs +++ b/datafusion/common/src/rounding.rs @@ -37,9 +37,6 @@ const FE_UPWARD: i32 = 0x0800; #[cfg(all(target_arch = "x86_64", not(target_os = "windows")))] const FE_DOWNWARD: i32 = 0x0400; -// Links `libc`, which provides the `fesetround`/`fegetround` symbols declared below. -// There is no path reference to the crate, so `unused_extern_crates` cannot see the use. -#[expect(unused_extern_crates)] #[cfg(all( any(target_arch = "x86_64", target_arch = "aarch64"), not(target_os = "windows") diff --git a/datafusion/core/src/lib.rs b/datafusion/core/src/lib.rs index a3d8bff5ccada..28ea4b4490c3e 100644 --- a/datafusion/core/src/lib.rs +++ b/datafusion/core/src/lib.rs @@ -763,6 +763,9 @@ //! [`Array`]: arrow::array::Array #![cfg_attr(doc, doc = include_str!("optimizer_rule_reference.md"))] +extern crate core; +#[cfg(feature = "sql")] +extern crate sqlparser; /// DataFusion crate version pub const DATAFUSION_VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/datafusion/core/tests/physical_optimizer/enforce_sorting.rs b/datafusion/core/tests/physical_optimizer/enforce_sorting.rs index dd67e4895b1ac..d94253a84aa5f 100644 --- a/datafusion/core/tests/physical_optimizer/enforce_sorting.rs +++ b/datafusion/core/tests/physical_optimizer/enforce_sorting.rs @@ -29,7 +29,7 @@ use crate::physical_optimizer::test_utils::{ spr_repartition_exec, stream_exec_ordered, union_exec, }; -use arrow::compute::SortOptions; +use arrow::compute::{SortOptions}; use arrow::datatypes::{DataType, SchemaRef}; use datafusion_common::config::{ConfigOptions, CsvOptions}; use datafusion_common::tree_node::{TreeNode, TransformedResult}; @@ -61,7 +61,7 @@ use datafusion_physical_optimizer::output_requirements::OutputRequirementExec; use datafusion_physical_optimizer::PhysicalOptimizerRule; use datafusion::prelude::*; use arrow::array::{record_batch, Array, ArrayRef, Int32Array, RecordBatch}; -use arrow::datatypes::Field; +use arrow::datatypes::{Field}; use arrow_schema::Schema; use datafusion_execution::TaskContext; use datafusion_catalog::streaming::StreamingTable; diff --git a/datafusion/datasource-json/src/source.rs b/datafusion/datasource-json/src/source.rs index ee39858e873e1..8632d6b942bc1 100644 --- a/datafusion/datasource-json/src/source.rs +++ b/datafusion/datasource-json/src/source.rs @@ -65,7 +65,7 @@ const JSON_CONVERTER_BUFFER_SIZE: usize = 2 * 1024 * 1024; /// A stream wrapper that holds SpawnedTask handles to keep them alive /// until the stream is fully consumed or dropped. /// -/// This makes the stream cancel-safe: when the stream is dropped, the tasks +/// This ensures cancel-safety: when the stream is dropped, the tasks /// are properly aborted via SpawnedTask's Drop implementation. struct JsonArrayStream { inner: ReceiverStream>, diff --git a/datafusion/expr/src/lib.rs b/datafusion/expr/src/lib.rs index b8d30346025c1..1033952642a2b 100644 --- a/datafusion/expr/src/lib.rs +++ b/datafusion/expr/src/lib.rs @@ -35,6 +35,8 @@ //! //! The [expr_fn] module contains functions for creating expressions. +extern crate core; + mod higher_order_function; mod literal; mod operation; diff --git a/datafusion/functions-aggregate/src/percentile_cont.rs b/datafusion/functions-aggregate/src/percentile_cont.rs index 8052247586df7..3a98900bbb446 100644 --- a/datafusion/functions-aggregate/src/percentile_cont.rs +++ b/datafusion/functions-aggregate/src/percentile_cont.rs @@ -63,7 +63,7 @@ use crate::utils::validate_percentile_expr; /// Precision multiplier for linear interpolation calculations. /// -/// This value of 1,000,000 was chosen to balance precision against overflow risk: +/// This value of 1,000,000 was chosen to balance precision with overflow safety: /// - Provides 6 decimal places of precision for the fractional component /// - Small enough to avoid overflow when multiplied with typical numeric values /// - Sufficient precision for most statistical applications diff --git a/datafusion/functions/benches/atan2.rs b/datafusion/functions/benches/atan2.rs index d1f2b3d1332ad..f1c9756a0cc08 100644 --- a/datafusion/functions/benches/atan2.rs +++ b/datafusion/functions/benches/atan2.rs @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +extern crate criterion; + use arrow::datatypes::{DataType, Field, Float32Type, Float64Type}; use arrow::util::bench_util::create_primitive_array; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/benches/get_field.rs b/datafusion/functions/benches/get_field.rs index a274bfb385d64..8a5fd0a1e2fa9 100644 --- a/datafusion/functions/benches/get_field.rs +++ b/datafusion/functions/benches/get_field.rs @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +extern crate criterion; + use arrow::array::{ArrayRef, Int32Builder, MapBuilder, StringBuilder}; use arrow::datatypes::{DataType, Field}; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/benches/nanvl.rs b/datafusion/functions/benches/nanvl.rs index 830a1ea3888ca..d3d2c7ebff998 100644 --- a/datafusion/functions/benches/nanvl.rs +++ b/datafusion/functions/benches/nanvl.rs @@ -15,6 +15,8 @@ // specific language governing permissions and limitations // under the License. +extern crate criterion; + use arrow::array::{ArrayRef, Float32Array, Float64Array}; use arrow::datatypes::{DataType, Field}; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/benches/power.rs b/datafusion/functions/benches/power.rs index d40f330d76c23..5336e42ebe59b 100644 --- a/datafusion/functions/benches/power.rs +++ b/datafusion/functions/benches/power.rs @@ -23,6 +23,8 @@ //! through a Float64 round-trip, which is measurably slower than the //! decimal kernel for the cases the kernel can handle. +extern crate criterion; + use arrow::array::{Decimal128Array, Int64Array}; use arrow::datatypes::{DataType, Field, FieldRef}; use criterion::{Criterion, criterion_group, criterion_main}; diff --git a/datafusion/functions/src/string/repeat.rs b/datafusion/functions/src/string/repeat.rs index 09ecfd00168eb..a53f1e2e4fc42 100644 --- a/datafusion/functions/src/string/repeat.rs +++ b/datafusion/functions/src/string/repeat.rs @@ -306,7 +306,7 @@ where // Doubling strategy: copy what we have so far until we reach the target while buffer.len() < src.len() * count { let copy_len = buffer.len().min(src.len() * count - buffer.len()); - // We are copying valid UTF-8 bytes that we already verified + // SAFETY: we're copying valid UTF-8 bytes that we already verified buffer.extend_from_within(..copy_len); } } diff --git a/datafusion/functions/src/unicode/character_length.rs b/datafusion/functions/src/unicode/character_length.rs index 2b99d942f6c0a..9f0d952a02636 100644 --- a/datafusion/functions/src/unicode/character_length.rs +++ b/datafusion/functions/src/unicode/character_length.rs @@ -158,10 +158,10 @@ where } else { let values: Vec<_> = (0..array.len()) .map(|i| { + // Safety: we are iterating with array.len() so the index is always valid if array.is_null(i) { T::default_value() } else { - // Safety: we are iterating with array.len() so the index is always valid let value = unsafe { array.value_unchecked(i) }; if value.is_empty() { T::default_value() diff --git a/datafusion/macros/src/user_doc.rs b/datafusion/macros/src/user_doc.rs index bbfe7877fc211..ce9e7d55ef103 100644 --- a/datafusion/macros/src/user_doc.rs +++ b/datafusion/macros/src/user_doc.rs @@ -21,6 +21,7 @@ )] #![cfg_attr(docsrs, feature(doc_cfg))] +extern crate proc_macro; use datafusion_doc::scalar_doc_sections::doc_sections_const; use proc_macro::TokenStream; use quote::quote; diff --git a/datafusion/optimizer/src/extract_equijoin_predicate.rs b/datafusion/optimizer/src/extract_equijoin_predicate.rs index 58f9a4cd42a2d..0a50761e8a9f7 100644 --- a/datafusion/optimizer/src/extract_equijoin_predicate.rs +++ b/datafusion/optimizer/src/extract_equijoin_predicate.rs @@ -95,7 +95,7 @@ impl OptimizerRule for ExtractEquijoinPredicate { && equijoin_predicates.is_empty() && non_equijoin_expr.is_some() { - // Checked in the outer `if` + // SAFETY: checked in the outer `if` let expr = non_equijoin_expr.clone().unwrap(); let (equijoin_predicates, non_equijoin_expr) = split_is_not_distinct_from_and_other_join_predicate( diff --git a/datafusion/physical-expr-common/src/binary_map.rs b/datafusion/physical-expr-common/src/binary_map.rs index 48568665d27a4..44ca35c7f8708 100644 --- a/datafusion/physical-expr-common/src/binary_map.rs +++ b/datafusion/physical-expr-common/src/binary_map.rs @@ -557,7 +557,7 @@ fn single_null_buffer(num_values: usize, null_index: usize) -> NullBuffer { null_builder.append_n_non_nulls(null_index); null_builder.append_null(); null_builder.append_n_non_nulls(num_values - null_index - 1); - // The appends above guarantee the inner builder has been constructed. + // SAFETY: inner builder must be constructed null_builder.finish().unwrap() } diff --git a/datafusion/physical-plan/src/sorts/cursor.rs b/datafusion/physical-plan/src/sorts/cursor.rs index f595ec6fcb06b..d71eaad663410 100644 --- a/datafusion/physical-plan/src/sorts/cursor.rs +++ b/datafusion/physical-plan/src/sorts/cursor.rs @@ -410,18 +410,16 @@ impl CursorValues for StringViewArray { #[inline(always)] fn compare(l: &Self, l_idx: usize, r: &Self, r_idx: usize) -> Ordering { + // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. + // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). + // And the bound is checked in is_finished, it is safe to call get_unchecked if l.data_buffers().is_empty() && r.data_buffers().is_empty() { - // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. - // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). - // And the bound is checked in is_finished, it is safe to call get_unchecked let l_view = unsafe { l.views().get_unchecked(l_idx) }; let r_view = unsafe { r.views().get_unchecked(r_idx) }; return StringViewArray::inline_key_fast(*l_view) .cmp(&StringViewArray::inline_key_fast(*r_view)); } - // SAFETY: Prior assertions guarantee that l_idx and r_idx are valid indices. - // Null-checks are assumed to have been handled in the wrapper (e.g., ArrayValues). unsafe { GenericByteViewArray::compare_unchecked(l, l_idx, r, r_idx) } } } diff --git a/datafusion/spark/src/function/string/length.rs b/datafusion/spark/src/function/string/length.rs index b3683dac5b83e..8c5539a0577d8 100644 --- a/datafusion/spark/src/function/string/length.rs +++ b/datafusion/spark/src/function/string/length.rs @@ -154,10 +154,10 @@ where } else { let values: Vec<_> = (0..array.len()) .map(|i| { + // Safety: we are iterating with array.len() so the index is always valid if array.is_null(i) { i32::default() } else { - // Safety: we are iterating with array.len() so the index is always valid let value = unsafe { array.value_unchecked(i) }; if value.is_empty() { i32::default() diff --git a/datafusion/wasmtest/src/lib.rs b/datafusion/wasmtest/src/lib.rs index 6289a0c3956fb..f545ccf19306a 100644 --- a/datafusion/wasmtest/src/lib.rs +++ b/datafusion/wasmtest/src/lib.rs @@ -22,6 +22,8 @@ )] #![cfg_attr(docsrs, feature(doc_cfg))] +extern crate wasm_bindgen; + use datafusion_common::ScalarValue; use datafusion_expr::lit; use datafusion_expr::simplify::SimplifyContext; From 101919d14adad5d4dc7d7c468ef6abbd2cdd2c55 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Thu, 6 Aug 2026 12:24:06 +0200 Subject: [PATCH 16/16] fix: use `Path::join` in `get_query_path` New code from `main` trips `clippy::pathbuf_init_then_push`. Co-Authored-By: Claude Opus 5 (1M context) --- benchmarks/src/clickbench.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/benchmarks/src/clickbench.rs b/benchmarks/src/clickbench.rs index a2e65aa5618a9..b6d118129a485 100644 --- a/benchmarks/src/clickbench.rs +++ b/benchmarks/src/clickbench.rs @@ -111,9 +111,7 @@ pub struct RunOpt { /// Get the SQL file path pub fn get_query_path(query_dir: &Path, query: usize) -> PathBuf { - let mut query_path = query_dir.to_path_buf(); - query_path.push(format!("q{query}.sql")); - query_path + query_dir.join(format!("q{query}.sql")) } /// Get the SQL statement from the specified query file