Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
8b250e6
feat: enable 19 more clippy lints that need no code changes
emilk Aug 13, 2026
5f53e4c
feat: enable clippy::unused_peekable
emilk Aug 13, 2026
068d3ad
feat: enable clippy::fallible_impl_from
emilk Aug 13, 2026
6be5234
feat: enable clippy::float_cmp_const
emilk Aug 13, 2026
ac09370
feat: enable clippy::lossy_float_literal
emilk Aug 13, 2026
8a2bdc3
feat: enable clippy::manual_midpoint
emilk Aug 13, 2026
999c2da
feat: enable clippy::literal_string_with_formatting_args
emilk Aug 13, 2026
1dbde56
feat: enable clippy::ref_as_ptr
emilk Aug 13, 2026
913310a
feat: enable clippy::unnecessary_safety_comment
emilk Aug 13, 2026
20f4453
feat: enable clippy::large_types_passed_by_value
emilk Aug 13, 2026
19eaa18
feat: enable clippy::unnecessary_box_returns
emilk Aug 13, 2026
32dd685
feat: enable clippy::option_as_ref_cloned
emilk Aug 13, 2026
a31db08
feat: enable clippy::iter_on_single_items
emilk Aug 13, 2026
72b7456
feat: enable clippy::string_lit_as_bytes
emilk Aug 13, 2026
5b80ab1
feat: enable clippy::unnecessary_debug_formatting
emilk Aug 13, 2026
316ea02
feat: enable clippy::option_option
emilk Aug 13, 2026
e534900
feat: enable clippy::single_option_map
emilk Aug 13, 2026
663f66a
feat: enable clippy::ignore_without_reason
emilk Aug 13, 2026
0f7e1ab
feat: enable clippy::branches_sharing_code
emilk Aug 13, 2026
20fc834
feat: enable clippy::needless_type_cast
emilk Aug 13, 2026
c4bbf00
feat: enable clippy::checked_conversions
emilk Aug 13, 2026
2758248
feat: enable clippy::filter_map_next
emilk Aug 13, 2026
fddce20
feat: enable clippy::large_digit_groups
emilk Aug 13, 2026
6272053
feat: enable clippy::manual_is_variant_and
emilk Aug 13, 2026
084783a
feat: enable clippy::duration_suboptimal_units
emilk Aug 13, 2026
215f20a
feat: enable clippy::elidable_lifetime_names
emilk Aug 13, 2026
460c0bd
feat: enable clippy::inconsistent_struct_constructor
emilk Aug 13, 2026
e19846a
feat: enable clippy::bool_to_int_with_if
emilk Aug 13, 2026
a8e2e35
feat: enable clippy::unnecessary_struct_initialization
emilk Aug 13, 2026
9dce16b
feat: enable clippy::manual_assert
emilk Aug 13, 2026
358f225
feat: enable clippy::rest_pat_in_fully_bound_structs
emilk Aug 13, 2026
e3a2747
feat: enable clippy::equatable_if_let
emilk Aug 13, 2026
685ea70
feat: enable clippy::explicit_deref_methods
emilk Aug 13, 2026
65ef16e
Revert "feat: enable clippy::unnecessary_safety_comment"
emilk Aug 13, 2026
76bde3a
refactor: use `==` instead of `matches!` where the values are comparable
emilk Aug 13, 2026
0d0bad8
Revert "feat: enable clippy::float_cmp_const"
emilk Aug 13, 2026
92ae8a0
Revert "feat: enable clippy::lossy_float_literal"
emilk Aug 13, 2026
42ef8b1
Revert "feat: enable clippy::fallible_impl_from"
emilk Aug 13, 2026
f65d360
refactor: address review feedback on duration units and the ignored test
emilk Aug 13, 2026
1755ad1
fix: apply iter_on_single_items to a new site from main
emilk Aug 13, 2026
2c28d49
Merge branch 'main' into emilk/more-clippy-lints
Dandandan Aug 15, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 56 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -217,59 +217,115 @@ zstd = { version = "0.13", default-features = false }
allow_attributes = "warn"
as_ptr_cast_mut = "warn"
assigning_clones = "warn"
bool_to_int_with_if = "warn"
branches_sharing_code = "warn"
checked_conversions = "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"
duration_suboptimal_units = "warn"
elidable_lifetime_names = "warn"
empty_enum_variants_with_brackets = "warn"
empty_enums = "warn"
equatable_if_let = "warn"
exit = "warn"
expl_impl_clone_on_copy = "warn"
explicit_deref_methods = "warn"
filter_map_next = "warn"
flat_map_option = "warn"
fn_params_excessive_bools = "warn"
fn_to_numeric_cast_any = "warn"
ignore_without_reason = "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"
ip_constant = "warn"
iter_filter_is_ok = "warn"
iter_filter_is_some = "warn"
iter_not_returning_iterator = "warn"
iter_on_empty_collections = "warn"
iter_on_single_items = "warn"
iter_without_into_iter = "warn"
large_digit_groups = "warn"
# Detects large stack-allocated futures that may cause stack overflow crashes (see threshold in clippy.toml)
large_futures = "warn"
large_include_file = "warn"
# Like `large_futures`, these guard against stack overflows
large_stack_arrays = "warn"
large_stack_frames = "warn"
large_types_passed_by_value = "warn"
linkedlist = "warn"
# Catches `"{foo}"` where the string is never actually formatted
literal_string_with_formatting_args = "warn"
macro_use_imports = "warn"
manual_assert = "warn"
manual_ilog2 = "warn"
manual_instant_elapsed = "warn"
manual_is_power_of_two = "warn"
manual_is_variant_and = "warn"
# `(a + b) / 2` can overflow; `a.midpoint(b)` cannot
manual_midpoint = "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"
needless_type_cast = "warn"
negative_feature_names = "warn"
# Prefer `std::sync::LazyLock` over the `lazy_static`/`once_cell` crates
non_std_lazy_statics = "warn"
non_zero_suggestions = "warn"
nonstandard_macro_braces = "warn"
option_as_ref_cloned = "warn"
option_option = "warn"
or_fun_call = "warn"
path_buf_push_overwrite = "warn"
pathbuf_init_then_push = "warn"
precedence_bits = "warn"
ptr_cast_constness = "warn"
pub_underscore_fields = "warn"
pub_without_shorthand = "warn"
rc_mutex = "warn"
ref_as_ptr = "warn"
ref_option_ref = "warn"
rest_pat_in_fully_bound_structs = "warn"
# Catches copy-paste bugs in `if`/`else if` chains
same_functions_in_if_condition = "warn"
same_length_and_capacity = "warn"
# Catches a `&self` argument that is only threaded through recursive calls
self_only_used_in_recursion = "warn"
# Avoids hashing the key twice
set_contains_or_insert = "warn"
single_option_map = "warn"
str_split_at_newline = "warn"
string_add_assign = "warn"
string_lit_as_bytes = "warn"
string_lit_chars_any = "warn"
suspicious_xor_used_as_pow = "warn"
trailing_empty_array = "warn"
trait_duplication_in_bounds = "warn"
transmute_ptr_to_ptr = "warn"
# Subtracting `Instant`s panics on overflow; use `saturating_duration_since`
unchecked_time_subtraction = "warn"
uninhabited_references = "warn"
uninlined_format_args = "warn"
unnecessary_box_returns = "warn"
# `{:?}` on a `Path` quotes and escapes it; `{}` on `.display()` does not
unnecessary_debug_formatting = "warn"
unnecessary_lazy_evaluations = "warn"
unnecessary_safety_doc = "warn"
unnecessary_self_imports = "warn"
unnecessary_struct_initialization = "warn"
unused_async = "warn"
unused_peekable = "warn"
unused_rounding = "warn"
used_underscore_binding = "warn"
verbose_file_reads = "warn"
Expand Down
8 changes: 8 additions & 0 deletions benchmarks/src/bin/benchmark_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1970,6 +1970,10 @@ description = "Run query one against CSV data."
}

#[tokio::test]
#[expect(
clippy::literal_string_with_formatting_args,
reason = "The `${VAR:-default}` braces are shell-style placeholders, not format args"
)]
async fn cli_subgroup_filter_is_used_for_benchmark_replacements() {
let temp = tempfile::tempdir().unwrap();

Expand All @@ -1992,6 +1996,10 @@ description = "Run query one against CSV data."
}

#[tokio::test]
#[expect(
clippy::literal_string_with_formatting_args,
reason = "The `${VAR:-default}` braces are shell-style placeholders, not format args"
)]
async fn benchmark_replacements_use_explicit_data_dir() {
let temp = tempfile::tempdir().unwrap();

Expand Down
18 changes: 8 additions & 10 deletions benchmarks/src/bin/mem_profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,10 @@ fn run_benchmark_as_child_process(
env::var("CARGO_TARGET_DIR").unwrap_or_else(|_| "target".to_string());
let command = format!("{target_dir}/{profile}/dfbench");
// Check whether benchmark binary exists
if !Path::new(&command).exists() {
panic!(
"Benchmark binary not found: `{command}`\nRun this command from the top-level `datafusion/` directory so `target/{profile}/dfbench` can be found.",
);
}
assert!(
Path::new(&command).exists(),
"Benchmark binary not found: `{command}`\nRun this command from the top-level `datafusion/` directory so `target/{profile}/dfbench` can be found.",
);
args.insert(0, command);
let mut results = vec![];

Expand Down Expand Up @@ -339,11 +338,10 @@ mod tests {
let stdout = String::from_utf8_lossy(&output.stdout);
let stderr = String::from_utf8_lossy(&output.stderr);

if !output.status.success() {
panic!(
"mem_profile failed\nstdout:\n{stdout}\nstderr:\n{stderr}---------------------",
);
}
assert!(
output.status.success(),
"mem_profile failed\nstdout:\n{stdout}\nstderr:\n{stderr}---------------------",
);

assert!(
stdout.contains("Peak RSS")
Expand Down
2 changes: 1 addition & 1 deletion benchmarks/src/nlj.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ impl RunOpt {
}
Err(e) => {
return Err(DataFusionError::Context(
"NLJ benchmark Q{query_id} failed with error:".to_string(),
format!("NLJ benchmark Q{query_id} failed with error:"),
Comment thread
emilk marked this conversation as resolved.
Box::new(e),
));
}
Expand Down
8 changes: 8 additions & 0 deletions benchmarks/src/sql_benchmark.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1761,6 +1761,10 @@ mod tests {
}

#[test]
#[expect(
clippy::literal_string_with_formatting_args,
reason = "The `${VAR:-default}` braces are shell-style placeholders, not format args"
)]
fn process_replacements_uses_default_for_missing_variable() {
let replacements = HashMap::new();

Expand Down Expand Up @@ -2300,6 +2304,10 @@ NULL|(empty)
}

#[tokio::test]
#[expect(
clippy::literal_string_with_formatting_args,
reason = "The `${VAR:-default}` braces are shell-style placeholders, not format args"
)]
async fn parser_applies_data_dir_replacement_in_load_query_file() {
let temp_dir = tempdir().expect("failed to create benchmark test directory");
let data_dir = temp_dir.path().join("non_default_data");
Expand Down
28 changes: 12 additions & 16 deletions datafusion-cli/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,16 +283,16 @@ fn convert_parquet_statistics(
val.max_opt().map(|v| v.to_string()),
),
(Statistics::ByteArray(val), ConvertedType::UTF8) => (
byte_array_to_string(val.min_opt()),
byte_array_to_string(val.max_opt()),
val.min_opt().map(byte_array_to_string),
val.max_opt().map(byte_array_to_string),
),
(Statistics::ByteArray(val), _) => (
val.min_opt().map(|v| v.to_string()),
val.max_opt().map(|v| v.to_string()),
),
(Statistics::FixedLenByteArray(val), ConvertedType::UTF8) => (
fixed_len_byte_array_to_string(val.min_opt()),
fixed_len_byte_array_to_string(val.max_opt()),
val.min_opt().map(fixed_len_byte_array_to_string),
val.max_opt().map(fixed_len_byte_array_to_string),
),
(Statistics::FixedLenByteArray(val), _) => (
val.min_opt().map(|v| v.to_string()),
Expand All @@ -302,21 +302,17 @@ fn convert_parquet_statistics(
}

/// Convert to a string if it has utf8 encoding, otherwise print bytes directly
fn byte_array_to_string(val: Option<&ByteArray>) -> Option<String> {
val.map(|v| {
v.as_utf8()
.map(|s| s.to_string())
.unwrap_or_else(|_e| v.to_string())
})
fn byte_array_to_string(val: &ByteArray) -> String {
val.as_utf8()
.map(|s| s.to_string())
.unwrap_or_else(|_e| val.to_string())
}

/// Convert to a string if it has utf8 encoding, otherwise print bytes directly
fn fixed_len_byte_array_to_string(val: Option<&FixedLenByteArray>) -> Option<String> {
val.map(|v| {
v.as_utf8()
.map(|s| s.to_string())
.unwrap_or_else(|_e| v.to_string())
})
fn fixed_len_byte_array_to_string(val: &FixedLenByteArray) -> String {
val.as_utf8()
.map(|s| s.to_string())
.unwrap_or_else(|_e| val.to_string())
}

#[derive(Debug)]
Expand Down
6 changes: 2 additions & 4 deletions datafusion-cli/src/helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,16 +308,14 @@ mod tests {
let mut validator = CliHelper::default();

// should be invalid in generic dialect
let result =
readline_direct(Cursor::new(r"select 1 # 2;".as_bytes()), &validator)?;
let result = readline_direct(Cursor::new(br"select 1 # 2;"), &validator)?;
assert!(
matches!(result, ValidationResult::Invalid(Some(e)) if e.contains("Invalid statement"))
);

// valid in postgresql dialect
validator.set_dialect(&Dialect::PostgreSQL);
let result =
readline_direct(Cursor::new(r"select 1 # 2;".as_bytes()), &validator)?;
let result = readline_direct(Cursor::new(br"select 1 # 2;"), &validator)?;
assert!(matches!(result, ValidationResult::Valid(None)));

Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,8 @@ impl IndexedFile {
let file_size = path.metadata()?.len();

let file = File::open(path).map_err(|e| {
DataFusionError::from(e).context(format!("Error opening file {path:?}"))
DataFusionError::from(e)
.context(format!("Error opening file {}", path.display()))
})?;

let options = ArrowReaderOptions::new()
Expand Down Expand Up @@ -583,7 +584,7 @@ impl ParquetFileReaderFactory for CachedParquetFileReaderFactory {
let metadata = self
.metadata
.get(&filename)
.expect("metadata for file not found: {filename}");
.unwrap_or_else(|| panic!("metadata for file not found: {filename}"));
Ok(Box::new(ParquetReaderWithCache {
filename,
metadata: Arc::clone(metadata),
Expand Down
12 changes: 8 additions & 4 deletions datafusion-examples/examples/data_io/parquet_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,8 @@ impl ParquetMetadataIndexBuilder {
let file_size = file.metadata()?.len();

let file = File::open(file).map_err(|e| {
DataFusionError::from(e).context(format!("Error opening file {file:?}"))
DataFusionError::from(e)
.context(format!("Error opening file {}", file.display()))
})?;

let reader = ParquetRecordBatchReaderBuilder::try_new(file)?;
Expand Down Expand Up @@ -621,12 +622,15 @@ fn read_dir(dir: &Path) -> Result<Vec<DirEntry>> {
let mut files = dir
.read_dir()
.map_err(|e| {
DataFusionError::from(e).context(format!("Error reading directory {dir:?}"))
DataFusionError::from(e)
.context(format!("Error reading directory {}", dir.display()))
})?
.map(|entry| {
entry.map_err(|e| {
DataFusionError::from(e)
.context(format!("Error reading directory entry in {dir:?}"))
DataFusionError::from(e).context(format!(
"Error reading directory entry in {}",
dir.display()
))
})
})
.collect::<Result<Vec<DirEntry>>>()?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ impl PhysicalExtensionCodec for ParentPhysicalExtensionCodec {
_ctx: &TaskContext,
_proto_converter: &dyn PhysicalProtoConverterExtension,
) -> Result<Arc<dyn ExecutionPlan>> {
if buf == "ParentExec".as_bytes() {
if buf == b"ParentExec" {
Ok(Arc::new(ParentExec {
input: inputs[0].clone(),
}))
Expand All @@ -176,7 +176,7 @@ impl PhysicalExtensionCodec for ParentPhysicalExtensionCodec {
_proto_converter: &dyn PhysicalProtoConverterExtension,
) -> Result<()> {
if node.is::<ParentExec>() {
buf.extend_from_slice("ParentExec".as_bytes());
buf.extend_from_slice(b"ParentExec");
Ok(())
} else {
internal_err!("Not supported")
Expand Down Expand Up @@ -258,7 +258,7 @@ impl PhysicalExtensionCodec for ChildPhysicalExtensionCodec {
_ctx: &TaskContext,
_proto_converter: &dyn PhysicalProtoConverterExtension,
) -> Result<Arc<dyn ExecutionPlan>> {
if buf == "ChildExec".as_bytes() {
if buf == b"ChildExec" {
Ok(Arc::new(ChildExec {}))
} else {
internal_err!("Not supported")
Expand All @@ -272,7 +272,7 @@ impl PhysicalExtensionCodec for ChildPhysicalExtensionCodec {
_proto_converter: &dyn PhysicalProtoConverterExtension,
) -> Result<()> {
if node.is::<ChildExec>() {
buf.extend_from_slice("ChildExec".as_bytes());
buf.extend_from_slice(b"ChildExec");
Ok(())
} else {
internal_err!("Not supported")
Expand Down
5 changes: 1 addition & 4 deletions datafusion/common/src/scalar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10327,22 +10327,19 @@ mod tests {
const SECS_IN_ONE_DAY: i32 = 86_400;
const MICROSECS_IN_ONE_DAY: i64 = 86_400_000_000;
for i in 0..vector_size {
let days = rng.random_range(0..5000);
if i % 4 == 0 {
let days = rng.random_range(0..5000);
// to not break second precision
let millis = rng.random_range(0..SECS_IN_ONE_DAY) * 1000;
intervals.push(ScalarValue::new_interval_dt(days, millis));
} else if i % 4 == 1 {
let days = rng.random_range(0..5000);
let millisec = rng.random_range(0..(MILLISECS_IN_ONE_DAY as i32));
intervals.push(ScalarValue::new_interval_dt(days, millisec));
} else if i % 4 == 2 {
let days = rng.random_range(0..5000);
// to not break microsec precision
let nanosec = rng.random_range(0..MICROSECS_IN_ONE_DAY) * 1000;
intervals.push(ScalarValue::new_interval_mdn(0, days, nanosec));
} else {
let days = rng.random_range(0..5000);
let nanosec = rng.random_range(0..NANOSECS_IN_ONE_DAY);
intervals.push(ScalarValue::new_interval_mdn(0, days, nanosec));
}
Expand Down
2 changes: 1 addition & 1 deletion datafusion/common/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,7 @@ pub mod datafusion_strsim {
let mut distance_b = i;

for (j, b_elem) in b.into_iter().enumerate() {
let cost = if a_elem == b_elem { 0usize } else { 1usize };
let cost = usize::from(a_elem != b_elem);
let distance_a = distance_b + cost;
distance_b = cache[j];
result = min(result + 1, min(distance_a, distance_b + 1));
Expand Down
4 changes: 4 additions & 0 deletions datafusion/core/benches/topk_aggregate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,10 @@ fn assert_string_results_match(
}
}

#[expect(
clippy::literal_string_with_formatting_args,
reason = "The `{rows}`/`{limit}` placeholders are substituted with `str::replace`"
)]
fn criterion_benchmark(c: &mut Criterion) {
let rt = Runtime::new().unwrap();
let limit = LIMIT;
Expand Down
Loading
Loading