From 851cfc72785f96d107cef81dfc209d7f752c3d63 Mon Sep 17 00:00:00 2001
From: Starnick4444 <nemes.bence1@gmail.com>
Date: Fri, 11 Apr 2025 11:22:08 +0200
Subject: [PATCH] refactor(eval): add keys_sorted() to NixAttrs

builtins.toXML relies on attr keys being sorted, which is an
implementation detail.
Cppnix creates sorted xml, and tests expect it too, so changed that
iter call site.
builtins.attrNames and attrValues should also returns sorted lists, so
changed that too.

Change-Id: Ic706afc6855d5d28932ade7482449c965b0ae94f
Reviewed-on: https://cl.snix.dev/c/snix/+/30307
Reviewed-by: Florian Klink <flokli@flokli.de>
Tested-by: besadii
---
 snix/eval/src/builtins/mod.rs    | 8 ++++----
 snix/eval/src/builtins/to_xml.rs | 2 +-
 snix/eval/src/value/attrs.rs     | 6 ++++++
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/snix/eval/src/builtins/mod.rs b/snix/eval/src/builtins/mod.rs
index 13d1d3df7..a99a96571 100644
--- a/snix/eval/src/builtins/mod.rs
+++ b/snix/eval/src/builtins/mod.rs
@@ -150,7 +150,7 @@ mod pure_builtins {
         let xs = set.to_attrs()?;
         let mut output = Vec::with_capacity(xs.len());
 
-        for (key, _val) in xs.iter() {
+        for (key, _val) in xs.iter_sorted() {
             output.push(Value::from(key.clone()));
         }
 
@@ -162,7 +162,7 @@ mod pure_builtins {
         let xs = set.to_attrs()?;
         let mut output = Vec::with_capacity(xs.len());
 
-        for (_key, val) in xs.iter() {
+        for (_key, val) in xs.iter_sorted() {
             output.push(val.clone());
         }
 
@@ -789,13 +789,13 @@ mod pure_builtins {
         if left_set.is_empty() {
             return Ok(Value::attrs(NixAttrs::empty()));
         }
-        let mut left_keys = left_set.keys();
+        let mut left_keys = left_set.keys_sorted();
 
         let right_set = y.to_attrs()?;
         if right_set.is_empty() {
             return Ok(Value::attrs(NixAttrs::empty()));
         }
-        let mut right_keys = right_set.keys();
+        let mut right_keys = right_set.keys_sorted();
 
         let mut out: BTreeMap<NixString, Value> = BTreeMap::new();
 
diff --git a/snix/eval/src/builtins/to_xml.rs b/snix/eval/src/builtins/to_xml.rs
index c91841926..46a711a4c 100644
--- a/snix/eval/src/builtins/to_xml.rs
+++ b/snix/eval/src/builtins/to_xml.rs
@@ -67,7 +67,7 @@ fn value_variant_to_xml<W: Write>(w: &mut XmlEmitter<W>, value: &Value) -> Resul
         Value::Attrs(attrs) => {
             w.write_open_tag("attrs", &[])?;
 
-            for elem in attrs.iter() {
+            for elem in attrs.iter_sorted() {
                 w.write_open_tag("attr", &[("name", &elem.0.to_str_lossy())])?;
                 value_variant_to_xml(w, elem.1)?;
                 w.write_closing_tag("attr")?;
diff --git a/snix/eval/src/value/attrs.rs b/snix/eval/src/value/attrs.rs
index 363cf3b1d..8df69abd4 100644
--- a/snix/eval/src/value/attrs.rs
+++ b/snix/eval/src/value/attrs.rs
@@ -318,6 +318,12 @@ impl NixAttrs {
         })
     }
 
+    /// Same as [Self::keys], but marks call sites which rely on the
+    /// iteration being lexicographic.
+    pub fn keys_sorted(&self) -> Keys {
+        self.keys()
+    }
+
     /// Implement construction logic of an attribute set, to encapsulate
     /// logic about attribute set optimisations inside of this module.
     pub fn construct(