lib: allow exactly() to take interval ranges as an argument

It turns out that the use case I had for `exactly()` was slightly more
subtle, and I need range support. In particular, usage of `exactly()`
in expressions used by `revsets.log-graph-prioritize` can and will
cause `jj log` to fail if the assertion fails; in my case, a particular
expression I use should be equal to zero-or-one revisions.

This (ab)uses string literal syntax to parse out a range that is like
a subset of Rust's range syntax; in particular unbounded endpoints like
`x..` or `..=y` are not supported. Ideally the grammar would be expanded
to handle this case more smoothly.

Signed-off-by: Austin Seipp <[email protected]>

Author: thoughtpolice

CHANGELOG.md

@@ -63,7 +63,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   deprecation from `jj 0.26`, as the community feedback was mostly negative.
 
 * The revset function `exactly(x, n)` will now evaluate `x` and error if it does
-  not have exactly `n` elements.
+  not have the range specified by `n`.
 
 * `jj util exec` now matches the exit status of the program it runs, and
   doesn't print anything.

docs/revsets.md

@@ -318,9 +318,13 @@ revsets (expressions) as arguments.
   set are descendants. The current implementation deals somewhat poorly with
   non-linear history.
 
-* `exactly(x, count)`: Evaluates `x`, and errors if it is not of exactly size
-  `count`. Otherwise, returns `x`. This is useful in particular with `count=1`
-  when you want to ensure that some revset expression has exactly one target.
+* `exactly(x, range)`: Evaluates `x`, and errors if it is not within `range`.
+  Otherwise, returns `x`. This is primarily useful to enforce invariants in
+  your revsets.
+    - `range` may be an integer literal, in which case `x` must always resolve
+      to exactly that number of revisions. e.g. `exactly(root(), 1)`
+    - `range` may be a half-open interval `n..x`, equivalent to `[n, x)` (`x` is excluded)
+    - `range` may be a closed interval `n..=x`, equivalent to `[n, x]` (`x` is included)
 
 * `merges()`: Merge commits.
 

lib/src/default_index/revset_engine.rs

@@ -994,32 +994,48 @@ impl EvaluationContext<'_> {
                 let candidate_set = self.evaluate(candidates)?;
                 Ok(Box::new(self.take_latest_revset(&*candidate_set, *count)?))
             }
-            ResolvedExpression::HasSize { candidates, count } => {
+            ResolvedExpression::HasSize { candidates, range } => {
                 let set = self.evaluate(candidates)?;
+                // Take at most count.end elements to check the range
+                // We take one extra to detect if there are more elements than allowed
                 let positions: Vec<_> = set
                     .positions()
                     .attach(index)
-                    .take(count.saturating_add(1))
+                    .take(range.end.saturating_add(1))
                     .try_collect()?;
-                if positions.len() != *count {
+
+                if !range.contains(&positions.len()) {
                     // https://github.com/jj-vcs/jj/pull/7252#pullrequestreview-3236259998
                     // in the default engine we have to evaluate the entire
                     // revset (which may be very large) to get an exact count;
                     // we would need to remove .take() above. instead just give
                     // a vaguely approximate error message
-                    let determiner = if positions.len() > *count {
-                        "more"
+                    let expected = if range.start == range.end.saturating_sub(1) {
+                        // Range represents exactly one value
+                        format!("exactly {} elements", range.start)
+                    } else {
+                        format!(
+                            "between {} and {} elements",
+                            range.start,
+                            range.end.saturating_sub(1)
+                        )
+                    };
+
+                    let actual = if positions.len() > range.end {
+                        "more".to_string()
                     } else {
-                        "less"
+                        format!("{}", positions.len())
                     };
+
                     return Err(RevsetEvaluationError::Other(
                         format!(
-                            "The revset was expected to have {count} elements, but {determiner} \
-                             were provided",
+                            "The revset was expected to have {expected}, but {actual} were \
+                             provided"
                         )
                         .into(),
                     ));
                 }
+
                 Ok(Box::new(EagerRevset { positions }))
             }
             ResolvedExpression::Coalesce(expression1, expression2) => {

lib/src/revset.rs

@@ -298,7 +298,7 @@ pub enum RevsetExpression<St: ExpressionState> {
     Bisect(Arc<Self>),
     HasSize {
         candidates: Arc<Self>,
-        count: usize,
+        range: Range<usize>,
     },
     Latest {
         candidates: Arc<Self>,
@@ -533,10 +533,10 @@ impl<St: ExpressionState> RevsetExpression<St> {
     }
 
     /// Commits in `self`, the number of which must be exactly equal to `count`.
-    pub fn has_size(self: &Arc<Self>, count: usize) -> Arc<Self> {
+    pub fn has_size(self: &Arc<Self>, range: Range<usize>) -> Arc<Self> {
         Arc::new(Self::HasSize {
             candidates: self.clone(),
-            count,
+            range,
         })
     }
 
@@ -736,7 +736,7 @@ pub enum ResolvedExpression {
     Bisect(Box<Self>),
     HasSize {
         candidates: Box<Self>,
-        count: usize,
+        range: Range<usize>,
     },
     Latest {
         candidates: Box<Self>,
@@ -948,10 +948,13 @@ static BUILTIN_FUNCTION_MAP: LazyLock<HashMap<&str, RevsetFunction>> = LazyLock:
         Ok(RevsetExpression::bisect(&expression))
     });
     map.insert("exactly", |diagnostics, function, context| {
-        let ([candidates_arg, count_arg], []) = function.expect_arguments()?;
+        let ([candidates_arg, range_arg], []) = function.expect_arguments()?;
         let candidates = lower_expression(diagnostics, candidates_arg, context)?;
-        let count = expect_literal("integer", count_arg)?;
-        Ok(candidates.has_size(count))
+        let range_str: String = expect_literal("string", range_arg)?;
+        let range = parse_range(&range_str)
+            .map_err(|msg| RevsetParseError::expression(msg, range_arg.span))?;
+
+        Ok(candidates.has_size(range))
     });
     map.insert("merges", |_diagnostics, function, _context| {
         function.expect_no_arguments()?;
@@ -1161,6 +1164,44 @@ pub fn expect_date_pattern(
     })
 }
 
+fn parse_range(s: &str) -> Result<Range<usize>, String> {
+    if let Ok(n) = s.parse::<usize>() {
+        return Ok(n..n + 1);
+    }
+
+    // inclusive range syntax (a..=b)
+    if let Some((start_str, end_str)) = s.split_once("..=") {
+        let start = start_str
+            .parse::<usize>()
+            .map_err(|_| format!("Invalid start of range: {start_str}"))?;
+        let end = end_str
+            .parse::<usize>()
+            .map_err(|_| format!("Invalid end of range: {end_str}"))?;
+        if start > end {
+            return Err(format!("Invalid range: start ({start}) > end ({end})"));
+        }
+        return Ok(start..end + 1);
+    }
+
+    // exclusive range syntax (a..b)
+    if let Some((start_str, end_str)) = s.split_once("..") {
+        let start = start_str
+            .parse::<usize>()
+            .map_err(|_| format!("Invalid start of range: {start_str}"))?;
+        let end = end_str
+            .parse::<usize>()
+            .map_err(|_| format!("Invalid end of range: {end_str}"))?;
+        if start > end {
+            return Err(format!("Invalid range: start ({start}) > end ({end})"));
+        }
+        return Ok(start..end);
+    }
+
+    Err(format!(
+        "Invalid range syntax: '{s}'. Expected format: 'n', 'n..m', or 'n..=m'"
+    ))
+}
+
 fn parse_remote_bookmarks_arguments(
     diagnostics: &mut RevsetDiagnostics,
     function: &FunctionCallNode,
@@ -1472,10 +1513,10 @@ fn try_transform_expression<St: ExpressionState, E>(
             RevsetExpression::Bisect(expression) => {
                 transform_rec(expression, pre, post)?.map(RevsetExpression::Bisect)
             }
-            RevsetExpression::HasSize { candidates, count } => {
+            RevsetExpression::HasSize { candidates, range } => {
                 transform_rec(candidates, pre, post)?.map(|candidates| RevsetExpression::HasSize {
                     candidates,
-                    count: *count,
+                    range: range.clone(),
                 })
             }
             RevsetExpression::Latest { candidates, count } => transform_rec(candidates, pre, post)?
@@ -1719,11 +1760,11 @@ where
             let expression = folder.fold_expression(expression)?;
             RevsetExpression::Bisect(expression).into()
         }
-        RevsetExpression::HasSize { candidates, count } => {
+        RevsetExpression::HasSize { candidates, range } => {
             let candidates = folder.fold_expression(candidates)?;
             RevsetExpression::HasSize {
                 candidates,
-                count: *count,
+                range: range.clone(),
             }
             .into()
         }
@@ -3047,9 +3088,9 @@ impl VisibilityResolutionContext<'_> {
                 candidates: self.resolve(candidates).into(),
                 count: *count,
             },
-            RevsetExpression::HasSize { candidates, count } => ResolvedExpression::HasSize {
+            RevsetExpression::HasSize { candidates, range } => ResolvedExpression::HasSize {
                 candidates: self.resolve(candidates).into(),
-                count: *count,
+                range: range.clone(),
             },
             RevsetExpression::Filter(_) | RevsetExpression::AsFilter(_) => {
                 // Top-level filter without intersection: e.g. "~author(_)" is represented as

lib/tests/test_revset.rs

@@ -3062,6 +3062,7 @@ fn test_evaluate_expression_exactly() {
     let commit1 = write_random_commit(mut_repo);
     let commit2 = write_random_commit_with_parents(mut_repo, &[&commit1]);
 
+    // basics
     assert!(try_evaluate_expression(mut_repo, "exactly(none(), 0)").is_ok());
     assert!(try_evaluate_expression(mut_repo, "exactly(none(), 1)").is_err());
     assert!(try_evaluate_expression(mut_repo, &format!("exactly({}, 1)", commit1.id())).is_ok());
@@ -3077,6 +3078,11 @@ fn test_evaluate_expression_exactly() {
         resolve_commit_ids(mut_repo, &format!("exactly({}, 1)", commit1.id())),
         vec![commit1.id().clone()]
     );
+    // ranges
+    assert!(try_evaluate_expression(mut_repo, r#"exactly(none(), "0..=1")"#).is_ok());
+    assert!(try_evaluate_expression(mut_repo, r#"exactly(none(), "1..=1")"#).is_err());
+    assert!(try_evaluate_expression(mut_repo, r#"exactly(root(), "1..2")"#).is_ok());
+    assert!(try_evaluate_expression(mut_repo, r#"exactly(root()::, "1..=3")"#).is_ok());
 }
 
 #[test]