Skip to content

Commit 644fdcd

Browse files
anakrishCopilot
andcommitted
refactor(interpreter,value): insert_if_absent; consolidate Serialize
- Two interpreter rule-output sites that read get_or_insert_with(p, || value.clone()) / get_or_insert_with(p, || output.clone()) now use the new Object::insert_if_absent helper — same semantics, no closure indirection. - Value::Serialize for Value::Object now delegates to Object::serialize so there is a single canonical serialization path. The duplicated copy in value.rs handled non-string-key stringification, but Object's own impl already does the same. - as_set / as_set_mut / as_object / as_object_mut doctest snippets updated to use Set / Object instead of BTreeSet / BTreeMap. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 339600f commit 644fdcd

4 files changed

Lines changed: 14 additions & 29 deletions

File tree

src/builtins/encoding.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ fn urlquery_encode_object(
308308

309309
{
310310
let mut pairs = url.query_pairs_mut();
311-
for (key, value) in obj.iter() {
311+
for (key, value) in obj.iter_sorted() {
312312
let key = ensure_string(name, &params[0], key)?;
313313
match value {
314314
Value::String(v) => {

src/builtins/strings.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,15 +199,15 @@ fn to_string(v: &Value, unescape: bool) -> String {
199199
}
200200
Value::Set(s) => {
201201
"{".to_owned()
202-
+ &s.iter()
202+
+ &s.iter_sorted()
203203
.map(|e| to_string(e, true))
204204
.collect::<Vec<String>>()
205205
.join(", ")
206206
+ "}"
207207
}
208208
Value::Object(o) => {
209209
"{".to_owned()
210-
+ &o.iter()
210+
+ &o.iter_sorted()
211211
.map(|(k, v)| to_string(k, true) + ": " + &to_string(v, true))
212212
.collect::<Vec<String>>()
213213
.join(", ")
@@ -568,7 +568,7 @@ fn replace_n(span: &Span, params: &[Ref<Expr>], args: &[Value], _strict: bool) -
568568
let mut s = ensure_string(name, &params[1], &args[1])?;
569569

570570
let span = params[0].span();
571-
for item in obj.as_ref().iter() {
571+
for item in obj.as_ref().iter_sorted() {
572572
match item {
573573
(Value::String(k), Value::String(v)) => {
574574
s = s.replace(k.as_ref(), v.as_ref()).into();

src/interpreter.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1693,7 +1693,7 @@ impl Interpreter {
16931693
// TODO: clean this assumption between Undefined vs Object.
16941694
obj.get_or_insert_with(p, Value::new_object);
16951695
} else {
1696-
let existing = obj.get_or_insert_with(p, || value.clone());
1696+
let existing = obj.insert_if_absent(p, value.clone());
16971697
if *existing != value {
16981698
bail!(span.error("complete rules should not produce multiple outputs"))
16991699
}
@@ -1823,7 +1823,7 @@ impl Interpreter {
18231823
// Non-set rule.
18241824
let key = Value::from_array(comps);
18251825
let obj_mut = ctx_mut.rule_value.as_object_mut()?;
1826-
let existing = obj_mut.get_or_insert_with(key, || output.clone());
1826+
let existing = obj_mut.insert_if_absent(key, output.clone());
18271827
if *existing != output {
18281828
bail!(rule_ref
18291829
.span()

src/value.rs

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use core::str::FromStr;
2424

2525
use anyhow::{anyhow, bail, Result};
2626
use serde::de::{self, Deserializer, Error as DeError, MapAccess, SeqAccess, Visitor};
27-
use serde::ser::{SerializeMap, Serializer};
27+
use serde::ser::Serializer;
2828
use serde::{Deserialize, Serialize};
2929

3030
use crate::*;
@@ -87,26 +87,15 @@ impl Serialize for Value {
8787
where
8888
S: Serializer,
8989
{
90-
use serde::ser::Error;
9190
match self {
9291
Value::Null => serializer.serialize_unit(),
9392
Value::Bool(b) => serializer.serialize_bool(*b),
9493
Value::String(s) => serializer.serialize_str(s.as_ref()),
9594
Value::Number(n) => n.serialize(serializer),
9695
Value::Array(a) => a.serialize(serializer),
97-
Value::Object(fields) => {
98-
let mut map = serializer.serialize_map(Some(fields.len()))?;
99-
for (k, v) in fields.iter_sorted() {
100-
match k {
101-
Value::String(_) => map.serialize_entry(k, v)?,
102-
_ => {
103-
let key_str = serde_json::to_string(k).map_err(Error::custom)?;
104-
map.serialize_entry(&key_str, v)?
105-
}
106-
}
107-
}
108-
map.end()
109-
}
96+
// Delegate to the Object/Set serializers — single canonical path,
97+
// handles non-string-key stringification internally.
98+
Value::Object(fields) => fields.serialize(serializer),
11099

111100
// display set as an array
112101
Value::Set(s) => s.serialize(serializer),
@@ -1241,13 +1230,12 @@ impl Value {
12411230
/// Cast value to [`&Set`] if [`Value::Set`].
12421231
/// ```
12431232
/// # use regorus::*;
1244-
/// # use std::collections::BTreeSet;
12451233
/// # fn main() -> anyhow::Result<()> {
12461234
/// let v = Value::from(
12471235
/// [Value::from("Hello")]
12481236
/// .iter()
12491237
/// .cloned()
1250-
/// .collect::<BTreeSet<Value>>(),
1238+
/// .collect::<Set>(),
12511239
/// );
12521240
/// assert_eq!(v.as_set()?.first(), Some(&Value::from("Hello")));
12531241
/// # Ok(())
@@ -1262,13 +1250,12 @@ impl Value {
12621250
/// Cast value to [`&mut Set`] if [`Value::Set`].
12631251
/// ```
12641252
/// # use regorus::*;
1265-
/// # use std::collections::BTreeSet;
12661253
/// # fn main() -> anyhow::Result<()> {
12671254
/// let mut v = Value::from(
12681255
/// [Value::from("Hello")]
12691256
/// .iter()
12701257
/// .cloned()
1271-
/// .collect::<BTreeSet<Value>>(),
1258+
/// .collect::<Set>(),
12721259
/// );
12731260
/// v.as_set_mut()?.insert(Value::from("World"));
12741261
/// # Ok(())
@@ -1283,13 +1270,12 @@ impl Value {
12831270
/// Cast value to [`&Object`] if [`Value::Object`].
12841271
/// ```
12851272
/// # use regorus::*;
1286-
/// # use std::collections::BTreeMap;
12871273
/// # fn main() -> anyhow::Result<()> {
12881274
/// let v = Value::from(
12891275
/// [(Value::from("Hello"), Value::from("World"))]
12901276
/// .iter()
12911277
/// .cloned()
1292-
/// .collect::<BTreeMap<Value, Value>>(),
1278+
/// .collect::<Object>(),
12931279
/// );
12941280
/// assert_eq!(
12951281
/// v.as_object()?.iter().next(),
@@ -1307,13 +1293,12 @@ impl Value {
13071293
/// Cast value to [`&mut Object`] if [`Value::Object`].
13081294
/// ```
13091295
/// # use regorus::*;
1310-
/// # use std::collections::BTreeMap;
13111296
/// # fn main() -> anyhow::Result<()> {
13121297
/// let mut v = Value::from(
13131298
/// [(Value::from("Hello"), Value::from("World"))]
13141299
/// .iter()
13151300
/// .cloned()
1316-
/// .collect::<BTreeMap<Value, Value>>(),
1301+
/// .collect::<Object>(),
13171302
/// );
13181303
/// v.as_object_mut()?.insert(Value::from("Good"), Value::from("Bye"));
13191304
/// # Ok(())

0 commit comments

Comments
 (0)