.fig export: a property name used with two types in a set writes both definitions

The set wrote one definition per name, so a variant whose property of
that name had another type named a definition the file didn't hold
(CodeRabbit review on #1152).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Ricardo
2026-10-23 12:00:22 -06:00
co-authored by Claude Opus 5.5
parent 4c6c4d9667
commit c19b7d0799
2 changed files with 37 additions and 6 deletions
+5 -6
View File
@@ -2004,18 +2004,17 @@ impl<'d> Exporter<'d> {
}
// The format keeps one default per property of a set (a variant's copy holds none): the first
// variant's, with a warning when another variant's differs.
let mut seen: BTreeMap<String, PropDef> = BTreeMap::new();
// A name used with two types is two properties (their definitions have their own ids).
let mut seen: BTreeMap<(String, &'static str), PropDef> = BTreeMap::new();
for v in &variants {
let NodeKind::Component { props, .. } = &v.kind else { continue };
for (name, d) in props {
if prop_type(d).is_none() {
continue;
}
match seen.get(name) {
let Some(ty) = prop_type(d) else { continue };
match seen.get(&(name.clone(), ty)) {
Some(first) if first != d => self.warn("variants with different defaults for one property were written with the first variant's"),
Some(_) => {}
None => {
seen.insert(name.clone(), d.clone());
seen.insert((name.clone(), ty), d.clone());
let index = defs.len();
defs.extend(self.prop_def(set.id, name, d, index));
}
+32
View File
@@ -1393,3 +1393,35 @@ fn variants_that_disagree_on_a_default_are_a_warning() {
let (_, warnings) = export(&doc, None).unwrap();
assert!(warnings.iter().any(|w| w.contains("different defaults")), "{warnings:?}");
}
#[test]
fn a_name_used_with_two_types_in_a_set_writes_both_definitions() {
let (mut doc, _) = set_with_properties();
let large = doc.nodes().find(|n| n.name == "Size=Large").unwrap().id;
if let NodeKind::Component { props, .. } = &mut doc.node_mut(large).unwrap().kind {
props.insert("Label".into(), PropDef::Boolean { default: true });
}
let (bytes, _) = export(&doc, None).unwrap();
let archive = Archive::open(&bytes).unwrap();
let (schema, message) = decode_canvas(&archive.canvas).unwrap();
let msg = schema.decode_message(schema.find("Message").unwrap(), &message).unwrap();
let changes = msg.get("nodeChanges").unwrap().list();
let key = |g: &Kv| format!("{}:{}", g.get("sessionID").unwrap().f64().unwrap(), g.get("localID").unwrap().f64().unwrap());
let set = changes.iter().find(|n| n.get("isStateGroup").is_some()).unwrap();
let written: BTreeSet<String> = set.get("componentPropDefs").unwrap().list().iter().map(|d| key(d.get("id").unwrap())).collect();
let labels: Vec<&str> = set
.get("componentPropDefs")
.unwrap()
.list()
.iter()
.filter(|d| d.get("name").and_then(Kv::str) == Some("Label"))
.filter_map(|d| d.get("type").and_then(Kv::str))
.collect();
assert_eq!(labels.len(), 2, "{labels:?}");
// Every variant copy names a definition the set holds.
for v in changes.iter().filter(|n| n.get("variantPropSpecs").is_some()) {
for d in v.get("componentPropDefs").unwrap().list() {
assert!(written.contains(&key(d.get("parentPropDefId").unwrap())));
}
}
}