merge: introduce a type alias for Merge<Option<TreeValue>>

Reasons to introduce this alias:

* Reduces complexity of a type, to silence Clippy warnings in the
  future if we use this type as a type parameter

* The type is used quite frequently, so it makes sense to have a name
  for it

* It's easier to visually scan for the end of the type when you don't
  have to match opening and closing angle brackets

cli/src/commands/mod.rs

@@ -37,7 +37,7 @@ use jj_lib::commit::Commit;
 use jj_lib::dag_walk::topo_order_reverse;
 use jj_lib::git_backend::GitBackend;
 use jj_lib::matchers::EverythingMatcher;
-use jj_lib::merge::Merge;
+use jj_lib::merge::{Merge, MergedTreeValue};
 use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder};
 use jj_lib::op_store::WorkspaceId;
 use jj_lib::repo::{ReadonlyRepo, Repo};
@@ -3053,7 +3053,7 @@ fn cmd_resolve(
 
 #[instrument(skip_all)]
 fn print_conflicted_paths(
-    conflicts: &[(RepoPath, Merge<Option<TreeValue>>)],
+    conflicts: &[(RepoPath, MergedTreeValue)],
     formatter: &mut dyn Formatter,
     workspace_command: &WorkspaceCommandHelper,
 ) -> Result<(), CommandError> {

cli/src/diff_util.rs

@@ -25,7 +25,7 @@ use jj_lib::commit::Commit;
 use jj_lib::diff::{Diff, DiffHunk};
 use jj_lib::files::DiffLine;
 use jj_lib::matchers::Matcher;
-use jj_lib::merge::Merge;
+use jj_lib::merge::{Merge, MergedTreeValue};
 use jj_lib::merged_tree::{MergedTree, TreeDiffIterator};
 use jj_lib::repo::{ReadonlyRepo, Repo};
 use jj_lib::repo_path::RepoPath;
@@ -345,7 +345,7 @@ fn show_color_words_diff_line(
 fn diff_content(
     repo: &Arc<ReadonlyRepo>,
     path: &RepoPath,
-    value: &Merge<Option<TreeValue>>,
+    value: &MergedTreeValue,
 ) -> Result<Vec<u8>, CommandError> {
     match value.as_resolved() {
         Some(None) => Ok(vec![]),
@@ -379,7 +379,7 @@ fn diff_content(
     }
 }
 
-fn basic_diff_file_type(values: &Merge<Option<TreeValue>>) -> String {
+fn basic_diff_file_type(values: &MergedTreeValue) -> String {
     match values.as_resolved() {
         Some(None) => {
             panic!("absent path in diff");
@@ -493,7 +493,7 @@ struct GitDiffPart {
 fn git_diff_part(
     repo: &Arc<ReadonlyRepo>,
     path: &RepoPath,
-    value: &Merge<Option<TreeValue>>,
+    value: &MergedTreeValue,
 ) -> Result<GitDiffPart, CommandError> {
     let mode;
     let hash;
@@ -886,7 +886,7 @@ pub fn show_types(
     })
 }
 
-fn diff_summary_char(value: &Merge<Option<TreeValue>>) -> char {
+fn diff_summary_char(value: &MergedTreeValue) -> char {
     match value.as_resolved() {
         Some(None) => '-',
         Some(Some(TreeValue::File { .. })) => 'F',

cli/src/merge_tools/builtin.rs

@@ -544,6 +544,7 @@ pub fn edit_merge_builtin(
 mod tests {
     use futures::executor::block_on;
     use jj_lib::conflicts::extract_as_single_hunk;
+    use jj_lib::merge::MergedTreeValue;
     use jj_lib::repo::Repo;
     use testutils::TestRepo;
 
@@ -710,7 +711,7 @@ mod tests {
             &[(&path, "right 1\nbase 2\nbase 3\nbase 4\nright 5\n")],
         );
 
-        fn to_file_id(tree_value: Merge<Option<TreeValue>>) -> Option<FileId> {
+        fn to_file_id(tree_value: MergedTreeValue) -> Option<FileId> {
             match tree_value.into_resolved() {
                 Ok(Some(TreeValue::File { id, executable: _ })) => Some(id.clone()),
                 other => {

cli/src/merge_tools/external.rs

@@ -13,7 +13,7 @@ use jj_lib::conflicts::{self, materialize_merge_result};
 use jj_lib::gitignore::GitIgnoreFile;
 use jj_lib::local_working_copy::{TreeState, TreeStateError};
 use jj_lib::matchers::Matcher;
-use jj_lib::merge::Merge;
+use jj_lib::merge::{Merge, MergedTreeValue};
 use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder};
 use jj_lib::repo_path::RepoPath;
 use jj_lib::settings::UserSettings;
@@ -291,7 +291,7 @@ pub fn run_mergetool_external(
     file_merge: Merge<Option<FileId>>,
     content: Merge<jj_lib::files::ContentHunk>,
     repo_path: &RepoPath,
-    conflict: Merge<Option<TreeValue>>,
+    conflict: MergedTreeValue,
     tree: &MergedTree,
 ) -> Result<MergedTreeId, ConflictResolveError> {
     let initial_output_content: Vec<u8> = if editor.merge_tool_edits_conflict_markers {

lib/src/conflicts.rs

@@ -20,11 +20,11 @@ use std::iter::zip;
 use futures::StreamExt;
 use itertools::Itertools;
 
-use crate::backend::{BackendResult, FileId, TreeValue};
+use crate::backend::{BackendResult, FileId};
 use crate::diff::{find_line_ranges, Diff, DiffHunk};
 use crate::files;
 use crate::files::{ContentHunk, MergeResult};
-use crate::merge::{Merge, MergeBuilder};
+use crate::merge::{Merge, MergeBuilder, MergedTreeValue};
 use crate::repo_path::RepoPath;
 use crate::store::Store;
 
@@ -89,7 +89,7 @@ pub async fn extract_as_single_hunk(
 }
 
 pub async fn materialize(
-    conflict: &Merge<Option<TreeValue>>,
+    conflict: &MergedTreeValue,
     store: &Store,
     path: &RepoPath,
     output: &mut dyn Write,

lib/src/local_working_copy.rs

@@ -53,7 +53,7 @@ use crate::lock::FileLock;
 use crate::matchers::{
     DifferenceMatcher, EverythingMatcher, IntersectionMatcher, Matcher, PrefixMatcher,
 };
-use crate::merge::{Merge, MergeBuilder};
+use crate::merge::{Merge, MergeBuilder, MergedTreeValue};
 use crate::merged_tree::{MergedTree, MergedTreeBuilder};
 use crate::op_store::{OperationId, WorkspaceId};
 use crate::repo_path::{FsPathParseError, RepoPath, RepoPathComponent, RepoPathJoin};
@@ -662,7 +662,7 @@ impl TreeState {
         &self,
         matcher: &dyn Matcher,
         current_tree: &MergedTree,
-        tree_entries_tx: Sender<(RepoPath, Merge<Option<TreeValue>>)>,
+        tree_entries_tx: Sender<(RepoPath, MergedTreeValue)>,
         file_states_tx: Sender<(RepoPath, FileState)>,
         present_files_tx: Sender<RepoPath>,
         directory_to_visit: DirectoryToVisit,
@@ -887,7 +887,7 @@ impl TreeState {
         maybe_current_file_state: Option<&FileState>,
         current_tree: &MergedTree,
         new_file_state: &FileState,
-    ) -> Result<Option<Merge<Option<TreeValue>>>, SnapshotError> {
+    ) -> Result<Option<MergedTreeValue>, SnapshotError> {
         let clean = match maybe_current_file_state {
             None => {
                 // untracked
@@ -922,9 +922,9 @@ impl TreeState {
         &self,
         repo_path: &RepoPath,
         disk_path: &Path,
-        current_tree_values: &Merge<Option<TreeValue>>,
+        current_tree_values: &MergedTreeValue,
         file_type: FileType,
-    ) -> Result<Merge<Option<TreeValue>>, SnapshotError> {
+    ) -> Result<MergedTreeValue, SnapshotError> {
         let executable = match file_type {
             FileType::Normal { executable } => executable,
             FileType::Symlink => {
@@ -1052,7 +1052,7 @@ impl TreeState {
         &self,
         disk_path: &Path,
         path: &RepoPath,
-        conflict: &Merge<Option<TreeValue>>,
+        conflict: &MergedTreeValue,
     ) -> Result<FileState, CheckoutError> {
         let mut file = OpenOptions::new()
             .write(true)

lib/src/merge.rs

@@ -414,7 +414,13 @@ impl<T: ContentHash> ContentHash for Merge<T> {
     }
 }
 
-impl Merge<Option<TreeValue>> {
+/// The value at a given path in a commit. It depends on the context whether it
+/// can be absent (`Merge::is_absent()`). For example, when getting the value at
+/// a specific path, it may be, but when iterating over entries in a tree, it
+/// shouldn't be.
+pub type MergedTreeValue = Merge<Option<TreeValue>>;
+
+impl MergedTreeValue {
     /// Create a `Merge` from a `backend::Conflict`, padding with `None` to
     /// make sure that there is exactly one more `adds()` than `removes()`.
     pub fn from_backend_conflict(conflict: backend::Conflict) -> Self {
@@ -497,7 +503,7 @@ impl<T> Merge<Option<T>>
 where
     T: Borrow<TreeValue>,
 {
-    /// If every non-`None` term of a `Merge<Option<TreeValue>>`
+    /// If every non-`None` term of a `MergedTreeValue`
     /// is a `TreeValue::Tree`, this converts it to
     /// a `Merge<Tree>`, with empty trees instead of
     /// any `None` terms. Otherwise, returns `None`.

lib/src/merged_tree.rs

@@ -26,7 +26,7 @@ use itertools::Itertools;
 
 use crate::backend::{BackendError, BackendResult, ConflictId, MergedTreeId, TreeId, TreeValue};
 use crate::matchers::{EverythingMatcher, Matcher};
-use crate::merge::{Merge, MergeBuilder};
+use crate::merge::{Merge, MergeBuilder, MergedTreeValue};
 use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
 use crate::store::Store;
 use crate::tree::{try_resolve_file_conflict, Tree, TreeMergeError};
@@ -50,11 +50,11 @@ pub enum MergedTreeVal<'a> {
     Resolved(Option<&'a TreeValue>),
     /// TODO: Make this a `Merge<Option<&'a TreeValue>>` (reference to the
     /// value) once we have removed the `MergedTree::Legacy` variant.
-    Conflict(Merge<Option<TreeValue>>),
+    Conflict(MergedTreeValue),
 }
 
 impl MergedTreeVal<'_> {
-    fn to_merge(&self) -> Merge<Option<TreeValue>> {
+    fn to_merge(&self) -> MergedTreeValue {
         match self {
             MergedTreeVal::Resolved(value) => Merge::resolved(value.cloned()),
             MergedTreeVal::Conflict(merge) => merge.clone(),
@@ -110,7 +110,7 @@ impl MergedTree {
         // build `2*num_removes + 1` trees
         let mut max_num_removes = 0;
         let store = tree.store();
-        let mut conflicts: Vec<(&RepoPath, Merge<Option<TreeValue>>)> = vec![];
+        let mut conflicts: Vec<(&RepoPath, MergedTreeValue)> = vec![];
         for (path, conflict_id) in &conflict_ids {
             let conflict = store.read_conflict(path, conflict_id)?;
             max_num_removes = max(max_num_removes, conflict.removes().len());
@@ -218,7 +218,7 @@ impl MergedTree {
     /// 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, Merge<Option<TreeValue>>)> {
+    pub fn conflicts(&self) -> impl Iterator<Item = (RepoPath, MergedTreeValue)> {
         ConflictIterator::new(self.clone())
     }
 
@@ -265,7 +265,7 @@ impl MergedTree {
     /// The value at the given path. The value can be `Resolved` even if
     /// `self` is a `Conflict`, which happens if the value at the path can be
     /// trivially merged.
-    pub fn path_value(&self, path: &RepoPath) -> Merge<Option<TreeValue>> {
+    pub fn path_value(&self, path: &RepoPath) -> MergedTreeValue {
         assert_eq!(self.dir(), &RepoPath::root());
         match path.split() {
             Some((dir, basename)) => match self.sub_tree_recursive(dir.components()) {
@@ -475,8 +475,8 @@ fn merge_trees(merge: &Merge<Tree>) -> Result<Merge<Tree>, TreeMergeError> {
 fn merge_tree_values(
     store: &Arc<Store>,
     path: &RepoPath,
-    values: Merge<Option<TreeValue>>,
-) -> Result<Merge<Option<TreeValue>>, TreeMergeError> {
+    values: MergedTreeValue,
+) -> Result<MergedTreeValue, TreeMergeError> {
     if let Some(resolved) = values.resolve_trivial() {
         return Ok(Merge::resolved(resolved.clone()));
     }
@@ -563,7 +563,7 @@ impl<'matcher> TreeEntriesIterator<'matcher> {
 }
 
 impl Iterator for TreeEntriesIterator<'_> {
-    type Item = (RepoPath, Merge<Option<TreeValue>>);
+    type Item = (RepoPath, MergedTreeValue);
 
     fn next(&mut self) -> Option<Self::Item> {
         while let Some(top) = self.stack.last_mut() {
@@ -622,7 +622,7 @@ impl<'a> ConflictEntriesNonRecursiveIterator<'a> {
 }
 
 impl<'a> Iterator for ConflictEntriesNonRecursiveIterator<'a> {
-    type Item = (&'a RepoPathComponent, Merge<Option<TreeValue>>);
+    type Item = (&'a RepoPathComponent, MergedTreeValue);
 
     fn next(&mut self) -> Option<Self::Item> {
         for basename in self.basename_iter.by_ref() {
@@ -684,7 +684,7 @@ impl ConflictIterator {
 }
 
 impl Iterator for ConflictIterator {
-    type Item = (RepoPath, Merge<Option<TreeValue>>);
+    type Item = (RepoPath, MergedTreeValue);
 
     fn next(&mut self) -> Option<Self::Item> {
         match self {
@@ -799,7 +799,7 @@ enum TreeDiffItem {
     // This is used for making sure that when a directory gets replaced by a file, we
     // yield the value for the addition of the file after we yield the values
     // for removing files in the directory.
-    File(RepoPath, Merge<Option<TreeValue>>, Merge<Option<TreeValue>>),
+    File(RepoPath, MergedTreeValue, MergedTreeValue),
 }
 
 impl<'matcher> TreeDiffIterator<'matcher> {
@@ -822,11 +822,7 @@ impl<'matcher> TreeDiffIterator<'matcher> {
     }
 
     /// Gets the given tree if `value` is a tree, otherwise an empty tree.
-    async fn tree(
-        tree: &MergedTree,
-        dir: &RepoPath,
-        values: &Merge<Option<TreeValue>>,
-    ) -> MergedTree {
+    async fn tree(tree: &MergedTree, dir: &RepoPath, values: &MergedTreeValue) -> MergedTree {
         let trees = if values.is_tree() {
             let builder: MergeBuilder<Tree> = futures::stream::iter(values.iter())
                 .then(|value| Self::single_tree(tree.store(), dir, value.as_ref()))
@@ -861,7 +857,7 @@ impl TreeDiffDirItem {
 }
 
 impl Iterator for TreeDiffIterator<'_> {
-    type Item = (RepoPath, Merge<Option<TreeValue>>, Merge<Option<TreeValue>>);
+    type Item = (RepoPath, MergedTreeValue, MergedTreeValue);
 
     fn next(&mut self) -> Option<Self::Item> {
         while let Some(top) = self.stack.last_mut() {
@@ -926,7 +922,7 @@ impl Iterator for TreeDiffIterator<'_> {
 /// (allowing path-level conflicts) or as multiple conflict-free trees.
 pub struct MergedTreeBuilder {
     base_tree_id: MergedTreeId,
-    overrides: BTreeMap<RepoPath, Merge<Option<TreeValue>>>,
+    overrides: BTreeMap<RepoPath, MergedTreeValue>,
 }
 
 impl MergedTreeBuilder {
@@ -944,7 +940,7 @@ impl MergedTreeBuilder {
     /// `Merge::absent()` to remove a value from the tree. When the base tree is
     /// a legacy tree, conflicts can be written either as a multi-way `Merge`
     /// value or as a resolved `Merge` value using `TreeValue::Conflict`.
-    pub fn set_or_remove(&mut self, path: RepoPath, values: Merge<Option<TreeValue>>) {
+    pub fn set_or_remove(&mut self, path: RepoPath, values: MergedTreeValue) {
         if let MergedTreeId::Merge(_) = &self.base_tree_id {
             assert!(!values
                 .iter()

lib/src/store.rs

@@ -24,11 +24,10 @@ use futures::executor::block_on;
 
 use crate::backend;
 use crate::backend::{
-    Backend, BackendResult, ChangeId, CommitId, ConflictId, FileId, MergedTreeId, SymlinkId,
-    TreeId, TreeValue,
+    Backend, BackendResult, ChangeId, CommitId, ConflictId, FileId, MergedTreeId, SymlinkId, TreeId,
 };
 use crate::commit::Commit;
-use crate::merge::Merge;
+use crate::merge::{Merge, MergedTreeValue};
 use crate::merged_tree::MergedTree;
 use crate::repo_path::RepoPath;
 use crate::tree::Tree;
@@ -229,7 +228,7 @@ impl Store {
         &self,
         path: &RepoPath,
         id: &ConflictId,
-    ) -> BackendResult<Merge<Option<TreeValue>>> {
+    ) -> BackendResult<MergedTreeValue> {
         let backend_conflict = block_on(self.backend.read_conflict(path, id))?;
         Ok(Merge::from_backend_conflict(backend_conflict))
     }
@@ -237,7 +236,7 @@ impl Store {
     pub fn write_conflict(
         &self,
         path: &RepoPath,
-        contents: &Merge<Option<TreeValue>>,
+        contents: &MergedTreeValue,
     ) -> BackendResult<ConflictId> {
         self.backend
             .write_conflict(path, &contents.clone().into_backend_conflict())

lib/src/tree.rs

@@ -29,7 +29,7 @@ use crate::backend::{
 };
 use crate::files::MergeResult;
 use crate::matchers::{EverythingMatcher, Matcher};
-use crate::merge::{trivial_merge, Merge};
+use crate::merge::{trivial_merge, Merge, MergedTreeValue};
 use crate::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
 use crate::store::Store;
 use crate::{backend, files};
@@ -403,7 +403,7 @@ fn merge_tree_value(
 pub fn try_resolve_file_conflict(
     store: &Store,
     filename: &RepoPath,
-    conflict: &Merge<Option<TreeValue>>,
+    conflict: &MergedTreeValue,
 ) -> Result<Option<TreeValue>, TreeMergeError> {
     // If there are any non-file or any missing parts in the conflict, we can't
     // merge it. We check early so we don't waste time reading file contents if