feat(builtins): add array.flatten, strings.split_n, strings.count, numbers.range_step improvements, time duration d/w/y, units.parse exponent guard - #78
Open
anakrish wants to merge 2 commits into
Conversation
microsoft#794) Bumps the per-dependency group in /bindings/ruby with 1 update: [rb_sys](https://github.com/oxidize-rb/rb-sys). Updates `rb_sys` from 0.9.128 to 0.9.130 - [Release notes](https://github.com/oxidize-rb/rb-sys/releases) - [Commits](oxidize-rb/rb-sys@v0.9.128...v0.9.130) --- updated-dependencies: - dependency-name: rb_sys dependency-version: 0.9.130 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: per-dependency ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…p improvements, time duration d/w/y, units exponent guard New builtins and fixes extracted from OPA v1.2.0 → v1.19.1 upgrade: - array.flatten: one-level flatten (nested arrays remain nested) - strings.split_n: split string into at most n pieces (positive n) or last |n| pieces (negative n); uses str::splitn for correct remainder semantics - strings.count: fix param index bug (params[0] was used for both args) and handle empty substring per OPA semantics (returns char_count + 1) - numbers.range_step: return Undefined for non-integer/non-positive step in non-strict mode; use subtracted step for descending ranges - time.parse_duration_ns: add 'd' (day), 'w' (week), 'y' (365-day year) units - units.parse / units.parse_bytes: reject exponents with >6 digits to prevent numeric overflow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds and aligns several builtins with OPA v1.19.1 semantics, including new builtins and behavior fixes (strings, arrays, numbers, time, and units parsing), plus documentation and new interpreter YAML test cases.
Changes:
- Added new builtins
array.flattenandstrings.split_n, and documented them. - Fixed builtin registration/parameter handling for
strings.splitandstrings.count, and adjustednumbers.range_stepstrict/non-strict behavior. - Extended
time.parse_duration_nsunit support (d/w/y) and added exponent-size guardrails forunits.parse(_bytes).
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/interpreter/cases/builtins/units/exponent.yaml | Adds regression tests for exponent-too-large errors and a normal exponent pass case. |
| tests/interpreter/cases/builtins/time/duration_units.yaml | Adds coverage for new d/w/y duration units and mixed-unit parsing. |
| tests/interpreter/cases/builtins/strings/strings_count.yaml | Adds semantic tests for strings.count, including empty-substring behavior. |
| tests/interpreter/cases/builtins/strings/split_n.yaml | Adds tests for strings.split_n limits, negative tail semantics, and strictness. |
| tests/interpreter/cases/builtins/numbers/range_step.yaml | Adds tests for numbers.range_step including descending and undefined cases. |
| tests/interpreter/cases/builtins/arrays/flatten.yaml | Adds tests for new array.flatten (shallow) and error/undefined behavior. |
| src/builtins/units.rs | Adds exponent-length guard to avoid pathological parses. |
| src/builtins/time/compat.rs | Adds day/week/year units to duration parsing. |
| src/builtins/strings.rs | Fixes split name, fixes strings_count param usage + semantics, and adds split_n. |
| src/builtins/numbers.rs | Adjusts range_step to return undefined in non-strict mode for invalid steps and refactors stepping logic. |
| src/builtins/arrays.rs | Registers and implements array.flatten with limit enforcement. |
| docs/builtins.md | Documents new/updated builtins in the builtins listing. |
| bindings/ruby/Gemfile.lock | Bumps rb_sys dependency version. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| 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); |
Comment on lines
+228
to
+232
| let mut parts: Vec<Value> = chars[..limit.saturating_sub(1)] | ||
| .iter() | ||
| .map(|c| Value::from(c.to_string())) | ||
| .collect(); | ||
| let rest: String = chars[limit.saturating_sub(1)..].iter().collect(); |
| "d" => DAY, | ||
| "w" => WEEK, | ||
| "y" => YEAR, | ||
| unkonwn => return Err(ParseDurationError::UnknownUnit(unkonwn.to_string())), |
Comment on lines
+150
to
154
| let step = if v1 <= v2 { | ||
| incr | ||
| } else { | ||
| Number::from(0u64).sub(&incr)? | ||
| }; |
Comment on lines
+4
to
+10
| cases: | ||
| - note: positive-limit | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = strings.split_n("a,b,c,d", ",", 2) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds several standalone builtin improvements targeting OPA v1.19.1 compatibility.
Changes
array.flatten— new builtin; flattens one level of nestingstrings.split_n— new builtin using Ruststr::splitnfor correct OPA semantics (positive n = at most n pieces)strings.count— fix parameter name registration bugstrings.split— fix name registration bugnumbers.range_step— returnundefined(not error) for no-progress or descending rangestime— addd(days),w(weeks),y(years) duration unit supportunits.parse/units.parse_bytes— guard against exponent-too-large valuesdocs/builtins.md— document new builtinsTests
Six new YAML test files under
tests/interpreter/cases/builtins/, all passing.Part of OPA v1.2.0 → v1.19.1 upgrade.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com