From d921c9fa552fe302ae6e264e3c4a2fffb5e65a5d Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Fri, 28 Aug 2026 11:17:08 -0500 Subject: [PATCH 1/2] feat(builtins): add strings.split_n; fix split error-message name strings.split_n splits a string into at most n pieces (positive n) or returns the last |n| pieces from a full split (negative n), using str::splitn for correct remainder semantics. Also fixes the long-standing typo in split() where the function registered its name as "replace" instead of "split", which produced misleading error messages (e.g. when split_n delegates to split for the negative-n tail path). - Register strings.split_n with arity 3 - Positive n: use str::splitn; handle empty-delimiter char-by-char with remainder to match OPA semantics - Negative n: full split, then take the last |n| pieces - Non-integer n in non-strict mode returns undefined (strict: error) - n == 0 returns empty array; n overflowing i64 positive treated as no limit (full split) - enforce_limit() on each piece to bound memory growth - Fix split() name string: "replace" -> "split" - Add YAML regression test covering all major code paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/builtins.md | 2 + src/builtins/strings.rs | 76 ++++++++++++++++++- .../cases/builtins/strings/split_n.yaml | 76 +++++++++++++++++++ 3 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 tests/interpreter/cases/builtins/strings/split_n.yaml diff --git a/docs/builtins.md b/docs/builtins.md index dddf124b9..ac06385d9 100644 --- a/docs/builtins.md +++ b/docs/builtins.md @@ -96,9 +96,11 @@ In future, each builtin will be associated with a feature (many builtins could b | [startswith](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-startswith) | _ | | [strings.any_prefix_match](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-stringsany_prefix_match) | _ | | [strings.any_suffix_match](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-stringsany_suffix_match) | _ | + | [strings.count](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-stringscount) | _ | | [strings.render_template](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-stringsrender_template) | _ | | [strings.replace_n](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-stringsreplace_n) | _ | | [strings.reverse](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-stringsreverse) | _ | + | [strings.split_n](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-stringssplit_n) | _ | | [substring](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-substring) | _ | | [trim](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-trim) | _ | | [trim_left](https://www.openpolicyagent.org/docs/latest/policy-reference/#builtin-strings-trim_left) | _ | diff --git a/src/builtins/strings.rs b/src/builtins/strings.rs index fd3a2ee8b..653d9a188 100644 --- a/src/builtins/strings.rs +++ b/src/builtins/strings.rs @@ -37,6 +37,7 @@ pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn m.insert("strings.count", (strings_count, 2)); m.insert("strings.replace_n", (replace_n, 2)); m.insert("strings.reverse", (reverse, 1)); + m.insert("strings.split_n", (split_n, 3)); m.insert("substring", (substring, 3)); m.insert("trim", (trim, 2)); m.insert("trim_left", (trim_left, 2)); @@ -149,7 +150,7 @@ fn replace(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> } fn split(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> Result { - let name = "replace"; + let name = "split"; ensure_args_count(span, name, params, args, 2)?; let s = ensure_string(name, ¶ms[0], &args[0])?; let delimiter = ensure_string(name, ¶ms[1], &args[1])?; @@ -180,6 +181,79 @@ fn split(span: &Span, params: &[Ref], args: &[Value], _strict: bool) -> Re Ok(Value::from(parts)) } +fn split_n(span: &Span, params: &[Ref], args: &[Value], strict: bool) -> Result { + let name = "strings.split_n"; + ensure_args_count(span, name, params, args, 3)?; + let s = ensure_string(name, ¶ms[0], &args[0])?; + let delimiter = ensure_string(name, ¶ms[1], &args[1])?; + let n = ensure_numeric(name, ¶ms[2], &args[2])?; + + if !n.is_integer() { + if strict { + bail!(params[2] + .span() + .error("strings.split_n expects an integer third argument")); + } + return Ok(Value::Undefined); + } + + let Some(n) = n.as_i64() else { + if n.is_positive() { + // n overflows i64 — treat as no limit (full split) + return split(span, params, args, strict); + } + return Ok(Value::from_array(Vec::new())); + }; + + if n == 0 { + return Ok(Value::from_array(Vec::new())); + } + + // For positive n: split into at most n pieces (last piece holds remainder). + // For negative n: return the last |n| pieces from a full split. + let result: Vec = if n > 0 { + let limit = usize::try_from(n).unwrap_or(usize::MAX); + if delimiter.is_empty() { + // Empty delimiter: yield individual chars; last piece holds any remainder. + let chars: Vec = s.chars().collect(); + if limit >= chars.len() { + chars + .iter() + .map(|c| { + enforce_limit()?; + Ok(Value::from(c.to_string())) + }) + .collect::>>()? + } else { + let mut parts: Vec = chars[..limit.saturating_sub(1)] + .iter() + .map(|c| Value::from(c.to_string())) + .collect(); + let rest: String = chars[limit.saturating_sub(1)..].iter().collect(); + parts.push(Value::from(rest)); + parts + } + } else { + s.splitn(limit, delimiter.as_ref()) + .map(|p| { + enforce_limit()?; + Ok(Value::String(p.into())) + }) + .collect::>>()? + } + } else { + let parts = match split(span, ¶ms[..2], &args[..2], strict)? { + Value::Array(parts) => parts.as_ref().clone(), + _ => return Ok(Value::Undefined), + }; + let count = usize::try_from(n.unsigned_abs()).unwrap_or(usize::MAX); + let start = parts.len().saturating_sub(count); + parts.into_iter().skip(start).collect() + }; + + Ok(Value::from_array(result)) +} + fn to_string(v: &Value, unescape: bool) -> String { match v { Value::Null => "null".to_owned(), diff --git a/tests/interpreter/cases/builtins/strings/split_n.yaml b/tests/interpreter/cases/builtins/strings/split_n.yaml new file mode 100644 index 000000000..0eaf28a10 --- /dev/null +++ b/tests/interpreter/cases/builtins/strings/split_n.yaml @@ -0,0 +1,76 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +cases: + - note: positive-limit + data: {} + modules: + - | + package test + x = strings.split_n("a,b,c,d", ",", 2) + query: data.test.x + want_result: ["a", "b,c,d"] + + - note: exact-limit + data: {} + modules: + - | + package test + x = strings.split_n("a,b,c", ",", 3) + query: data.test.x + want_result: ["a", "b", "c"] + + - note: limit-exceeds-parts + data: {} + modules: + - | + package test + x = strings.split_n("a,b", ",", 10) + query: data.test.x + want_result: ["a", "b"] + + - note: zero-limit + data: {} + modules: + - | + package test + x = strings.split_n("a,b,c", ",", 0) + query: data.test.x + want_result: [] + + - note: negative-limit-tail + data: {} + modules: + - | + package test + x = strings.split_n("a,b,c,d", ",", -2) + query: data.test.x + want_result: ["c", "d"] + + - note: empty-string + data: {} + modules: + - | + package test + x = strings.split_n("", ",", 5) + query: data.test.x + want_result: [""] + + - note: non-integer-n-undefined + strict: false + data: {} + modules: + - | + package test + x = strings.split_n("a,b", ",", 1.5) + query: data.test.x + no_result: true + + - note: wrong-arg-count + data: {} + modules: + - | + package test + x = strings.split_n("a,b", ",") + query: data.test.x + error: "`strings.split_n` expects 3 arguments" From b7e650b49ced4540c6b4309fe9453c140a5deeec Mon Sep 17 00:00:00 2001 From: Anand Krishnamoorthi Date: Fri, 28 Aug 2026 13:41:38 -0500 Subject: [PATCH 2/2] fix(strings): address split_n review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix fallback for n > i64::MAX: pass only first two args to split() instead of all three; split() expects exactly 2 operands and ensure_args_count was rejecting the extra one. - Add enforce_limit() in empty-delimiter branch to prevent unbounded memory growth: call it in the per-character map closure and before the final remainder push. - Add YAML tests: split_n("abcd", "", 3) → ["a","b","cd"] (remainder) and split_n("a,b,c", ",", 1) → ["a,b,c"] (n=1 returns whole string). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/builtins/strings.rs | 12 ++++++++---- .../cases/builtins/strings/split_n.yaml | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/builtins/strings.rs b/src/builtins/strings.rs index 653d9a188..eeaf65ba8 100644 --- a/src/builtins/strings.rs +++ b/src/builtins/strings.rs @@ -199,8 +199,8 @@ fn split_n(span: &Span, params: &[Ref], args: &[Value], strict: bool) -> R let Some(n) = n.as_i64() else { if n.is_positive() { - // n overflows i64 — treat as no limit (full split) - return split(span, params, args, strict); + // n overflows i64 — treat as no limit (full split); pass only the first 2 args + return split(span, ¶ms[..2], &args[..2], strict); } return Ok(Value::from_array(Vec::new())); }; @@ -227,9 +227,13 @@ fn split_n(span: &Span, params: &[Ref], args: &[Value], strict: bool) -> R } else { let mut parts: Vec = chars[..limit.saturating_sub(1)] .iter() - .map(|c| Value::from(c.to_string())) - .collect(); + .map(|c| { + enforce_limit()?; + Ok(Value::from(c.to_string())) + }) + .collect::>>()?; let rest: String = chars[limit.saturating_sub(1)..].iter().collect(); + enforce_limit()?; parts.push(Value::from(rest)); parts } diff --git a/tests/interpreter/cases/builtins/strings/split_n.yaml b/tests/interpreter/cases/builtins/strings/split_n.yaml index 0eaf28a10..38db3febf 100644 --- a/tests/interpreter/cases/builtins/strings/split_n.yaml +++ b/tests/interpreter/cases/builtins/strings/split_n.yaml @@ -74,3 +74,21 @@ cases: x = strings.split_n("a,b", ",") query: data.test.x error: "`strings.split_n` expects 3 arguments" + + - note: empty-delimiter-with-remainder + data: {} + modules: + - | + package test + x = strings.split_n("abcd", "", 3) + query: data.test.x + want_result: ["a", "b", "cd"] + + - note: limit-one-returns-whole-string + data: {} + modules: + - | + package test + x = strings.split_n("a,b,c", ",", 1) + query: data.test.x + want_result: ["a,b,c"]