Enable more clippy lints (part 2/3)

Cargo.toml

@@ -283,24 +283,19 @@ explicit_iter_loop = "allow"                 # 189 hits
 float_cmp = "allow"                          # 8 hits; exact float comparisons are often intentional here
 from_iter_instead_of_collect = "allow"       # 51 hits
 if_not_else = "allow"                        # 133 hits
-ignored_unit_patterns = "allow"              # 52 hits
 implicit_clone = "allow"                     # 198 hits
 implicit_hasher = "allow"                    # 17 hits; some sites feed arrow APIs that require the default hasher
 inline_always = "allow"                      # 45 hits
 items_after_statements = "allow"             # 171 hits
-manual_string_new = "allow"                  # 84 hits
 many_single_char_names = "allow"             # 12 hits; short names are idiomatic in the numeric kernels
 map_unwrap_or = "allow"                      # 198 hits
-match_bool = "allow"                         # 46 hits
 match_same_arms = "allow"                    # 261 hits
 match_wildcard_for_single_variants = "allow" # 132 hits
 missing_errors_doc = "allow"                 # 1807 hits
-missing_fields_in_debug = "allow"            # 29 hits
 missing_panics_doc = "allow"                 # 244 hits
 must_use_candidate = "allow"                 # 2726 hits
 needless_raw_string_hashes = "allow"         # 540 hits
 redundant_closure_for_method_calls = "allow" # 686 hits
-redundant_else = "allow"                     # 48 hits
 return_self_not_must_use = "allow"           # 644 hits
 semicolon_if_nothing_returned = "allow"      # 1353 hits
 similar_names = "allow"                      # 228 hits; too many false positives, e.g. `expr`/`exprs`
@@ -311,7 +306,6 @@ too_many_lines = "allow"                     # 484 hits
 trivially_copy_pass_by_ref = "allow"         # 74 hits
 unnecessary_literal_bound = "allow"          # 471 hits
 unnecessary_wraps = "allow"                  # 427 hits
-unnested_or_patterns = "allow"               # 68 hits
 unreadable_literal = "allow"                 # 502 hits
 unused_self = "allow"                        # 69 hits
 used_underscore_items = "allow"              # 28 hits

benchmarks/src/cancellation.rs

@@ -127,12 +127,12 @@ fn run_test(wait_time: u64, store: Arc<dyn ObjectStore>) -> Result<Duration> {
                 let store = Arc::clone(&store);
                 tokio::select! {
                     biased;
-                    _ = async move {
+                    () = async move {
                         datafusion(store).await.unwrap();
                     } => {
                         println!("matched case doing work");
                     },
-                    _ = captured_token.cancelled() => {
+                    () = captured_token.cancelled() => {
                         println!("Received shutdown request");
                         return;
                     },

benchmarks/src/sql_benchmark.rs

@@ -231,52 +231,49 @@ impl SqlBenchmark {
             let mut local_result = vec![];
 
             for query in run_queries {
-                match save_results {
-                    true => {
-                        debug!(
-                            "Running query (saving results) {}-{}: {query}",
-                            self.group, self.subgroup
-                        );
+                if save_results {
+                    debug!(
+                        "Running query (saving results) {}-{}: {query}",
+                        self.group, self.subgroup
+                    );
 
-                        let df = ctx.sql(query).await?;
-                        if !self.expect.is_empty() {
-                            let physical_plan = df.create_physical_plan().await?;
-                            self.validate_expected_plan(&physical_plan)?;
-                        }
+                    let df = ctx.sql(query).await?;
+                    if !self.expect.is_empty() {
+                        let physical_plan = df.create_physical_plan().await?;
+                        self.validate_expected_plan(&physical_plan)?;
+                    }
 
-                        let result_schema = Arc::new(df.schema().as_arrow().clone());
-                        let mut batches = df.collect().await?;
-                        let trimmed = query.trim_start();
-
-                        // save the output for select/with queries
-                        if starts_with_ignore_ascii_case(trimmed, "select")
-                            || starts_with_ignore_ascii_case(trimmed, "with")
-                        {
-                            if batches.is_empty() {
-                                batches.push(RecordBatch::new_empty(result_schema));
-                            }
-                            let row_count_for_query =
-                                batches.iter().map(RecordBatch::num_rows).sum::<usize>();
-                            debug!(
-                                "Persisting {} batches ({} rows)...",
-                                batches.len(),
-                                row_count_for_query
-                            );
-
-                            result_count = row_count_for_query;
-                            local_result = batches;
+                    let result_schema = Arc::new(df.schema().as_arrow().clone());
+                    let mut batches = df.collect().await?;
+                    let trimmed = query.trim_start();
+
+                    // save the output for select/with queries
+                    if starts_with_ignore_ascii_case(trimmed, "select")
+                        || starts_with_ignore_ascii_case(trimmed, "with")
+                    {
+                        if batches.is_empty() {
+                            batches.push(RecordBatch::new_empty(result_schema));
                         }
-                    }
-                    false => {
+                        let row_count_for_query =
+                            batches.iter().map(RecordBatch::num_rows).sum::<usize>();
                         debug!(
-                            "Running query (ignoring results) {}-{}: {query}",
-                            self.group, self.subgroup
+                            "Persisting {} batches ({} rows)...",
+                            batches.len(),
+                            row_count_for_query
                         );
 
-                        result_count = self
-                            .execute_sql_without_result_buffering(query, ctx)
-                            .await?;
+                        result_count = row_count_for_query;
+                        local_result = batches;
                     }
+                } else {
+                    debug!(
+                        "Running query (ignoring results) {}-{}: {query}",
+                        self.group, self.subgroup
+                    );
+
+                    result_count = self
+                        .execute_sql_without_result_buffering(query, ctx)
+                        .await?;
                 }
             }
 
@@ -509,7 +506,7 @@ impl SqlBenchmark {
 
         while let Some(result) = reader_result {
             match result {
-                Ok(_) => {
+                Ok(()) => {
                     if !is_blank_or_comment_line(&line) {
                         // boxing required because of recursion
                         Box::pin(self.process_line(ctx, &mut reader, &mut line)).await?;
@@ -824,7 +821,7 @@ impl BenchmarkDirective {
 
         loop {
             match reader_result {
-                Some(Ok(_)) => {
+                Some(Ok(())) => {
                     if is_comment_line(line) {
                         // comment, ignore
                     } else if is_blank_line(line) {
@@ -956,7 +953,7 @@ impl BenchmarkDirective {
 
         loop {
             match reader_result {
-                Some(Ok(_)) => {
+                Some(Ok(())) => {
                     if line.trim() == "----" {
                         found_break = true;
                         break;
@@ -1045,7 +1042,7 @@ impl BenchmarkDirective {
 
         loop {
             match reader_result {
-                Some(Ok(_)) => {
+                Some(Ok(())) => {
                     if line.trim() == "----" {
                         found_break = true;
                         break;
@@ -1108,7 +1105,7 @@ impl BenchmarkDirective {
 
         loop {
             match reader_result {
-                Some(Ok(_)) => {
+                Some(Ok(())) => {
                     if is_comment_line(line) {
                         // Clear the line buffer for the next iteration.
                         line.clear();
@@ -1449,7 +1446,7 @@ fn read_query_from_reader(
 
     loop {
         match reader_result {
-            Some(Ok(_)) => {
+            Some(Ok(())) => {
                 if is_comment_line(&line) {
                     // comment, ignore
                 } else if is_blank_line(&line) {

datafusion-cli/src/exec.rs

@@ -69,7 +69,7 @@ pub async fn exec_from_lines(
     reader: &mut BufReader<File>,
     print_options: &PrintOptions,
 ) -> Result<()> {
-    let mut query = "".to_owned();
+    let mut query = String::new();
 
     for line in reader.lines() {
         match line {
@@ -79,10 +79,10 @@ pub async fn exec_from_lines(
                 query.push_str(line);
                 if line.ends_with(';') {
                     match exec_and_print(ctx, print_options, query).await {
-                        Ok(_) => {}
+                        Ok(()) => {}
                         Err(err) => eprintln!("{err}"),
                     }
-                    query = "".to_string();
+                    query = String::new();
                 } else {
                     query.push('\n');
                 }
@@ -175,7 +175,7 @@ pub async fn exec_from_repl(
                     rl.add_history_entry(line.trim_end())?;
                     tokio::select! {
                         res = exec_and_print(ctx, print_options, line) => match res {
-                            Ok(_) => {}
+                            Ok(()) => {}
                             Err(err) => eprintln!("{err}"),
                         },
                         _ = signal::ctrl_c() => {

datafusion-cli/src/functions.rs

@@ -665,8 +665,7 @@ impl TableFunctionImpl for StatisticsCacheFunc {
         {
             for (path, entry) in file_statistics_cache.list_entries() {
                 path_arr.push(path.path.to_string());
-                table_arr
-                    .push(path.table.map_or_else(|| "".to_string(), |t| t.to_string()));
+                table_arr.push(path.table.map_or_else(String::new, |t| t.to_string()));
                 file_modified_arr
                     .push(Some(entry.value.meta.last_modified.timestamp_millis()));
                 file_size_bytes_arr.push(entry.value.meta.size);

datafusion-cli/src/object_storage/instrumented.rs

@@ -499,7 +499,7 @@ impl fmt::Debug for RequestDetails {
             .field("size", &self.size)
             .field("range", &self.range)
             .field("extra_display", &self.extra_display)
-            .finish()
+            .finish_non_exhaustive()
     }
 }
 

datafusion-cli/src/print_format.rs

@@ -128,10 +128,9 @@ fn format_batches_with_maxrows<W: std::io::Write>(
                     filtered_batches.push(sliced_batch);
                     over_limit = true;
                     break;
-                } else {
-                    filtered_batches.push(batch.clone());
-                    row_count += batch.num_rows();
                 }
+                filtered_batches.push(batch.clone());
+                row_count += batch.num_rows();
             }
 
             let formatted =

datafusion-examples/examples/data_io/parquet_encrypted_with_kms.rs

@@ -200,7 +200,7 @@ async fn read_encrypted_with_sql(ctx: &SessionContext, table_path: &str) -> Resu
 extensions_options! {
     struct EncryptionConfig {
         /// Comma-separated list of columns to encrypt
-        pub encrypted_columns: String, default = "".to_owned()
+        pub encrypted_columns: String, default = String::new()
     }
 }
 

datafusion-examples/examples/udf/simple_udtf.rs

@@ -110,10 +110,9 @@ impl TableProvider for LocalCsvTable {
                     let batch_lines = max_return_lines - lines;
                     batches.push(batch.slice(0, batch_lines));
                     break;
-                } else {
-                    batches.push(batch.clone());
-                    lines += batch_lines;
                 }
+                batches.push(batch.clone());
+                lines += batch_lines;
             }
             batches
         } else {

datafusion/catalog-listing/src/helpers.rs

@@ -213,12 +213,14 @@ pub async fn list_partitions(
                 depth: depth + 1,
                 files: None,
             };
-            match depth < max_depth {
-                true => match futures.len() < CONCURRENCY_LIMIT {
-                    true => futures.push(child.list(store)),
-                    false => pending.push(child.list(store)),
-                },
-                false => out.push(child),
+            if depth < max_depth {
+                if futures.len() < CONCURRENCY_LIMIT {
+                    futures.push(child.list(store))
+                } else {
+                    pending.push(child.list(store))
+                }
+            } else {
+                out.push(child)
             }
         }
     }

datafusion/catalog-listing/src/table.rs

@@ -425,12 +425,11 @@ fn derive_common_ordering_from_files(file_groups: &[FileGroup]) -> Option<LexOrd
                             "Cannot derive common ordering: no common prefix between orderings {current:?} and {ordering:?}"
                         );
                         return None;
-                    } else {
-                        let ordering =
-                            LexOrdering::new(current.as_ref()[..prefix_len].to_vec())
-                                .expect("prefix_len > 0, so ordering must be valid");
-                        CurrentOrderingState::SomeOrdering(ordering)
                     }
+                    let ordering =
+                        LexOrdering::new(current.as_ref()[..prefix_len].to_vec())
+                            .expect("prefix_len > 0, so ordering must be valid");
+                    CurrentOrderingState::SomeOrdering(ordering)
                 }
                 // If one file has ordering and another doesn't, no common ordering
                 // Return None and log a trace message explaining why

datafusion/common/src/column.rs

@@ -37,6 +37,10 @@ pub struct Column {
     pub spans: Spans,
 }
 
+#[expect(
+    clippy::missing_fields_in_debug,
+    reason = "this Debug output appears in user-facing error messages; `spans` is diagnostic bookkeeping and a `..` would only add noise"
+)]
 impl fmt::Debug for Column {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Column")

datafusion/common/src/config.rs

@@ -2000,7 +2000,7 @@ config_namespace! {
         /// instead of being converted into a [`std::fmt::Error`]
         pub safe: bool, default = true
         /// Format string for nulls
-        pub null: String, default = "".into()
+        pub null: String, default = String::new()
         /// Date format for date arrays
         pub date_format: Option<String>, default = Some("%Y-%m-%d".to_string())
         /// Format for DateTime arrays
@@ -3399,7 +3399,7 @@ impl Default for ConfigFileEncryptionProperties {
 config_namespace_with_hashmap! {
     pub struct ColumnEncryptionProperties {
         /// Per column encryption key
-        pub column_key_as_hex: String, default = "".to_string()
+        pub column_key_as_hex: String, default = String::new()
         /// Per column encryption key metadata
         pub column_metadata_as_hex: Option<String>, default = None
     }
@@ -3578,7 +3578,7 @@ pub struct ConfigFileDecryptionProperties {
 config_namespace_with_hashmap! {
     pub struct ColumnDecryptionProperties {
         /// Per column encryption key
-        pub column_key_as_hex: String, default = "".to_string()
+        pub column_key_as_hex: String, default = String::new()
     }
 }
 
@@ -4548,7 +4548,7 @@ mod tests {
 
         let parsed_metadata = table_config.parquet.key_value_metadata.clone();
         assert_eq!(parsed_metadata.get("should not exist1"), None);
-        assert_eq!(parsed_metadata.get("key1"), Some(&Some("".into())));
+        assert_eq!(parsed_metadata.get("key1"), Some(&Some(String::new())));
         assert_eq!(parsed_metadata.get("key2"), Some(&Some("value2".into())));
         assert_eq!(
             parsed_metadata.get("key3"),

datafusion/common/src/dfschema.rs

@@ -395,7 +395,7 @@ impl DFSchema {
                 // field to lookup is qualified but current field is unqualified.
                 (Some(_), None) => false,
                 // field to lookup is unqualified, no need to compare qualifier
-                (None, Some(_)) | (None, None) => f.name() == name,
+                (None, Some(_) | None) => f.name() == name,
             })
             .map(|(idx, _)| idx);
         matches.next()