From ce03d8eb1236903b8835d4716729b7a48242db6d Mon Sep 17 00:00:00 2001
From: Stefan Holderbach <sholderbach@users.noreply.github.com>
Date: Wed, 23 Nov 2022 05:04:04 +0100
Subject: [PATCH] Error on negative argument to `last` (#7184)

# Description

- Error on negative argument to `last`
- Add test for negative value in last

Follow-up for #7178

# User-Facing Changes

Breaking change:

even before #7178 `last` returned an empty `list<any>` when given
negative indices.
Now this is an
[error](https://docs.rs/nu-protocol/latest/nu_protocol/enum.ShellError.html#variant.NeedsPositiveValue)

Note:
In #7136 we are considering supporting negative indexing

# Tests + Formatting

+ 1 failure test

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
---
 crates/nu-command/src/filters/last.rs    | 20 ++++++++++++--------
 crates/nu-command/tests/commands/last.rs | 13 +++++++++++++
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/crates/nu-command/src/filters/last.rs b/crates/nu-command/src/filters/last.rs
index 4b4c8e140..969846c60 100644
--- a/crates/nu-command/src/filters/last.rs
+++ b/crates/nu-command/src/filters/last.rs
@@ -77,14 +77,18 @@ impl Command for Last {
         let span = call.head;
 
         let rows: Option<i64> = call.opt(engine_state, stack, 0)?;
-        let to_keep = rows.unwrap_or(1).try_into().unwrap_or(0);
-
-        // early exit for `last 0`
-        if to_keep == 0 {
-            return Ok(Vec::<Value>::new()
-                .into_pipeline_data(engine_state.ctrlc.clone())
-                .set_metadata(metadata));
-        }
+        let to_keep = match rows.unwrap_or(1) {
+            0 => {
+                // early exit for `last 0`
+                return Ok(Vec::<Value>::new()
+                    .into_pipeline_data(engine_state.ctrlc.clone())
+                    .set_metadata(metadata));
+            }
+            i if i < 0 => {
+                return Err(ShellError::NeedsPositiveValue(span));
+            }
+            i => i as usize,
+        };
 
         // only keep last `to_keep` rows in memory
         let mut buf = VecDeque::<_>::new();
diff --git a/crates/nu-command/tests/commands/last.rs b/crates/nu-command/tests/commands/last.rs
index ba0baae99..92c7fc888 100644
--- a/crates/nu-command/tests/commands/last.rs
+++ b/crates/nu-command/tests/commands/last.rs
@@ -78,3 +78,16 @@ fn gets_last_row_as_list_when_amount_given() {
 
     assert_eq!(actual.out, "list<int>");
 }
+
+#[test]
+fn last_errors_on_negative_index() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+                [1, 2, 3]
+                | last -2
+            "#
+    ));
+
+    assert!(actual.err.contains("use a positive value"));
+}