Output graph files to target directory and improve Graphviz output (N…

…omicFoundation#1167)

The graph files `.mmd` and `.dot` files are not committed to the
repository and hence writing them to the source directory (inside the
`generated` folder) does not provide any benefit and even has some
drawbacks (eg. build errors when switching branches and test are no
longer present). This PR changes the output directory to be inside the
`target` directory in the `solidity_cargo_tests` crate directory.

These output files are opt-in via a `RENDER_GRAPHS` environment
variable. The performance impact of their generation is not too big, but
they are generally unnecessary unless we're debugging binding rules.

There are also some tweaks and added information to the `.dot` file
output to improve graph readability and make tracing nodes to rules
easier.

crates/solidity/outputs/cargo/tests/src/bindings_output/graph/graphviz.rs

@@ -6,6 +6,7 @@ use slang_solidity::cst::KindTypes;
 use super::super::runner::ParsedPart;
 
 const VARIABLE_DEBUG_ATTR: &str = "debug_msgb_variable";
+const LOCATION_DEBUG_ATTR: &str = "debug_msgb_location";
 
 pub(crate) fn render(parsed_parts: &[ParsedPart<'_>]) -> String {
     let mut result = Vec::new();
@@ -63,6 +64,36 @@ impl<'a> DotSubGraph<'a> {
             format!("{graph_id}N{index}", graph_id = self.graph_id,)
         }
     }
+
+    fn node_label(&self, node: GraphNodeRef, node_type: Option<&str>) -> String {
+        let graph_node = &self.graph[node];
+        let mut node_label = if let Some(symbol) = graph_node.attributes.get("symbol") {
+            format!("{symbol} [{index}]", index = node.index())
+        } else if let Some(variable) = graph_node.attributes.get(VARIABLE_DEBUG_ATTR) {
+            format!("{variable} [{index}]", index = node.index())
+        } else {
+            format!("[{index}]", index = node.index())
+        };
+        match node_type {
+            Some("push_scoped_symbol") => {
+                node_label += " \u{25ef}";
+            }
+            Some("pop_scoped_symbol") => {
+                node_label += " \u{2b24}";
+            }
+            _ => {
+                if graph_node.attributes.get("is_exported").is_some() {
+                    // shorten the label to fit in a circle
+                    if let Some(dot_index) = node_label.find('.') {
+                        node_label = String::from(&node_label[dot_index + 1..dot_index + 4]);
+                    } else if node_label.len() > 3 {
+                        node_label = String::from(&node_label[..3]);
+                    }
+                }
+            }
+        }
+        node_label
+    }
 }
 
 impl<'a> fmt::Display for DotSubGraph<'a> {
@@ -82,10 +113,10 @@ impl<'a> fmt::Display for DotSubGraph<'a> {
         writeln!(f)?;
         writeln!(
             f,
-            "subgraph cluster_{graph_id} {{\nlabel = \"{title}\"",
-            graph_id = self.graph_id,
-            title = self.title
+            "subgraph cluster_{graph_id} {{",
+            graph_id = self.graph_id
         )?;
+        writeln!(f, "label = \"{title}\"", title = self.title)?;
 
         for node in self.graph.iter_nodes() {
             if special_node(node) {
@@ -94,30 +125,23 @@ impl<'a> fmt::Display for DotSubGraph<'a> {
             }
 
             let graph_node = &self.graph[node];
-            let mut node_label = if let Some(symbol) = graph_node.attributes.get("symbol") {
-                symbol.to_string()
-            } else if let Some(variable) = graph_node.attributes.get(VARIABLE_DEBUG_ATTR) {
-                variable.to_string()
-            } else {
-                format!("{}", node.index())
-            };
-
             let node_type = graph_node
                 .attributes
                 .get("type")
                 .and_then(|x| x.as_str().ok());
+            let node_label = self.node_label(node, node_type);
             let node_id = self.node_id(node);
 
+            if let Some(location) = graph_node.attributes.get(LOCATION_DEBUG_ATTR) {
+                writeln!(f, "\t// {location}")?;
+            }
             match node_type {
                 Some("push_symbol" | "push_scoped_symbol") => {
                     let extra_attrs = if graph_node.attributes.get("is_reference").is_some() {
                         ", penwidth = 2, color = limegreen, fontcolor = limegreen"
                     } else {
                         ", color = lightgreen, fontcolor = lightgreen, style = dashed"
                     };
-                    if node_type == Some("push_scoped_symbol") {
-                        node_label += " \u{25ef}";
-                    }
                     writeln!(
                         f,
                         "\t{node_id} [label = \"{node_label}\", shape = invhouse{extra_attrs}]"
@@ -140,9 +164,6 @@ impl<'a> fmt::Display for DotSubGraph<'a> {
                     } else {
                         ", color = coral, fontcolor = coral, style = dashed"
                     };
-                    if node_type == Some("pop_scoped_symbol") {
-                        node_label += " \u{2b24}";
-                    }
                     writeln!(
                         f,
                         "\t{node_id} [label = \"{node_label}\", shape = house{extra_attrs}]"
@@ -153,19 +174,23 @@ impl<'a> fmt::Display for DotSubGraph<'a> {
                 }
                 _ => {
                     let extra_attrs = if graph_node.attributes.get("is_exported").is_some() {
-                        ", shape = circle, width = 1, penwidth = 2, fixedsize = true, color = purple"
+                        ", shape = circle, width = 0.5, penwidth = 2, fixedsize = true, color = purple"
                     } else {
                         ""
                     };
                     writeln!(f, "\t{node_id} [label = \"{node_label}\"{extra_attrs}]")?;
                 }
             }
 
-            for (sink, _edge) in graph_node.iter_edges() {
+            for (sink, edge) in graph_node.iter_edges() {
                 if special_node(sink) {
                     // we already rendered the edges going to special nodes
                     continue;
                 }
+
+                if let Some(location) = edge.attributes.get(LOCATION_DEBUG_ATTR) {
+                    writeln!(f, "\t// {location}")?;
+                }
                 writeln!(f, "\t{node_id} -> {sink_id}", sink_id = self.node_id(sink))?;
             }
         }

crates/solidity/outputs/cargo/tests/src/bindings_output/runner.rs

@@ -1,7 +1,8 @@
+use std::fs;
+
 use anyhow::Result;
 use infra_utils::cargo::CargoWorkspace;
 use infra_utils::codegen::CodegenFileSystem;
-use infra_utils::github::GitHub;
 use infra_utils::paths::PathExtensions;
 use metaslang_graph_builder::graph::Graph;
 use slang_solidity::cst::KindTypes;
@@ -26,6 +27,10 @@ pub fn run(group_name: &str, test_name: &str) -> Result<()> {
         .join("bindings_output")
         .join(group_name)
         .join(test_name);
+    let target_dir = CargoWorkspace::locate_source_crate("solidity_testing_snapshots")?
+        .join("target/bindings_output")
+        .join(group_name)
+        .join(test_name);
 
     let mut fs = CodegenFileSystem::new(&test_dir)?;
 
@@ -74,26 +79,23 @@ pub fn run(group_name: &str, test_name: &str) -> Result<()> {
             "failure"
         };
 
-        if !GitHub::is_running_in_ci() {
-            // Don't run this in CI, since the graph outputs are not committed
-            // to the repository and hence we cannot verify their contents,
-            // which is what `fs.write_file` does in CI.
+        // Render graph outputs only if the __SLANG_BINDINGS_RENDER_GRAPHS__
+        // environment variable is set.
+        if std::env::var("__SLANG_BINDINGS_RENDER_GRAPHS__").is_ok() {
+            fs::create_dir_all(&target_dir)?;
+
             let graph_output = render_mermaid_graph(&parsed_parts);
             match last_graph_output {
                 Some(ref last) if last == &graph_output => (),
                 _ => {
-                    let snapshot_path = test_dir
-                        .join("generated")
-                        .join(format!("{version}-{status}.mmd"));
+                    let snapshot_path = target_dir.join(format!("{version}-{status}.mmd"));
 
-                    fs.write_file(snapshot_path, &graph_output)?;
+                    fs::write(snapshot_path, &graph_output)?;
                     last_graph_output = Some(graph_output);
 
                     let dot_output = render_graphviz_graph(&parsed_parts);
-                    let dot_output_path = test_dir
-                        .join("generated")
-                        .join(format!("{version}-{status}.dot"));
-                    fs.write_file(dot_output_path, &dot_output)?;
+                    let dot_output_path = target_dir.join(format!("{version}-{status}.dot"));
+                    fs::write(dot_output_path, &dot_output)?;
                 }
             };
         }