M3: CSS export never names a calculation it leaves out either (#1280)

The CSS export left out a colour calculation that resolves to nothing but still wrote an alias to
it as var(--name); where no block defines that property, whatever reads the alias is invalid.
CSS (and Tailwind) and Penpot now share one rule, Variables::flattener: an alias that leads to a
calculation some mode of its collection leaves out goes out as the value it shows in the mode,
or is left out when it shows nothing.

crates/doc/src/styles.rs

@@ -271,6 +271,12 @@ pub struct Variable {
 pub const CODE_PLATFORMS: [&str; 3] = ["WEB", "iOS", "ANDROID"];
 
 impl Variable {
+    /// Its value in `mode`, or its first value when it has none there: an export writes every
+    /// mode of the collection, given a value in each or not.
+    pub fn value_or_first(&self, mode: &str) -> Option<&VariableValue> {
+        self.values.get(mode).or_else(|| self.values.values().next())
+    }
+
     /// Whether pickers offer this variable for properties of `scope`.
     pub fn in_scope(&self, scope: &str) -> bool {
         self.scopes.as_ref().is_none_or(|list| list.iter().any(|s| s == scope || s == ALL_SCOPES || (s == "ALL_FILLS" && scope.ends_with("_FILL"))))
@@ -401,6 +407,36 @@ impl Variables {
         }
     }
 
+    /// Values as an export without colour calculations writes them, references included: see
+    /// [`Flattener::value`].
+    pub fn flattener(&self) -> Flattener<'_> {
+        let mut out = BTreeSet::new();
+        for var in &self.variables {
+            let Some(coll) = self.collection_by_id(&var.collection) else { continue };
+            if coll.modes.iter().any(|m| var.value_or_first(&m.id).is_some_and(|v| self.flattened(var, &m.id, v).is_none())) {
+                out.insert(var.id.as_str());
+            }
+        }
+        // Then every alias that leads to one, back along alias chains.
+        let mut readers: BTreeMap<&str, Vec<&str>> = BTreeMap::new();
+        for var in &self.variables {
+            for value in var.values.values() {
+                if let VariableValue::Alias(target) = value {
+                    readers.entry(target.as_str()).or_default().push(var.id.as_str());
+                }
+            }
+        }
+        let mut stack: Vec<&str> = out.iter().copied().collect();
+        while let Some(id) = stack.pop() {
+            for reader in readers.get(id).into_iter().flatten() {
+                if out.insert(reader) {
+                    stack.push(reader);
+                }
+            }
+        }
+        Flattener { vars: self, unreferenceable: out }
+    }
+
     /// The stored value of `id` in its collection's current mode (the mode chosen when it has a
     /// value for it, else the default mode, else any value).
     fn value_in(&self, id: &str, modes: &BTreeMap<CollectionId, ModeId>) -> Option<&VariableValue> {
@@ -476,6 +512,28 @@ impl Variables {
 
 /// Linear per-channel interpolation (`t` clamped and NaN-guarded: a command validates, a crafted
 /// file must still resolve to a sane colour).
+/// Values as an export that leaves out what resolves to nothing writes them ([`Variables::flattener`]).
+pub struct Flattener<'v> {
+    vars: &'v Variables,
+    /// Colour calculations that resolve to nothing in some mode of their collection, and every
+    /// alias that leads to one.
+    unreferenceable: BTreeSet<&'v str>,
+}
+
+impl Flattener<'_> {
+    /// `val`, a value of `var` in `mode`, as [`Variables::flattened`] writes it, and an alias that
+    /// leads to a calculation some mode leaves out also as the value it shows here: a reference to
+    /// it would read nothing whenever that mode applies. None: it shows nothing in this mode.
+    pub fn value(&self, var: &Variable, mode: &str, val: &VariableValue) -> Option<VariableValue> {
+        match self.vars.flattened(var, mode, val)? {
+            VariableValue::Alias(target) if self.unreferenceable.contains(target.as_str()) => {
+                self.vars.resolve(&var.id, &BTreeMap::from([(var.collection.clone(), mode.to_string())]))
+            }
+            other => Some(other),
+        }
+    }
+}
+
 fn mix_colors(a: &Color, b: &Color, t: f32) -> Color {
     let t = if t.is_finite() { t.clamp(0.0, 1.0) } else { 0.0 };
     // Weighted at both ends so t = 0 and t = 1 give each operand exactly: `x + (y - x) * t` can

crates/engine/src/cmd/variables_io.rs

@@ -70,6 +70,7 @@ fn css_safe(text: &str) -> String {
 /// aliases are `var(--name)` and colour calculations resolve to their colour in the mode.
 fn css(doc: &Document, colls: &[&Collection], tailwind: bool) -> String {
     let vars = &doc.variables;
+    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) };
@@ -90,8 +91,9 @@ fn css(doc: &Document, colls: &[&Collection], tailwind: bool) -> String {
             out.push_str(&format!("/* {} · {} */\n{sel} {{\n", css_safe(&c.name), css_safe(&m.name)));
             for v in vars.variables.iter().filter(|v| v.collection == c.id) {
                 // A calculation is written as the colour it resolves to in this mode (no color-mix()
-                // or relative-colour output).
-                let flat = v.values.get(&m.id).and_then(|x| vars.flattened(v, &m.id, x));
+                // or relative-colour output), and so is an alias to one some mode leaves out:
+                // `var()` of a property no block defines would invalidate whatever reads it.
+                let flat = v.values.get(&m.id).and_then(|x| flattener.value(v, &m.id, x));
                 let value = match flat.as_ref() {
                     Some(VariableValue::Color(col)) => col.to_css(),
                     Some(VariableValue::Number(x)) if roles.role(v) == crate::token_roles::NumberRole::Unitless => trim_num(*x),

crates/engine/src/variables_colour_tests.rs

@@ -395,6 +395,43 @@ fn rest_and_css_exports_write_calculations_as_their_colours() {
     assert!(text.contains("rgba(255, 0, 0, 0.5") && text.contains("rgba(0, 0, 255, 0.5"), "both modes in CSS: {text}");
 }
 
+/// A hand-edited file can hold a calculation that resolves to nothing (one reading a variable
+/// that isn't there). The CSS export leaves it out, so no property may name it: an alias to it
+/// goes out as the colour it shows, or is left out when that is nothing.
+#[test]
+fn css_never_names_a_calculation_it_leaves_out() {
+    let mut s = Session::new();
+    ex(&mut s, "variable.createCollection", json!({"name": "T", "modes": ["Light", "Dark"]}));
+    let ink = json!({"collection": "T", "name": "ink", "type": "color", "values": {"Light": "#FF0000", "Dark": "#0000FF"}});
+    ex(&mut s, "variable.create", ink);
+    for name in ["lost", "half"] {
+        ex(&mut s, "variable.create", json!({"collection": "T", "name": name, "type": "color", "value": "#000000"}));
+    }
+    for (name, to) in [("lost-too", "lost"), ("half-too", "half"), ("ink-too", "ink")] {
+        ex(&mut s, "variable.create", json!({"collection": "T", "name": name, "type": "color", "value": {"alias": to}}));
+    }
+    // As a hand-edited file could hold them: `lost` reads a variable that isn't there in both
+    // modes, `half` only in Dark.
+    let doc = s.doc_mut().unwrap();
+    let ink = doc.variables.get("ink").unwrap().id.clone();
+    let modes: Vec<String> = doc.variables.collection("T").unwrap().modes.iter().map(|m| m.id.clone()).collect();
+    let gone = figcraft_doc::VariableValue::ColorAlpha { variable: "gone".into(), opacity: 0.5 };
+    let half = figcraft_doc::VariableValue::ColorAlpha { variable: ink, opacity: 0.5 };
+    for v in &mut doc.variables.variables {
+        match v.name.as_str() {
+            "lost" => v.values = modes.iter().map(|m| (m.clone(), gone.clone())).collect(),
+            "half" => v.values = [(modes[0].clone(), half.clone()), (modes[1].clone(), gone.clone())].into_iter().collect(),
+            _ => {}
+        }
+    }
+    let css = ex(&mut s, "variable.export", json!({"format": "css"}))["text"].as_str().unwrap().to_string();
+    let (light, dark) = css.split_once("[data-").unwrap();
+    assert!(!css.contains("var(--lost)") && !css.contains("var(--half)"), "no var() of a property a block leaves out: {css}");
+    assert!(!css.contains("--lost-too"), "an alias to what shows nothing is left out: {css}");
+    assert!(light.contains("--half-too: rgba(255, 0, 0, 0.5") && !dark.contains("--half-too"), "the colour it shows, where it shows one: {css}");
+    assert!(css.contains("--ink-too: var(--ink);"), "an alias that always resolves stays a reference: {css}");
+}
+
 /// A calculation is a variable's value, not a literal an expression may hold: nothing would check
 /// what it reads. Every way an expression arrives refuses one.
 #[test]

crates/format/src/penpot/export.rs

@@ -8,7 +8,7 @@ use figcraft_doc::proto::{Action, Direction, Easing, Interaction, OverlayPositio
 use figcraft_doc::{
     BoolOp, CellAlign, ContentAlign, CrossAlign, Document, EffectKind, GridAlign, GridKind, GridTrack, HConstraint, LayoutMode, LetterSpacing,
     LineHeight, MainAlign, Node, NodeId, NodeKind, Sizing, StrokeAlign, StrokeCap, TextAlignH, TextAlignV, TextAutoResize, TextCase, TextDecoration,
-    TextStyle, VConstraint, Variable, VariableType, VariableValue, Variables,
+    TextStyle, VConstraint, VariableType, VariableValue,
 };
 use serde_json::{Map, Value, json};
 
@@ -1720,7 +1720,7 @@ impl<'a> Exporter<'a> {
         if variables.variables.is_empty() {
             return None;
         }
-        let unreferenceable = unreferenceable(variables);
+        let flattener = variables.flattener();
         let mut root = Map::new();
         let mut themes = Vec::new();
         let mut order = Vec::new();
@@ -1732,26 +1732,14 @@ impl<'a> Exporter<'a> {
                 let set_name = if many { format!("{}/{}", collection.name, mode.name) } else { collection.name.clone() };
                 let mut tokens = Map::new();
                 for var in variables.variables.iter().filter(|v| v.collection == collection.id) {
-                    let Some(value) = mode_value(var, &mode.id) else { continue };
+                    let Some(value) = var.value_or_first(&mode.id) else { continue };
                     // Penpot tokens have no shape for colour calculations: the colour they resolve
-                    // to in this mode goes out as a plain colour token.
-                    let Some(value) = variables.flattened(var, &mode.id, value) else {
+                    // to in this mode goes out as a plain colour token, and so does an alias to one
+                    // some set leaves out (its `{name}` would dangle whenever that set is active).
+                    let Some(value) = flattener.value(var, &mode.id, value) else {
                         self.warn(crate::CALCULATIONS_LEFT_OUT);
                         continue;
                     };
-                    // A reference to a token some set leaves out would dangle whenever that set is
-                    // active: such an alias goes out as the colour it shows here instead.
-                    let value = match value {
-                        VariableValue::Alias(target) if unreferenceable.contains(target.as_str()) => {
-                            let here = BTreeMap::from([(collection.id.clone(), mode.id.clone())]);
-                            let Some(shown) = variables.resolve(&var.id, &here) else {
-                                self.warn(crate::CALCULATIONS_LEFT_OUT);
-                                continue;
-                            };
-                            shown
-                        }
-                        other => other,
-                    };
                     let (kind, value) = match &value {
                         VariableValue::Alias(target) => {
                             let Some(target_name) = self.token_name(target) else { continue };
@@ -1805,40 +1793,6 @@ impl<'a> Exporter<'a> {
     }
 }
 
-/// The value a variable's token takes in `mode` (its first value when it has none for that mode).
-fn mode_value<'v>(var: &'v Variable, mode: &str) -> Option<&'v VariableValue> {
-    var.values.get(mode).or_else(|| var.values.values().next())
-}
-
-/// Variables a token reference must not name: colour calculations some token set leaves out (they
-/// resolve to nothing in a mode of their collection) and every alias that leads to one.
-fn unreferenceable(variables: &Variables) -> BTreeSet<&str> {
-    let mut out = BTreeSet::new();
-    for var in &variables.variables {
-        let Some(coll) = variables.collection_by_id(&var.collection) else { continue };
-        if coll.modes.iter().any(|m| mode_value(var, &m.id).is_some_and(|v| variables.flattened(var, &m.id, v).is_none())) {
-            out.insert(var.id.as_str());
-        }
-    }
-    let mut readers: BTreeMap<&str, Vec<&str>> = BTreeMap::new();
-    for var in &variables.variables {
-        for value in var.values.values() {
-            if let VariableValue::Alias(target) = value {
-                readers.entry(target.as_str()).or_default().push(var.id.as_str());
-            }
-        }
-    }
-    let mut stack: Vec<&str> = out.iter().copied().collect();
-    while let Some(id) = stack.pop() {
-        for reader in readers.get(id).into_iter().flatten() {
-            if out.insert(reader) {
-                stack.push(reader);
-            }
-        }
-    }
-    out
-}
-
 /// A layer's visibility, lock, opacity, blend, proportion lock and constraints as Penpot fields.
 fn appearance_fields(node: &Node, fields: &mut Map<String, Value>) {
     if !node.visible {