Enable clippy::if_not_else

Prefer the positive condition first: `if x { .. } else { .. }` rather than
`if !x { .. } else { .. }`, so the reader does not have to negate mentally
to follow which branch runs.

Mostly `cargo clippy --fix`. Comments that described the negated branch were
moved to the branch they actually describe, and thirteen sites in
datafusion-sql were done by hand because one suggestion in that crate did not
type-check, which made rustfix roll back the whole crate.

Author: emilk

Cargo.toml

@@ -282,7 +282,6 @@ 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
 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

benchmarks/src/sql_benchmark.rs

@@ -374,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)

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-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/catalog-listing/src/helpers.rs

@@ -392,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/src/streaming.rs

@@ -158,7 +158,9 @@ impl StreamingTable {
         _filters: &[Expr],
         limit: Option<usize>,
     ) -> Result<Arc<dyn ExecutionPlan>> {
-        let physical_sort = if !self.sort_order.is_empty() {
+        let physical_sort = if self.sort_order.is_empty() {
+            vec![]
+        } else {
             let df_schema = DFSchema::try_from(Arc::clone(&self.schema))?;
             let eqp = state.execution_props();
 
@@ -182,8 +184,6 @@ impl StreamingTable {
             } else {
                 original_sort_exprs
             }
-        } else {
-            vec![]
         };
 
         let exec = StreamingTableExec::try_new(

datafusion/common/src/config.rs

@@ -4469,12 +4469,12 @@ mod tests {
     #[cfg(feature = "parquet_encryption")]
     impl parquet::encryption::decrypt::KeyRetriever for ParquetEncryptionKeyRetriever {
         fn retrieve_key(&self, key_metadata: &[u8]) -> parquet::errors::Result<Vec<u8>> {
-            if !key_metadata.is_empty() {
-                Ok(b"1234567890123450".to_vec())
-            } else {
+            if key_metadata.is_empty() {
                 Err(parquet::errors::ParquetError::General(
                     "Key metadata not provided".to_string(),
                 ))
+            } else {
+                Ok(b"1234567890123450".to_vec())
             }
         }
     }

datafusion/common/src/datatype.rs

@@ -181,10 +181,10 @@ impl FieldExt for Field {
     fn renamed(self, new_name: &str) -> Self {
         // check if this is a new name before allocating a new Field / copying
         // the existing one
-        if self.name() != new_name {
-            self.with_name(new_name)
-        } else {
+        if self.name() == new_name {
             self
+        } else {
+            self.with_name(new_name)
         }
     }
 
@@ -214,10 +214,10 @@ impl FieldExt for Field {
     }
 
     fn into_list_item(self) -> Self {
-        if self.name() != Field::LIST_FIELD_DEFAULT_NAME {
-            self.with_name(Field::LIST_FIELD_DEFAULT_NAME)
-        } else {
+        if self.name() == Field::LIST_FIELD_DEFAULT_NAME {
             self
+        } else {
+            self.with_name(Field::LIST_FIELD_DEFAULT_NAME)
         }
     }
 }

datafusion/common/src/dfschema.rs

@@ -631,14 +631,7 @@ impl DFSchema {
     /// encoded UTF8 array to be equivalent to a plain UTF8 array.
     pub fn has_equivalent_names_and_types(&self, other: &Self) -> Result<()> {
         // case 1 : schema length mismatch
-        if self.fields().len() != other.fields().len() {
-            _plan_err!(
-                "Schema mismatch: the schema length are not same \
-            Expected schema length: {}, got: {}",
-                self.fields().len(),
-                other.fields().len()
-            )
-        } else {
+        if self.fields().len() == other.fields().len() {
             // case 2 : schema length match, but fields mismatch
             // check if the fields name are the same and have the same data types
             self.fields()
@@ -663,6 +656,13 @@ impl DFSchema {
                         Ok(())
                     }
                 })
+        } else {
+            _plan_err!(
+                "Schema mismatch: the schema length are not same \
+            Expected schema length: {}, got: {}",
+                self.fields().len(),
+                other.fields().len()
+            )
         }
     }
 
@@ -1319,14 +1319,7 @@ impl SchemaExt for Schema {
     // It is only used by insert into cases.
     fn logically_equivalent_names_and_types(&self, other: &Self) -> Result<()> {
         // case 1 : schema length mismatch
-        if self.fields().len() != other.fields().len() {
-            _plan_err!(
-                "Inserting query must have the same schema length as the table. \
-            Expected table schema length: {}, got: {}",
-                self.fields().len(),
-                other.fields().len()
-            )
-        } else {
+        if self.fields().len() == other.fields().len() {
             // case 2 : schema length match, but fields mismatch
             // check if the fields name are the same and have the same data types
             self.fields()
@@ -1345,6 +1338,13 @@ impl SchemaExt for Schema {
                         Ok(())
                     }
                 })
+        } else {
+            _plan_err!(
+                "Inserting query must have the same schema length as the table. \
+            Expected table schema length: {}, got: {}",
+                self.fields().len(),
+                other.fields().len()
+            )
         }
     }
 }