From ecaed7f0ae5f496a6dcaf3ec979e9bd8d78aa160 Mon Sep 17 00:00:00 2001
From: moonlander <111705932+astral-l@users.noreply.github.com>
Date: Tue, 27 Feb 2024 15:05:26 +0000
Subject: [PATCH] add --signed flag for binary into int conversions (#11902)

<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->
- adds a `--signed` flag to `into int` to allow parsing binary values as
signed integers, the integer size depends on the length of the binary
value

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->
- attempting to convert binary values larger than 8 bytes into integers
now throws an error, with or without `--signed`

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->
- wrote 3 tests and 1 example for `into int --signed` usage
- added an example for unsigned binary `into int`

# 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.
-->
- will add examples from this PR to `into int` documentation
---
 crates/nu-command/src/conversions/into/int.rs | 69 ++++++++++++++++---
 .../tests/commands/conversions/into/int.rs    | 18 +++++
 crates/nu-command/tests/commands/into_int.rs  | 35 ++++++++++
 3 files changed, 111 insertions(+), 11 deletions(-)

diff --git a/crates/nu-command/src/conversions/into/int.rs b/crates/nu-command/src/conversions/into/int.rs
index afe90f7fcd..30b08d0bc1 100644
--- a/crates/nu-command/src/conversions/into/int.rs
+++ b/crates/nu-command/src/conversions/into/int.rs
@@ -11,6 +11,7 @@ use nu_utils::get_system_locale;
 struct Arguments {
     radix: u32,
     cell_paths: Option<Vec<CellPath>>,
+    signed: bool,
     little_endian: bool,
 }
 
@@ -79,6 +80,11 @@ impl Command for SubCommand {
                 "byte encode endian, available options: native(default), little, big",
                 Some('e'),
             )
+            .switch(
+                "signed",
+                "always treat input number as a signed number",
+                Some('s'),
+            )
             .rest(
                 "rest",
                 SyntaxShape::CellPath,
@@ -148,9 +154,12 @@ impl Command for SubCommand {
             None => cfg!(target_endian = "little"),
         };
 
+        let signed = call.has_flag(engine_state, stack, "signed")?;
+
         let args = Arguments {
             radix,
             little_endian,
+            signed,
             cell_paths,
         };
         operate(action, args, input, call.head, engine_state.ctrlc.clone())
@@ -221,12 +230,23 @@ impl Command for SubCommand {
                 example: "'0010132' | into int --radix 8",
                 result: Some(Value::test_int(4186)),
             },
+            Example {
+                description: "Convert binary value to int",
+                example: "0x[10] | into int",
+                result: Some(Value::test_int(16)),
+            },
+            Example {
+                description: "Convert binary value to signed int",
+                example: "0x[a0] | into int --signed",
+                result: Some(Value::test_int(-96)),
+            },
         ]
     }
 }
 
 fn action(input: &Value, args: &Arguments, span: Span) -> Value {
     let radix = args.radix;
+    let signed = args.signed;
     let little_endian = args.little_endian;
     let val_span = input.span();
     match input {
@@ -307,21 +327,42 @@ fn action(input: &Value, args: &Arguments, span: Span) -> Value {
             use byteorder::{BigEndian, ByteOrder, LittleEndian};
 
             let mut val = val.to_vec();
+            let size = val.len();
 
-            if little_endian {
-                while val.len() < 8 {
-                    val.push(0);
+            if size == 0 {
+                return Value::int(0, span);
+            }
+
+            if size > 8 {
+                return Value::error(
+                    ShellError::IncorrectValue {
+                        msg: format!("binary input is too large to convert to int ({size} bytes)"),
+                        val_span,
+                        call_span: span,
+                    },
+                    span,
+                );
+            }
+
+            match (little_endian, signed) {
+                (true, true) => Value::int(LittleEndian::read_int(&val, size), span),
+                (false, true) => Value::int(BigEndian::read_int(&val, size), span),
+                (true, false) => {
+                    while val.len() < 8 {
+                        val.push(0);
+                    }
+                    val.resize(8, 0);
+
+                    Value::int(LittleEndian::read_i64(&val), span)
                 }
-                val.resize(8, 0);
+                (false, false) => {
+                    while val.len() < 8 {
+                        val.insert(0, 0);
+                    }
+                    val.resize(8, 0);
 
-                Value::int(LittleEndian::read_i64(&val), val_span)
-            } else {
-                while val.len() < 8 {
-                    val.insert(0, 0);
+                    Value::int(BigEndian::read_i64(&val), span)
                 }
-                val.resize(8, 0);
-
-                Value::int(BigEndian::read_i64(&val), val_span)
             }
         }
         // Propagate errors by explicitly matching them before the final case.
@@ -497,6 +538,7 @@ mod test {
             &Arguments {
                 radix: 10,
                 cell_paths: None,
+                signed: false,
                 little_endian: false,
             },
             Span::test_data(),
@@ -512,6 +554,7 @@ mod test {
             &Arguments {
                 radix: 10,
                 cell_paths: None,
+                signed: false,
                 little_endian: false,
             },
             Span::test_data(),
@@ -527,6 +570,7 @@ mod test {
             &Arguments {
                 radix: 16,
                 cell_paths: None,
+                signed: false,
                 little_endian: false,
             },
             Span::test_data(),
@@ -543,6 +587,7 @@ mod test {
             &Arguments {
                 radix: 10,
                 cell_paths: None,
+                signed: false,
                 little_endian: false,
             },
             Span::test_data(),
@@ -565,6 +610,7 @@ mod test {
             &Arguments {
                 radix: 10,
                 cell_paths: None,
+                signed: false,
                 little_endian: false,
             },
             Span::test_data(),
@@ -587,6 +633,7 @@ mod test {
             &Arguments {
                 radix: 10,
                 cell_paths: None,
+                signed: false,
                 little_endian: false,
             },
             Span::test_data(),
diff --git a/crates/nu-command/tests/commands/conversions/into/int.rs b/crates/nu-command/tests/commands/conversions/into/int.rs
index 850ad6361e..6e290636fd 100644
--- a/crates/nu-command/tests/commands/conversions/into/int.rs
+++ b/crates/nu-command/tests/commands/conversions/into/int.rs
@@ -23,3 +23,21 @@ fn convert_into_int_big_endian() {
     let actual = nu!(r#"0x[01 00 00 00 00 00 00 00] | into int --endian big"#);
     assert_eq!(actual.out, "72057594037927936");
 }
+
+#[test]
+fn convert_into_signed_int_little_endian() {
+    let actual = nu!(r#"0x[ff 00 00 00 00 00 00 00] | into int --endian little --signed"#);
+    assert_eq!(actual.out, "255");
+
+    let actual = nu!(r#"0x[00 00 00 00 00 00 00 ff] | into int --endian little --signed"#);
+    assert_eq!(actual.out, "-72057594037927936");
+}
+
+#[test]
+fn convert_into_signed_int_big_endian() {
+    let actual = nu!(r#"0x[00 00 00 00 00 00 00 ff] | into int --endian big"#);
+    assert_eq!(actual.out, "255");
+
+    let actual = nu!(r#"0x[ff 00 00 00 00 00 00 00] | into int --endian big"#);
+    assert_eq!(actual.out, "-72057594037927936");
+}
diff --git a/crates/nu-command/tests/commands/into_int.rs b/crates/nu-command/tests/commands/into_int.rs
index 87b47c90c1..4ad8750242 100644
--- a/crates/nu-command/tests/commands/into_int.rs
+++ b/crates/nu-command/tests/commands/into_int.rs
@@ -31,6 +31,41 @@ fn into_int_binary() {
     assert!(actual.out.contains("16843009"));
 }
 
+#[test]
+fn into_int_binary_signed() {
+    let actual = nu!("echo 0x[f0] | into int --signed");
+
+    assert!(actual.out.contains("-16"));
+}
+
+#[test]
+fn into_int_binary_back_and_forth() {
+    let actual = nu!("echo 0x[f0] | into int | into binary | to nuon");
+
+    assert!(actual.out.contains("0x[F000000000000000]"));
+}
+
+#[test]
+fn into_int_binary_signed_back_and_forth() {
+    let actual = nu!("echo 0x[f0] | into int --signed | into binary | to nuon");
+
+    assert!(actual.out.contains("0x[F0FFFFFFFFFFFFFF]"));
+}
+
+#[test]
+fn into_int_binary_empty() {
+    let actual = nu!("echo 0x[] | into int");
+
+    assert!(actual.out.contains('0'))
+}
+
+#[test]
+fn into_int_binary_signed_empty() {
+    let actual = nu!("echo 0x[] | into int --signed");
+
+    assert!(actual.out.contains('0'))
+}
+
 #[test]
 #[ignore]
 fn into_int_datetime1() {