Enable more lints

Cargo.toml

@@ -210,20 +210,72 @@ 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"
+clear_with_drain = "warn"
+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"
+exit = "warn"
+flat_map_option = "warn"
+fn_to_numeric_cast_any = "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_instant_elapsed = "warn"
+manual_is_power_of_two = "warn"
+match_wild_err_arm = "warn"
+mem_forget = "warn"
+mismatching_type_param_order = "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"
+pathbuf_init_then_push = "warn"
+precedence_bits = "warn"
+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_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"
+verbose_file_reads = "warn"
+wildcard_dependencies = "warn"
+zero_sized_map_values = "warn"
 
 [workspace.lints.rust]
 unexpected_cfgs = { level = "warn", check-cfg = [

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

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;

datafusion/common/src/cse.rs

@@ -808,7 +808,7 @@ mod test {
         ) -> HashSet<u64> {
             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;

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"
             );
         }

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();

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")]

datafusion/core/tests/physical_optimizer/pushdown_utils.rs

@@ -417,7 +417,7 @@ pub fn format_execution_plan(plan: &Arc<dyn ExecutionPlan>) -> Vec<String> {
 }
 
 fn format_lines(s: &str) -> Vec<String> {
-    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<dyn ExecutionPlan>) -> String {

datafusion/core/tests/physical_optimizer/test_utils.rs

@@ -818,7 +818,7 @@ pub fn format_execution_plan(plan: &Arc<dyn ExecutionPlan>) -> Vec<String> {
 }
 
 fn format_lines(s: &str) -> Vec<String> {
-    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)

datafusion/datasource-arrow/src/source.rs

@@ -444,7 +444,7 @@ impl From<ArrowSource> for Arc<dyn FileSource> {
 
 #[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());

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 =

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::<HashSet<Expr>>();
         projection.extend(join_key_items);

datafusion/ffi/src/tests/utils.rs

@@ -83,6 +83,7 @@ pub fn get_module() -> Result<ForeignLibraryModule> {
     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)

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<Tz> {
-    let tz = tz.unwrap_or_else(|| "+00");
+fn parse_tz(tz: Option<&str>) -> Result<Tz> {
+    let tz = tz.unwrap_or("+00");
 
     Tz::from_str(tz)
         .map_err(|op| exec_datafusion_err!("failed to parse timezone {tz}: {:?}", op))

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::<Column>().map(|c| c.index())
                 })
             })

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::<Column>().map(|c| c.index())
                     })
                 })

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,

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(),
                         )),