From 2a3d402d0c05c9bad218c981f94d2c6a08921420 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Tue, 28 Mar 2023 09:50:28 -0700
Subject: [PATCH] revset: also resolve branches(), tags(), etc. when resolving
 symbols

This is another step towards removing the `Repo` argument from
`Index::evaluate_revset()`.
---
 lib/src/default_revset_engine.rs | 61 +++++---------------------------
 lib/src/revset.rs                | 50 ++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 53 deletions(-)

diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs
index 43487229d..b4891866f 100644
--- a/lib/src/default_revset_engine.rs
+++ b/lib/src/default_revset_engine.rs
@@ -505,6 +505,14 @@ fn internal_evaluate<'index>(
     expression: &RevsetExpression,
 ) -> Result<Box<dyn InternalRevset<'index> + 'index>, RevsetError> {
     match expression {
+        RevsetExpression::Symbol(_)
+        | RevsetExpression::Branches(_)
+        | RevsetExpression::RemoteBranches { .. }
+        | RevsetExpression::Tags
+        | RevsetExpression::GitRefs
+        | RevsetExpression::GitHead => {
+            panic!("Expression '{expression:?}' should have been resolved by caller");
+        }
         RevsetExpression::None => Ok(Box::new(EagerRevset::empty())),
         RevsetExpression::All => {
             // Since `all()` does not include hidden commits, some of the logical
@@ -517,9 +525,6 @@ fn internal_evaluate<'index>(
             internal_evaluate(repo, &RevsetExpression::visible_heads().ancestors())
         }
         RevsetExpression::Commits(commit_ids) => Ok(revset_for_commit_ids(repo, commit_ids)),
-        RevsetExpression::Symbol(symbol) => {
-            panic!("Symbol '{}' should have been resolved by caller", symbol);
-        }
         RevsetExpression::Children(roots) => {
             let root_set = internal_evaluate(repo, roots)?;
             let candidates_expression = roots.descendants();
@@ -611,56 +616,6 @@ fn internal_evaluate<'index>(
             repo,
             &repo.view().public_heads().iter().cloned().collect_vec(),
         )),
-        RevsetExpression::Branches(needle) => {
-            let mut commit_ids = vec![];
-            for (branch_name, branch_target) in repo.view().branches() {
-                if !branch_name.contains(needle) {
-                    continue;
-                }
-                if let Some(local_target) = &branch_target.local_target {
-                    commit_ids.extend(local_target.adds());
-                }
-            }
-            Ok(revset_for_commit_ids(repo, &commit_ids))
-        }
-        RevsetExpression::RemoteBranches {
-            branch_needle,
-            remote_needle,
-        } => {
-            let mut commit_ids = vec![];
-            for (branch_name, branch_target) in repo.view().branches() {
-                if !branch_name.contains(branch_needle) {
-                    continue;
-                }
-                for (remote_name, remote_target) in branch_target.remote_targets.iter() {
-                    if remote_name.contains(remote_needle) {
-                        commit_ids.extend(remote_target.adds());
-                    }
-                }
-            }
-            Ok(revset_for_commit_ids(repo, &commit_ids))
-        }
-        RevsetExpression::Tags => {
-            let mut commit_ids = vec![];
-            for ref_target in repo.view().tags().values() {
-                commit_ids.extend(ref_target.adds());
-            }
-            Ok(revset_for_commit_ids(repo, &commit_ids))
-        }
-        RevsetExpression::GitRefs => {
-            let mut commit_ids = vec![];
-            for ref_target in repo.view().git_refs().values() {
-                commit_ids.extend(ref_target.adds());
-            }
-            Ok(revset_for_commit_ids(repo, &commit_ids))
-        }
-        RevsetExpression::GitHead => {
-            let mut commit_ids = vec![];
-            if let Some(ref_target) = repo.view().git_head() {
-                commit_ids.extend(ref_target.adds());
-            }
-            Ok(revset_for_commit_ids(repo, &commit_ids))
-        }
         RevsetExpression::Latest { candidates, count } => {
             let candidate_set = internal_evaluate(repo, candidates)?;
             Ok(take_latest_revset(repo, candidate_set.as_ref(), *count))
diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 5ff9799b7..4459a806e 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -1603,6 +1603,56 @@ pub fn resolve_symbols(
                         resolve_symbol(repo, symbol, workspace_ctx.map(|ctx| ctx.workspace_id))?;
                     Some(RevsetExpression::commits(commit_ids))
                 }
+                RevsetExpression::Branches(needle) => {
+                    let mut commit_ids = vec![];
+                    for (branch_name, branch_target) in repo.view().branches() {
+                        if !branch_name.contains(needle) {
+                            continue;
+                        }
+                        if let Some(local_target) = &branch_target.local_target {
+                            commit_ids.extend(local_target.adds());
+                        }
+                    }
+                    Some(RevsetExpression::commits(commit_ids))
+                }
+                RevsetExpression::RemoteBranches {
+                    branch_needle,
+                    remote_needle,
+                } => {
+                    let mut commit_ids = vec![];
+                    for (branch_name, branch_target) in repo.view().branches() {
+                        if !branch_name.contains(branch_needle) {
+                            continue;
+                        }
+                        for (remote_name, remote_target) in branch_target.remote_targets.iter() {
+                            if remote_name.contains(remote_needle) {
+                                commit_ids.extend(remote_target.adds());
+                            }
+                        }
+                    }
+                    Some(RevsetExpression::commits(commit_ids))
+                }
+                RevsetExpression::Tags => {
+                    let mut commit_ids = vec![];
+                    for ref_target in repo.view().tags().values() {
+                        commit_ids.extend(ref_target.adds());
+                    }
+                    Some(RevsetExpression::commits(commit_ids))
+                }
+                RevsetExpression::GitRefs => {
+                    let mut commit_ids = vec![];
+                    for ref_target in repo.view().git_refs().values() {
+                        commit_ids.extend(ref_target.adds());
+                    }
+                    Some(RevsetExpression::commits(commit_ids))
+                }
+                RevsetExpression::GitHead => {
+                    let mut commit_ids = vec![];
+                    if let Some(ref_target) = repo.view().git_head() {
+                        commit_ids.extend(ref_target.adds());
+                    }
+                    Some(RevsetExpression::commits(commit_ids))
+                }
                 _ => None,
             })
         })?