fix include merging for when a file imports another file more than once directly

Author: Strum355

server/main/src/commands/merged_includes.rs

@@ -51,7 +51,7 @@ impl VirtualMergedDocument {
 
         for node in nodes {
             let graph = self.graph.borrow();
-            let path = graph.get_node(node.0);
+            let path = graph.get_node(node.child);
 
             if sources.contains_key(&path) {
                 continue;
@@ -103,7 +103,7 @@ impl Invokeable for VirtualMergedDocument {
 
             let mut source_mapper = SourceMapper::new(all_sources.len());
             let graph = self.graph.borrow();
-            let view = merge_views::generate_merge_list(&tree, &all_sources, &graph, &mut source_mapper);
+            let view = merge_views::MergeViewBuilder::new(&tree, &all_sources, &graph, &mut source_mapper).build();
             return Ok(serde_json::value::Value::String(view));
         }
         return Err(format_err!(

server/main/src/dfs.rs

@@ -56,21 +56,21 @@ impl<'a> Iterator for Dfs<'a> {
     fn next(&mut self) -> Option<Result<FilialTuple, error::CycleError>> {
         let parent = self.cycle.last().map(|p| p.node);
 
-        if let Some(node) = self.stack.pop() {
+        if let Some(child) = self.stack.pop() {
             self.cycle.push(VisitCount {
-                node,
-                children: self.graph.graph.edges(node).count(),
+                node: child,
+                children: self.graph.graph.edges(child).count(),
                 touch: 1,
             });
 
-            let mut children = self.graph.child_node_indexes(node);
+            let mut children: Vec<NodeIndex> = self.graph.child_node_indexes(child).collect();
 
             if !children.is_empty() {
                 // sort by line number in parent
                 children.sort_by(|x, y| {
                     let graph = &self.graph.graph;
-                    let edge1 = graph.edge_weight(graph.find_edge(node, *x).unwrap()).unwrap();
-                    let edge2 = graph.edge_weight(graph.find_edge(node, *y).unwrap()).unwrap();
+                    let edge1 = graph.edge_weight(graph.find_edge(child, *x).unwrap()).unwrap();
+                    let edge2 = graph.edge_weight(graph.find_edge(child, *y).unwrap()).unwrap();
 
                     edge2.line.cmp(&edge1.line)
                 });
@@ -87,7 +87,7 @@ impl<'a> Iterator for Dfs<'a> {
                 self.reset_path_to_branch();
             }
 
-            return Some(Ok((node, parent)));
+            return Some(Ok(FilialTuple { child, parent }));
         }
         None
     }
@@ -189,8 +189,8 @@ mod dfs_test {
                 collection.push(i.unwrap());
             }
 
-            let nodes: Vec<NodeIndex> = collection.iter().map(|n| n.0).collect();
-            let parents: Vec<Option<NodeIndex>> = collection.iter().map(|n| n.1).collect();
+            let nodes: Vec<NodeIndex> = collection.iter().map(|n| n.child).collect();
+            let parents: Vec<Option<NodeIndex>> = collection.iter().map(|n| n.parent).collect();
             //          0
             //        /  \
             //      1     2
@@ -237,8 +237,8 @@ mod dfs_test {
                 collection.push(i.unwrap());
             }
 
-            let nodes: Vec<NodeIndex> = collection.iter().map(|n| n.0).collect();
-            let parents: Vec<Option<NodeIndex>> = collection.iter().map(|n| n.1).collect();
+            let nodes: Vec<NodeIndex> = collection.iter().map(|n| n.child).collect();
+            let parents: Vec<Option<NodeIndex>> = collection.iter().map(|n| n.parent).collect();
             //          0
             //        /  \
             //      1     2

server/main/src/graph.rs

@@ -1,6 +1,7 @@
 use petgraph::stable_graph::EdgeIndex;
 use petgraph::stable_graph::NodeIndex;
 use petgraph::stable_graph::StableDiGraph;
+use petgraph::visit::EdgeRef;
 use petgraph::Direction;
 
 use std::{
@@ -53,20 +54,27 @@ impl CachedStableGraph {
         }
     }
 
+    // Returns the `PathBuf` for a given `NodeIndex`
     pub fn get_node(&self, node: NodeIndex) -> PathBuf {
         PathBuf::from_str(&self.graph[node]).unwrap()
     }
 
-    pub fn get_edge_meta(&self, parent: NodeIndex, child: NodeIndex) -> &IncludePosition {
-        self.graph.edge_weight(self.graph.find_edge(parent, child).unwrap()).unwrap()
-    }
-
-    #[allow(dead_code)]
-    pub fn remove_node(&mut self, name: &Path) {
-        let idx = self.cache.remove(name);
-        if let Some(idx) = idx {
-            self.graph.remove_node(idx);
-        }
+    /// returns an iterator over all the `IncludePosition`'s between a parent and its child for all the positions
+    /// that the child may be imported into the parent, in order of import.
+    pub fn get_edge_metas(&self, parent: NodeIndex, child: NodeIndex) -> impl Iterator<Item = IncludePosition> + '_ {
+        let mut edges = self
+            .graph
+            .edges(parent)
+            .filter_map(move |edge| {
+                let target = self.graph.edge_endpoints(edge.id()).unwrap().1;
+                if target != child {
+                    return None;
+                }
+                Some(self.graph[edge.id()])
+            })
+            .collect::<Vec<IncludePosition>>();
+        edges.sort_by(|x, y| x.line.cmp(&y.line));
+        edges.into_iter()
     }
 
     pub fn add_node(&mut self, name: &Path) -> NodeIndex {
@@ -83,61 +91,36 @@ impl CachedStableGraph {
         self.graph.add_edge(parent, child, meta)
     }
 
-    pub fn remove_edge(&mut self, parent: NodeIndex, child: NodeIndex) {
-        let edge = self.graph.find_edge(parent, child).unwrap();
-        self.graph.remove_edge(edge);
-    }
-
-    #[allow(dead_code)]
-    pub fn edge_weights(&self, node: NodeIndex) -> Vec<IncludePosition> {
-        self.graph.edges(node).map(|e| e.weight().clone()).collect()
-    }
-
-    #[allow(dead_code)]
-    pub fn child_node_names(&self, node: NodeIndex) -> Vec<PathBuf> {
-        self.graph
-            .neighbors(node)
-            .map(|n| self.reverse_index.get(&n).unwrap().clone())
-            .collect()
-    }
-
-    pub fn child_node_meta(&self, node: NodeIndex) -> Vec<(PathBuf, IncludePosition)> {
+    pub fn remove_edge(&mut self, parent: NodeIndex, child: NodeIndex, position: IncludePosition) {
         self.graph
-            .neighbors(node)
-            .map(|n| {
-                let edge = self.graph.find_edge(node, n).unwrap();
-                let edge_meta = self.graph.edge_weight(edge).unwrap();
-                return (self.reverse_index.get(&n).unwrap().clone(), edge_meta.clone());
-            })
-            .collect()
+            .edges(parent)
+            .find(|edge| self.graph.edge_endpoints(edge.id()).unwrap().1 == child && *edge.weight() == position)
+            .map(|edge| edge.id())
+            .and_then(|edge| self.graph.remove_edge(edge));
     }
 
-    pub fn child_node_indexes(&self, node: NodeIndex) -> Vec<NodeIndex> {
-        self.graph.neighbors(node).collect()
+    pub fn child_node_metas(&self, node: NodeIndex) -> impl Iterator<Item = (PathBuf, IncludePosition)> + '_ {
+        self.graph.neighbors(node).map(move |n| {
+            let edge = self.graph.find_edge(node, n).unwrap();
+            let edge_meta = self.graph.edge_weight(edge).unwrap();
+            return (self.reverse_index.get(&n).unwrap().clone(), *edge_meta);
+        })
     }
 
-    #[allow(dead_code)]
-    pub fn parent_node_names(&self, node: NodeIndex) -> Vec<PathBuf> {
-        self.graph
-            .neighbors_directed(node, Direction::Incoming)
-            .map(|n| self.reverse_index.get(&n).unwrap().clone())
-            .collect()
-    }
-
-    pub fn parent_node_indexes(&self, node: NodeIndex) -> Vec<NodeIndex> {
-        self.graph.neighbors_directed(node, Direction::Incoming).collect()
-    }
-
-    #[allow(dead_code)]
-    pub fn get_include_meta(&self, node: NodeIndex) -> Vec<IncludePosition> {
-        self.graph.edges(node).map(|e| e.weight().clone()).collect()
+    pub fn child_node_indexes(&self, node: NodeIndex) -> impl Iterator<Item = NodeIndex> + '_ {
+        self.graph.neighbors(node)
     }
 
     pub fn collect_root_ancestors(&self, node: NodeIndex) -> Vec<NodeIndex> {
         let mut visited = HashSet::new();
         self.get_root_ancestors(node, node, &mut visited)
     }
 
+    // TODO: impl Iterator
+    fn parent_node_indexes(&self, node: NodeIndex) -> Vec<NodeIndex> {
+        self.graph.neighbors_directed(node, Direction::Incoming).collect()
+    }
+
     fn get_root_ancestors(&self, initial: NodeIndex, node: NodeIndex, visited: &mut HashSet<NodeIndex>) -> Vec<NodeIndex> {
         if node == initial && !visited.is_empty() {
             return vec![];
@@ -163,10 +146,36 @@ impl CachedStableGraph {
     }
 }
 
+#[cfg(test)]
+impl CachedStableGraph {
+    fn parent_node_names(&self, node: NodeIndex) -> Vec<PathBuf> {
+        self.graph
+            .neighbors_directed(node, Direction::Incoming)
+            .map(|n| self.reverse_index.get(&n).unwrap().clone())
+            .collect()
+    }
+
+    fn child_node_names(&self, node: NodeIndex) -> Vec<PathBuf> {
+        self.graph
+            .neighbors(node)
+            .map(|n| self.reverse_index.get(&n).unwrap().clone())
+            .collect()
+    }
+
+    fn remove_node(&mut self, name: &Path) {
+        let idx = self.cache.remove(name);
+        if let Some(idx) = idx {
+            self.graph.remove_node(idx);
+        }
+    }
+}
+
 #[cfg(test)]
 mod graph_test {
     use std::path::PathBuf;
 
+    use petgraph::graph::NodeIndex;
+
     use crate::{graph::CachedStableGraph, IncludePosition};
 
     #[test]
@@ -182,7 +191,7 @@ mod graph_test {
         assert_eq!(children.len(), 1);
         assert_eq!(children[0], Into::<PathBuf>::into("banana".to_string()));
 
-        let children = graph.child_node_indexes(idx1);
+        let children: Vec<NodeIndex> = graph.child_node_indexes(idx1).collect();
         assert_eq!(children.len(), 1);
         assert_eq!(children[0], idx2);
 
@@ -207,10 +216,33 @@ mod graph_test {
         graph.remove_node(&PathBuf::from("sample"));
         assert_eq!(graph.graph.node_count(), 1);
         assert!(graph.find_node(&PathBuf::from("sample")).is_none());
+
         let neighbors = graph.child_node_names(idx2);
         assert_eq!(neighbors.len(), 0);
     }
 
+    #[test]
+    #[logging_macro::log_scope]
+    fn test_double_import() {
+        let mut graph = CachedStableGraph::new();
+
+        let idx0 = graph.add_node(&PathBuf::from("0"));
+        let idx1 = graph.add_node(&PathBuf::from("1"));
+
+        graph.add_edge(idx0, idx1, IncludePosition { line: 2, start: 0, end: 0 });
+        graph.add_edge(idx0, idx1, IncludePosition { line: 4, start: 0, end: 0 });
+
+        //     0
+        //    / \
+        //   1   1
+
+        assert_eq!(2, graph.get_edge_metas(idx0, idx1).count());
+
+        let mut edge_metas = graph.get_edge_metas(idx0, idx1);
+        assert_eq!(Some(IncludePosition { line: 2, start: 0, end: 0 }), edge_metas.next());
+        assert_eq!(Some(IncludePosition { line: 4, start: 0, end: 0 }), edge_metas.next());
+    }
+
     #[test]
     #[logging_macro::log_scope]
     fn test_collect_root_ancestors() {
@@ -287,10 +319,8 @@ mod graph_test {
             graph.add_edge(idx1, idx3, IncludePosition { line: 5, start: 0, end: 0 });
 
             //       0
-            //       |
-            //       1
-            //       \
-            //     2  \
+            //        \
+            //     2   1
             //      \ /
             //       3