Enable more clippy lints

Cargo.toml

@@ -282,25 +282,19 @@ explicit_into_iter_loop = "allow"            # 55 hits
 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 +305,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();
 
-                        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;
+                    // 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?;
                 }
             }
 
@@ -377,15 +374,15 @@ impl SqlBenchmark {
 
         // Get the first result query (assuming only one for now)
         let query = &self.result_queries[0];
-        let formatted_actual_results = if !query.query.trim().is_empty() {
-            let results = ctx.sql(&query.query).await?.collect().await?;
-            format_record_batches(&results)
-        } else {
+        let formatted_actual_results = if query.query.trim().is_empty() {
             let actual_results = self
                 .last_results
                 .as_ref()
                 .expect("last_results should be present after successful run");
             format_record_batches(actual_results)
+        } else {
+            let results = ctx.sql(&query.query).await?.collect().await?;
+            format_record_batches(&results)
         }?;
 
         Self::compare_results(query, &formatted_actual_results, &query.expected_result)
@@ -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/helper.rs

@@ -49,10 +49,10 @@ pub struct CliHelper {
 
 impl CliHelper {
     pub fn new(dialect: &Dialect, color: bool) -> Self {
-        let highlighter: Box<dyn Highlighter> = if !color {
-            Box::new(NoSyntaxHighlighter {})
-        } else {
+        let highlighter: Box<dyn Highlighter> = if color {
             Box::new(SyntaxHighlighter::new(dialect))
+        } else {
+            Box::new(NoSyntaxHighlighter {})
         };
         Self {
             completer: FilenameCompleter::new(),

datafusion-cli/src/main.rs

@@ -407,10 +407,10 @@ fn parse_batch_size(size: &str) -> Result<usize, String> {
 }
 
 fn parse_command(command: &str) -> Result<String, String> {
-    if !command.is_empty() {
-        Ok(command.to_string())
-    } else {
+    if command.is_empty() {
         Err("-c flag expects only non empty commands".to_string())
+    } else {
+        Ok(command.to_string())
     }
 }
 

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/custom_data_source/default_column_values.rs

@@ -317,11 +317,11 @@ impl PhysicalExprAdapter for DefaultValuePhysicalExprAdapter {
         }
 
         // Replace columns with their default literals if any
-        let rewritten = if !replacements.is_empty() {
+        let rewritten = if replacements.is_empty() {
+            expr
+        } else {
             let refs: HashMap<_, _> = replacements.iter().map(|(k, v)| (*k, v)).collect();
             replace_columns_with_literals(expr, &refs)?
-        } else {
-            expr
         };
 
         // Apply the default adapter as a fallback for other schema adaptations

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)
             }
         }
     }
@@ -390,10 +392,10 @@ pub async fn pruned_partition_list<'a>(
     file_extension: &'a str,
     partition_cols: &'a [(String, DataType)],
 ) -> Result<BoxStream<'a, Result<PartitionedFile>>> {
-    let prefix = if !partition_cols.is_empty() {
-        evaluate_partition_prefix(partition_cols, filters)
-    } else {
+    let prefix = if partition_cols.is_empty() {
         None
+    } else {
+        evaluate_partition_prefix(partition_cols, filters)
     };
 
     let objects = table_path

datafusion/catalog-listing/src/table.rs

@@ -422,12 +422,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