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 <aseipp@pobox.com>
This commit is contained in:
Austin Seipp 2025-09-24 18:21:54 -05:00
parent df2c1ed474
commit 643c41bc4a
5 changed files with 92 additions and 25 deletions

View file

@ -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.

View file

@ -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.

View file

@ -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 {
"less"
format!(
"between {} and {} elements",
range.start,
range.end.saturating_sub(1)
)
};
let actual = if positions.len() > range.end {
"more".to_string()
} else {
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) => {

View file

@ -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

View file

@ -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]