revset: migrate file() predicate to be based on FilesetExpression

cli/src/cli_util.rs

@@ -36,6 +36,7 @@ use indexmap::{IndexMap, IndexSet};
 use itertools::Itertools;
 use jj_lib::backend::{ChangeId, CommitId, MergedTreeId, TreeValue};
 use jj_lib::commit::Commit;
+use jj_lib::fileset::FilesetExpression;
 use jj_lib::git_backend::GitBackend;
 use jj_lib::gitignore::{GitIgnoreError, GitIgnoreFile};
 use jj_lib::hex_util::to_reverse_hex;
@@ -1223,8 +1224,9 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
         // are millions of commits added to the repo, assuming the revset engine can
         // efficiently skip non-conflicting commits. Filter out empty commits mostly so
         // `jj new <conflicted commit>` doesn't result in a message about new conflicts.
-        let conflicts = RevsetExpression::filter(RevsetFilterPredicate::HasConflict)
-            .intersection(&RevsetExpression::filter(RevsetFilterPredicate::File(None)));
+        let conflicts = RevsetExpression::filter(RevsetFilterPredicate::HasConflict).intersection(
+            &RevsetExpression::filter(RevsetFilterPredicate::File(FilesetExpression::all())),
+        );
         let removed_conflicts_expr = new_heads.range(&old_heads).intersection(&conflicts);
         let added_conflicts_expr = old_heads.range(&new_heads).intersection(&conflicts);
 

cli/src/commands/log.rs

@@ -14,6 +14,7 @@
 
 use itertools::Itertools;
 use jj_lib::backend::CommitId;
+use jj_lib::fileset::FilesetExpression;
 use jj_lib::repo::Repo;
 use jj_lib::revset::{self, RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt};
 use jj_lib::revset_graph::{
@@ -89,14 +90,14 @@ pub(crate) fn cmd_log(
             workspace_command.attach_revset_evaluator(RevsetExpression::all())?
         };
         if !args.paths.is_empty() {
-            let repo_paths: Vec<_> = args
+            let file_expressions: Vec<_> = args
                 .paths
                 .iter()
                 .map(|path_arg| workspace_command.parse_file_path(path_arg))
+                .map_ok(FilesetExpression::prefix_path)
                 .try_collect()?;
-            expression.intersect_with(&RevsetExpression::filter(RevsetFilterPredicate::File(
-                Some(repo_paths),
-            )));
+            let expr = FilesetExpression::union_all(file_expressions);
+            expression.intersect_with(&RevsetExpression::filter(RevsetFilterPredicate::File(expr)));
         }
         expression
     };

lib/src/default_index/revset_engine.rs

@@ -28,7 +28,7 @@ use super::rev_walk::{EagerRevWalk, PeekableRevWalk, RevWalk, RevWalkBuilder};
 use super::revset_graph_iterator::RevsetGraphWalk;
 use crate::backend::{ChangeId, CommitId, MillisSinceEpoch};
 use crate::default_index::{AsCompositeIndex, CompositeIndex, IndexEntry, IndexPosition};
-use crate::matchers::{EverythingMatcher, Matcher, PrefixMatcher, Visit};
+use crate::matchers::{Matcher, Visit};
 use crate::repo_path::RepoPath;
 use crate::revset::{
     ResolvedExpression, ResolvedPredicateExpression, Revset, RevsetEvaluationError,
@@ -1038,13 +1038,8 @@ fn build_predicate_fn(
                     || pattern.matches(&commit.committer().email)
             })
         }
-        RevsetFilterPredicate::File(paths) => {
-            // TODO: Add support for globs and other formats
-            let matcher: Rc<dyn Matcher> = if let Some(paths) = paths {
-                Rc::new(PrefixMatcher::new(paths))
-            } else {
-                Rc::new(EverythingMatcher)
-            };
+        RevsetFilterPredicate::File(expr) => {
+            let matcher: Rc<dyn Matcher> = expr.to_matcher().into();
             box_pure_predicate_fn(move |index, pos| {
                 let entry = index.entry_by_pos(pos);
                 has_diff_from_parent(&store, index, &entry, matcher.as_ref())

lib/src/revset.rs

@@ -33,6 +33,7 @@ use thiserror::Error;
 
 use crate::backend::{BackendError, BackendResult, ChangeId, CommitId};
 use crate::commit::Commit;
+use crate::fileset::FilesetExpression;
 use crate::git;
 use crate::hex_util::to_forward_hex;
 use crate::object_id::{HexPrefix, PrefixResolution};
@@ -318,8 +319,8 @@ pub enum RevsetFilterPredicate {
     Author(StringPattern),
     /// Commits with committer's name or email containing the needle.
     Committer(StringPattern),
-    /// Commits modifying the paths specified by the pattern.
-    File(Option<Vec<RepoPathBuf>>), // TODO: embed matcher expression?
+    /// Commits modifying the paths specified by the fileset.
+    File(FilesetExpression),
     /// Commits with conflicts
     HasConflict,
 }
@@ -1330,12 +1331,15 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
     });
     map.insert("empty", |name, arguments_pair, _state| {
         expect_no_arguments(name, arguments_pair)?;
-        Ok(RevsetExpression::filter(RevsetFilterPredicate::File(None)).negated())
+        Ok(
+            RevsetExpression::filter(RevsetFilterPredicate::File(FilesetExpression::all()))
+                .negated(),
+        )
     });
     map.insert("file", |name, arguments_pair, state| {
         let arguments_span = arguments_pair.as_span();
         if let Some(ctx) = state.workspace_ctx {
-            let paths: Vec<_> = arguments_pair
+            let file_expressions: Vec<_> = arguments_pair
                 .into_inner()
                 .map(|arg| -> Result<_, RevsetParseError> {
                     let span = arg.as_span();
@@ -1345,19 +1349,18 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
                             RevsetParseError::invalid_arguments(name, "Invalid file pattern", span)
                                 .with_source(e)
                         })?;
-                    Ok(path)
+                    Ok(FilesetExpression::prefix_path(path))
                 })
                 .try_collect()?;
-            if paths.is_empty() {
+            if file_expressions.is_empty() {
                 Err(RevsetParseError::invalid_arguments(
                     name,
                     "Expected at least 1 argument",
                     arguments_span,
                 ))
             } else {
-                Ok(RevsetExpression::filter(RevsetFilterPredicate::File(Some(
-                    paths,
-                ))))
+                let expr = FilesetExpression::union_all(file_expressions);
+                Ok(RevsetExpression::filter(RevsetFilterPredicate::File(expr)))
             }
         } else {
             Err(RevsetParseError::with_span(
@@ -2955,9 +2958,9 @@ mod tests {
                 StringPattern::Substring("arg1".to_string())
             ))
             .minus(&RevsetExpression::filter(RevsetFilterPredicate::File(
-                Some(vec![
-                    RepoPathBuf::from_internal_string("arg1"),
-                    RepoPathBuf::from_internal_string("arg2"),
+                FilesetExpression::union_all(vec![
+                    FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("arg1")),
+                    FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("arg2")),
                 ])
             )))
             .minus(&RevsetExpression::visible_heads()))
@@ -3317,25 +3320,28 @@ mod tests {
         );
         assert_eq!(
             parse_with_workspace("empty()", &WorkspaceId::default()),
-            Ok(RevsetExpression::filter(RevsetFilterPredicate::File(None)).negated())
+            Ok(
+                RevsetExpression::filter(RevsetFilterPredicate::File(FilesetExpression::all()))
+                    .negated()
+            )
         );
         assert!(parse_with_workspace("empty(foo)", &WorkspaceId::default()).is_err());
         assert!(parse_with_workspace("file()", &WorkspaceId::default()).is_err());
         assert_eq!(
             parse_with_workspace("file(foo)", &WorkspaceId::default()),
-            Ok(RevsetExpression::filter(RevsetFilterPredicate::File(Some(
-                vec![RepoPathBuf::from_internal_string("foo")]
-            ))))
+            Ok(RevsetExpression::filter(RevsetFilterPredicate::File(
+                FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("foo"))
+            )))
         );
         assert_eq!(
             parse_with_workspace("file(foo, bar, baz)", &WorkspaceId::default()),
-            Ok(RevsetExpression::filter(RevsetFilterPredicate::File(Some(
-                vec![
-                    RepoPathBuf::from_internal_string("foo"),
-                    RepoPathBuf::from_internal_string("bar"),
-                    RepoPathBuf::from_internal_string("baz"),
-                ]
-            ))))
+            Ok(RevsetExpression::filter(RevsetFilterPredicate::File(
+                FilesetExpression::union_all(vec![
+                    FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("foo")),
+                    FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("bar")),
+                    FilesetExpression::prefix_path(RepoPathBuf::from_internal_string("baz")),
+                ])
+            )))
         );
     }
 
@@ -4023,7 +4029,7 @@ mod tests {
         insta::assert_debug_snapshot!(optimize(parse("~empty()").unwrap()), @r###"
         Filter(
             File(
-                None,
+                All,
             ),
         )
         "###);
@@ -4260,10 +4266,10 @@ mod tests {
             ),
             Filter(
                 File(
-                    Some(
-                        [
+                    Pattern(
+                        PrefixPath(
                             "bar",
-                        ],
+                        ),
                     ),
                 ),
             ),
@@ -4282,10 +4288,10 @@ mod tests {
                 ),
                 Filter(
                     File(
-                        Some(
-                            [
+                        Pattern(
+                            PrefixPath(
                                 "bar",
-                            ],
+                            ),
                         ),
                     ),
                 ),
@@ -4315,10 +4321,10 @@ mod tests {
             ),
             Filter(
                 File(
-                    Some(
-                        [
+                    Pattern(
+                        PrefixPath(
                             "bar",
-                        ],
+                        ),
                     ),
                 ),
             ),

lib/tests/test_revset.rs

@@ -18,6 +18,7 @@ use assert_matches::assert_matches;
 use itertools::Itertools;
 use jj_lib::backend::{CommitId, MillisSinceEpoch, Signature, Timestamp};
 use jj_lib::commit::Commit;
+use jj_lib::fileset::FilesetExpression;
 use jj_lib::git;
 use jj_lib::git_backend::GitBackend;
 use jj_lib::object_id::ObjectId;
@@ -2671,10 +2672,9 @@ fn test_evaluate_expression_file() {
 
     let resolve = |file_path: &RepoPath| -> Vec<CommitId> {
         let mut_repo = &*mut_repo;
-        let expression =
-            RevsetExpression::filter(RevsetFilterPredicate::File(Some(
-                vec![file_path.to_owned()],
-            )));
+        let expression = RevsetExpression::filter(RevsetFilterPredicate::File(
+            FilesetExpression::prefix_path(file_path.to_owned()),
+        ));
         let revset = expression.evaluate_programmatic(mut_repo).unwrap();
         revset.iter().collect()
     };