Invert &Options to Option<&T>

Elide the reference for `Copy` type (`usize`) Use the canonical deref where possible. * `&Box` -> `&` * `&String` -> `&str` * `&PathBuf` -> `&Path` Skips the ctrl-C handler for now.
nushell · Sep 12, 2023 · 94b9d60 · 94b9d60
1 parent d53b0a9
commit 94b9d60
Show file tree

Hide file tree

Showing 24 changed files with 87 additions and 83 deletions.
diff --git a/crates/nu-cmd-dataframe/src/dataframe/eager/sql_expr.rs b/crates/nu-cmd-dataframe/src/dataframe/eager/sql_expr.rs
@@ -125,7 +125,7 @@ pub fn parse_sql_expr(expr: &SqlExpr) -> Result<Expr> {
     })
 }
 
-fn apply_window_spec(expr: Expr, window_type: &Option<WindowType>) -> Result<Expr> {
+fn apply_window_spec(expr: Expr, window_type: Option<&WindowType>) -> Result<Expr> {
     Ok(match &window_type {
         Some(wtype) => match wtype {
             WindowType::WindowSpec(window_spec) => {
@@ -168,13 +168,13 @@ fn parse_sql_function(sql_function: &SQLFunction) -> Result<Expr> {
             sql_function.distinct,
         ) {
             ("sum", [FunctionArgExpr::Expr(expr)], false) => {
-                apply_window_spec(parse_sql_expr(expr)?, &sql_function.over)?.sum()
+                apply_window_spec(parse_sql_expr(expr)?, sql_function.over.as_ref())?.sum()
             }
             ("count", [FunctionArgExpr::Expr(expr)], false) => {
-                apply_window_spec(parse_sql_expr(expr)?, &sql_function.over)?.count()
+                apply_window_spec(parse_sql_expr(expr)?, sql_function.over.as_ref())?.count()
             }
             ("count", [FunctionArgExpr::Expr(expr)], true) => {
-                apply_window_spec(parse_sql_expr(expr)?, &sql_function.over)?.n_unique()
+                apply_window_spec(parse_sql_expr(expr)?, sql_function.over.as_ref())?.n_unique()
             }
             // Special case for wildcard args to count function.
             ("count", [FunctionArgExpr::Wildcard], false) => lit(1i32).count(),

diff --git a/crates/nu-cmd-extra/src/extra/bits/mod.rs b/crates/nu-cmd-extra/src/extra/bits/mod.rs
@@ -44,7 +44,7 @@ enum InputNumType {
     SignedEight,
 }
 
-fn get_number_bytes(number_bytes: &Option<Spanned<String>>) -> NumberBytes {
+fn get_number_bytes(number_bytes: Option<&Spanned<String>>) -> NumberBytes {
     match number_bytes.as_ref() {
         None => NumberBytes::Eight,
         Some(size) => match size.item.as_str() {

diff --git a/crates/nu-cmd-extra/src/extra/bits/not.rs b/crates/nu-cmd-extra/src/extra/bits/not.rs
@@ -57,7 +57,7 @@ impl Command for BitsNot {
         let signed = call.has_flag("signed");
         let number_bytes: Option<Spanned<String>> =
             call.get_flag(engine_state, stack, "number-bytes")?;
-        let bytes_len = get_number_bytes(&number_bytes);
+        let bytes_len = get_number_bytes(number_bytes.as_ref());
         if let NumberBytes::Invalid = bytes_len {
             if let Some(val) = number_bytes {
                 return Err(ShellError::UnsupportedInput(

diff --git a/crates/nu-cmd-extra/src/extra/bits/rotate_left.rs b/crates/nu-cmd-extra/src/extra/bits/rotate_left.rs
@@ -60,7 +60,7 @@ impl Command for BitsRol {
         let signed = call.has_flag("signed");
         let number_bytes: Option<Spanned<String>> =
             call.get_flag(engine_state, stack, "number-bytes")?;
-        let bytes_len = get_number_bytes(&number_bytes);
+        let bytes_len = get_number_bytes(number_bytes.as_ref());
         if let NumberBytes::Invalid = bytes_len {
             if let Some(val) = number_bytes {
                 return Err(ShellError::UnsupportedInput(

diff --git a/crates/nu-cmd-extra/src/extra/bits/rotate_right.rs b/crates/nu-cmd-extra/src/extra/bits/rotate_right.rs
@@ -60,7 +60,7 @@ impl Command for BitsRor {
         let signed = call.has_flag("signed");
         let number_bytes: Option<Spanned<String>> =
             call.get_flag(engine_state, stack, "number-bytes")?;
-        let bytes_len = get_number_bytes(&number_bytes);
+        let bytes_len = get_number_bytes(number_bytes.as_ref());
         if let NumberBytes::Invalid = bytes_len {
             if let Some(val) = number_bytes {
                 return Err(ShellError::UnsupportedInput(

diff --git a/crates/nu-cmd-extra/src/extra/bits/shift_left.rs b/crates/nu-cmd-extra/src/extra/bits/shift_left.rs
@@ -60,7 +60,7 @@ impl Command for BitsShl {
         let signed = call.has_flag("signed");
         let number_bytes: Option<Spanned<String>> =
             call.get_flag(engine_state, stack, "number-bytes")?;
-        let bytes_len = get_number_bytes(&number_bytes);
+        let bytes_len = get_number_bytes(number_bytes.as_ref());
         if let NumberBytes::Invalid = bytes_len {
             if let Some(val) = number_bytes {
                 return Err(ShellError::UnsupportedInput(

diff --git a/crates/nu-cmd-extra/src/extra/bits/shift_right.rs b/crates/nu-cmd-extra/src/extra/bits/shift_right.rs
@@ -60,7 +60,7 @@ impl Command for BitsShr {
         let signed = call.has_flag("signed");
         let number_bytes: Option<Spanned<String>> =
             call.get_flag(engine_state, stack, "number-bytes")?;
-        let bytes_len = get_number_bytes(&number_bytes);
+        let bytes_len = get_number_bytes(number_bytes.as_ref());
         if let NumberBytes::Invalid = bytes_len {
             if let Some(val) = number_bytes {
                 return Err(ShellError::UnsupportedInput(

diff --git a/crates/nu-cmd-extra/src/extra/filters/roll/mod.rs b/crates/nu-cmd-extra/src/extra/filters/roll/mod.rs
@@ -48,7 +48,7 @@ enum HorizontalDirection {
 
 fn horizontal_rotate_value(
     value: Value,
-    by: &Option<usize>,
+    by: Option<usize>,
     cells_only: bool,
     direction: &HorizontalDirection,
 ) -> Result<Value, ShellError> {

diff --git a/crates/nu-cmd-extra/src/extra/filters/roll/roll_left.rs b/crates/nu-cmd-extra/src/extra/filters/roll/roll_left.rs
@@ -106,7 +106,7 @@ impl Command for RollLeft {
         let cells_only = call.has_flag("cells-only");
         let value = input.into_value(call.head);
         let rotated_value =
-            horizontal_rotate_value(value, &by, cells_only, &HorizontalDirection::Left)?;
+            horizontal_rotate_value(value, by, cells_only, &HorizontalDirection::Left)?;
 
         Ok(rotated_value.into_pipeline_data().set_metadata(metadata))
     }

diff --git a/crates/nu-cmd-extra/src/extra/filters/roll/roll_right.rs b/crates/nu-cmd-extra/src/extra/filters/roll/roll_right.rs
@@ -106,7 +106,7 @@ impl Command for RollRight {
         let cells_only = call.has_flag("cells-only");
         let value = input.into_value(call.head);
         let rotated_value =
-            horizontal_rotate_value(value, &by, cells_only, &HorizontalDirection::Right)?;
+            horizontal_rotate_value(value, by, cells_only, &HorizontalDirection::Right)?;
 
         Ok(rotated_value.into_pipeline_data().set_metadata(metadata))
     }

diff --git a/crates/nu-cmd-extra/src/extra/formats/to/html.rs b/crates/nu-cmd-extra/src/extra/formats/to/html.rs
@@ -164,7 +164,7 @@ impl Command for ToHtml {
 
 fn get_theme_from_asset_file(
     is_dark: bool,
-    theme: &Option<Spanned<String>>,
+    theme: Option<&Spanned<String>>,
 ) -> Result<HashMap<&'static str, String>, ShellError> {
     let theme_name = match theme {
         Some(s) => &s.item,
@@ -301,7 +301,7 @@ fn to_html(
             None => head,
         };
 
-        let color_hm = get_theme_from_asset_file(dark, &theme);
+        let color_hm = get_theme_from_asset_file(dark, theme.as_ref());
         let color_hm = match color_hm {
             Ok(c) => c,
             _ => {

diff --git a/crates/nu-cmd-extra/src/extra/platform/ansi/link.rs b/crates/nu-cmd-extra/src/extra/platform/ansi/link.rs
@@ -98,12 +98,12 @@ fn operate(
 
     if column_paths.is_empty() {
         input.map(
-            move |v| process_value(&v, &text),
+            move |v| process_value(&v, text.as_deref()),
             engine_state.ctrlc.clone(),
         )
     } else {
         input.map(
-            move |v| process_each_path(v, &column_paths, &text, command_span),
+            move |v| process_each_path(v, &column_paths, text.as_deref(), command_span),
             engine_state.ctrlc.clone(),
         )
     }
@@ -112,7 +112,7 @@ fn operate(
 fn process_each_path(
     mut value: Value,
     column_paths: &[CellPath],
-    text: &Option<String>,
+    text: Option<&str>,
     command_span: Span,
 ) -> Value {
     for path in column_paths {
@@ -124,11 +124,11 @@ fn process_each_path(
     value
 }
 
-fn process_value(value: &Value, text: &Option<String>) -> Value {
+fn process_value(value: &Value, text: Option<&str>) -> Value {
     let span = value.span();
     match value {
         Value::String { val, .. } => {
-            let text = text.as_deref().unwrap_or(val.as_str());
+            let text = text.unwrap_or(val.as_str());
             let result = add_osc_link(text, val.as_str());
             Value::string(result, span)
         }

diff --git a/crates/nu-command/src/debug/metadata.rs b/crates/nu-command/src/debug/metadata.rs
@@ -54,23 +54,33 @@ impl Command for Metadata {
                         } => {
                             let origin = stack.get_var_with_origin(*var_id, *span)?;
 
-                            Ok(build_metadata_record(&origin, &input.metadata(), head)
-                                .into_pipeline_data())
+                            Ok(
+                                build_metadata_record(&origin, input.metadata().as_deref(), head)
+                                    .into_pipeline_data(),
+                            )
                         }
                         _ => {
                             let val: Value = call.req(engine_state, stack, 0)?;
-                            Ok(build_metadata_record(&val, &input.metadata(), head)
-                                .into_pipeline_data())
+                            Ok(
+                                build_metadata_record(&val, input.metadata().as_deref(), head)
+                                    .into_pipeline_data(),
+                            )
                         }
                     }
                 } else {
                     let val: Value = call.req(engine_state, stack, 0)?;
-                    Ok(build_metadata_record(&val, &input.metadata(), head).into_pipeline_data())
+                    Ok(
+                        build_metadata_record(&val, input.metadata().as_deref(), head)
+                            .into_pipeline_data(),
+                    )
                 }
             }
             Some(_) => {
                 let val: Value = call.req(engine_state, stack, 0)?;
-                Ok(build_metadata_record(&val, &input.metadata(), head).into_pipeline_data())
+                Ok(
+                    build_metadata_record(&val, input.metadata().as_deref(), head)
+                        .into_pipeline_data(),
+                )
             }
             None => {
                 let mut record = Record::new();
@@ -109,11 +119,7 @@ impl Command for Metadata {
     }
 }
 
-fn build_metadata_record(
-    arg: &Value,
-    metadata: &Option<Box<PipelineMetadata>>,
-    head: Span,
-) -> Value {
+fn build_metadata_record(arg: &Value, metadata: Option<&PipelineMetadata>, head: Span) -> Value {
     let mut record = Record::new();
 
     let span = arg.span();
@@ -128,7 +134,7 @@ fn build_metadata_record(
         ),
     );
 
-    if let Some(x) = metadata.as_deref() {
+    if let Some(x) = metadata {
         match x {
             PipelineMetadata {
                 data_source: DataSource::Ls,

diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs
@@ -87,15 +87,15 @@ impl Command for Save {
         match input {
             PipelineData::ExternalStream { stdout: None, .. } => {
                 // Open files to possibly truncate them
-                let _ = get_files(&path, &stderr_path, append, force)?;
+                let _ = get_files(&path, stderr_path.as_ref(), append, force)?;
                 Ok(PipelineData::empty())
             }
             PipelineData::ExternalStream {
                 stdout: Some(stream),
                 stderr,
                 ..
             } => {
-                let (file, stderr_file) = get_files(&path, &stderr_path, append, force)?;
+                let (file, stderr_file) = get_files(&path, stderr_path.as_ref(), append, force)?;
 
                 // delegate a thread to redirect stderr to result.
                 let handler = stderr.map(|stderr_stream| match stderr_file {
@@ -127,7 +127,7 @@ impl Command for Save {
             PipelineData::ListStream(ls, _)
                 if raw || prepare_path(&path, append, force)?.0.extension().is_none() =>
             {
-                let (mut file, _) = get_files(&path, &stderr_path, append, force)?;
+                let (mut file, _) = get_files(&path, stderr_path.as_ref(), append, force)?;
                 for val in ls {
                     file.write_all(&value_to_bytes(val)?)
                         .map_err(|err| ShellError::IOError(err.to_string()))?;
@@ -143,7 +143,7 @@ impl Command for Save {
                     input_to_bytes(input, Path::new(&path.item), raw, engine_state, stack, span)?;
 
                 // Only open file after successful conversion
-                let (mut file, _) = get_files(&path, &stderr_path, append, force)?;
+                let (mut file, _) = get_files(&path, stderr_path.as_ref(), append, force)?;
 
                 file.write_all(&bytes)
                     .map_err(|err| ShellError::IOError(err.to_string()))?;
@@ -317,7 +317,7 @@ fn open_file(path: &Path, span: Span, append: bool) -> Result<File, ShellError>
 /// Get output file and optional stderr file
 fn get_files(
     path: &Spanned<PathBuf>,
-    stderr_path: &Option<Spanned<PathBuf>>,
+    stderr_path: Option<&Spanned<PathBuf>>,
     append: bool,
     force: bool,
 ) -> Result<(File, Option<File>), ShellError> {

diff --git a/crates/nu-command/src/filters/split_by.rs b/crates/nu-command/src/filters/split_by.rs
@@ -130,7 +130,7 @@ pub fn split_by(
                 item: v.as_string()?,
                 span: name,
             });
-            Ok(split(&splitter, input, name)?)
+            Ok(split(splitter.as_ref(), input, name)?)
         }
         // This uses the same format as the 'requires a column name' error in sort_utils.rs
         None => Err(ShellError::GenericError(
@@ -144,7 +144,7 @@ pub fn split_by(
 }
 
 pub fn split(
-    column_name: &Option<Spanned<String>>,
+    column_name: Option<&Spanned<String>>,
     values: PipelineData,
     span: Span,
 ) -> Result<PipelineData, ShellError> {
@@ -156,32 +156,29 @@ pub fn split(
 
     match grouper {
         Grouper::ByColumn(Some(column_name)) => {
-            let block =
-                Box::new(
-                    move |_, row: &Value| match row.get_data_by_key(&column_name.item) {
-                        Some(group_key) => Ok(group_key.as_string()?),
-                        None => Err(ShellError::CantFindColumn {
-                            col_name: column_name.item.to_string(),
-                            span: column_name.span,
-                            src_span: row.span(),
-                        }),
-                    },
-                );
+            let block = move |_, row: &Value| match row.get_data_by_key(&column_name.item) {
+                Some(group_key) => Ok(group_key.as_string()?),
+                None => Err(ShellError::CantFindColumn {
+                    col_name: column_name.item.to_string(),
+                    span: column_name.span,
+                    src_span: row.span(),
+                }),
+            };
 
-            data_split(values, &Some(block), span)
+            data_split(values, Some(&block), span)
         }
         Grouper::ByColumn(None) => {
-            let block = Box::new(move |_, row: &Value| row.as_string());
+            let block = move |_, row: &Value| row.as_string();
 
-            data_split(values, &Some(block), span)
+            data_split(values, Some(&block), span)
         }
     }
 }
 
 #[allow(clippy::type_complexity)]
 fn data_group(
     values: &Value,
-    grouper: &Option<Box<dyn Fn(usize, &Value) -> Result<String, ShellError> + Send>>,
+    grouper: Option<&dyn Fn(usize, &Value) -> Result<String, ShellError>>,
     span: Span,
 ) -> Result<Value, ShellError> {
     let mut groups: IndexMap<String, Vec<Value>> = IndexMap::new();
@@ -209,7 +206,7 @@ fn data_group(
 #[allow(clippy::type_complexity)]
 pub fn data_split(
     value: PipelineData,
-    splitter: &Option<Box<dyn Fn(usize, &Value) -> Result<String, ShellError> + Send>>,
+    splitter: Option<&dyn Fn(usize, &Value) -> Result<String, ShellError>>,
     span: Span,
 ) -> Result<PipelineData, ShellError> {
     let mut splits = indexmap::IndexMap::new();

diff --git a/crates/nu-command/src/filters/uniq.rs b/crates/nu-command/src/filters/uniq.rs
@@ -234,7 +234,7 @@ fn sort_attributes(val: Value) -> Value {
 
 fn generate_key(item: &ValueCounter) -> Result<String, ShellError> {
     let value = sort_attributes(item.val_to_compare.clone()); //otherwise, keys could be different for Records
-    value_to_string(&value, Span::unknown(), 0, &None)
+    value_to_string(&value, Span::unknown(), 0, None)
 }
 
 fn generate_results_with_count(head: Span, uniq_values: Vec<ValueCounter>) -> Vec<Value> {