Make FromValue take owned Values (#10900)

# Description
Changes `FromValue` to take owned `Value`s instead of borrowed `Value`s.
This eliminates some unnecessary clones (e.g., in `call_ext.rs`).

# User-Facing Changes
Breaking API change for `nu_protocol`.

crates/nu-cmd-dataframe/src/dataframe/utils.rs

@@ -1,15 +1,16 @@
 use nu_protocol::{FromValue, ShellError, Value};
 
 pub fn extract_strings(value: Value) -> Result<Vec<String>, ShellError> {
+    let span = value.span();
     match (
-        <String as FromValue>::from_value(&value),
+        <String as FromValue>::from_value(value.clone()),
-        <Vec<String> as FromValue>::from_value(&value),
+        <Vec<String> as FromValue>::from_value(value),
     ) {
         (Ok(col), Err(_)) => Ok(vec![col]),
         (Err(_), Ok(cols)) => Ok(cols),
         _ => Err(ShellError::IncompatibleParametersSingle {
             msg: "Expected a string or list of strings".into(),
-            span: value.span(),
+            span,
         }),
     }
 }

crates/nu-command/src/filters/drop/column.rs

@@ -157,7 +157,7 @@ fn get_cellpath_columns(keep_cols: Vec<String>, span: Span) -> Vec<CellPath> {
     let mut output = vec![];
     for keep_col in keep_cols {
         let val = Value::string(keep_col, span);
-        let cell_path = match CellPath::from_value(&val) {
+        let cell_path = match CellPath::from_value(val) {
             Ok(v) => v,
             Err(_) => return vec![],
         };

crates/nu-command/src/filters/drop/nth.rs

@@ -3,8 +3,8 @@ use nu_engine::CallExt;
 use nu_protocol::ast::{Call, RangeInclusion};
 use nu_protocol::engine::{Command, EngineState, Stack};
 use nu_protocol::{
-    Category, Example, FromValue, IntoInterruptiblePipelineData, PipelineData, PipelineIterator,
+    Category, Example, IntoInterruptiblePipelineData, PipelineData, PipelineIterator, Range,
-    Range, ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value,
+    ShellError, Signature, Span, Spanned, SyntaxShape, Type, Value,
 };
 
 #[derive(Clone)]
@@ -185,9 +185,7 @@ fn extract_int_or_range(
     let value = call.req::<Value>(engine_state, stack, 0)?;
 
     let int_opt = value.as_int().map(Either::Left).ok();
-    let range_opt: Result<nu_protocol::Range, ShellError> = FromValue::from_value(&value);
+    let range_opt = value.as_range().map(|r| Either::Right(r.clone())).ok();
-
-    let range_opt = range_opt.map(Either::Right).ok();
 
     int_opt
         .or(range_opt)

crates/nu-command/src/filters/insert.rs

@@ -122,7 +122,7 @@ fn insert(
 
     // Replace is a block, so set it up and run it instead of using it as the replacement
     if replacement.as_block().is_ok() {
-        let capture_block: Closure = FromValue::from_value(&replacement)?;
+        let capture_block = Closure::from_value(replacement)?;
         let block = engine_state.get_block(capture_block.block_id).clone();
 
         let mut stack = stack.captures_to_stack(&capture_block.captures);

crates/nu-command/src/filters/update.rs

@@ -119,7 +119,7 @@ fn update(
 
     // Replace is a block, so set it up and run it instead of using it as the replacement
     if replacement.as_block().is_ok() {
-        let capture_block: Closure = FromValue::from_value(&replacement)?;
+        let capture_block = Closure::from_value(replacement)?;
         let block = engine_state.get_block(capture_block.block_id).clone();
 
         let mut stack = stack.captures_to_stack(&capture_block.captures);

crates/nu-command/src/filters/upsert.rs

@@ -141,7 +141,7 @@ fn upsert(
 
     // Replace is a block, so set it up and run it instead of using it as the replacement
     if replacement.as_block().is_ok() {
-        let capture_block: Closure = FromValue::from_value(&replacement)?;
+        let capture_block = Closure::from_value(replacement)?;
         let block = engine_state.get_block(capture_block.block_id).clone();
 
         let mut stack = stack.captures_to_stack(&capture_block.captures);

crates/nu-engine/src/call_ext.rs

@@ -71,7 +71,7 @@ impl CallExt for Call {
     ) -> Result<Option<T>, ShellError> {
         if let Some(expr) = self.get_flag_expr(name) {
             let result = eval_expression(engine_state, stack, &expr)?;
-            FromValue::from_value(&result).map(Some)
+            FromValue::from_value(result).map(Some)
         } else {
             Ok(None)
         }
@@ -84,7 +84,7 @@ impl CallExt for Call {
     ) -> Result<Option<T>, ShellError> {
         if let Some(expr) = self.get_flag_expr(name) {
             let result = eval_constant(working_set, &expr)?;
-            FromValue::from_value(&result).map(Some)
+            FromValue::from_value(result).map(Some)
         } else {
             Ok(None)
         }
@@ -100,7 +100,7 @@ impl CallExt for Call {
 
         for expr in self.positional_iter().skip(starting_pos) {
             let result = eval_expression(engine_state, stack, expr)?;
-            output.push(FromValue::from_value(&result)?);
+            output.push(FromValue::from_value(result)?);
         }
 
         Ok(output)
@@ -115,7 +115,7 @@ impl CallExt for Call {
 
         for expr in self.positional_iter().skip(starting_pos) {
             let result = eval_constant(working_set, expr)?;
-            output.push(FromValue::from_value(&result)?);
+            output.push(FromValue::from_value(result)?);
         }
 
         Ok(output)
@@ -129,7 +129,7 @@ impl CallExt for Call {
     ) -> Result<Option<T>, ShellError> {
         if let Some(expr) = self.positional_nth(pos) {
             let result = eval_expression(engine_state, stack, expr)?;
-            FromValue::from_value(&result).map(Some)
+            FromValue::from_value(result).map(Some)
         } else {
             Ok(None)
         }
@@ -143,7 +143,7 @@ impl CallExt for Call {
     ) -> Result<T, ShellError> {
         if let Some(expr) = self.positional_nth(pos) {
             let result = eval_expression(engine_state, stack, expr)?;
-            FromValue::from_value(&result)
+            FromValue::from_value(result)
         } else if self.positional_len() == 0 {
             Err(ShellError::AccessEmptyContent { span: self.head })
         } else {
@@ -161,7 +161,7 @@ impl CallExt for Call {
     ) -> Result<T, ShellError> {
         if let Some(expr) = self.positional_nth(pos) {
             let result = eval_constant(working_set, expr)?;
-            FromValue::from_value(&result)
+            FromValue::from_value(result)
         } else if self.positional_len() == 0 {
             Err(ShellError::AccessEmptyContent { span: self.head })
         } else {
@@ -180,7 +180,7 @@ impl CallExt for Call {
     ) -> Result<T, ShellError> {
         if let Some(expr) = self.get_parser_info(name) {
             let result = eval_expression(engine_state, stack, expr)?;
-            FromValue::from_value(&result)
+            FromValue::from_value(result)
         } else if self.parser_info.is_empty() {
             Err(ShellError::AccessEmptyContent { span: self.head })
         } else {

crates/nu-plugin/src/protocol/evaluated_call.rs

@@ -238,7 +238,7 @@ impl EvaluatedCall {
     /// ```
     pub fn get_flag<T: FromValue>(&self, name: &str) -> Result<Option<T>, ShellError> {
         if let Some(value) = self.get_flag_value(name) {
-            FromValue::from_value(&value).map(Some)
+            FromValue::from_value(value).map(Some)
         } else {
             Ok(None)
         }
@@ -272,7 +272,7 @@ impl EvaluatedCall {
         self.positional
             .iter()
             .skip(starting_pos)
-            .map(|value| FromValue::from_value(value))
+            .map(|value| FromValue::from_value(value.clone()))
             .collect()
     }
 
@@ -283,7 +283,7 @@ impl EvaluatedCall {
     /// or an error that can be passed back to the shell on error.
     pub fn opt<T: FromValue>(&self, pos: usize) -> Result<Option<T>, ShellError> {
         if let Some(value) = self.nth(pos) {
-            FromValue::from_value(&value).map(Some)
+            FromValue::from_value(value).map(Some)
         } else {
             Ok(None)
         }
@@ -296,7 +296,7 @@ impl EvaluatedCall {
     /// be passed back to the shell.
     pub fn req<T: FromValue>(&self, pos: usize) -> Result<T, ShellError> {
         if let Some(value) = self.nth(pos) {
-            FromValue::from_value(&value)
+            FromValue::from_value(value)
         } else if self.positional.is_empty() {
             Err(ShellError::AccessEmptyContent { span: self.head })
         } else {

crates/nu-protocol/src/value/from_value.rs

@@ -1,6 +1,5 @@
 use std::collections::HashMap;
 use std::path::PathBuf;
-use std::str::FromStr;
 
 use crate::ast::{CellPath, MatchPattern, PathMember};
 use crate::engine::{Block, Closure};
@@ -8,22 +7,22 @@ use crate::{Range, Record, ShellError, Spanned, Value};
 use chrono::{DateTime, FixedOffset};
 
 pub trait FromValue: Sized {
-    fn from_value(v: &Value) -> Result<Self, ShellError>;
+    fn from_value(v: Value) -> Result<Self, ShellError>;
 }
 
 impl FromValue for Value {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
-        Ok(v.clone())
+        Ok(v)
     }
 }
 
 impl FromValue for Spanned<i64> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
-            Value::Int { val, .. } => Ok(Spanned { item: *val, span }),
+            Value::Int { val, .. } => Ok(Spanned { item: val, span }),
-            Value::Filesize { val, .. } => Ok(Spanned { item: *val, span }),
+            Value::Filesize { val, .. } => Ok(Spanned { item: val, span }),
-            Value::Duration { val, .. } => Ok(Spanned { item: *val, span }),
+            Value::Duration { val, .. } => Ok(Spanned { item: val, span }),
 
             v => Err(ShellError::CantConvert {
                 to_type: "int".into(),
@@ -36,11 +35,11 @@ impl FromValue for Spanned<i64> {
 }
 
 impl FromValue for i64 {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::Int { val, .. } => Ok(*val),
+            Value::Int { val, .. } => Ok(val),
-            Value::Filesize { val, .. } => Ok(*val),
+            Value::Filesize { val, .. } => Ok(val),
-            Value::Duration { val, .. } => Ok(*val),
+            Value::Duration { val, .. } => Ok(val),
 
             v => Err(ShellError::CantConvert {
                 to_type: "int".into(),
@@ -53,14 +52,14 @@ impl FromValue for i64 {
 }
 
 impl FromValue for Spanned<f64> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
             Value::Int { val, .. } => Ok(Spanned {
-                item: *val as f64,
+                item: val as f64,
                 span,
             }),
-            Value::Float { val, .. } => Ok(Spanned { item: *val, span }),
+            Value::Float { val, .. } => Ok(Spanned { item: val, span }),
 
             v => Err(ShellError::CantConvert {
                 to_type: "float".into(),
@@ -73,10 +72,10 @@ impl FromValue for Spanned<f64> {
 }
 
 impl FromValue for f64 {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::Float { val, .. } => Ok(*val),
+            Value::Float { val, .. } => Ok(val),
-            Value::Int { val, .. } => Ok(*val as f64),
+            Value::Int { val, .. } => Ok(val as f64),
             v => Err(ShellError::CantConvert {
                 to_type: "float".into(),
                 from_type: v.get_type().to_string(),
@@ -88,7 +87,7 @@ impl FromValue for f64 {
 }
 
 impl FromValue for Spanned<usize> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
             Value::Int { val, .. } => {
@@ -96,7 +95,7 @@ impl FromValue for Spanned<usize> {
                     Err(ShellError::NeedsPositiveValue(span))
                 } else {
                     Ok(Spanned {
-                        item: *val as usize,
+                        item: val as usize,
                         span,
                     })
                 }
@@ -106,7 +105,7 @@ impl FromValue for Spanned<usize> {
                     Err(ShellError::NeedsPositiveValue(span))
                 } else {
                     Ok(Spanned {
-                        item: *val as usize,
+                        item: val as usize,
                         span,
                     })
                 }
@@ -116,7 +115,7 @@ impl FromValue for Spanned<usize> {
                     Err(ShellError::NeedsPositiveValue(span))
                 } else {
                     Ok(Spanned {
-                        item: *val as usize,
+                        item: val as usize,
                         span,
                     })
                 }
@@ -133,28 +132,28 @@ impl FromValue for Spanned<usize> {
 }
 
 impl FromValue for usize {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
             Value::Int { val, .. } => {
                 if val.is_negative() {
                     Err(ShellError::NeedsPositiveValue(span))
                 } else {
-                    Ok(*val as usize)
+                    Ok(val as usize)
                 }
             }
             Value::Filesize { val, .. } => {
                 if val.is_negative() {
                     Err(ShellError::NeedsPositiveValue(span))
                 } else {
-                    Ok(*val as usize)
+                    Ok(val as usize)
                 }
             }
             Value::Duration { val, .. } => {
                 if val.is_negative() {
                     Err(ShellError::NeedsPositiveValue(span))
                 } else {
-                    Ok(*val as usize)
+                    Ok(val as usize)
                 }
             }
 
@@ -169,11 +168,11 @@ impl FromValue for usize {
 }
 
 impl FromValue for String {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         // FIXME: we may want to fail a little nicer here
         match v {
             Value::CellPath { val, .. } => Ok(val.into_string()),
-            Value::String { val, .. } => Ok(val.clone()),
+            Value::String { val, .. } => Ok(val),
             v => Err(ShellError::CantConvert {
                 to_type: "string".into(),
                 from_type: v.get_type().to_string(),
@@ -185,11 +184,12 @@ impl FromValue for String {
 }
 
 impl FromValue for Spanned<String> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
+        let span = v.span();
         Ok(Spanned {
             item: match v {
                 Value::CellPath { val, .. } => val.into_string(),
-                Value::String { val, .. } => val.clone(),
+                Value::String { val, .. } => val,
                 v => {
                     return Err(ShellError::CantConvert {
                         to_type: "string".into(),
@@ -199,19 +199,19 @@ impl FromValue for Spanned<String> {
                     })
                 }
             },
-            span: v.span(),
+            span,
         })
     }
 }
 
 impl FromValue for Vec<String> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         // FIXME: we may want to fail a little nicer here
         match v {
             Value::List { vals, .. } => vals
-                .iter()
+                .into_iter()
                 .map(|val| match val {
-                    Value::String { val, .. } => Ok(val.clone()),
+                    Value::String { val, .. } => Ok(val),
                     c => Err(ShellError::CantConvert {
                         to_type: "string".into(),
                         from_type: c.get_type().to_string(),
@@ -231,16 +231,16 @@ impl FromValue for Vec<String> {
 }
 
 impl FromValue for Vec<Spanned<String>> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         // FIXME: we may want to fail a little nicer here
         match v {
             Value::List { vals, .. } => vals
-                .iter()
+                .into_iter()
                 .map(|val| {
                     let val_span = val.span();
                     match val {
                         Value::String { val, .. } => Ok(Spanned {
-                            item: val.clone(),
+                            item: val,
                             span: val_span,
                         }),
                         c => Err(ShellError::CantConvert {
@@ -263,12 +263,12 @@ impl FromValue for Vec<Spanned<String>> {
 }
 
 impl FromValue for Vec<bool> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
             Value::List { vals, .. } => vals
-                .iter()
+                .into_iter()
                 .map(|val| match val {
-                    Value::Bool { val, .. } => Ok(*val),
+                    Value::Bool { val, .. } => Ok(val),
                     c => Err(ShellError::CantConvert {
                         to_type: "bool".into(),
                         from_type: c.get_type().to_string(),
@@ -288,13 +288,13 @@ impl FromValue for Vec<bool> {
 }
 
 impl FromValue for CellPath {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
-            Value::CellPath { val, .. } => Ok(val.clone()),
+            Value::CellPath { val, .. } => Ok(val),
             Value::String { val, .. } => Ok(CellPath {
                 members: vec![PathMember::String {
-                    val: val.clone(),
+                    val,
                     span,
                     optional: false,
                 }],
@@ -305,7 +305,7 @@ impl FromValue for CellPath {
                 } else {
                     Ok(CellPath {
                         members: vec![PathMember::Int {
-                            val: *val as usize,
+                            val: val as usize,
                             span,
                             optional: false,
                         }],
@@ -323,9 +323,9 @@ impl FromValue for CellPath {
 }
 
 impl FromValue for bool {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::Bool { val, .. } => Ok(*val),
+            Value::Bool { val, .. } => Ok(val),
             v => Err(ShellError::CantConvert {
                 to_type: "bool".into(),
                 from_type: v.get_type().to_string(),
@@ -337,10 +337,10 @@ impl FromValue for bool {
 }
 
 impl FromValue for Spanned<bool> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
-            Value::Bool { val, .. } => Ok(Spanned { item: *val, span }),
+            Value::Bool { val, .. } => Ok(Spanned { item: val, span }),
             v => Err(ShellError::CantConvert {
                 to_type: "bool".into(),
                 from_type: v.get_type().to_string(),
@@ -352,9 +352,9 @@ impl FromValue for Spanned<bool> {
 }
 
 impl FromValue for DateTime<FixedOffset> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::Date { val, .. } => Ok(*val),
+            Value::Date { val, .. } => Ok(val),
             v => Err(ShellError::CantConvert {
                 to_type: "date".into(),
                 from_type: v.get_type().to_string(),
@@ -366,10 +366,10 @@ impl FromValue for DateTime<FixedOffset> {
 }
 
 impl FromValue for Spanned<DateTime<FixedOffset>> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
-            Value::Date { val, .. } => Ok(Spanned { item: *val, span }),
+            Value::Date { val, .. } => Ok(Spanned { item: val, span }),
             v => Err(ShellError::CantConvert {
                 to_type: "date".into(),
                 from_type: v.get_type().to_string(),
@@ -381,9 +381,9 @@ impl FromValue for Spanned<DateTime<FixedOffset>> {
 }
 
 impl FromValue for Range {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::Range { val, .. } => Ok((**val).clone()),
+            Value::Range { val, .. } => Ok(*val),
             v => Err(ShellError::CantConvert {
                 to_type: "range".into(),
                 from_type: v.get_type().to_string(),
@@ -395,13 +395,10 @@ impl FromValue for Range {
 }
 
 impl FromValue for Spanned<Range> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
-            Value::Range { val, .. } => Ok(Spanned {
+            Value::Range { val, .. } => Ok(Spanned { item: *val, span }),
-                item: (**val).clone(),
-                span,
-            }),
             v => Err(ShellError::CantConvert {
                 to_type: "range".into(),
                 from_type: v.get_type().to_string(),
@@ -413,10 +410,10 @@ impl FromValue for Spanned<Range> {
 }
 
 impl FromValue for Vec<u8> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::Binary { val, .. } => Ok(val.clone()),
+            Value::Binary { val, .. } => Ok(val),
-            Value::String { val, .. } => Ok(val.bytes().collect()),
+            Value::String { val, .. } => Ok(val.into_bytes()),
             v => Err(ShellError::CantConvert {
                 to_type: "binary data".into(),
                 from_type: v.get_type().to_string(),
@@ -428,15 +425,12 @@ impl FromValue for Vec<u8> {
 }
 
 impl FromValue for Spanned<Vec<u8>> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
-            Value::Binary { val, .. } => Ok(Spanned {
+            Value::Binary { val, .. } => Ok(Spanned { item: val, span }),
-                item: val.clone(),
-                span,
-            }),
             Value::String { val, .. } => Ok(Spanned {
-                item: val.bytes().collect(),
+                item: val.into_bytes(),
                 span,
             }),
             v => Err(ShellError::CantConvert {
@@ -450,12 +444,11 @@ impl FromValue for Spanned<Vec<u8>> {
 }
 
 impl FromValue for Spanned<PathBuf> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
             Value::String { val, .. } => Ok(Spanned {
-                item: PathBuf::from_str(val)
+                item: val.into(),
-                    .map_err(|err| ShellError::FileNotFoundCustom(err.to_string(), span))?,
                 span,
             }),
             v => Err(ShellError::CantConvert {
@@ -469,10 +462,10 @@ impl FromValue for Spanned<PathBuf> {
 }
 
 impl FromValue for Vec<Value> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         // FIXME: we may want to fail a little nicer here
         match v {
-            Value::List { vals, .. } => Ok(vals.clone()),
+            Value::List { vals, .. } => Ok(vals),
             v => Err(ShellError::CantConvert {
                 to_type: "Vector of values".into(),
                 from_type: v.get_type().to_string(),
@@ -484,9 +477,9 @@ impl FromValue for Vec<Value> {
 }
 
 impl FromValue for Record {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::Record { val, .. } => Ok(val.clone()),
+            Value::Record { val, .. } => Ok(val),
             v => Err(ShellError::CantConvert {
                 to_type: "Record".into(),
                 from_type: v.get_type().to_string(),
@@ -498,11 +491,11 @@ impl FromValue for Record {
 }
 
 impl FromValue for Closure {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::Closure { val, .. } => Ok(val.clone()),
+            Value::Closure { val, .. } => Ok(val),
             Value::Block { val, .. } => Ok(Closure {
-                block_id: *val,
+                block_id: val,
                 captures: HashMap::new(),
             }),
             v => Err(ShellError::CantConvert {
@@ -516,9 +509,9 @@ impl FromValue for Closure {
 }
 
 impl FromValue for Block {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::Block { val, .. } => Ok(Block { block_id: *val }),
+            Value::Block { val, .. } => Ok(Block { block_id: val }),
             v => Err(ShellError::CantConvert {
                 to_type: "Block".into(),
                 from_type: v.get_type().to_string(),
@@ -530,13 +523,10 @@ impl FromValue for Block {
 }
 
 impl FromValue for Spanned<Closure> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
-            Value::Closure { val, .. } => Ok(Spanned {
+            Value::Closure { val, .. } => Ok(Spanned { item: val, span }),
-                item: val.clone(),
-                span,
-            }),
             v => Err(ShellError::CantConvert {
                 to_type: "Closure".into(),
                 from_type: v.get_type().to_string(),
@@ -548,13 +538,10 @@ impl FromValue for Spanned<Closure> {
 }
 
 impl FromValue for Spanned<MatchPattern> {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         let span = v.span();
         match v {
-            Value::MatchPattern { val, .. } => Ok(Spanned {
+            Value::MatchPattern { val, .. } => Ok(Spanned { item: *val, span }),
-                item: *val.clone(),
-                span,
-            }),
             v => Err(ShellError::CantConvert {
                 to_type: "Match pattern".into(),
                 from_type: v.get_type().to_string(),
@@ -566,9 +553,9 @@ impl FromValue for Spanned<MatchPattern> {
 }
 
 impl FromValue for MatchPattern {
-    fn from_value(v: &Value) -> Result<Self, ShellError> {
+    fn from_value(v: Value) -> Result<Self, ShellError> {
         match v {
-            Value::MatchPattern { val, .. } => Ok(*val.clone()),
+            Value::MatchPattern { val, .. } => Ok(*val),
             v => Err(ShellError::CantConvert {
                 to_type: "Match pattern".into(),
                 from_type: v.get_type().to_string(),