From c32aa9a62a7871e5e920c29e65590c93f15e3171 Mon Sep 17 00:00:00 2001
From: Jonathan Tan <jonathantanmy@google.com>
Date: Mon, 2 Sep 2024 19:29:25 -0700
Subject: [PATCH] RevsetExpressionEvaluator: extract resolve() from evaluate()

This was noticed at work while implementing an API, when I needed to
resolve an expression without evaluating it (because the result of
resolution needs to be part of another expression, and is not used
directly as an iterator of commit IDs).
---
 cli/src/revset_util.rs | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs
index 6d3eb93b9..461ddd822 100644
--- a/cli/src/revset_util.rs
+++ b/cli/src/revset_util.rs
@@ -25,6 +25,7 @@ use jj_lib::id_prefix::IdPrefixContext;
 use jj_lib::repo::Repo;
 use jj_lib::revset;
 use jj_lib::revset::DefaultSymbolResolver;
+use jj_lib::revset::ResolvedExpression;
 use jj_lib::revset::Revset;
 use jj_lib::revset::RevsetAliasesMap;
 use jj_lib::revset::RevsetCommitRef;
@@ -90,14 +91,19 @@ impl<'repo> RevsetExpressionEvaluator<'repo> {
         self.expression = self.expression.intersection(other);
     }
 
-    /// Evaluates the expression.
-    pub fn evaluate(&self) -> Result<Box<dyn Revset + 'repo>, UserRevsetEvaluationError> {
+    /// Resolves the expression.
+    pub fn resolve(&self) -> Result<ResolvedExpression, UserRevsetEvaluationError> {
         let symbol_resolver = default_symbol_resolver(
             self.repo,
             self.extensions.symbol_resolvers(),
             self.id_prefix_context,
         );
-        evaluate(self.repo, &symbol_resolver, self.expression.clone())
+        resolve(self.repo, &symbol_resolver, self.expression.clone())
+    }
+
+    /// Evaluates the expression.
+    pub fn evaluate(&self) -> Result<Box<dyn Revset + 'repo>, UserRevsetEvaluationError> {
+        evaluate_resolved(self.repo, &self.resolve()?)
     }
 
     /// Evaluates the expression to an iterator over commit ids. Entries are
@@ -175,17 +181,31 @@ pub fn load_revset_aliases(
     Ok(aliases_map)
 }
 
+fn resolve(
+    repo: &dyn Repo,
+    symbol_resolver: &DefaultSymbolResolver,
+    expression: Rc<RevsetExpression>,
+) -> Result<ResolvedExpression, UserRevsetEvaluationError> {
+    revset::optimize(expression)
+        .resolve_user_expression(repo, symbol_resolver)
+        .map_err(UserRevsetEvaluationError::Resolution)
+}
+
+fn evaluate_resolved<'a>(
+    repo: &'a dyn Repo,
+    resolved_expression: &ResolvedExpression,
+) -> Result<Box<dyn Revset + 'a>, UserRevsetEvaluationError> {
+    resolved_expression
+        .evaluate(repo)
+        .map_err(UserRevsetEvaluationError::Evaluation)
+}
+
 pub fn evaluate<'a>(
     repo: &'a dyn Repo,
     symbol_resolver: &DefaultSymbolResolver,
     expression: Rc<RevsetExpression>,
 ) -> Result<Box<dyn Revset + 'a>, UserRevsetEvaluationError> {
-    let resolved = revset::optimize(expression)
-        .resolve_user_expression(repo, symbol_resolver)
-        .map_err(UserRevsetEvaluationError::Resolution)?;
-    resolved
-        .evaluate(repo)
-        .map_err(UserRevsetEvaluationError::Evaluation)
+    evaluate_resolved(repo, &resolve(repo, symbol_resolver, expression)?)
 }
 
 /// Wraps the given `IdPrefixContext` in `SymbolResolver` to be passed in to