Skip to content

Commit 3979082

Browse files
anakrishCopilot
andcommitted
refactor(builtins): migrate to Object/Set API
Migrates ~85 sites across src/builtins/ to the Object/Set API: - objects.rs uses Object::get_or_insert_with for the merge path. - sets.rs uses Set algebra methods (union/intersection/difference) returning fresh Set; replaces the previous Rc<BTreeSet> Deref + local BTreeSet pattern. - graph.rs uses explicit iter_sorted().rev() where DoubleEndedIterator is required. - utils.rs ensure_object / ensure_set return Rc<Object> / Rc<Set>. - azure_policy template functions migrate to Object construction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 10261b5 commit 3979082

6 files changed

Lines changed: 30 additions & 38 deletions

File tree

src/builtins/azure_policy/template_functions_collection.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
88
use crate::ast::{Expr, Ref};
99
use crate::builtins;
10+
use crate::collections::Object;
1011
use crate::lexer::Span;
1112
use crate::value::Value;
1213
use crate::Rc;
1314

14-
use alloc::collections::BTreeMap;
1515
use alloc::vec::Vec;
1616
use anyhow::Result;
1717

@@ -72,7 +72,7 @@ fn fn_intersection(
7272
// Intersection of objects: keep key-value pairs from the first
7373
// object only when the key exists in every other object AND
7474
// the value is equal across all of them.
75-
let mut result: BTreeMap<Value, Value> = first.as_ref().clone();
75+
let mut result: Object = first.as_ref().clone();
7676
for arg in rest {
7777
let Value::Object(ref other) = *arg else {
7878
return Ok(Value::Undefined);
@@ -114,7 +114,7 @@ fn fn_union(_span: &Span, _params: &[Ref<Expr>], args: &[Value], _strict: bool)
114114
Value::Object(_) => {
115115
// Union of objects: recursive merge. Nested objects are merged
116116
// recursively; all other types (including arrays) use last-writer-wins.
117-
let mut result = BTreeMap::<Value, Value>::new();
117+
let mut result = Object::new();
118118
for arg in args {
119119
let Value::Object(ref obj) = *arg else {
120120
return Ok(Value::Undefined);
@@ -264,7 +264,7 @@ fn fn_create_object(
264264
);
265265
}
266266

267-
let mut map = BTreeMap::<Value, Value>::new();
267+
let mut map = Object::new();
268268

269269
for pair in args.chunks(2) {
270270
#[allow(clippy::pattern_type_mismatch)]
@@ -280,9 +280,9 @@ fn fn_create_object(
280280

281281
/// Recursively merge two objects. Nested objects are merged; everything
282282
/// else (including arrays) uses the value from `incoming`.
283-
fn merge_objects(base: &BTreeMap<Value, Value>, overlay: &BTreeMap<Value, Value>) -> Value {
283+
fn merge_objects(base: &Object, overlay: &Object) -> Value {
284284
let mut result = base.clone();
285-
for (k, v) in overlay {
285+
for (k, v) in overlay.iter() {
286286
#[allow(clippy::needless_borrowed_reference)]
287287
let merged = match (result.get(k), v) {
288288
(Some(&Value::Object(ref prev)), &Value::Object(ref next)) => merge_objects(prev, next),

src/builtins/azure_policy/template_functions_misc.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@
77
88
use crate::ast::{Expr, Ref};
99
use crate::builtins;
10+
use crate::collections::Object;
1011
use crate::lexer::Span;
1112
use crate::value::Value;
1213
use crate::Rc;
1314

14-
use alloc::collections::BTreeMap;
1515
use alloc::string::{String, ToString as _};
1616
use alloc::vec::Vec;
1717
use anyhow::Result;
@@ -85,7 +85,7 @@ fn fn_items(_span: &Span, _params: &[Ref<Expr>], args: &[Value], _strict: bool)
8585
};
8686
let mut result = Vec::with_capacity(obj.len());
8787
for (k, v) in obj.as_ref() {
88-
let mut entry = BTreeMap::<Value, Value>::new();
88+
let mut entry = Object::new();
8989
entry.insert(Value::from("key"), k.clone());
9090
entry.insert(Value::from("value"), v.clone());
9191
result.push(Value::Object(Rc::new(entry)));

src/builtins/graph.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::lexer::Span;
1010
use crate::value::Value;
1111
use crate::*;
1212

13-
use alloc::collections::{BTreeMap, BTreeSet};
13+
use alloc::collections::BTreeSet;
1414

1515
use anyhow::{bail, Result};
1616

@@ -80,7 +80,7 @@ fn reachable(span: &Span, params: &[Ref<Expr>], args: &[Value], strict: bool) ->
8080
}
8181

8282
fn visit(
83-
graph: &BTreeMap<Value, Value>,
83+
graph: &Object,
8484
visited: &mut BTreeSet<Value>,
8585
node: &Value,
8686
path: &mut Vec<Value>,
@@ -127,7 +127,7 @@ fn visit(
127127
arr.len()
128128
}
129129
Some(Value::Set(set)) => {
130-
for n in set.iter().rev() {
130+
for n in set.iter_sorted().rev() {
131131
visit(graph, visited, n, path, paths)?;
132132
}
133133
set.len()
@@ -173,7 +173,7 @@ fn reachable_paths(
173173
}
174174
}
175175
Value::Set(set) => {
176-
for node in set.iter() {
176+
for node in set.iter_sorted() {
177177
visit(&graph, &mut visited, node, &mut path, &mut paths)?;
178178
}
179179
}

src/builtins/objects.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ fn merge_filters(
205205
let vref = match f {
206206
Value::Object(obj) => {
207207
let obj = Rc::make_mut(obj);
208-
let entry = obj.entry(p.clone()).or_insert_with(Value::new_object);
208+
let entry = obj.get_or_insert_with(p.clone(), Value::new_object);
209209
// Guard filter map growth when creating nested objects.
210210
enforce_limit()?;
211211
entry

src/builtins/sets.rs

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,9 @@ use crate::builtins;
88
use crate::builtins::utils::{ensure_args_count, ensure_set};
99
use crate::lexer::Span;
1010
use crate::value::Value;
11+
use crate::Set;
1112
use crate::*;
1213

13-
use alloc::collections::BTreeSet;
14-
1514
use anyhow::{bail, Result};
1615

1716
pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn>) {
@@ -24,19 +23,19 @@ pub fn register(m: &mut builtins::BuiltinsMap<&'static str, builtins::BuiltinFcn
2423
pub fn intersection(expr1: &Expr, expr2: &Expr, v1: Value, v2: Value) -> Result<Value> {
2524
let s1 = ensure_set("intersection", expr1, v1)?;
2625
let s2 = ensure_set("intersection", expr2, v2)?;
27-
Ok(Value::from_set(s1.intersection(&s2).cloned().collect()))
26+
Ok(s1.intersection(&s2).into_value())
2827
}
2928

3029
pub fn union(expr1: &Expr, expr2: &Expr, v1: Value, v2: Value) -> Result<Value> {
3130
let s1 = ensure_set("union", expr1, v1)?;
3231
let s2 = ensure_set("union", expr2, v2)?;
33-
Ok(Value::from_set(s1.union(&s2).cloned().collect()))
32+
Ok(s1.union(&s2).into_value())
3433
}
3534

3635
pub fn difference(expr1: &Expr, expr2: &Expr, v1: Value, v2: Value) -> Result<Value> {
3736
let s1 = ensure_set("difference", expr1, v1)?;
3837
let s2 = ensure_set("difference", expr2, v2)?;
39-
Ok(Value::from_set(s1.difference(&s2).cloned().collect()))
38+
Ok(s1.difference(&s2).into_value())
4039
}
4140

4241
fn binary_set_union(
@@ -49,7 +48,7 @@ fn binary_set_union(
4948
ensure_args_count(span, name, params, args, 2)?;
5049
let left = ensure_set(name, &params[0], args[0].clone())?;
5150
let right = ensure_set(name, &params[1], args[1].clone())?;
52-
Ok(Value::from_set(left.union(&right).cloned().collect()))
51+
Ok(left.union(&right).into_value())
5352
}
5453

5554
fn binary_set_intersection(
@@ -62,9 +61,7 @@ fn binary_set_intersection(
6261
ensure_args_count(span, name, params, args, 2)?;
6362
let left = ensure_set(name, &params[0], args[0].clone())?;
6463
let right = ensure_set(name, &params[1], args[1].clone())?;
65-
Ok(Value::from_set(
66-
left.intersection(&right).cloned().collect(),
67-
))
64+
Ok(left.intersection(&right).into_value())
6865
}
6966

7067
fn intersection_of_set_of_sets(
@@ -77,8 +74,7 @@ fn intersection_of_set_of_sets(
7774
ensure_args_count(span, name, params, args, 1)?;
7875
let set = ensure_set(name, &params[0], args[0].clone())?;
7976

80-
let mut res = BTreeSet::new();
81-
let mut first = true;
77+
let mut res: Option<Set> = None;
8278

8379
for s in set.iter() {
8480
let s = match s {
@@ -88,15 +84,13 @@ fn intersection_of_set_of_sets(
8884
),
8985
};
9086

91-
if first {
92-
res.clone_from(s);
93-
first = false;
94-
} else {
95-
res = res.intersection(s).cloned().collect();
96-
}
87+
res = Some(match res {
88+
None => (**s).clone(),
89+
Some(prev) => prev.intersection(s),
90+
});
9791
}
9892

99-
Ok(Value::from_set(res))
93+
Ok(res.unwrap_or_default().into_value())
10094
}
10195

10296
fn union_of_set_of_sets(
@@ -109,7 +103,7 @@ fn union_of_set_of_sets(
109103
ensure_args_count(span, name, params, args, 1)?;
110104
let set = ensure_set(name, &params[0], args[0].clone())?;
111105

112-
let mut res = BTreeSet::new();
106+
let mut res = Set::new();
113107

114108
for s in set.iter() {
115109
let s = match s {
@@ -119,8 +113,8 @@ fn union_of_set_of_sets(
119113
),
120114
};
121115

122-
res = res.union(s).cloned().collect();
116+
res = res.union(s);
123117
}
124118

125-
Ok(Value::from_set(res))
119+
Ok(res.into_value())
126120
}

src/builtins/utils.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ use crate::Rc;
99
use crate::Value;
1010
use crate::*;
1111

12-
use alloc::collections::{BTreeMap, BTreeSet};
13-
1412
use anyhow::{bail, Result};
1513

1614
#[inline]
@@ -158,7 +156,7 @@ pub fn ensure_array(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<Vec<Value>>> {
158156
})
159157
}
160158

161-
pub fn ensure_set(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<BTreeSet<Value>>> {
159+
pub fn ensure_set(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<Set>> {
162160
Ok(match v {
163161
Value::Set(s) => s,
164162
_ => {
@@ -168,7 +166,7 @@ pub fn ensure_set(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<BTreeSet<Value>>
168166
})
169167
}
170168

171-
pub fn ensure_object(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<BTreeMap<Value, Value>>> {
169+
pub fn ensure_object(fcn: &str, arg: &Expr, v: Value) -> Result<Rc<Object>> {
172170
Ok(match v {
173171
Value::Object(o) => o,
174172
_ => {

0 commit comments

Comments
 (0)