merged_tree: add an iterator over conflicts

With `MergedTree`, we can iterate over conflicts by descending into
only the subdirectories that cannot be trivially resolved. We assume
that the trees have previously been resolved as much as possible, so
we don't attempt to resolve conflicts again.

lib/src/conflicts.rs

@@ -87,6 +87,11 @@ impl<T> Conflict<T> {
         &self.adds
     }
 
+    /// Whether this conflict is resolved. Does not resolve trivial conflicts.
+    pub fn is_resolved(&self) -> bool {
+        self.removes.is_empty()
+    }
+
     /// Returns the resolved value, if this conflict is resolved. Does not
     /// resolve trivial conflicts.
     pub fn as_resolved(&self) -> Option<&T> {

lib/src/merged_tree.rs

@@ -16,18 +16,20 @@
 
 use std::cmp::max;
 use std::sync::Arc;
+use std::{iter, vec};
 
 use itertools::Itertools;
 
 use crate::backend;
-use crate::backend::TreeValue;
+use crate::backend::{ConflictId, TreeValue};
 use crate::conflicts::Conflict;
-use crate::repo_path::{RepoPath, RepoPathComponent};
+use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
 use crate::store::Store;
 use crate::tree::{try_resolve_file_conflict, Tree, TreeMergeError};
 use crate::tree_builder::TreeBuilder;
 
 /// Presents a view of a merged set of trees.
+#[derive(Clone, Debug)]
 pub enum MergedTree {
     /// A single tree, possibly with path-level conflicts.
     Legacy(Tree),
@@ -130,6 +132,22 @@ impl MergedTree {
         ))
     }
 
+    /// This tree's directory
+    pub fn dir(&self) -> &RepoPath {
+        match self {
+            MergedTree::Legacy(tree) => tree.dir(),
+            MergedTree::Merge(conflict) => conflict.adds()[0].dir(),
+        }
+    }
+
+    /// The `Store` associated with this tree.
+    pub fn store(&self) -> &Arc<Store> {
+        match self {
+            MergedTree::Legacy(tree) => tree.store(),
+            MergedTree::Merge(conflict) => conflict.adds()[0].store(),
+        }
+    }
+
     /// The value at the given basename. The value can be `Resolved` even if
     /// `self` is a `Conflict`, which happens if the value at the path can be
     /// trivially merged. Does not recurse, so if `basename` refers to a Tree,
@@ -167,6 +185,22 @@ impl MergedTree {
             MergedTree::Merge(conflict) => merge_trees(conflict),
         }
     }
+
+    /// An iterator over the conflicts in this tree, including subtrees.
+    /// Recurses into subtrees and yields conflicts in those, but only if
+    /// all sides are trees, so tree/file conflicts will be reported as a single
+    /// conflict, not one for each path in the tree.
+    // TODO: Restrict this by a matcher (or add a separate method for that).
+    pub fn conflicts(&self) -> impl Iterator<Item = (RepoPath, Conflict<Option<TreeValue>>)> {
+        ConflictIterator::new(self.clone())
+    }
+}
+
+fn all_tree_conflict_names(conflict: &Conflict<Tree>) -> impl Iterator<Item = &RepoPathComponent> {
+    itertools::chain(conflict.removes(), conflict.adds())
+        .map(|tree| tree.data().names())
+        .kmerge()
+        .dedup()
 }
 
 fn merge_trees(conflict: &Conflict<Tree>) -> Result<Conflict<Tree>, TreeMergeError> {
@@ -174,11 +208,6 @@ fn merge_trees(conflict: &Conflict<Tree>) -> Result<Conflict<Tree>, TreeMergeErr
         return Ok(Conflict::resolved(tree.clone()));
     }
 
-    let base_names = itertools::chain(conflict.removes(), conflict.adds())
-        .map(|tree| tree.data().names())
-        .kmerge()
-        .dedup();
-
     let base_tree = &conflict.adds()[0];
     let store = base_tree.store();
     let dir = base_tree.dir();
@@ -187,7 +216,7 @@ fn merge_trees(conflict: &Conflict<Tree>) -> Result<Conflict<Tree>, TreeMergeErr
     // any conflicts.
     let mut new_tree = backend::Tree::default();
     let mut conflicts = vec![];
-    for basename in base_names {
+    for basename in all_tree_conflict_names(conflict) {
         let path_conflict = conflict.map(|tree| tree.value(basename).cloned());
         let path_conflict = merge_tree_values(store, dir, path_conflict)?;
         if let Some(value) = path_conflict.as_resolved() {
@@ -257,3 +286,137 @@ fn merge_tree_values(
         }
     }
 }
+
+struct ConflictEntriesNonRecursiveIterator<'a> {
+    merged_tree: &'a MergedTree,
+    basename_iter: Box<dyn Iterator<Item = &'a RepoPathComponent> + 'a>,
+}
+
+impl<'a> ConflictEntriesNonRecursiveIterator<'a> {
+    fn new(merged_tree: &'a MergedTree) -> Self {
+        let basename_iter: Box<dyn Iterator<Item = &'a RepoPathComponent> + 'a> = match merged_tree
+        {
+            MergedTree::Legacy(tree) => Box::new(
+                tree.entries_non_recursive()
+                    .filter(|entry| matches!(entry.value(), &TreeValue::Conflict(_)))
+                    .map(|entry| entry.name()),
+            ),
+            MergedTree::Merge(conflict) => {
+                if conflict.is_resolved() {
+                    Box::new(iter::empty())
+                } else {
+                    Box::new(all_tree_conflict_names(conflict))
+                }
+            }
+        };
+        ConflictEntriesNonRecursiveIterator {
+            merged_tree,
+            basename_iter,
+        }
+    }
+}
+
+impl<'a> Iterator for ConflictEntriesNonRecursiveIterator<'a> {
+    type Item = (&'a RepoPathComponent, Conflict<Option<TreeValue>>);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        for basename in self.basename_iter.by_ref() {
+            match self.merged_tree.value(basename) {
+                MergedTreeValue::Resolved(_) => {}
+                MergedTreeValue::Conflict(conflict) => {
+                    return Some((basename, conflict));
+                }
+            }
+        }
+        None
+    }
+}
+
+/// The state for the non-recursive iteration over the conflicted entries in a
+/// single directory.
+struct ConflictsDirItem {
+    entry_iterator: ConflictEntriesNonRecursiveIterator<'static>,
+    // On drop, tree must outlive entry_iterator
+    tree: Box<MergedTree>,
+}
+
+impl ConflictsDirItem {
+    fn new(tree: MergedTree) -> Self {
+        // Put the tree in a box so it doesn't move if `ConflictsDirItem` moves.
+        let tree = Box::new(tree);
+        let entry_iterator = ConflictEntriesNonRecursiveIterator::new(&tree);
+        let entry_iterator: ConflictEntriesNonRecursiveIterator<'static> =
+            unsafe { std::mem::transmute(entry_iterator) };
+        Self {
+            entry_iterator,
+            tree,
+        }
+    }
+}
+
+enum ConflictIterator {
+    Legacy {
+        store: Arc<Store>,
+        conflicts_iter: vec::IntoIter<(RepoPath, ConflictId)>,
+    },
+    Merge {
+        stack: Vec<ConflictsDirItem>,
+    },
+}
+
+impl ConflictIterator {
+    fn new(tree: MergedTree) -> Self {
+        match tree {
+            MergedTree::Legacy(tree) => ConflictIterator::Legacy {
+                store: tree.store().clone(),
+                conflicts_iter: tree.conflicts().into_iter(),
+            },
+            MergedTree::Merge(_) => ConflictIterator::Merge {
+                stack: vec![ConflictsDirItem::new(tree)],
+            },
+        }
+    }
+}
+
+impl Iterator for ConflictIterator {
+    type Item = (RepoPath, Conflict<Option<TreeValue>>);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        match self {
+            ConflictIterator::Legacy {
+                store,
+                conflicts_iter,
+            } => {
+                if let Some((path, conflict_id)) = conflicts_iter.next() {
+                    // TODO: propagate errors
+                    let conflict = store.read_conflict(&path, &conflict_id).unwrap();
+                    Some((path, conflict))
+                } else {
+                    None
+                }
+            }
+            ConflictIterator::Merge { stack } => {
+                while let Some(top) = stack.last_mut() {
+                    if let Some((basename, conflict)) = top.entry_iterator.next() {
+                        let path = top.tree.dir().join(basename);
+                        // TODO: propagate errors
+                        if let Some(tree_conflict) =
+                            conflict.to_tree_conflict(top.tree.store(), &path).unwrap()
+                        {
+                            // If all sides are trees or missing, descend into the merged tree
+                            stack.push(ConflictsDirItem::new(MergedTree::Merge(tree_conflict)));
+                        } else {
+                            // Otherwise this is a conflict between files, trees, etc. If they could
+                            // be automatically resolved, they should have been when the top-level
+                            // tree conflict was written, so we assume that they can't be.
+                            return Some((path, conflict));
+                        }
+                    } else {
+                        stack.pop();
+                    }
+                }
+                None
+            }
+        }
+    }
+}

lib/tests/test_merged_tree.rs

@@ -18,6 +18,7 @@ use jj_lib::conflicts::Conflict;
 use jj_lib::merged_tree::{MergedTree, MergedTreeValue};
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
+use jj_lib::tree::merge_trees;
 use testutils::{write_file, write_normal_file, TestRepo};
 
 fn file_value(file_id: &FileId) -> TreeValue {
@@ -293,3 +294,195 @@ fn test_resolve_with_conflict() {
         Conflict::new(vec![expected_base1], vec![expected_side1, expected_side2])
     )
 }
+
+#[test]
+fn test_conflict_iterator() {
+    let test_repo = TestRepo::init(true);
+    let repo = &test_repo.repo;
+
+    let unchanged_path = RepoPath::from_internal_string("dir/subdir/unchanged");
+    let trivial_path = RepoPath::from_internal_string("dir/subdir/trivial");
+    let trivial_hunk_path = RepoPath::from_internal_string("dir/non_trivial");
+    let file_conflict_path = RepoPath::from_internal_string("dir/subdir/file_conflict");
+    let modify_delete_path = RepoPath::from_internal_string("dir/subdir/modify_delete");
+    let same_add_path = RepoPath::from_internal_string("dir/subdir/same_add");
+    let different_add_path = RepoPath::from_internal_string("dir/subdir/different_add");
+    let dir_file_path = RepoPath::from_internal_string("dir/subdir/dir_file");
+    let added_dir_path = RepoPath::from_internal_string("dir/new_dir");
+    let modify_delete_dir_path = RepoPath::from_internal_string("dir/modify_delete_dir");
+    let base1 = testutils::create_tree(
+        repo,
+        &[
+            (&unchanged_path, "unchanged"),
+            (&trivial_path, "base"),
+            (&trivial_hunk_path, "line1\nline2\nline3\n"),
+            (&file_conflict_path, "base"),
+            (&modify_delete_path, "base"),
+            // no same_add_path
+            // no different_add_path
+            (&dir_file_path, "base"),
+            // no added_dir_path
+            (
+                &modify_delete_dir_path.join(&RepoPathComponent::from("base")),
+                "base",
+            ),
+        ],
+    );
+    let side1 = testutils::create_tree(
+        repo,
+        &[
+            (&unchanged_path, "unchanged"),
+            (&trivial_path, "base"),
+            (&file_conflict_path, "side1"),
+            (&trivial_hunk_path, "line1 side1\nline2\nline3\n"),
+            (&modify_delete_path, "modified"),
+            (&same_add_path, "same"),
+            (&different_add_path, "side1"),
+            (&dir_file_path, "side1"),
+            (
+                &added_dir_path.join(&RepoPathComponent::from("side1")),
+                "side1",
+            ),
+            (
+                &modify_delete_dir_path.join(&RepoPathComponent::from("side1")),
+                "side1",
+            ),
+        ],
+    );
+    let side2 = testutils::create_tree(
+        repo,
+        &[
+            (&unchanged_path, "unchanged"),
+            (&trivial_path, "side2"),
+            (&file_conflict_path, "side2"),
+            (&trivial_hunk_path, "line1\nline2\nline3 side2\n"),
+            // no modify_delete_path
+            (&same_add_path, "same"),
+            (&different_add_path, "side2"),
+            (&dir_file_path.join(&RepoPathComponent::from("dir")), "new"),
+            (
+                &added_dir_path.join(&RepoPathComponent::from("side2")),
+                "side2",
+            ),
+            // no modify_delete_dir_path
+        ],
+    );
+
+    let tree = MergedTree::new(Conflict::new(
+        vec![base1.clone()],
+        vec![side1.clone(), side2.clone()],
+    ));
+    let conflicts = tree.conflicts().collect_vec();
+    let conflict_at = |path: &RepoPath| {
+        Conflict::new(
+            vec![base1.path_value(path)],
+            vec![side1.path_value(path), side2.path_value(path)],
+        )
+    };
+    // We initially also get a conflict in trivial_hunk_path because we had
+    // forgotten to resolve conflicts
+    assert_eq!(
+        conflicts,
+        vec![
+            (trivial_hunk_path.clone(), conflict_at(&trivial_hunk_path)),
+            (different_add_path.clone(), conflict_at(&different_add_path)),
+            (dir_file_path.clone(), conflict_at(&dir_file_path)),
+            (file_conflict_path.clone(), conflict_at(&file_conflict_path)),
+            (modify_delete_path.clone(), conflict_at(&modify_delete_path)),
+        ]
+    );
+
+    // After we resolve conflicts, there are only non-trivial conflicts left
+    let tree = MergedTree::Merge(tree.resolve().unwrap());
+    let conflicts = tree.conflicts().collect_vec();
+    assert_eq!(
+        conflicts,
+        vec![
+            (different_add_path.clone(), conflict_at(&different_add_path)),
+            (dir_file_path.clone(), conflict_at(&dir_file_path)),
+            (file_conflict_path.clone(), conflict_at(&file_conflict_path)),
+            (modify_delete_path.clone(), conflict_at(&modify_delete_path)),
+        ]
+    );
+
+    let merged_legacy_tree = merge_trees(&side1, &base1, &side2).unwrap();
+    let legacy_conflicts = MergedTree::legacy(merged_legacy_tree)
+        .conflicts()
+        .collect_vec();
+    assert_eq!(legacy_conflicts, conflicts);
+}
+#[test]
+fn test_conflict_iterator_higher_arity() {
+    let test_repo = TestRepo::init(true);
+    let repo = &test_repo.repo;
+
+    let two_sided_path = RepoPath::from_internal_string("dir/2-sided");
+    let three_sided_path = RepoPath::from_internal_string("dir/3-sided");
+    let base1 = testutils::create_tree(
+        repo,
+        &[(&two_sided_path, "base1"), (&three_sided_path, "base1")],
+    );
+    let base2 = testutils::create_tree(
+        repo,
+        &[(&two_sided_path, "base2"), (&three_sided_path, "base2")],
+    );
+    let side1 = testutils::create_tree(
+        repo,
+        &[(&two_sided_path, "side1"), (&three_sided_path, "side1")],
+    );
+    let side2 = testutils::create_tree(
+        repo,
+        &[(&two_sided_path, "base1"), (&three_sided_path, "side2")],
+    );
+    let side3 = testutils::create_tree(
+        repo,
+        &[(&two_sided_path, "side3"), (&three_sided_path, "side3")],
+    );
+
+    let tree = MergedTree::new(Conflict::new(
+        vec![base1.clone(), base2.clone()],
+        vec![side1.clone(), side2.clone(), side3.clone()],
+    ));
+    let conflicts = tree.conflicts().collect_vec();
+    let conflict_at = |path: &RepoPath| {
+        Conflict::new(
+            vec![base1.path_value(path), base2.path_value(path)],
+            vec![
+                side1.path_value(path),
+                side2.path_value(path),
+                side3.path_value(path),
+            ],
+        )
+    };
+    // Both paths have the full, unsimplified conflict (3-sided)
+    assert_eq!(
+        conflicts,
+        vec![
+            (two_sided_path.clone(), conflict_at(&two_sided_path)),
+            (three_sided_path.clone(), conflict_at(&three_sided_path))
+        ]
+    );
+    // Iterating over conflicts in a legacy tree yields the simplified conflict at
+    // each path
+    let merged_legacy_tree = merge_trees(&side1, &base1, &side2).unwrap();
+    let merged_legacy_tree = merge_trees(&merged_legacy_tree, &base2, &side3).unwrap();
+    let legacy_conflicts = MergedTree::legacy(merged_legacy_tree)
+        .conflicts()
+        .collect_vec();
+    assert_eq!(
+        legacy_conflicts,
+        vec![
+            (
+                two_sided_path.clone(),
+                Conflict::new(
+                    vec![base2.path_value(&two_sided_path)],
+                    vec![
+                        side1.path_value(&two_sided_path),
+                        side3.path_value(&two_sided_path),
+                    ],
+                )
+            ),
+            (three_sided_path.clone(), conflict_at(&three_sided_path))
+        ]
+    );
+}