Fix roll panic on empty lists and records (#19285)

## Description
`roll` panics when the input is empty. On current main:

> [] | roll down
panicked: assertion failed: k <= self.len()

> {} | roll left
panicked: assertion failed: mid <= self.len()

> [] | roll up --by 3
panicked: attempt to calculate the remainder with a divisor of zero

I found these by running commands against empty and edge-case input. The
cause is in `vertical_rotate_value` and `horizontal_rotate_value`. Both
do `n % len`, which divides by zero when the list or record is empty.
Without `--by`, the default rotation of 1 on an empty slice also
asserts.

The fix uses `checked_rem`, so an empty list or record gets a rotation
of 0 and is returned unchanged. I added tests for `roll up`, `roll
down`, `roll left` and `roll right` on empty input, one for empty
records inside a list, and one for a table where only one row is empty.

I ran the `roll` tests locally and they pass (13). I haven't run the
full test suite, so I'm relying on CI for that.


## User-facing changes (Release notes)
Fixed a panic when using `roll up`, `roll down`, `roll left` or `roll
right` on an empty list or record.

## Additional notes
n/a

---------

Signed-off-by: manoj-1407 <manojkumar148700@gmail.com>
Signed-off-by: G. Manoj Kumar <manojkumar148700@gmail.com>
This commit is contained in:
Manoj authored and GitHub committed 2026-10-21 06:10:21 -05:00
1 parent dd0a08cd4b
commit 64915f00e4
2 files changed
+49 -2

No files matched your search

@@ -25,7 +25,8 @@ fn vertical_rotate_value(
let span = value.span();
match value {
Value::List { mut vals, .. } => {
let rotations = by.map(|n| n % vals.len()).unwrap_or(1);
// `checked_rem` is `None` for an empty list, so there is nothing to rotate.
let rotations = by.unwrap_or(1).checked_rem(vals.len()).unwrap_or(0);
let values = vals.to_mut().as_mut_slice();
match direction {
@@ -56,7 +57,8 @@ fn horizontal_rotate_value(
let span = value.span();
match value {
Value::Record { val: record, .. } => {
let rotations = by.map(|n| n % record.len()).unwrap_or(1);
// `checked_rem` is `None` for an empty record, so there is nothing to rotate.
let rotations = by.unwrap_or(1).checked_rem(record.len()).unwrap_or(0);
let (mut cols, mut vals): (Vec<_>, Vec<_>) = record.into_owned().into_iter().unzip();
if !cells_only {
+45
View File
@@ -26,6 +26,23 @@ mod rows {
test().run_with_data(code, table()).expect_value_eq("HERE")
}
#[test]
fn roll_down_empty_list_is_unchanged() -> Result {
test().run("[] | roll down").expect_value_eq(test_list![])
}
#[test]
fn roll_up_empty_list_is_unchanged() -> Result {
test().run("[] | roll up").expect_value_eq(test_list![])
}
#[test]
fn roll_up_empty_list_with_by_is_unchanged() -> Result {
test()
.run("[] | roll up --by 3")
.expect_value_eq(test_list![])
}
}
mod columns {
@@ -72,6 +89,34 @@ mod columns {
.expect_value_eq(expected_value.1)
}
#[test]
fn roll_left_empty_record_is_unchanged() -> Result {
test()
.run("{} | roll left")
.expect_value_eq(test_record! {})
}
#[test]
fn roll_right_empty_record_is_unchanged() -> Result {
test()
.run("{} | roll right")
.expect_value_eq(test_record! {})
}
#[test]
fn roll_left_empty_record_in_a_list_is_unchanged() -> Result {
test()
.run("[{} {}] | roll left --by 2 | to nuon")
.expect_value_eq("[{}, {}]")
}
#[test]
fn roll_left_skips_empty_row_in_a_table() -> Result {
test()
.run("[{a: 1, b: 2} {}] | roll left | to nuon")
.expect_value_eq("[{b: 2, a: 1}, {}]")
}
#[test]
fn four_in_bitstring_left_shifted_with_three_bits_should_be_32_in_decimal() -> Result {
let four_bitstring = "00000100";