Give each variable a unique custom property in CSS export (#1738)

Found while testing crates/engine/src/cmd/variables_io.rs:~80-105 (css, non-tailwind `name` closure using css_var_of) with crates/engine/src/dev/codegen/css.rs:41 (css_var_name)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/engine/src/cmd/variables_io.rs

@@ -8,7 +8,7 @@ use figcraft_doc::{Collection, CollectionId, Document, Mode, ModeId, Variable, V
 use serde_json::{Value, json};
 
 use super::CommandSpec;
-use crate::codegen::{css_var_name, css_var_of};
+use crate::codegen::{CssNames, css_var_name};
 use crate::library::{is_remote_collection, local_collections};
 use crate::params::{get, string};
 use crate::{EngineError, Result, Session, bad, spec};
@@ -73,7 +73,9 @@ fn css(doc: &Document, colls: &[&Collection], tailwind: bool) -> String {
     let flattener = vars.flattener();
     // Tailwind theme names, unique per file and the same ones page code reads.
     let theme = crate::tailwind::Theme::new(doc);
-    let name = |v: &Variable| if tailwind { theme.name(v) } else { css_var_of(v) };
+    // CSS names, unique per file and the same ones generated code reads.
+    let names = CssNames::new(doc);
+    let name = |v: &Variable| if tailwind { theme.name(v) } else { names.name(v) };
     // Lengths need a unit where CSS reads them (`border-radius: var(--radius)`); opacity and
     // font weight don't.
     let roles = crate::token_roles::Roles::new(doc);

crates/engine/src/dev/codegen.rs

@@ -30,8 +30,8 @@ pub(crate) use compose::{compose, kotlin_color};
 pub(crate) use css::Tokens;
 #[cfg(test)]
 pub(crate) use css::css_decls;
+pub use css::{CssNames, css_var_name, css_var_of};
 use css::{css, tok};
-pub use css::{css_var_name, css_var_of};
 pub(crate) use flutter::{FLUTTER_IMPORT, Place, flutter, flutter_indent};
 use html::html;
 pub(crate) use html::{Markup, class_name};

crates/engine/src/dev/codegen/css.rs

@@ -1,3 +1,7 @@
+use std::collections::{HashMap, HashSet};
+
+use figcraft_doc::VariableId;
+
 use super::{
     ContentAlign, CrossAlign, Document, EffectKind, LayoutMode, LetterSpacing, LineHeight, MainAlign, Node, NodeId, NodeKind, PADDING, PaintKind,
     TextAlignH, TextCase, TextDecoration, Variable, css_borders, num,
@@ -13,13 +17,18 @@ pub(crate) struct Tokens {
     pub tailwind: bool,
     /// The file's theme names (computed on first use).
     theme: Option<crate::tailwind::Theme>,
+    /// The file's CSS names (computed on first use).
+    names: Option<CssNames>,
 }
 
 impl Tokens {
     /// `var(--name, value)`, remembering the variable.
     fn var(&mut self, doc: &Document, variable: &Variable, value: String) -> String {
-        let name =
-            if self.tailwind { self.theme.get_or_insert_with(|| crate::tailwind::Theme::new(doc)).name(variable) } else { css_var_of(variable) };
+        let name = if self.tailwind {
+            self.theme.get_or_insert_with(|| crate::tailwind::Theme::new(doc)).name(variable)
+        } else {
+            self.names.get_or_insert_with(|| CssNames::new(doc)).name(variable)
+        };
         let out = format!("var({name}, {value})");
         if !self.vars.iter().any(|(n, _)| *n == name) {
             self.vars.push((name, value));
@@ -60,6 +69,45 @@ pub fn css_var_of(v: &Variable) -> String {
     if valid { inner.chars().take(80).collect() } else { css_var_name(inner) }
 }
 
+/// The file's variables' CSS custom properties, unique in the file: [`css_var_of`], and where an
+/// earlier variable already has that name, the name with a numeric suffix (`--brand-primary-2`).
+pub struct CssNames {
+    names: HashMap<VariableId, String>,
+}
+
+impl CssNames {
+    pub fn new(doc: &Document) -> Self {
+        let vars = &doc.variables.variables;
+        let mut names = HashMap::with_capacity(vars.len());
+        // Every plain name is reserved first, so a suffixed name never takes another's plain one;
+        // the next suffix per base keeps this linear.
+        let plain: Vec<String> = vars.iter().map(css_var_of).collect();
+        let mut taken: HashSet<String> = HashSet::with_capacity(vars.len());
+        let all: HashSet<&str> = plain.iter().map(String::as_str).collect();
+        let mut next: HashMap<&str, usize> = HashMap::new();
+        for (v, base) in vars.iter().zip(&plain) {
+            let mut name = base.clone();
+            if !taken.insert(name.clone()) {
+                let k = next.entry(base.as_str()).or_insert(1);
+                loop {
+                    *k += 1;
+                    name = format!("{base}-{k}");
+                    if !all.contains(name.as_str()) && taken.insert(name.clone()) {
+                        break;
+                    }
+                }
+            }
+            names.insert(v.id.clone(), name);
+        }
+        CssNames { names }
+    }
+
+    /// The custom property for `v` (its [`css_var_of`] name for a variable not in the file).
+    pub fn name(&self, v: &Variable) -> String {
+        self.names.get(&v.id).cloned().unwrap_or_else(|| css_var_of(v))
+    }
+}
+
 /// `value`, or a reference to the variable bound to the first of `fields` that has one.
 pub(super) fn tok(doc: &Document, n: &Node, fields: &[&str], value: String, t: &mut Tokens) -> String {
     match fields.iter().find_map(|f| crate::devinspect::bound_variable(doc, n, f)) {

crates/engine/src/variables_io_tests.rs

@@ -639,3 +639,26 @@ fn stored_variable_ids_reset_only_the_inherited_parent_and_collection() {
         assert_eq!(result.overrides, std::collections::BTreeSet::from([light]));
     }
 }
+
+#[test]
+fn css_export_gives_clashing_variable_names_their_own_property() {
+    let mut s = Session::new();
+    ex(&mut s, "variable.createCollection", json!({"name": "Brand"}));
+    ex(&mut s, "variable.create", json!({"collection": "Brand", "name": "Brand/Primary", "type": "color", "value": "#ff0000"}));
+    let blue = ex(&mut s, "variable.create", json!({"collection": "Brand", "name": "Brand Primary", "type": "color", "value": "#0000ff"}));
+    ex(&mut s, "variable.create", json!({"collection": "Brand", "name": "link", "type": "color", "value": {"alias": blue["id"]}}));
+    let css = ex(&mut s, "variable.export", json!({"format": "css"}))["text"].as_str().unwrap_or_default().to_string();
+    let decls: Vec<(&str, &str)> =
+        css.lines().filter_map(|l| l.trim().strip_prefix("--").and_then(|d| d.strip_suffix(';')).and_then(|d| d.split_once(": "))).collect();
+    let mut names: Vec<&str> = decls.iter().map(|(n, _)| *n).collect();
+    names.sort_unstable();
+    names.dedup();
+    assert_eq!(names.len(), decls.len(), "{css}");
+    let red = decls.iter().find(|(_, v)| *v == "#FF0000").map(|(n, _)| *n);
+    let blue = decls.iter().find(|(_, v)| *v == "#0000FF").map(|(n, _)| *n);
+    // The first keeps its plain name; the alias reads the one its target is declared under.
+    assert_eq!(red, Some("brand-primary"), "{css}");
+    let blue = blue.unwrap_or_default();
+    assert_ne!(blue, "brand-primary", "{css}");
+    assert!(css.contains(&format!("--link: var(--{blue});")), "{css}");
+}