-
Notifications
You must be signed in to change notification settings - Fork 1
feat(builtins): add array.flatten #86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,15 +4,17 @@ | |
|
|
||
| use crate::ast::{Expr, Ref}; | ||
| use crate::builtins; | ||
| use crate::builtins::utils::{ensure_args_count, ensure_array, ensure_numeric}; | ||
| use crate::builtins::utils::{enforce_limit, ensure_args_count, ensure_array, ensure_numeric}; | ||
| use crate::lexer::Span; | ||
| use crate::Rc; | ||
| use crate::Value; | ||
| use crate::Vec; | ||
|
|
||
| use anyhow::Result; | ||
|
|
||
| pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn>) { | ||
| m.insert("array.concat", (concat, 2)); | ||
| m.insert("array.flatten", (flatten, 1)); | ||
| m.insert("array.reverse", (reverse, 1)); | ||
| m.insert("array.slice", (slice, 3)); | ||
| } | ||
|
|
@@ -68,3 +70,28 @@ fn slice(span: &Span, params: &[Ref<Expr>], args: &[Value], _strict: bool) -> Re | |
| let slice = array.as_slice().get(start..stop).unwrap_or_default(); | ||
| Ok(Value::from(slice.to_vec())) | ||
| } | ||
|
|
||
| fn flatten(span: &Span, params: &[Ref<Expr>], args: &[Value], _strict: bool) -> Result<Value> { | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Fixed: Added comprehensive RVM YAML test suite (7 cases) at |
||
| let name = "array.flatten"; | ||
| ensure_args_count(span, name, params, args, 1)?; | ||
| let array = ensure_array(name, ¶ms[0], args[0].clone())?; | ||
| let mut flattened = Vec::new(); | ||
|
|
||
| for value in array.iter() { | ||
| // `pattern_type_mismatch` requires the explicit `&`/`ref` here, which in | ||
| // turn triggers `needless_borrowed_reference`; the two lints conflict for | ||
| // this shape, so silence the latter (see template_functions_collection.rs). | ||
| #[allow(clippy::needless_borrowed_reference)] | ||
| if let &Value::Array(ref nested) = value { | ||
| for nested_value in nested.iter() { | ||
| flattened.push(nested_value.clone()); | ||
| enforce_limit()?; | ||
| } | ||
| } else { | ||
| flattened.push(value.clone()); | ||
| enforce_limit()?; | ||
| } | ||
| } | ||
|
|
||
| Ok(Value::from_array(flattened)) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
|
|
||
| cases: | ||
| - note: shallow | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([[1, 2], [3, 4]]) | ||
| query: data.test.x | ||
| want_result: [1, 2, 3, 4] | ||
|
|
||
| - note: mixed-depth | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([[1, [2, 3]], 4, [5]]) | ||
| query: data.test.x | ||
| want_result: [1, [2, 3], 4, 5] | ||
|
|
||
| - note: empty | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([]) | ||
| query: data.test.x | ||
| want_result: [] | ||
|
|
||
| - note: all-scalars | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([1, 2, 3]) | ||
| query: data.test.x | ||
| want_result: [1, 2, 3] | ||
|
|
||
| - note: undefined-element-propagates | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| r { false } | ||
| x = array.flatten([r]) | ||
| query: data.test.x | ||
| no_result: true | ||
|
|
||
| - note: wrong-type | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten("not-an-array") | ||
| query: data.test.x | ||
| error: "`array.flatten` expects array argument." | ||
|
|
||
| - note: too-many-args | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([1], [2]) | ||
| query: data.test.x | ||
| error: "`array.flatten` expects 1 argument" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| # Copyright (c) Microsoft Corporation. | ||
| # Licensed under the MIT License. | ||
|
|
||
| # RVM coverage for `array.flatten`, mirroring | ||
| # tests/interpreter/cases/builtins/arrays/flatten.yaml so the interpreter and | ||
| # RVM execution paths stay in parity for this builtin. | ||
|
|
||
| cases: | ||
| - note: shallow | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([[1, 2], [3, 4]]) | ||
| query: data.test.x | ||
| want_result: [1, 2, 3, 4] | ||
|
|
||
| - note: mixed-depth | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([[1, [2, 3]], 4, [5]]) | ||
| query: data.test.x | ||
| want_result: [1, [2, 3], 4, 5] | ||
|
|
||
| - note: empty | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([]) | ||
| query: data.test.x | ||
| want_result: [] | ||
|
|
||
| - note: all-scalars | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([1, 2, 3]) | ||
| query: data.test.x | ||
| want_result: [1, 2, 3] | ||
|
|
||
| - note: undefined-element-propagates | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| r if { false } | ||
| x = array.flatten([r]) | ||
| query: data.test.x | ||
| want_result: "#undefined" | ||
|
|
||
| - note: wrong-type | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten("not-an-array") | ||
| query: data.test.x | ||
| want_result: "#undefined" | ||
| # RVM defaults `strict_builtin_errors` to false (the interpreter defaults | ||
| # to true), so the type-check `bail!` raised by `ensure_array` is | ||
| # swallowed to Undefined here rather than surfacing as an error. This is | ||
| # a pre-existing, general divergence in default settings that applies to | ||
| # every builtin's error path, not something specific to `array.flatten`. | ||
| allow_interpreter_incorrect_behavior: true | ||
|
|
||
| - note: too-many-args | ||
| data: {} | ||
| modules: | ||
| - | | ||
| package test | ||
| x = array.flatten([1], [2], [3]) | ||
| query: data.test.x | ||
| want_result: "#undefined" | ||
| # Same non-strict-builtin-errors divergence as `wrong-type` above: the | ||
| # `ensure_args_count` arity-check `bail!` is swallowed to Undefined under | ||
| # RVM's default settings instead of surfacing as an error. (Two args are | ||
| # deliberately avoided here because `array.flatten(a, b)` with exactly | ||
| # one extra argument is valid Rego out-param call syntax equivalent to | ||
| # `b := array.flatten(a)`; three args unambiguously exceeds that.) | ||
| allow_interpreter_incorrect_behavior: true | ||
|
Comment on lines
+77
to
+84
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Fixed: Corrected RVM test harness — the arity-error cases now use |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Fixed: PR description updated via GitHub API to match the actual correct implementation — array.flatten is non-recursive, single-arg per official OPA spec. Implementation and tests are accurate.