You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/08/13 03:00:08 UTC
(datafusion) 01/01: Enable more lints (#24066)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24066-591ae10c98064a49a90d30af398b92aece72487f
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 3985bd5b4ab399a38382417ce1e0d432f3589767
Author: Emil Ernerfeldt <em...@gmail.com>
AuthorDate: Thu Aug 13 02:59:50 2026 +0000
Enable more lints (#24066)
## Which issue does this PR close?
- Part of #18467
- Follow-up to https://github.com/apache/datafusion/pull/24076
## Rationale for this change
There are a lot of lints we can enable to, which different abilities to:
* Simplify the code
* Write more efficient code
* Catch bugs
* etc
## What changes are included in this PR?
One commit per new lint (maybe easiest to review commit by commit!).
I hope to add even more lints in later PRsq
## Are these changes tested?
Covered by existing tests plus the clippy CI job.
## Are there any user-facing changes?
No.
---------
Co-authored-by: Claude Opus 5 (1M context) <no...@anthropic.com>
---
Cargo.toml | 51 ++++++++++++++++++++++
benchmarks/src/clickbench.rs | 4 +-
datafusion-cli/src/lib.rs | 2 +-
datafusion/common/src/cse.rs | 2 +-
datafusion/common/src/utils/proxy.rs | 2 +-
datafusion/core/benches/parquet_query_sql.rs | 6 +--
datafusion/core/src/lib.rs | 2 +-
.../tests/physical_optimizer/pushdown_utils.rs | 2 +-
.../core/tests/physical_optimizer/test_utils.rs | 2 +-
datafusion/datasource-arrow/src/source.rs | 30 ++++---------
datafusion/datasource-parquet/src/metadata.rs | 4 +-
datafusion/expr/src/logical_plan/builder.rs | 2 +-
datafusion/ffi/src/tests/utils.rs | 1 +
datafusion/functions-nested/src/range.rs | 8 ++--
.../src/equivalence/properties/mod.rs | 2 +-
datafusion/physical-plan/src/aggregates/mod.rs | 2 +-
datafusion/physical-plan/src/analyze.rs | 2 +-
datafusion/proto/src/logical_plan/mod.rs | 5 +--
18 files changed, 81 insertions(+), 48 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index a0482f85f2..440b5ae389 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"
+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"
# 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 = [
diff --git a/benchmarks/src/clickbench.rs b/benchmarks/src/clickbench.rs
index a2e65aa561..b6d118129a 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
diff --git a/datafusion-cli/src/lib.rs b/datafusion-cli/src/lib.rs
index f0b0bc23fd..51202818d3 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/common/src/cse.rs b/datafusion/common/src/cse.rs
index 93169d6a02..64dad4f5c0 100644
--- a/datafusion/common/src/cse.rs
+++ b/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;
diff --git a/datafusion/common/src/utils/proxy.rs b/datafusion/common/src/utils/proxy.rs
index 846c928515..7661cfa491 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"
);
}
diff --git a/datafusion/core/benches/parquet_query_sql.rs b/datafusion/core/benches/parquet_query_sql.rs
index 2e7794bfd1..9ed2618392 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/core/src/lib.rs b/datafusion/core/src/lib.rs
index 3170f4be7f..28ea4b4490 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")]
diff --git a/datafusion/core/tests/physical_optimizer/pushdown_utils.rs b/datafusion/core/tests/physical_optimizer/pushdown_utils.rs
index 0c2286527d..e11573b95f 100644
--- a/datafusion/core/tests/physical_optimizer/pushdown_utils.rs
+++ b/datafusion/core/tests/physical_optimizer/pushdown_utils.rs
@@ -434,7 +434,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 {
diff --git a/datafusion/core/tests/physical_optimizer/test_utils.rs b/datafusion/core/tests/physical_optimizer/test_utils.rs
index 0835497f34..6b10efffa9 100644
--- a/datafusion/core/tests/physical_optimizer/test_utils.rs
+++ b/datafusion/core/tests/physical_optimizer/test_utils.rs
@@ -838,7 +838,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)
diff --git a/datafusion/datasource-arrow/src/source.rs b/datafusion/datasource-arrow/src/source.rs
index f51e1c1009..dba99a9758 100644
--- a/datafusion/datasource-arrow/src/source.rs
+++ b/datafusion/datasource-arrow/src/source.rs
@@ -512,7 +512,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};
@@ -528,11 +528,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());
@@ -572,11 +569,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());
@@ -613,11 +607,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());
@@ -678,11 +669,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());
diff --git a/datafusion/datasource-parquet/src/metadata.rs b/datafusion/datasource-parquet/src/metadata.rs
index 38db1e22ee..c01811c087 100644
--- a/datafusion/datasource-parquet/src/metadata.rs
+++ b/datafusion/datasource-parquet/src/metadata.rs
@@ -1636,8 +1636,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 =
diff --git a/datafusion/expr/src/logical_plan/builder.rs b/datafusion/expr/src/logical_plan/builder.rs
index a3d2c6e17a..4b728fe447 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::<HashSet<Expr>>();
projection.extend(join_key_items);
diff --git a/datafusion/ffi/src/tests/utils.rs b/datafusion/ffi/src/tests/utils.rs
index 3119ab96d4..3834ed82fe 100644
--- a/datafusion/ffi/src/tests/utils.rs
+++ b/datafusion/ffi/src/tests/utils.rs
@@ -81,6 +81,7 @@ fn load_module(lib_path: &Path) -> 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)
diff --git a/datafusion/functions-nested/src/range.rs b/datafusion/functions-nested/src/range.rs
index 65d9244ecd..0a02a8b7bb 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<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))
diff --git a/datafusion/physical-expr/src/equivalence/properties/mod.rs b/datafusion/physical-expr/src/equivalence/properties/mod.rs
index 22b3382f50..54269e07f9 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::<Column>().map(|c| c.index())
})
})
diff --git a/datafusion/physical-plan/src/aggregates/mod.rs b/datafusion/physical-plan/src/aggregates/mod.rs
index a39c6f3486..d0aadc2847 100644
--- a/datafusion/physical-plan/src/aggregates/mod.rs
+++ b/datafusion/physical-plan/src/aggregates/mod.rs
@@ -1377,7 +1377,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())
})
})
diff --git a/datafusion/physical-plan/src/analyze.rs b/datafusion/physical-plan/src/analyze.rs
index d1519828c2..92f7479547 100644
--- a/datafusion/physical-plan/src/analyze.rs
+++ b/datafusion/physical-plan/src/analyze.rs
@@ -307,7 +307,7 @@ impl ExecutionPlan for AnalyzeExec {
}
drop(input_stream);
- let duration = Instant::now() - start;
+ let duration = start.elapsed();
create_output_batch(
verbose,
show_statistics,
diff --git a/datafusion/proto/src/logical_plan/mod.rs b/datafusion/proto/src/logical_plan/mod.rs
index 2efff1cbde..08c1a59e46 100644
--- a/datafusion/proto/src/logical_plan/mod.rs
+++ b/datafusion/proto/src/logical_plan/mod.rs
@@ -19,7 +19,6 @@ use std::collections::HashMap;
use std::fmt::Debug;
use std::sync::Arc;
-use crate::protobuf::logical_plan_node::LogicalPlanType::CustomScan;
use crate::protobuf::{
ColumnUnnestListItem, ColumnUnnestListRecursion, CteWorkTableScanNode,
CustomTableScanNode, DmlNode, SortExprNodeCollection, dml_node,
@@ -702,7 +701,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;
@@ -1532,7 +1531,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(
table_name.clone(),
)),
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org