move-Command Tests, Refactor, Fix (#11904)

fixes #11783 

# Description
Firstly Tests for the `move` Command have been added. Afterwards some
duplicate Code has been removed and finally an error-message has been
added for when a column is tried to be moved based on itself. This
should fix #11783 .

To reiterate, the example of the initial issue now plays out as follows:
```shell
> {a: 1} | move a --after a                             
Error: nu:🐚:incompatible_parameters

  × Incompatible parameters.
   ╭─[entry #1:1:15]
 1 │ {a: 1} | move a --after a
   ·               ┬         ┬
   ·               │         ╰── relative to itself
   ·               ╰── Column cannot be moved
   ╰────
``` 

# User-Facing Changes
The error message shown above.

# Tests + Formatting
I added some Tests for the behavior of the command. If I should add
more, please let me know but I added everything that came to mind when
thinking about the command.

---------

Co-authored-by: dannou812 <dannou281@gmail.com>

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

@@ -194,35 +194,25 @@ fn move_record_columns(
 ) -> Result<Value, ShellError> {
     let mut column_idx: Vec<usize> = Vec::with_capacity(columns.len());
 
-    // Check if before/after column exist
-    match &before_or_after.item {
-        BeforeOrAfter::After(after) => {
-            if !record.contains(after) {
-                return Err(ShellError::GenericError {
-                    error: "Cannot move columns".into(),
-                    msg: "column does not exist".into(),
-                    span: Some(before_or_after.span),
-                    help: None,
-                    inner: vec![],
-                });
-            }
-        }
-        BeforeOrAfter::Before(before) => {
-            if !record.contains(before) {
-                return Err(ShellError::GenericError {
-                    error: "Cannot move columns".into(),
-                    msg: "column does not exist".into(),
-                    span: Some(before_or_after.span),
-                    help: None,
-                    inner: vec![],
-                });
-            }
-        }
+    let pivot = match &before_or_after.item {
+        BeforeOrAfter::Before(before) => before,
+        BeforeOrAfter::After(after) => after,
+    };
+
+    // check if pivot exists
+    if !record.contains(pivot) {
+        return Err(ShellError::GenericError {
+            error: "Cannot move columns".into(),
+            msg: "column does not exist".into(),
+            span: Some(before_or_after.span),
+            help: None,
+            inner: vec![],
+        });
     }
 
     // Find indices of columns to be moved
     for column in columns.iter() {
-        if let Some(idx) = record.index_of(&column.coerce_str()?) {
+        if let Some(idx) = record.index_of(column.coerce_string()?) {
             column_idx.push(idx);
         } else {
             return Err(ShellError::GenericError {
@@ -233,47 +223,44 @@ fn move_record_columns(
                 inner: vec![],
             });
         }
+
+        let column_as_string = column.coerce_string()?;
+        // check if column is also pivot
+        if &column_as_string == pivot {
+            return Err(ShellError::IncompatibleParameters {
+                left_message: "Column cannot be moved".to_string(),
+                left_span: column.span(),
+                right_message: "relative to itself".to_string(),
+                right_span: before_or_after.span,
+            });
+        }
     }
 
     let mut out = Record::with_capacity(record.len());
 
     for (i, (inp_col, inp_val)) in record.iter().enumerate() {
-        match &before_or_after.item {
-            BeforeOrAfter::After(after) if after == inp_col => {
-                out.push(inp_col.clone(), inp_val.clone());
-
-                for idx in column_idx.iter() {
-                    if let Some((col, val)) = record.get_index(*idx) {
-                        out.push(col.clone(), val.clone());
-                    } else {
-                        return Err(ShellError::NushellFailedSpanned {
-                            msg: "Error indexing input columns".to_string(),
-                            label: "originates from here".to_string(),
-                            span,
-                        });
-                    }
-                }
-            }
-            BeforeOrAfter::Before(before) if before == inp_col => {
-                for idx in column_idx.iter() {
-                    if let Some((col, val)) = record.get_index(*idx) {
-                        out.push(col.clone(), val.clone());
-                    } else {
-                        return Err(ShellError::NushellFailedSpanned {
-                            msg: "Error indexing input columns".to_string(),
-                            label: "originates from here".to_string(),
-                            span,
-                        });
-                    }
-                }
-
+        if inp_col == pivot {
+            if matches!(&before_or_after.item, BeforeOrAfter::After(..)) {
                 out.push(inp_col.clone(), inp_val.clone());
             }
-            _ => {
-                if !column_idx.contains(&i) {
-                    out.push(inp_col.clone(), inp_val.clone());
+
+            for idx in column_idx.iter() {
+                if let Some((col, val)) = record.get_index(*idx) {
+                    out.push(col.clone(), val.clone());
+                } else {
+                    return Err(ShellError::NushellFailedSpanned {
+                        msg: "Error indexing input columns".to_string(),
+                        label: "originates from here".to_string(),
+                        span,
+                    });
                 }
             }
+
+            if matches!(&before_or_after.item, BeforeOrAfter::Before(..)) {
+                out.push(inp_col.clone(), inp_val.clone());
+            }
+        } else if !column_idx.contains(&i) {
+            out.push(inp_col.clone(), inp_val.clone());
         }
     }
 
@@ -284,10 +271,194 @@ fn move_record_columns(
 mod test {
     use super::*;
 
+    // helper
+    fn get_test_record(columns: Vec<&str>, values: Vec<i64>) -> Record {
+        let test_span = Span::test_data();
+        Record::from_raw_cols_vals(
+            columns.iter().map(|col| col.to_string()).collect(),
+            values.iter().map(|val| Value::test_int(*val)).collect(),
+            test_span,
+            test_span,
+        )
+        .unwrap()
+    }
+
     #[test]
     fn test_examples() {
         use crate::test_examples;
 
         test_examples(Move {})
     }
+
+    #[test]
+    fn move_after_with_single_column() {
+        let test_span = Span::test_data();
+        let test_record = get_test_record(vec!["a", "b", "c", "d"], vec![1, 2, 3, 4]);
+        let after: Spanned<BeforeOrAfter> = Spanned {
+            item: BeforeOrAfter::After("c".to_string()),
+            span: test_span,
+        };
+        let columns = [Value::test_string("a")];
+
+        // corresponds to: {a: 1, b: 2, c: 3, d: 4} | move a --after c
+        let result = move_record_columns(&test_record, &columns, &after, test_span);
+
+        assert!(result.is_ok());
+
+        let result_rec_tmp = result.unwrap();
+        let result_record = result_rec_tmp.as_record().unwrap();
+
+        assert_eq!(*result_record.get_index(0).unwrap().0, "b".to_string());
+        assert_eq!(*result_record.get_index(1).unwrap().0, "c".to_string());
+        assert_eq!(*result_record.get_index(2).unwrap().0, "a".to_string());
+        assert_eq!(*result_record.get_index(3).unwrap().0, "d".to_string());
+    }
+
+    #[test]
+    fn move_after_with_multiple_columns() {
+        let test_span = Span::test_data();
+        let test_record = get_test_record(vec!["a", "b", "c", "d", "e"], vec![1, 2, 3, 4, 5]);
+        let after: Spanned<BeforeOrAfter> = Spanned {
+            item: BeforeOrAfter::After("c".to_string()),
+            span: test_span,
+        };
+        let columns = [Value::test_string("b"), Value::test_string("e")];
+
+        // corresponds to: {a: 1, b: 2, c: 3, d: 4, e: 5} | move b e --after c
+        let result = move_record_columns(&test_record, &columns, &after, test_span);
+
+        assert!(result.is_ok());
+
+        let result_rec_tmp = result.unwrap();
+        let result_record = result_rec_tmp.as_record().unwrap();
+
+        assert_eq!(*result_record.get_index(0).unwrap().0, "a".to_string());
+        assert_eq!(*result_record.get_index(1).unwrap().0, "c".to_string());
+        assert_eq!(*result_record.get_index(2).unwrap().0, "b".to_string());
+        assert_eq!(*result_record.get_index(3).unwrap().0, "e".to_string());
+        assert_eq!(*result_record.get_index(4).unwrap().0, "d".to_string());
+    }
+
+    #[test]
+    fn move_before_with_single_column() {
+        let test_span = Span::test_data();
+        let test_record = get_test_record(vec!["a", "b", "c", "d"], vec![1, 2, 3, 4]);
+        let after: Spanned<BeforeOrAfter> = Spanned {
+            item: BeforeOrAfter::Before("b".to_string()),
+            span: test_span,
+        };
+        let columns = [Value::test_string("d")];
+
+        // corresponds to: {a: 1, b: 2, c: 3, d: 4} | move d --before b
+        let result = move_record_columns(&test_record, &columns, &after, test_span);
+
+        assert!(result.is_ok());
+
+        let result_rec_tmp = result.unwrap();
+        let result_record = result_rec_tmp.as_record().unwrap();
+
+        assert_eq!(*result_record.get_index(0).unwrap().0, "a".to_string());
+        assert_eq!(*result_record.get_index(1).unwrap().0, "d".to_string());
+        assert_eq!(*result_record.get_index(2).unwrap().0, "b".to_string());
+        assert_eq!(*result_record.get_index(3).unwrap().0, "c".to_string());
+    }
+
+    #[test]
+    fn move_before_with_multiple_columns() {
+        let test_span = Span::test_data();
+        let test_record = get_test_record(vec!["a", "b", "c", "d", "e"], vec![1, 2, 3, 4, 5]);
+        let after: Spanned<BeforeOrAfter> = Spanned {
+            item: BeforeOrAfter::Before("b".to_string()),
+            span: test_span,
+        };
+        let columns = [Value::test_string("c"), Value::test_string("e")];
+
+        // corresponds to: {a: 1, b: 2, c: 3, d: 4, e: 5} | move c e --before b
+        let result = move_record_columns(&test_record, &columns, &after, test_span);
+
+        assert!(result.is_ok());
+
+        let result_rec_tmp = result.unwrap();
+        let result_record = result_rec_tmp.as_record().unwrap();
+
+        assert_eq!(*result_record.get_index(0).unwrap().0, "a".to_string());
+        assert_eq!(*result_record.get_index(1).unwrap().0, "c".to_string());
+        assert_eq!(*result_record.get_index(2).unwrap().0, "e".to_string());
+        assert_eq!(*result_record.get_index(3).unwrap().0, "b".to_string());
+        assert_eq!(*result_record.get_index(4).unwrap().0, "d".to_string());
+    }
+
+    #[test]
+    fn move_with_multiple_columns_reorders_columns() {
+        let test_span = Span::test_data();
+        let test_record = get_test_record(vec!["a", "b", "c", "d", "e"], vec![1, 2, 3, 4, 5]);
+        let after: Spanned<BeforeOrAfter> = Spanned {
+            item: BeforeOrAfter::After("e".to_string()),
+            span: test_span,
+        };
+        let columns = [
+            Value::test_string("d"),
+            Value::test_string("c"),
+            Value::test_string("a"),
+        ];
+
+        // corresponds to: {a: 1, b: 2, c: 3, d: 4, e: 5} | move d c a --after e
+        let result = move_record_columns(&test_record, &columns, &after, test_span);
+
+        assert!(result.is_ok());
+
+        let result_rec_tmp = result.unwrap();
+        let result_record = result_rec_tmp.as_record().unwrap();
+
+        assert_eq!(*result_record.get_index(0).unwrap().0, "b".to_string());
+        assert_eq!(*result_record.get_index(1).unwrap().0, "e".to_string());
+        assert_eq!(*result_record.get_index(2).unwrap().0, "d".to_string());
+        assert_eq!(*result_record.get_index(3).unwrap().0, "c".to_string());
+        assert_eq!(*result_record.get_index(4).unwrap().0, "a".to_string());
+    }
+
+    #[test]
+    fn move_fails_when_pivot_not_present() {
+        let test_span = Span::test_data();
+        let test_record = get_test_record(vec!["a", "b"], vec![1, 2]);
+        let after: Spanned<BeforeOrAfter> = Spanned {
+            item: BeforeOrAfter::Before("non-existent".to_string()),
+            span: test_span,
+        };
+        let columns = [Value::test_string("a")];
+
+        let result = move_record_columns(&test_record, &columns, &after, test_span);
+
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn move_fails_when_column_not_present() {
+        let test_span = Span::test_data();
+        let test_record = get_test_record(vec!["a", "b"], vec![1, 2]);
+        let after: Spanned<BeforeOrAfter> = Spanned {
+            item: BeforeOrAfter::Before("b".to_string()),
+            span: test_span,
+        };
+        let columns = [Value::test_string("a"), Value::test_string("non-existent")];
+
+        let result = move_record_columns(&test_record, &columns, &after, test_span);
+
+        assert!(result.is_err());
+    }
+
+    #[test]
+    fn move_fails_when_column_is_also_pivot() {
+        let test_span = Span::test_data();
+        let test_record = get_test_record(vec!["a", "b", "c", "d"], vec![1, 2, 3, 4]);
+        let after: Spanned<BeforeOrAfter> = Spanned {
+            item: BeforeOrAfter::After("b".to_string()),
+            span: test_span,
+        };
+        let columns = [Value::test_string("b"), Value::test_string("d")];
+
+        let result = move_record_columns(&test_record, &columns, &after, test_span);
+
+        assert!(result.is_err());
+    }
 }