revset: add at_operation(op, expression)

This can be used in order to refer old working-copy commit, for example. If
we find it's useful, maybe we can add an infix syntax later.

Closes #1283

CHANGELOG.md

@@ -24,6 +24,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 * New command `jj simplify-parents` will remove redundant parent edges.
 
+* New `at_operation(op, expr)` revset can be used in order to query revisions
+  based on historical state.
+
 ### Fixed bugs
 
 * Error on `trunk()` revset resolution is now handled gracefully.

docs/revsets.md

@@ -306,6 +306,12 @@ given [string pattern](#string-patterns).
 
 * `working_copies()`: The working copy commits across all the workspaces.
 
+* `at_operation(op, x)`: Evaluates `x` at the specified [operation][]. For
+  example, `at_operation(@-, visible_heads())` will return all heads which were
+  visible at the previous operation.
+
+[operation]: glossary.md#operation
+
 ??? examples
 
     Given this history:

lib/src/revset.rs

@@ -46,7 +46,10 @@ use crate::object_id::HexPrefix;
 use crate::object_id::PrefixResolution;
 use crate::op_store::RemoteRefState;
 use crate::op_store::WorkspaceId;
+use crate::op_walk;
+use crate::repo::ReadonlyRepo;
 use crate::repo::Repo;
+use crate::repo::RepoLoaderError;
 use crate::repo_path::RepoPathUiConverter;
 use crate::revset_parser;
 pub use crate::revset_parser::expect_literal;
@@ -208,6 +211,13 @@ pub enum RevsetExpression {
     Filter(RevsetFilterPredicate),
     /// Marker for subtree that should be intersected as filter.
     AsFilter(Rc<Self>),
+    /// Resolves symbols and visibility at the specified operation.
+    AtOperation {
+        operation: String,
+        candidates: Rc<Self>,
+        /// Copy of `repo.view().heads()`, should be set by `resolve_symbols()`.
+        visible_heads: Option<Vec<CommitId>>,
+    },
     Present(Rc<Self>),
     NotIn(Rc<Self>),
     Union(Rc<Self>, Rc<Self>),
@@ -429,13 +439,17 @@ impl RevsetExpression {
         Rc::new(Self::Difference(self.clone(), other.clone()))
     }
 
-    /// Resolve a programmatically created revset expression. In particular, the
-    /// expression must not contain any symbols (bookmarks, tags, change/commit
-    /// prefixes). Callers must not include `RevsetExpression::symbol()` in
-    /// the expression, and should instead resolve symbols to `CommitId`s and
-    /// pass them into `RevsetExpression::commits()`. Similarly, the expression
-    /// must not contain any `RevsetExpression::remote_symbol()` or
+    /// Resolve a programmatically created revset expression.
+    ///
+    /// In particular, the expression must not contain any symbols (bookmarks,
+    /// tags, change/commit prefixes). Callers must not include
+    /// `RevsetExpression::symbol()` in the expression, and should instead
+    /// resolve symbols to `CommitId`s and pass them into
+    /// `RevsetExpression::commits()`. Similarly, the expression must not
+    /// contain any `RevsetExpression::remote_symbol()` or
     /// `RevsetExpression::working_copy()`, unless they're known to be valid.
+    /// The expression must not contain `RevsetExpression::AtOperation` even if
+    /// it's known to be valid. It can fail at loading operation data.
     pub fn resolve_programmatic(self: Rc<Self>, repo: &dyn Repo) -> ResolvedExpression {
         let symbol_resolver = FailingSymbolResolver;
         resolve_symbols(repo, self, &symbol_resolver)
@@ -825,6 +839,20 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
         let expression = lower_expression(diagnostics, arg, context)?;
         Ok(Rc::new(RevsetExpression::Present(expression)))
     });
+    map.insert("at_operation", |diagnostics, function, context| {
+        let [op_arg, cand_arg] = function.expect_exact_arguments()?;
+        // TODO: Parse "opset" here if we add proper language support.
+        let operation =
+            revset_parser::expect_expression_with(diagnostics, op_arg, |_diagnostics, node| {
+                Ok(node.span.as_str().to_owned())
+            })?;
+        let candidates = lower_expression(diagnostics, cand_arg, context)?;
+        Ok(Rc::new(RevsetExpression::AtOperation {
+            operation,
+            candidates,
+            visible_heads: None,
+        }))
+    });
     map
 });
 
@@ -1132,6 +1160,17 @@ fn try_transform_expression<E>(
             RevsetExpression::AsFilter(candidates) => {
                 transform_rec(candidates, pre, post)?.map(RevsetExpression::AsFilter)
             }
+            RevsetExpression::AtOperation {
+                operation,
+                candidates,
+                visible_heads,
+            } => transform_rec(candidates, pre, post)?.map(|candidates| {
+                RevsetExpression::AtOperation {
+                    operation: operation.clone(),
+                    candidates,
+                    visible_heads: visible_heads.clone(),
+                }
+            }),
             RevsetExpression::Present(candidates) => {
                 transform_rec(candidates, pre, post)?.map(RevsetExpression::Present)
             }
@@ -1486,6 +1525,24 @@ pub fn walk_revs<'index>(
         .evaluate_programmatic(repo)
 }
 
+fn reload_repo_at_operation(
+    repo: &dyn Repo,
+    op_str: &str,
+) -> Result<Arc<ReadonlyRepo>, RevsetResolutionError> {
+    // TODO: Maybe we should ensure that the resolved operation is an ancestor
+    // of the current operation. If it weren't, there might be commits unknown
+    // to the outer repo.
+    let base_repo = repo.base_repo();
+    let operation = op_walk::resolve_op_with_repo(base_repo, op_str)
+        .map_err(|err| RevsetResolutionError::Other(err.into()))?;
+    base_repo.reload_at(&operation).map_err(|err| match err {
+        RepoLoaderError::Backend(err) => RevsetResolutionError::StoreError(err),
+        RepoLoaderError::IndexRead(_)
+        | RepoLoaderError::OpHeadResolution(_)
+        | RepoLoaderError::OpStore(_) => RevsetResolutionError::Other(err.into()),
+    })
+}
+
 fn resolve_remote_bookmark(repo: &dyn Repo, name: &str, remote: &str) -> Option<Vec<CommitId>> {
     let view = repo.view();
     let target = match (name, remote) {
@@ -1858,6 +1915,22 @@ fn resolve_symbols(
     Ok(try_transform_expression(
         &expression,
         |expression| match expression.as_ref() {
+            // 'at_operation(op, x)' switches symbol resolution contexts.
+            RevsetExpression::AtOperation {
+                operation,
+                candidates,
+                visible_heads: _,
+            } => {
+                let repo = reload_repo_at_operation(repo, operation)?;
+                let candidates =
+                    resolve_symbols(repo.as_ref(), candidates.clone(), symbol_resolver)?;
+                let visible_heads = Some(repo.view().heads().iter().cloned().collect());
+                Ok(Some(Rc::new(RevsetExpression::AtOperation {
+                    operation: operation.clone(),
+                    candidates,
+                    visible_heads,
+                })))
+            }
             // 'present(x)' opens new symbol resolution scope to map error to 'none()'.
             RevsetExpression::Present(candidates) => {
                 resolve_symbols(repo, candidates.clone(), symbol_resolver)
@@ -1898,9 +1971,6 @@ fn resolve_symbols(
 /// return type `ResolvedExpression` is stricter than `RevsetExpression`,
 /// and isn't designed for such transformation.
 fn resolve_visibility(repo: &dyn Repo, expression: &RevsetExpression) -> ResolvedExpression {
-    // If we add "operation" scope (#1283), visible_heads might be translated to
-    // `RevsetExpression::WithinOperation(visible_heads, expression)` node to
-    // evaluate filter predicates and "all()" against that scope.
     let context = VisibilityResolutionContext {
         visible_heads: &repo.view().heads().iter().cloned().collect_vec(),
     };
@@ -1969,6 +2039,17 @@ impl VisibilityResolutionContext<'_> {
                     predicate: self.resolve_predicate(expression),
                 }
             }
+            RevsetExpression::AtOperation {
+                operation: _,
+                candidates,
+                visible_heads,
+            } => {
+                let visible_heads = visible_heads
+                    .as_ref()
+                    .expect("visible_heads should have been resolved by caller");
+                let context = VisibilityResolutionContext { visible_heads };
+                context.resolve(candidates)
+            }
             RevsetExpression::Present(_) => {
                 panic!("Expression '{expression:?}' should have been resolved by caller");
             }
@@ -2045,6 +2126,10 @@ impl VisibilityResolutionContext<'_> {
                 ResolvedPredicateExpression::Filter(predicate.clone())
             }
             RevsetExpression::AsFilter(candidates) => self.resolve_predicate(candidates),
+            // Filters should be intersected with all() within the at-op repo.
+            RevsetExpression::AtOperation { .. } => {
+                ResolvedPredicateExpression::Set(self.resolve(expression).into())
+            }
             RevsetExpression::Present(_) => {
                 panic!("Expression '{expression:?}' should have been resolved by caller")
             }
@@ -3010,6 +3095,15 @@ mod tests {
             optimize(parse("present(bookmarks() & all())").unwrap()),
             @r###"Present(CommitRef(Bookmarks(Substring(""))))"###);
 
+        insta::assert_debug_snapshot!(
+            optimize(parse("at_operation(@-, bookmarks() & all())").unwrap()), @r#"
+        AtOperation {
+            operation: "@-",
+            candidates: CommitRef(Bookmarks(Substring(""))),
+            visible_heads: None,
+        }
+        "#);
+
         insta::assert_debug_snapshot!(
             optimize(parse("~bookmarks() & all()").unwrap()),
             @r###"NotIn(CommitRef(Bookmarks(Substring(""))))"###);
@@ -3480,6 +3574,23 @@ mod tests {
             Filter(Author(Substring("baz"))),
         )
         "###);
+
+        // Filter node shouldn't move across at_operation() boundary.
+        insta::assert_debug_snapshot!(
+            optimize(parse("author(foo) & bar & at_operation(@-, committer(baz))").unwrap()),
+            @r#"
+        Intersection(
+            Intersection(
+                CommitRef(Symbol("bar")),
+                AtOperation {
+                    operation: "@-",
+                    candidates: Filter(Committer(Substring("baz"))),
+                    visible_heads: None,
+                },
+            ),
+            Filter(Author(Substring("foo"))),
+        )
+        "#);
     }
 
     #[test]

lib/tests/test_revset.rs

@@ -949,6 +949,13 @@ fn test_resolve_symbol_git_refs() {
 }
 
 fn resolve_commit_ids(repo: &dyn Repo, revset_str: &str) -> Vec<CommitId> {
+    try_resolve_commit_ids(repo, revset_str).unwrap()
+}
+
+fn try_resolve_commit_ids(
+    repo: &dyn Repo,
+    revset_str: &str,
+) -> Result<Vec<CommitId>, RevsetResolutionError> {
     let settings = testutils::user_settings();
     let aliases_map = RevsetAliasesMap::default();
     let revset_extensions = RevsetExtensions::default();
@@ -961,10 +968,8 @@ fn resolve_commit_ids(repo: &dyn Repo, revset_str: &str) -> Vec<CommitId> {
     );
     let expression = optimize(parse(&mut RevsetDiagnostics::new(), revset_str, &context).unwrap());
     let symbol_resolver = DefaultSymbolResolver::new(repo, revset_extensions.symbol_resolvers());
-    let expression = expression
-        .resolve_user_expression(repo, &symbol_resolver)
-        .unwrap();
-    expression.evaluate(repo).unwrap().iter().collect()
+    let expression = expression.resolve_user_expression(repo, &symbol_resolver)?;
+    Ok(expression.evaluate(repo).unwrap().iter().collect())
 }
 
 fn resolve_commit_ids_in_workspace(
@@ -2894,6 +2899,136 @@ fn test_evaluate_expression_committer() {
     );
 }
 
+#[test]
+fn test_evaluate_expression_at_operation() {
+    let settings = testutils::user_settings();
+    let test_repo = TestRepo::init();
+    let repo0 = &test_repo.repo;
+    let root_commit = repo0.store().root_commit();
+
+    let mut tx = repo0.start_transaction(&settings);
+    let commit1_op1 = create_random_commit(tx.repo_mut(), &settings)
+        .set_description("commit1@op1")
+        .write()
+        .unwrap();
+    let commit2_op1 = create_random_commit(tx.repo_mut(), &settings)
+        .set_description("commit2@op1")
+        .write()
+        .unwrap();
+    tx.repo_mut()
+        .set_local_bookmark_target("commit1_ref", RefTarget::normal(commit1_op1.id().clone()));
+    let repo1 = tx.commit("test");
+
+    let mut tx = repo1.start_transaction(&settings);
+    let commit1_op2 = tx
+        .repo_mut()
+        .rewrite_commit(&settings, &commit1_op1)
+        .set_description("commit1@op2")
+        .write()
+        .unwrap();
+    let commit3_op2 = create_random_commit(tx.repo_mut(), &settings)
+        .set_description("commit3@op2")
+        .write()
+        .unwrap();
+    tx.repo_mut().rebase_descendants(&settings).unwrap();
+    let repo2 = tx.commit("test");
+
+    let mut tx = repo2.start_transaction(&settings);
+    let _commit4_op3 = create_random_commit(tx.repo_mut(), &settings)
+        .set_description("commit4@op3")
+        .write()
+        .unwrap();
+
+    // Symbol resolution:
+    assert_eq!(
+        resolve_commit_ids(repo2.as_ref(), "at_operation(@, commit1_ref)"),
+        vec![commit1_op2.id().clone()]
+    );
+    assert_eq!(
+        resolve_commit_ids(repo2.as_ref(), "at_operation(@-, commit1_ref)"),
+        vec![commit1_op1.id().clone()]
+    );
+    assert_matches!(
+        try_resolve_commit_ids(repo2.as_ref(), "at_operation(@--, commit1_ref)"),
+        Err(RevsetResolutionError::NoSuchRevision { .. })
+    );
+    assert_eq!(
+        resolve_commit_ids(repo2.as_ref(), "present(at_operation(@--, commit1_ref))"),
+        vec![]
+    );
+
+    // Visibility resolution:
+    assert_eq!(
+        resolve_commit_ids(repo2.as_ref(), "at_operation(@, all())"),
+        vec![
+            commit3_op2.id().clone(),
+            commit1_op2.id().clone(),
+            commit2_op1.id().clone(),
+            root_commit.id().clone(),
+        ]
+    );
+    assert_eq!(
+        resolve_commit_ids(repo2.as_ref(), "at_operation(@-, all())"),
+        vec![
+            commit2_op1.id().clone(),
+            commit1_op1.id().clone(),
+            root_commit.id().clone(),
+        ]
+    );
+
+    // Operation is resolved relative to the outer ReadonlyRepo.
+    assert_eq!(
+        resolve_commit_ids(repo2.as_ref(), "at_operation(@-, at_operation(@-, all()))"),
+        resolve_commit_ids(repo2.as_ref(), "at_operation(@--, all())")
+    );
+
+    // TODO: It might make more sense to resolve "@" to the current MutableRepo
+    // state. However, doing that isn't easy because there's no Operation object
+    // representing a MutableRepo state. For now, "@" is resolved to the base
+    // operation.
+    assert_eq!(
+        resolve_commit_ids(tx.repo(), "at_operation(@, all())"),
+        vec![
+            commit3_op2.id().clone(),
+            commit1_op2.id().clone(),
+            commit2_op1.id().clone(),
+            root_commit.id().clone(),
+        ]
+    );
+
+    // Filter should be evaluated within the at-op repo. Note that this can
+    // populate hidden commits without explicitly referring them by commit refs.
+    // It might be better to intersect inner results again with the outer repo.
+    assert_eq!(
+        resolve_commit_ids(repo2.as_ref(), "at_operation(@-, description('commit'))"),
+        vec![commit2_op1.id().clone(), commit1_op1.id().clone()]
+    );
+    // For the same reason, commit1_op1 isn't filtered out. The following query
+    // is effectively evaluated as "description('commit1') & commit1_op1".
+    assert_eq!(
+        resolve_commit_ids(
+            repo2.as_ref(),
+            "description('commit1') & at_operation(@-, description('commit'))"
+        ),
+        vec![commit1_op1.id().clone()]
+    );
+    // If we have an explicit ::visible_heads(), commit1_op1 is filtered out.
+    assert_eq!(
+        resolve_commit_ids(
+            repo2.as_ref(),
+            "::visible_heads() & description('commit1') & at_operation(@-, description('commit'))"
+        ),
+        vec![]
+    );
+
+    // Bad operation:
+    // TODO: should we suppress NoSuchOperation error by present()?
+    assert_matches!(
+        try_resolve_commit_ids(repo2.as_ref(), "at_operation(000000000000-, all())"),
+        Err(RevsetResolutionError::Other(_))
+    );
+}
+
 #[test]
 fn test_evaluate_expression_union() {
     let settings = testutils::user_settings();