fix(core): make ${_param.…} substitution single-pass and order-independent

libraries/core/src/descriptor/expand.rs

@@ -650,12 +650,49 @@ fn substitute_params_in_node(node: &mut Node, params: &BTreeMap<String, String>)
     }
 }
 
+/// Replace every `${_param.<key>}` token with its parameter value in a single
+/// left-to-right pass.
+///
+/// A previous implementation looped over the params calling `String::replace`
+/// on the accumulating result, which had two problems: the outcome depended on
+/// `BTreeMap` key ordering, and a parameter *value* that itself contained a
+/// `${_param.…}` token (or a literal one a user wanted to keep) was expanded
+/// transitively. Scanning once and never re-examining substituted text makes
+/// substitution simultaneous and order-independent. Unknown keys are left
+/// verbatim, matching the old behavior of only replacing keys present in
+/// `params`.
 fn substitute_params_in_str(s: &str, params: &BTreeMap<String, String>) -> String {
-    let mut result = s.to_string();
-    for (key, value) in params {
-        let pattern = format!("${{_param.{key}}}");
-        result = result.replace(&pattern, value);
+    const PREFIX: &str = "${_param.";
+    let mut result = String::with_capacity(s.len());
+    let mut rest = s;
+    while let Some(start) = rest.find(PREFIX) {
+        result.push_str(&rest[..start]);
+        let after_prefix = &rest[start + PREFIX.len()..];
+        match after_prefix.find('}') {
+            Some(end) => {
+                let key = &after_prefix[..end];
+                match params.get(key) {
+                    Some(value) => result.push_str(value),
+                    // Unknown key: emit the token unchanged rather than dropping it.
+                    None => {
+                        result.push_str(PREFIX);
+                        result.push_str(key);
+                        result.push('}');
+                    }
+                }
+                // Continue *after* the closing brace so a substituted value is
+                // never re-scanned for further tokens.
+                rest = &after_prefix[end + 1..];
+            }
+            // No closing brace: emit the prefix literally and continue past it
+            // (guarantees progress, so the loop always terminates).
+            None => {
+                result.push_str(PREFIX);
+                rest = after_prefix;
+            }
+        }
     }
+    result.push_str(rest);
     result
 }
 
@@ -1639,6 +1676,50 @@ nodes:
         assert_eq!(proc.args.as_deref(), Some("--speed 2.0 --verbose"));
     }
 
+    #[test]
+    fn substitute_params_basic_and_unknown() {
+        let params = BTreeMap::from([
+            ("speed".to_string(), "2.0".to_string()),
+            ("name".to_string(), "robot".to_string()),
+        ]);
+        assert_eq!(
+            substitute_params_in_str("--speed ${_param.speed} --name ${_param.name}", &params),
+            "--speed 2.0 --name robot"
+        );
+        // Multiple occurrences of the same key are all replaced.
+        assert_eq!(
+            substitute_params_in_str("${_param.speed}/${_param.speed}", &params),
+            "2.0/2.0"
+        );
+        // An unknown key is left verbatim, not dropped.
+        assert_eq!(
+            substitute_params_in_str("${_param.missing}", &params),
+            "${_param.missing}"
+        );
+        // A dangling prefix without a closing brace is emitted literally.
+        assert_eq!(
+            substitute_params_in_str("prefix ${_param.speed", &params),
+            "prefix ${_param.speed"
+        );
+    }
+
+    #[test]
+    fn substitute_params_is_order_independent_and_non_transitive() {
+        // A parameter value that itself looks like a `${_param.…}` token must be
+        // inserted verbatim, never re-expanded — and the result must not depend
+        // on `BTreeMap` key ordering. With the old chained-`replace` approach,
+        // `a`'s value `${_param.b}` was expanded to `x` because `a` sorts first.
+        let params = BTreeMap::from([
+            ("a".to_string(), "${_param.b}".to_string()),
+            ("b".to_string(), "x".to_string()),
+        ]);
+        assert_eq!(
+            substitute_params_in_str("${_param.a}", &params),
+            "${_param.b}"
+        );
+        assert_eq!(substitute_params_in_str("${_param.b}", &params), "x");
+    }
+
     // ---- Feature 5: module-level build ----
 
     #[test]