From 7dfb43e50f30708e9e64004047acc9a1212efd0a Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Fri, 28 Aug 2026 11:17:40 -0500 Subject: [PATCH 1/2] fix(builtins): numbers.range_step descending ranges and undefined step Two semantic fixes for numbers.range_step: 1. Non-positive or non-integer step: previously returned an error even in non-strict mode; now returns undefined in non-strict mode (matches OPA behaviour) and only errors in strict mode. 2. Descending ranges: the previous implementation attempted to compute num_elements via i64 arithmetic and bail if it overflowed. This failed for large integers because as_i64() returns None for bignums. Replace with a direction-aware step: negate the caller-supplied positive step when v1 > v2 so the loop walks downward naturally, removing the overflow-prone element-count pre-computation. - Rework the step guard to check non-integer/non-positive first and short-circuit with undefined when not strict - Compute step = incr or -incr based on v1 <= v2 comparison - Use Vec::new() instead of with_capacity (element count no longer pre-computed, still bounded by enforce_limit) - Add YAML regression test: ascending, step-2, descending, single- element, non-integer-step-undefined, zero-step-undefined Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/builtins/numbers.rs | 22 +++---- .../cases/builtins/numbers/range_step.yaml | 59 +++++++++++++++++++ 2 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 tests/interpreter/cases/builtins/numbers/range_step.yaml diff --git a/src/builtins/numbers.rs b/src/builtins/numbers.rs index 2c55fcb7f..594ef3287 100644 --- a/src/builtins/numbers.rs +++ b/src/builtins/numbers.rs @@ -140,22 +140,24 @@ fn range_step(span: &Span, params: &[Ref], args: &[Value], strict: bool) - _ => (), } - if strict && (!incr.is_integer() || incr <= Number::from(0u64)) { - bail!(params[2].span().error("step must be a positive integer")) + if !incr.is_integer() || incr <= Number::from(0u64) { + if strict { + bail!(params[2].span().error("step must be a positive integer")) + } + return Ok(Value::Undefined); } - let (incr, num_elements) = match (v2.sub(&v1)?.as_i64(), incr.as_i64()) { - (Some(v), Some(incr)) if v >= 0 => (incr, v / incr + 1), - (Some(v), Some(incr)) => (-incr, -v / incr + 1), - _ => bail!(span.error("could not determine number of elements")), + let step = if v1 <= v2 { + incr + } else { + Number::from(0u64).sub(&incr)? }; - let mut values = Vec::with_capacity(num_elements as usize); - let incr = Number::from(incr); + let mut values = Vec::new(); let mut v = v1; - while (v <= v2 && incr.is_positive()) || (v >= v2 && !incr.is_positive()) { + while (v <= v2 && step.is_positive()) || (v >= v2 && !step.is_positive()) { values.push(Value::from(v.clone())); - v.add_assign(&incr)?; + v.add_assign(&step)?; // Guard vector growth as the stepped range accumulates. enforce_limit()?; } diff --git a/tests/interpreter/cases/builtins/numbers/range_step.yaml b/tests/interpreter/cases/builtins/numbers/range_step.yaml new file mode 100644 index 000000000..2642fbb08 --- /dev/null +++ b/tests/interpreter/cases/builtins/numbers/range_step.yaml @@ -0,0 +1,59 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +cases: + - note: ascending + data: {} + modules: + - | + package test + x = numbers.range_step(1, 5, 1) + query: data.test.x + want_result: [1, 2, 3, 4, 5] + + - note: step-2 + data: {} + modules: + - | + package test + x = numbers.range_step(0, 10, 2) + query: data.test.x + want_result: [0, 2, 4, 6, 8, 10] + + - note: descending + data: {} + modules: + - | + package test + x = numbers.range_step(5, 1, 1) + query: data.test.x + want_result: [5, 4, 3, 2, 1] + + - note: single-element + data: {} + modules: + - | + package test + x = numbers.range_step(3, 3, 1) + query: data.test.x + want_result: [3] + + - note: non-integer-step-undefined + strict: false + data: {} + modules: + - | + package test + x = numbers.range_step(1, 5, 0.5) + query: data.test.x + no_result: true + + - note: zero-step-undefined + strict: false + data: {} + modules: + - | + package test + x = numbers.range_step(1, 5, 0) + query: data.test.x + no_result: true From 06f7dc563d1f15323e08941ae32e5c15b05a174b Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Fri, 28 Aug 2026 13:43:05 -0500 Subject: [PATCH 2/2] docs: clarify numbers.range_step descending range behavior OPA v1.19.1 automatically negates the step when start > stop to produce a descending range. Our implementation matches this. Update builtins.md to document it rather than leaving the description blank. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/builtins.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/builtins.md b/docs/builtins.md index dddf124b9..324402597 100644 --- a/docs/builtins.md +++ b/docs/builtins.md @@ -31,7 +31,7 @@ In future, each builtin will be associated with a feature (many builtins could b | [x - y](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-numbers-minus) | _ | | [x * y](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-numbers-mul) | _ | | [numbers.range](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-numbers-numbersrange) | _ | - | [numbers.range_step](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-numbers-numbersrange_step) | _ | + | [numbers.range_step](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-numbers-numbersrange_step) | Supported. When `start > stop`, step is negated automatically to produce a descending range, matching OPA behavior. | | [x + y](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-numbers-plus) | _ | | [rand.intn](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-numbers-randintn) | _ | | [x % y](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-numbers-rem) | _ |