revset: pass diagnostics receiver around

Stacking at AliasExpanded node looks wonky. If we migrate error handling to
Diagnostics API, it might make sense to remove AliasExpanded node and add
node.aliases: vec![(id, span), ..] field instead.

Some closure arguments are inlined in order to help type inference.
This commit is contained in:
Yuya Nishihara 2024-09-19 15:02:10 +09:00
parent 4b477fa59e
commit 8b1760ca5d
8 changed files with 223 additions and 130 deletions

View file

@ -31,6 +31,7 @@ use jj_lib::object_id::ObjectId;
use jj_lib::repo::Repo;
use jj_lib::revset::FunctionCallNode;
use jj_lib::revset::PartialSymbolResolver;
use jj_lib::revset::RevsetDiagnostics;
use jj_lib::revset::RevsetExpression;
use jj_lib::revset::RevsetFilterExtension;
use jj_lib::revset::RevsetFilterPredicate;
@ -185,6 +186,7 @@ impl RevsetFilterExtension for EvenDigitsFilter {
}
fn even_digits(
_diagnostics: &mut RevsetDiagnostics,
function: &FunctionCallNode,
_context: &RevsetParseContext,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {

View file

@ -93,6 +93,7 @@ use jj_lib::repo_path::RepoPathUiConverter;
use jj_lib::repo_path::UiPathParseError;
use jj_lib::revset;
use jj_lib::revset::RevsetAliasesMap;
use jj_lib::revset::RevsetDiagnostics;
use jj_lib::revset::RevsetExpression;
use jj_lib::revset::RevsetExtensions;
use jj_lib::revset::RevsetFilterPredicate;
@ -682,15 +683,21 @@ impl WorkspaceCommandEnvironment {
fn load_immutable_heads_expression(
&self,
_ui: &Ui,
ui: &Ui,
) -> Result<Rc<RevsetExpression>, CommandError> {
revset_util::parse_immutable_heads_expression(&self.revset_parse_context())
.map_err(|e| config_error_with_message("Invalid `revset-aliases.immutable_heads()`", e))
let mut diagnostics = RevsetDiagnostics::new();
let expression = revset_util::parse_immutable_heads_expression(
&mut diagnostics,
&self.revset_parse_context(),
)
.map_err(|e| config_error_with_message("Invalid `revset-aliases.immutable_heads()`", e))?;
print_parse_diagnostics(ui, "In `revset-aliases.immutable_heads()`", &diagnostics)?;
Ok(expression)
}
fn load_short_prefixes_expression(
&self,
_ui: &Ui,
ui: &Ui,
) -> Result<Option<Rc<RevsetExpression>>, CommandError> {
let revset_string = self
.settings()
@ -700,10 +707,14 @@ impl WorkspaceCommandEnvironment {
if revset_string.is_empty() {
Ok(None)
} else {
let (expression, modifier) =
revset::parse_with_modifier(&revset_string, &self.revset_parse_context()).map_err(
|err| config_error_with_message("Invalid `revsets.short-prefixes`", err),
)?;
let mut diagnostics = RevsetDiagnostics::new();
let (expression, modifier) = revset::parse_with_modifier(
&mut diagnostics,
&revset_string,
&self.revset_parse_context(),
)
.map_err(|err| config_error_with_message("Invalid `revsets.short-prefixes`", err))?;
print_parse_diagnostics(ui, "In `revsets.short-prefixes`", &diagnostics)?;
let (None | Some(RevsetModifier::All)) = modifier;
Ok(Some(revset::optimize(expression)))
}
@ -1316,26 +1327,31 @@ impl WorkspaceCommandHelper {
fn parse_revset_with_modifier(
&self,
_ui: &Ui,
ui: &Ui,
revision_arg: &RevisionArg,
) -> Result<(RevsetExpressionEvaluator<'_>, Option<RevsetModifier>), CommandError> {
let mut diagnostics = RevsetDiagnostics::new();
let context = self.revset_parse_context();
let (expression, modifier) = revset::parse_with_modifier(revision_arg.as_ref(), &context)?;
let (expression, modifier) =
revset::parse_with_modifier(&mut diagnostics, revision_arg.as_ref(), &context)?;
print_parse_diagnostics(ui, "In revset expression", &diagnostics)?;
Ok((self.attach_revset_evaluator(expression), modifier))
}
/// Parses the given revset expressions and concatenates them all.
pub fn parse_union_revsets(
&self,
_ui: &Ui,
ui: &Ui,
revision_args: &[RevisionArg],
) -> Result<RevsetExpressionEvaluator<'_>, CommandError> {
let mut diagnostics = RevsetDiagnostics::new();
let context = self.revset_parse_context();
let expressions: Vec<_> = revision_args
.iter()
.map(|arg| revset::parse_with_modifier(arg.as_ref(), &context))
.map(|arg| revset::parse_with_modifier(&mut diagnostics, arg.as_ref(), &context))
.map_ok(|(expression, None | Some(RevsetModifier::All))| expression)
.try_collect()?;
print_parse_diagnostics(ui, "In revset expression", &diagnostics)?;
let expression = RevsetExpression::union_all(&expressions);
Ok(self.attach_revset_evaluator(expression))
}

View file

@ -17,8 +17,10 @@ use std::io::Write as _;
use jj_lib::object_id::ObjectId;
use jj_lib::revset;
use jj_lib::revset::RevsetDiagnostics;
use crate::cli_util::CommandHelper;
use crate::command_error::print_parse_diagnostics;
use crate::command_error::CommandError;
use crate::revset_util;
use crate::ui::Ui;
@ -38,7 +40,9 @@ pub fn cmd_debug_revset(
let workspace_ctx = workspace_command.revset_parse_context();
let repo = workspace_command.repo().as_ref();
let expression = revset::parse(&args.revision, &workspace_ctx)?;
let mut diagnostics = RevsetDiagnostics::new();
let expression = revset::parse(&mut diagnostics, &args.revision, &workspace_ctx)?;
print_parse_diagnostics(ui, "In revset expression", &diagnostics)?;
writeln!(ui.stdout(), "-- Parsed:")?;
writeln!(ui.stdout(), "{expression:#?}")?;
writeln!(ui.stdout())?;

View file

@ -43,6 +43,7 @@ use jj_lib::repo::Repo;
use jj_lib::repo_path::RepoPathUiConverter;
use jj_lib::revset;
use jj_lib::revset::Revset;
use jj_lib::revset::RevsetDiagnostics;
use jj_lib::revset::RevsetExpression;
use jj_lib::revset::RevsetModifier;
use jj_lib::revset::RevsetParseContext;
@ -799,10 +800,13 @@ fn evaluate_user_revset<'repo>(
span: pest::Span<'_>,
revset: &str,
) -> Result<Box<dyn Revset + 'repo>, TemplateParseError> {
let (expression, modifier) =
revset::parse_with_modifier(revset, &language.revset_parse_context).map_err(|err| {
TemplateParseError::expression("In revset expression", span).with_source(err)
})?;
let mut inner_diagnostics = RevsetDiagnostics::new(); // TODO
let (expression, modifier) = revset::parse_with_modifier(
&mut inner_diagnostics,
revset,
&language.revset_parse_context,
)
.map_err(|err| TemplateParseError::expression("In revset expression", span).with_source(err))?;
let (None | Some(RevsetModifier::All)) = modifier;
evaluate_revset_expression(language, span, expression)

View file

@ -28,6 +28,7 @@ use jj_lib::revset::DefaultSymbolResolver;
use jj_lib::revset::Revset;
use jj_lib::revset::RevsetAliasesMap;
use jj_lib::revset::RevsetCommitRef;
use jj_lib::revset::RevsetDiagnostics;
use jj_lib::revset::RevsetEvaluationError;
use jj_lib::revset::RevsetExpression;
use jj_lib::revset::RevsetExtensions;
@ -201,13 +202,14 @@ pub fn default_symbol_resolver<'a>(
/// Parses user-configured expression defining the heads of the immutable set.
/// Includes the root commit.
pub fn parse_immutable_heads_expression(
diagnostics: &mut RevsetDiagnostics,
context: &RevsetParseContext,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
let (_, _, immutable_heads_str) = context
.aliases_map()
.get_function(USER_IMMUTABLE_HEADS, 0)
.unwrap();
let heads = revset::parse(immutable_heads_str, context)?;
let heads = revset::parse(diagnostics, immutable_heads_str, context)?;
Ok(heads.union(&RevsetExpression::root()))
}

View file

@ -54,6 +54,7 @@ pub use crate::revset_parser::ExpressionKind;
pub use crate::revset_parser::ExpressionNode;
pub use crate::revset_parser::FunctionCallNode;
pub use crate::revset_parser::RevsetAliasesMap;
pub use crate::revset_parser::RevsetDiagnostics;
pub use crate::revset_parser::RevsetParseError;
pub use crate::revset_parser::RevsetParseErrorKind;
pub use crate::revset_parser::UnaryOp;
@ -565,104 +566,113 @@ impl ResolvedExpression {
}
}
pub type RevsetFunction =
fn(&FunctionCallNode, &RevsetParseContext) -> Result<Rc<RevsetExpression>, RevsetParseError>;
pub type RevsetFunction = fn(
&mut RevsetDiagnostics,
&FunctionCallNode,
&RevsetParseContext,
) -> Result<Rc<RevsetExpression>, RevsetParseError>;
static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy::new(|| {
// Not using maplit::hashmap!{} or custom declarative macro here because
// code completion inside macro is quite restricted.
let mut map: HashMap<&'static str, RevsetFunction> = HashMap::new();
map.insert("parents", |function, context| {
map.insert("parents", |diagnostics, function, context| {
let [arg] = function.expect_exact_arguments()?;
let expression = lower_expression(arg, context)?;
let expression = lower_expression(diagnostics, arg, context)?;
Ok(expression.parents())
});
map.insert("children", |function, context| {
map.insert("children", |diagnostics, function, context| {
let [arg] = function.expect_exact_arguments()?;
let expression = lower_expression(arg, context)?;
let expression = lower_expression(diagnostics, arg, context)?;
Ok(expression.children())
});
map.insert("ancestors", |function, context| {
map.insert("ancestors", |diagnostics, function, context| {
let ([heads_arg], [depth_opt_arg]) = function.expect_arguments()?;
let heads = lower_expression(heads_arg, context)?;
let heads = lower_expression(diagnostics, heads_arg, context)?;
let generation = if let Some(depth_arg) = depth_opt_arg {
let depth = expect_literal("integer", depth_arg)?;
let depth = expect_literal(diagnostics, "integer", depth_arg)?;
0..depth
} else {
GENERATION_RANGE_FULL
};
Ok(heads.ancestors_range(generation))
});
map.insert("descendants", |function, context| {
map.insert("descendants", |diagnostics, function, context| {
let ([roots_arg], [depth_opt_arg]) = function.expect_arguments()?;
let roots = lower_expression(roots_arg, context)?;
let roots = lower_expression(diagnostics, roots_arg, context)?;
let generation = if let Some(depth_arg) = depth_opt_arg {
let depth = expect_literal("integer", depth_arg)?;
let depth = expect_literal(diagnostics, "integer", depth_arg)?;
0..depth
} else {
GENERATION_RANGE_FULL
};
Ok(roots.descendants_range(generation))
});
map.insert("connected", |function, context| {
map.insert("connected", |diagnostics, function, context| {
let [arg] = function.expect_exact_arguments()?;
let candidates = lower_expression(arg, context)?;
let candidates = lower_expression(diagnostics, arg, context)?;
Ok(candidates.connected())
});
map.insert("reachable", |function, context| {
map.insert("reachable", |diagnostics, function, context| {
let [source_arg, domain_arg] = function.expect_exact_arguments()?;
let sources = lower_expression(source_arg, context)?;
let domain = lower_expression(domain_arg, context)?;
let sources = lower_expression(diagnostics, source_arg, context)?;
let domain = lower_expression(diagnostics, domain_arg, context)?;
Ok(sources.reachable(&domain))
});
map.insert("none", |function, _context| {
map.insert("none", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::none())
});
map.insert("all", |function, _context| {
map.insert("all", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::all())
});
map.insert("working_copies", |function, _context| {
map.insert("working_copies", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::working_copies())
});
map.insert("heads", |function, context| {
map.insert("heads", |diagnostics, function, context| {
let [arg] = function.expect_exact_arguments()?;
let candidates = lower_expression(arg, context)?;
let candidates = lower_expression(diagnostics, arg, context)?;
Ok(candidates.heads())
});
map.insert("roots", |function, context| {
map.insert("roots", |diagnostics, function, context| {
let [arg] = function.expect_exact_arguments()?;
let candidates = lower_expression(arg, context)?;
let candidates = lower_expression(diagnostics, arg, context)?;
Ok(candidates.roots())
});
map.insert("visible_heads", |function, _context| {
map.insert("visible_heads", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::visible_heads())
});
map.insert("root", |function, _context| {
map.insert("root", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::root())
});
map.insert("bookmarks", |function, _context| {
map.insert("bookmarks", |diagnostics, function, _context| {
let ([], [opt_arg]) = function.expect_arguments()?;
let pattern = if let Some(arg) = opt_arg {
expect_string_pattern(arg)?
expect_string_pattern(diagnostics, arg)?
} else {
StringPattern::everything()
};
Ok(RevsetExpression::bookmarks(pattern))
});
map.insert("remote_bookmarks", |function, _context| {
parse_remote_bookmarks_arguments(function, None)
});
map.insert("tracked_remote_bookmarks", |function, _context| {
parse_remote_bookmarks_arguments(function, Some(RemoteRefState::Tracking))
});
map.insert("untracked_remote_bookmarks", |function, _context| {
parse_remote_bookmarks_arguments(function, Some(RemoteRefState::New))
map.insert("remote_bookmarks", |diagnostics, function, _context| {
parse_remote_bookmarks_arguments(diagnostics, function, None)
});
map.insert(
"tracked_remote_bookmarks",
|diagnostics, function, _context| {
parse_remote_bookmarks_arguments(diagnostics, function, Some(RemoteRefState::Tracking))
},
);
map.insert(
"untracked_remote_bookmarks",
|diagnostics, function, _context| {
parse_remote_bookmarks_arguments(diagnostics, function, Some(RemoteRefState::New))
},
);
// TODO: Remove in jj 0.28+
map.insert("branches", map["bookmarks"]);
@ -673,56 +683,56 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
map["untracked_remote_bookmarks"],
);
map.insert("tags", |function, _context| {
map.insert("tags", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::tags())
});
map.insert("git_refs", |function, _context| {
map.insert("git_refs", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::git_refs())
});
map.insert("git_head", |function, _context| {
map.insert("git_head", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::git_head())
});
map.insert("latest", |function, context| {
map.insert("latest", |diagnostics, function, context| {
let ([candidates_arg], [count_opt_arg]) = function.expect_arguments()?;
let candidates = lower_expression(candidates_arg, context)?;
let candidates = lower_expression(diagnostics, candidates_arg, context)?;
let count = if let Some(count_arg) = count_opt_arg {
expect_literal("integer", count_arg)?
expect_literal(diagnostics, "integer", count_arg)?
} else {
1
};
Ok(candidates.latest(count))
});
map.insert("merges", |function, _context| {
map.insert("merges", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::filter(
RevsetFilterPredicate::ParentCount(2..u32::MAX),
))
});
map.insert("description", |function, _context| {
map.insert("description", |diagnostics, function, _context| {
let [arg] = function.expect_exact_arguments()?;
let pattern = expect_string_pattern(arg)?;
let pattern = expect_string_pattern(diagnostics, arg)?;
Ok(RevsetExpression::filter(
RevsetFilterPredicate::Description(pattern),
))
});
map.insert("author", |function, _context| {
map.insert("author", |diagnostics, function, _context| {
let [arg] = function.expect_exact_arguments()?;
let pattern = expect_string_pattern(arg)?;
let pattern = expect_string_pattern(diagnostics, arg)?;
Ok(RevsetExpression::filter(RevsetFilterPredicate::Author(
pattern,
)))
});
map.insert("author_date", |function, context| {
map.insert("author_date", |diagnostics, function, context| {
let [arg] = function.expect_exact_arguments()?;
let pattern = expect_date_pattern(arg, context.date_pattern_context())?;
let pattern = expect_date_pattern(diagnostics, arg, context.date_pattern_context())?;
Ok(RevsetExpression::filter(RevsetFilterPredicate::AuthorDate(
pattern,
)))
});
map.insert("mine", |function, context| {
map.insert("mine", |_diagnostics, function, context| {
function.expect_no_arguments()?;
// Email address domains are inherently caseinsensitive, and the localparts
// are generally (although not universally) treated as caseinsensitive too, so
@ -731,25 +741,25 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
StringPattern::exact_i(&context.user_email),
)))
});
map.insert("committer", |function, _context| {
map.insert("committer", |diagnostics, function, _context| {
let [arg] = function.expect_exact_arguments()?;
let pattern = expect_string_pattern(arg)?;
let pattern = expect_string_pattern(diagnostics, arg)?;
Ok(RevsetExpression::filter(RevsetFilterPredicate::Committer(
pattern,
)))
});
map.insert("committer_date", |function, context| {
map.insert("committer_date", |diagnostics, function, context| {
let [arg] = function.expect_exact_arguments()?;
let pattern = expect_date_pattern(arg, context.date_pattern_context())?;
let pattern = expect_date_pattern(diagnostics, arg, context.date_pattern_context())?;
Ok(RevsetExpression::filter(
RevsetFilterPredicate::CommitterDate(pattern),
))
});
map.insert("empty", |function, _context| {
map.insert("empty", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::is_empty())
});
map.insert("file", |function, context| {
map.insert("file", |diagnostics, function, context| {
let ctx = context.workspace.as_ref().ok_or_else(|| {
RevsetParseError::with_span(
RevsetParseErrorKind::FsPathWithoutWorkspace,
@ -759,14 +769,14 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
// TODO: emit deprecation warning if multiple arguments are passed
let ([arg], args) = function.expect_some_arguments()?;
let file_expressions = itertools::chain([arg], args)
.map(|arg| expect_fileset_expression(arg, ctx.path_converter))
.map(|arg| expect_fileset_expression(diagnostics, arg, ctx.path_converter))
.try_collect()?;
let expr = FilesetExpression::union_all(file_expressions);
Ok(RevsetExpression::filter(RevsetFilterPredicate::File(expr)))
});
map.insert("diff_contains", |function, context| {
map.insert("diff_contains", |diagnostics, function, context| {
let ([text_arg], [files_opt_arg]) = function.expect_arguments()?;
let text = expect_string_pattern(text_arg)?;
let text = expect_string_pattern(diagnostics, text_arg)?;
let files = if let Some(files_arg) = files_opt_arg {
let ctx = context.workspace.as_ref().ok_or_else(|| {
RevsetParseError::with_span(
@ -774,7 +784,7 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
files_arg.span,
)
})?;
expect_fileset_expression(files_arg, ctx.path_converter)?
expect_fileset_expression(diagnostics, files_arg, ctx.path_converter)?
} else {
// TODO: defaults to CLI path arguments?
// https://github.com/martinvonz/jj/issues/2933#issuecomment-1925870731
@ -784,13 +794,13 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
RevsetFilterPredicate::DiffContains { text, files },
))
});
map.insert("conflict", |function, _context| {
map.insert("conflict", |_diagnostics, function, _context| {
function.expect_no_arguments()?;
Ok(RevsetExpression::filter(RevsetFilterPredicate::HasConflict))
});
map.insert("present", |function, context| {
map.insert("present", |diagnostics, function, context| {
let [arg] = function.expect_exact_arguments()?;
let expression = lower_expression(arg, context)?;
let expression = lower_expression(diagnostics, arg, context)?;
Ok(Rc::new(RevsetExpression::Present(expression)))
});
map
@ -798,55 +808,73 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
/// Parses the given `node` as a fileset expression.
pub fn expect_fileset_expression(
diagnostics: &mut RevsetDiagnostics,
node: &ExpressionNode,
path_converter: &RepoPathUiConverter,
) -> Result<FilesetExpression, RevsetParseError> {
// Alias handling is a bit tricky. The outermost expression `alias` is
// substituted, but inner expressions `x & alias` aren't. If this seemed
// weird, we can either transform AST or turn off revset aliases completely.
revset_parser::expect_expression_with(node, |node| {
let mut inner_diagnostics = FilesetDiagnostics::new(); // TODO
fileset::parse(&mut inner_diagnostics, node.span.as_str(), path_converter).map_err(|err| {
RevsetParseError::expression("In fileset expression", node.span).with_source(err)
})
revset_parser::expect_expression_with(diagnostics, node, |diagnostics, node| {
let mut inner_diagnostics = FilesetDiagnostics::new();
let expression = fileset::parse(&mut inner_diagnostics, node.span.as_str(), path_converter)
.map_err(|err| {
RevsetParseError::expression("In fileset expression", node.span).with_source(err)
})?;
diagnostics.extend_with(inner_diagnostics, |diag| {
RevsetParseError::expression("In fileset expression", node.span).with_source(diag)
});
Ok(expression)
})
}
pub fn expect_string_pattern(node: &ExpressionNode) -> Result<StringPattern, RevsetParseError> {
let parse_pattern = |value: &str, kind: Option<&str>| match kind {
Some(kind) => StringPattern::from_str_kind(value, kind),
None => Ok(StringPattern::Substring(value.to_owned())),
};
revset_parser::expect_pattern_with("string pattern", node, parse_pattern)
pub fn expect_string_pattern(
diagnostics: &mut RevsetDiagnostics,
node: &ExpressionNode,
) -> Result<StringPattern, RevsetParseError> {
revset_parser::expect_pattern_with(
diagnostics,
"string pattern",
node,
|_diagnostics, value, kind| match kind {
Some(kind) => StringPattern::from_str_kind(value, kind),
None => Ok(StringPattern::Substring(value.to_owned())),
},
)
}
pub fn expect_date_pattern(
diagnostics: &mut RevsetDiagnostics,
node: &ExpressionNode,
context: &DatePatternContext,
) -> Result<DatePattern, RevsetParseError> {
let parse_pattern =
|value: &str, kind: Option<&str>| -> Result<_, Box<dyn std::error::Error + Send + Sync>> {
revset_parser::expect_pattern_with(
diagnostics,
"date pattern",
node,
|_diagnostics, value, kind| -> Result<_, Box<dyn std::error::Error + Send + Sync>> {
match kind {
None => Err("Date pattern must specify 'after' or 'before'".into()),
Some(kind) => Ok(context.parse_relative(value, kind)?),
}
};
revset_parser::expect_pattern_with("date pattern", node, parse_pattern)
},
)
}
fn parse_remote_bookmarks_arguments(
diagnostics: &mut RevsetDiagnostics,
function: &FunctionCallNode,
remote_ref_state: Option<RemoteRefState>,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
let ([], [bookmark_opt_arg, remote_opt_arg]) =
function.expect_named_arguments(&["", "remote"])?;
let bookmark_pattern = if let Some(bookmark_arg) = bookmark_opt_arg {
expect_string_pattern(bookmark_arg)?
expect_string_pattern(diagnostics, bookmark_arg)?
} else {
StringPattern::everything()
};
let remote_pattern = if let Some(remote_arg) = remote_opt_arg {
expect_string_pattern(remote_arg)?
expect_string_pattern(diagnostics, remote_arg)?
} else {
StringPattern::everything()
};
@ -859,12 +887,13 @@ fn parse_remote_bookmarks_arguments(
/// Resolves function call by using the given function map.
fn lower_function_call(
diagnostics: &mut RevsetDiagnostics,
function: &FunctionCallNode,
context: &RevsetParseContext,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
let function_map = &context.extensions.function_map;
if let Some(func) = function_map.get(function.name) {
func(function, context)
func(diagnostics, function, context)
} else {
Err(RevsetParseError::with_span(
RevsetParseErrorKind::NoSuchFunction {
@ -879,6 +908,7 @@ fn lower_function_call(
/// Transforms the given AST `node` into expression that describes DAG
/// operation. Function calls will be resolved at this stage.
pub fn lower_expression(
diagnostics: &mut RevsetDiagnostics,
node: &ExpressionNode,
context: &RevsetParseContext,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
@ -914,7 +944,7 @@ pub fn lower_expression(
Ok(RevsetExpression::root().range(&RevsetExpression::visible_heads()))
}
ExpressionKind::Unary(op, arg_node) => {
let arg = lower_expression(arg_node, context)?;
let arg = lower_expression(diagnostics, arg_node, context)?;
match op {
UnaryOp::Negate => Ok(arg.negated()),
UnaryOp::DagRangePre => Ok(arg.ancestors()),
@ -926,8 +956,8 @@ pub fn lower_expression(
}
}
ExpressionKind::Binary(op, lhs_node, rhs_node) => {
let lhs = lower_expression(lhs_node, context)?;
let rhs = lower_expression(rhs_node, context)?;
let lhs = lower_expression(diagnostics, lhs_node, context)?;
let rhs = lower_expression(diagnostics, rhs_node, context)?;
match op {
BinaryOp::Intersection => Ok(lhs.intersection(&rhs)),
BinaryOp::Difference => Ok(lhs.minus(&rhs)),
@ -938,11 +968,13 @@ pub fn lower_expression(
ExpressionKind::UnionAll(nodes) => {
let expressions: Vec<_> = nodes
.iter()
.map(|node| lower_expression(node, context))
.map(|node| lower_expression(diagnostics, node, context))
.try_collect()?;
Ok(RevsetExpression::union_all(&expressions))
}
ExpressionKind::FunctionCall(function) => lower_function_call(function, context),
ExpressionKind::FunctionCall(function) => {
lower_function_call(diagnostics, function, context)
}
ExpressionKind::Modifier(modifier) => {
let name = modifier.name;
Err(RevsetParseError::expression(
@ -951,31 +983,40 @@ pub fn lower_expression(
))
}
ExpressionKind::AliasExpanded(id, subst) => {
lower_expression(subst, context).map_err(|e| e.within_alias_expansion(*id, node.span))
let mut inner_diagnostics = RevsetDiagnostics::new();
let expression = lower_expression(&mut inner_diagnostics, subst, context)
.map_err(|e| e.within_alias_expansion(*id, node.span))?;
diagnostics.extend_with(inner_diagnostics, |diag| {
diag.within_alias_expansion(*id, node.span)
});
Ok(expression)
}
}
}
pub fn parse(
diagnostics: &mut RevsetDiagnostics,
revset_str: &str,
context: &RevsetParseContext,
) -> Result<Rc<RevsetExpression>, RevsetParseError> {
let node = revset_parser::parse_program(revset_str)?;
let node = dsl_util::expand_aliases(node, context.aliases_map)?;
lower_expression(&node, context)
lower_expression(diagnostics, &node, context)
.map_err(|err| err.extend_function_candidates(context.aliases_map.function_names()))
}
pub fn parse_with_modifier(
diagnostics: &mut RevsetDiagnostics,
revset_str: &str,
context: &RevsetParseContext,
) -> Result<(Rc<RevsetExpression>, Option<RevsetModifier>), RevsetParseError> {
let node = revset_parser::parse_program(revset_str)?;
let node = dsl_util::expand_aliases(node, context.aliases_map)?;
revset_parser::expect_program_with(
diagnostics,
&node,
|node| lower_expression(node, context),
|name, span| match name {
|diagnostics, node| lower_expression(diagnostics, node, context),
|_diagnostics, name, span| match name {
"all" => Ok(RevsetModifier::All),
_ => Err(RevsetParseError::with_span(
RevsetParseErrorKind::NoSuchModifier(name.to_owned()),
@ -2178,7 +2219,7 @@ mod tests {
&extensions,
None,
);
super::parse(revset_str, &context)
super::parse(&mut RevsetDiagnostics::new(), revset_str, &context)
}
fn parse_with_aliases_and_workspace(
@ -2207,7 +2248,7 @@ mod tests {
&extensions,
Some(workspace_ctx),
);
super::parse(revset_str, &context)
super::parse(&mut RevsetDiagnostics::new(), revset_str, &context)
}
fn parse_with_modifier(
@ -2232,7 +2273,7 @@ mod tests {
&extensions,
None,
);
super::parse_with_modifier(revset_str, &context)
super::parse_with_modifier(&mut RevsetDiagnostics::new(), revset_str, &context)
}
fn insta_settings() -> insta::Settings {

View file

@ -39,6 +39,7 @@ use crate::dsl_util::AliasExpandError;
use crate::dsl_util::AliasExpandableExpression;
use crate::dsl_util::AliasId;
use crate::dsl_util::AliasesMap;
use crate::dsl_util::Diagnostics;
use crate::dsl_util::ExpressionFolder;
use crate::dsl_util::FoldableExpression;
use crate::dsl_util::InvalidArguments;
@ -126,6 +127,10 @@ impl Rule {
}
}
/// Manages diagnostic messages emitted during revset parsing and function-call
/// resolution.
pub type RevsetDiagnostics = Diagnostics<RevsetParseError>;
#[derive(Debug, Error)]
#[error("{pest_error}")]
pub struct RevsetParseError {
@ -764,32 +769,41 @@ impl AliasDefinitionParser for RevsetAliasParser {
/// Applies the given functions to the top-level expression body node with an
/// optional modifier. Alias expansion nodes are unwrapped accordingly.
pub(super) fn expect_program_with<B, M>(
diagnostics: &mut RevsetDiagnostics,
node: &ExpressionNode,
parse_body: impl FnOnce(&ExpressionNode) -> Result<B, RevsetParseError>,
parse_modifier: impl FnOnce(&str, pest::Span<'_>) -> Result<M, RevsetParseError>,
parse_body: impl FnOnce(&mut RevsetDiagnostics, &ExpressionNode) -> Result<B, RevsetParseError>,
parse_modifier: impl FnOnce(
&mut RevsetDiagnostics,
&str,
pest::Span<'_>,
) -> Result<M, RevsetParseError>,
) -> Result<(B, Option<M>), RevsetParseError> {
expect_expression_with(node, |node| match &node.kind {
expect_expression_with(diagnostics, node, |diagnostics, node| match &node.kind {
ExpressionKind::Modifier(modifier) => {
let parsed_modifier = parse_modifier(modifier.name, modifier.name_span)?;
Ok((parse_body(&modifier.body)?, Some(parsed_modifier)))
let parsed_modifier = parse_modifier(diagnostics, modifier.name, modifier.name_span)?;
let parsed_body = parse_body(diagnostics, &modifier.body)?;
Ok((parsed_body, Some(parsed_modifier)))
}
_ => Ok((parse_body(node)?, None)),
_ => Ok((parse_body(diagnostics, node)?, None)),
})
}
pub(super) fn expect_pattern_with<T, E: Into<Box<dyn error::Error + Send + Sync>>>(
diagnostics: &mut RevsetDiagnostics,
type_name: &str,
node: &ExpressionNode,
parse_pattern: impl FnOnce(&str, Option<&str>) -> Result<T, E>,
parse_pattern: impl FnOnce(&mut RevsetDiagnostics, &str, Option<&str>) -> Result<T, E>,
) -> Result<T, RevsetParseError> {
let wrap_error = |err: E| {
RevsetParseError::expression(format!("Invalid {type_name}"), node.span).with_source(err)
};
expect_expression_with(node, |node| match &node.kind {
ExpressionKind::Identifier(name) => parse_pattern(name, None).map_err(wrap_error),
ExpressionKind::String(name) => parse_pattern(name, None).map_err(wrap_error),
expect_expression_with(diagnostics, node, |diagnostics, node| match &node.kind {
ExpressionKind::Identifier(name) => {
parse_pattern(diagnostics, name, None).map_err(wrap_error)
}
ExpressionKind::String(name) => parse_pattern(diagnostics, name, None).map_err(wrap_error),
ExpressionKind::StringPattern { kind, value } => {
parse_pattern(value, Some(kind)).map_err(wrap_error)
parse_pattern(diagnostics, value, Some(kind)).map_err(wrap_error)
}
_ => Err(RevsetParseError::expression(
format!("Expected expression of {type_name}"),
@ -799,6 +813,7 @@ pub(super) fn expect_pattern_with<T, E: Into<Box<dyn error::Error + Send + Sync>
}
pub fn expect_literal<T: FromStr>(
diagnostics: &mut RevsetDiagnostics,
type_name: &str,
node: &ExpressionNode,
) -> Result<T, RevsetParseError> {
@ -808,7 +823,7 @@ pub fn expect_literal<T: FromStr>(
node.span,
)
};
expect_expression_with(node, |node| match &node.kind {
expect_expression_with(diagnostics, node, |_diagnostics, node| match &node.kind {
ExpressionKind::Identifier(name) => name.parse().map_err(|_| make_error()),
ExpressionKind::String(name) => name.parse().map_err(|_| make_error()),
_ => Err(make_error()),
@ -818,13 +833,20 @@ pub fn expect_literal<T: FromStr>(
/// Applies the give function to the innermost `node` by unwrapping alias
/// expansion nodes.
pub(super) fn expect_expression_with<T>(
diagnostics: &mut RevsetDiagnostics,
node: &ExpressionNode,
f: impl FnOnce(&ExpressionNode) -> Result<T, RevsetParseError>,
f: impl FnOnce(&mut RevsetDiagnostics, &ExpressionNode) -> Result<T, RevsetParseError>,
) -> Result<T, RevsetParseError> {
if let ExpressionKind::AliasExpanded(id, subst) = &node.kind {
expect_expression_with(subst, f).map_err(|e| e.within_alias_expansion(*id, node.span))
let mut inner_diagnostics = RevsetDiagnostics::new();
let expression = expect_expression_with(&mut inner_diagnostics, subst, f)
.map_err(|e| e.within_alias_expansion(*id, node.span))?;
diagnostics.extend_with(inner_diagnostics, |diag| {
diag.within_alias_expansion(*id, node.span)
});
Ok(expression)
} else {
f(node)
f(diagnostics, node)
}
}

View file

@ -43,6 +43,7 @@ use jj_lib::revset::FailingSymbolResolver;
use jj_lib::revset::ResolvedExpression;
use jj_lib::revset::Revset;
use jj_lib::revset::RevsetAliasesMap;
use jj_lib::revset::RevsetDiagnostics;
use jj_lib::revset::RevsetExpression;
use jj_lib::revset::RevsetExtensions;
use jj_lib::revset::RevsetFilterPredicate;
@ -70,7 +71,7 @@ fn resolve_symbol_with_extensions(
let now = chrono::Local::now();
let context =
RevsetParseContext::new(&aliases_map, String::new(), now.into(), extensions, None);
let expression = parse(symbol, &context).unwrap();
let expression = parse(&mut RevsetDiagnostics::new(), symbol, &context).unwrap();
assert_matches!(*expression, RevsetExpression::CommitRef(_));
let symbol_resolver = DefaultSymbolResolver::new(repo, extensions.symbol_resolvers());
match expression.resolve_user_expression(repo, &symbol_resolver)? {
@ -211,7 +212,8 @@ fn test_resolve_symbol_commit_id() {
None,
);
assert_matches!(
optimize(parse("present(04)", &context).unwrap()).resolve_user_expression(repo.as_ref(), &symbol_resolver),
optimize(parse(&mut RevsetDiagnostics::new(), "present(04)", &context).unwrap())
.resolve_user_expression(repo.as_ref(), &symbol_resolver),
Err(RevsetResolutionError::AmbiguousCommitIdPrefix(s)) if s == "04"
);
assert_eq!(
@ -880,7 +882,7 @@ fn resolve_commit_ids(repo: &dyn Repo, revset_str: &str) -> Vec<CommitId> {
&revset_extensions,
None,
);
let expression = optimize(parse(revset_str, &context).unwrap());
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)
@ -912,7 +914,7 @@ fn resolve_commit_ids_in_workspace(
&extensions,
Some(workspace_ctx),
);
let expression = optimize(parse(revset_str, &context).unwrap());
let expression = optimize(parse(&mut RevsetDiagnostics::new(), revset_str, &context).unwrap());
let symbol_resolver =
DefaultSymbolResolver::new(repo, &([] as [&Box<dyn SymbolResolverExtension>; 0]));
let expression = expression