From 403d6a7691718e1debe4a4c2eab75ec3aa6dfc17 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@cs.stanford.edu>
Date: Tue, 18 Aug 2026 17:50:42 -0700
Subject: [PATCH] better typing

---
 rust/pspp/src/spv/read/graph.rs | 145 +++++++++++++++++++++-----------
 1 file changed, 96 insertions(+), 49 deletions(-)

diff --git a/rust/pspp/src/spv/read/graph.rs b/rust/pspp/src/spv/read/graph.rs
index 61ca63f5f0..d3fc583df7 100644
--- a/rust/pspp/src/spv/read/graph.rs
+++ b/rust/pspp/src/spv/read/graph.rs
@@ -148,9 +148,7 @@ pub struct Visualization {
     embedded_sources: Vec<EmbeddedSource>,
     categorical_domains: Vec<CategoricalDomain>,
     interval_domains: Vec<IntervalDomain>,
-    graphs: Vec<Graph>,
-    label_frames: Vec<LabelFrame>,
-    containers: Vec<Container>,
+    containers: Vec<AnyContainer>,
     styles: Vec<Style>,
     style_cycles: Vec<StyleCycle>,
     locations: Vec<Location>,
@@ -175,8 +173,6 @@ impl<'de> Deserialize<'de> for Visualization {
         let mut embedded_sources = Vec::new();
         let mut categorical_domains = Vec::new();
         let mut interval_domains = Vec::new();
-        let mut graphs = Vec::new();
-        let mut label_frames = Vec::new();
         let mut containers = Vec::new();
         let mut styles = Vec::new();
         let mut style_cycles = Vec::new();
@@ -197,9 +193,13 @@ impl<'de> Deserialize<'de> for Visualization {
                     categorical_domains.push(categorical_domain)
                 }
                 VisChild::IntervalDomain(interval_domain) => interval_domains.push(interval_domain),
-                VisChild::Graph(graph) => graphs.push(graph),
-                VisChild::LabelFrame(label_frame) => label_frames.push(label_frame),
-                VisChild::Container(container) => containers.push(container),
+                VisChild::Graph(graph) => containers.push(AnyContainer::Graph(graph)),
+                VisChild::LabelFrame(label_frame) => {
+                    containers.push(AnyContainer::LabelFrame(label_frame))
+                }
+                VisChild::Container(container) => {
+                    containers.push(AnyContainer::Container(container))
+                }
                 VisChild::StyleCycle(style_cycle) => style_cycles.push(style_cycle),
                 VisChild::Style(style) => styles.push(style),
                 VisChild::Location(location) => locations.push(location),
@@ -218,8 +218,6 @@ impl<'de> Deserialize<'de> for Visualization {
             embedded_sources,
             categorical_domains,
             interval_domains,
-            graphs,
-            label_frames,
             containers,
             styles,
             style_cycles,
@@ -358,7 +356,7 @@ impl Visualization {
             });
             for child in &container.children {
                 match child {
-                    ContainerChild::Graph(graph) => {
+                    AnyContainer::Graph(graph) => {
                         let index = frames.len();
                         frames[container_index].children.push(index);
                         if let Some(id) = &graph.id {
@@ -373,7 +371,7 @@ impl Visualization {
                             position: EnumMap::default(),
                         });
                     }
-                    ContainerChild::LabelFrame(label_frame) => {
+                    AnyContainer::LabelFrame(label_frame) => {
                         let index = frames.len();
                         frames[container_index].children.push(index);
                         if let Some(id) = &label_frame.id {
@@ -388,7 +386,7 @@ impl Visualization {
                             position: EnumMap::default(),
                         });
                     }
-                    ContainerChild::Legend(legend) => {
+                    AnyContainer::Legend(legend) => {
                         let index = frames.len();
                         frames[container_index].children.push(index);
                         if let Some(id) = &legend.id {
@@ -403,45 +401,50 @@ impl Visualization {
                             position: EnumMap::default(),
                         });
                     }
-                    ContainerChild::Container(container) => {
+                    AnyContainer::Container(container) => {
                         decode_container(container_index, container, frames, ids)
                     }
-                    _ => (),
                 }
             }
         }
-        for graph in &self.graphs {
-            let index = frames.len();
-            frames[0].children.push(index);
-            if let Some(id) = &graph.id {
-                ids.insert(id, index);
-            }
-            frames.push(Frame {
-                id: graph.id.clone(),
-                frame_type: FrameType::Graph(graph),
-                parent: Some(0),
-                children: Vec::new(),
-                locations: EnumMap::default(),
-                position: EnumMap::default(),
-            });
-        }
-        for label_frame in &self.label_frames {
-            let index = frames.len();
-            frames[0].children.push(index);
-            if let Some(id) = &label_frame.id {
-                ids.insert(id, index);
-            }
-            frames.push(Frame {
-                id: label_frame.id.clone(),
-                frame_type: FrameType::LabelFrame(label_frame),
-                parent: Some(0),
-                children: Vec::new(),
-                locations: EnumMap::default(),
-                position: EnumMap::default(),
-            });
-        }
+
         for container in &self.containers {
-            decode_container(0, container, &mut frames, &mut ids)
+            match container {
+                AnyContainer::Graph(graph) => {
+                    let index = frames.len();
+                    frames[0].children.push(index);
+                    if let Some(id) = &graph.id {
+                        ids.insert(id, index);
+                    }
+                    frames.push(Frame {
+                        id: graph.id.clone(),
+                        frame_type: FrameType::Graph(graph),
+                        parent: Some(0),
+                        children: Vec::new(),
+                        locations: EnumMap::default(),
+                        position: EnumMap::default(),
+                    });
+                }
+                AnyContainer::LabelFrame(label_frame) => {
+                    let index = frames.len();
+                    frames[0].children.push(index);
+                    if let Some(id) = &label_frame.id {
+                        ids.insert(id, index);
+                    }
+                    frames.push(Frame {
+                        id: label_frame.id.clone(),
+                        frame_type: FrameType::LabelFrame(label_frame),
+                        parent: Some(0),
+                        children: Vec::new(),
+                        locations: EnumMap::default(),
+                        position: EnumMap::default(),
+                    });
+                }
+                AnyContainer::Container(container) => {
+                    decode_container(0, container, &mut frames, &mut ids)
+                }
+                AnyContainer::Legend(_) => todo!(),
+            }
         }
 
         for frame in &mut frames {
@@ -1227,6 +1230,14 @@ impl Visualization {
     }
 }
 
+#[derive(Debug)]
+enum AnyContainer {
+    Container(Container),
+    Graph(Graph),
+    Legend(Legend),
+    LabelFrame(LabelFrame),
+}
+
 struct FontMetrics {
     layout: RefCell<pango::Layout>,
 }
@@ -4287,9 +4298,45 @@ impl Locations for LabelFrame {
     }
 }
 
+#[derive(Debug)]
+struct Container {
+    id: Option<String>,
+    locations: Vec<Location>,
+    children: Vec<AnyContainer>,
+}
+
+impl<'de> Deserialize<'de> for Container {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let raw_container = RawContainer::deserialize(deserializer)?;
+        let mut locations = Vec::new();
+        let mut children = Vec::new();
+        for child in raw_container.children {
+            match child {
+                ContainerChild::Location(location) => locations.push(location),
+                ContainerChild::Container(container) => {
+                    children.push(AnyContainer::Container(container))
+                }
+                ContainerChild::Graph(graph) => children.push(AnyContainer::Graph(graph)),
+                ContainerChild::Legend(legend) => children.push(AnyContainer::Legend(legend)),
+                ContainerChild::LabelFrame(label_frame) => {
+                    children.push(AnyContainer::LabelFrame(label_frame))
+                }
+            }
+        }
+        Ok(Self {
+            id: raw_container.id,
+            locations,
+            children,
+        })
+    }
+}
+
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "camelCase")]
-struct Container {
+struct RawContainer {
     #[serde(rename = "@id")]
     id: Option<String>,
     #[serde(rename = "$value")]
@@ -4297,7 +4344,7 @@ struct Container {
     children: Vec<ContainerChild>,
 }
 
-impl Locations for Container {
+impl Locations for RawContainer {
     fn locations(&self) -> Vec<&Location> {
         self.children
             .iter()
@@ -4400,7 +4447,7 @@ impl<'a> FrameType<'a> {
     fn locations(&self) -> Vec<&Location> {
         match self {
             FrameType::Visualization(visualization) => visualization.locations(),
-            FrameType::Container(container) => container.locations(),
+            FrameType::Container(container) => container.locations.iter().collect(),
             FrameType::Graph(graph) => graph.locations.iter().collect(),
             FrameType::LabelFrame(label_frame) => label_frame.locations(),
             FrameType::Legend(legend) => legend.locations(),
-- 
2.47.3