ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

fileset: pass diagnostics receiver around, add printing function

Browse source 
CLI tests will be added later.
This commit is contained in:
Yuya Nishihara 2024-09-19 15:02:10 +09:00
parent df8967970e
commit 4b477fa59e
8 changed files with 91 additions and 38 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
cli/src/cli_util.rs
View file

@ -58,6 +58,7 @@ use jj_lib::commit::Commit;
use jj_lib::dag_walk;
use jj_lib::file_util;
use jj_lib::fileset;
use jj_lib::fileset::FilesetDiagnostics;
use jj_lib::fileset::FilesetExpression;
use jj_lib::git;
use jj_lib::git_backend::GitBackend;
@ -131,6 +132,7 @@ use crate::command_error::config_error_with_message;
use crate::command_error::handle_command_result;
use crate::command_error::internal_error;
use crate::command_error::internal_error_with_message;
use crate::command_error::print_parse_diagnostics;
use crate::command_error::user_error;
use crate::command_error::user_error_with_hint;
use crate::command_error::user_error_with_message;
@ -1082,25 +1084,30 @@ impl WorkspaceCommandHelper {
/// Parses the given fileset expressions and concatenates them all.
pub fn parse_union_filesets(
&self,
_ui: &Ui,
ui: &Ui,
file_args: &[String], // TODO: introduce FileArg newtype?
) -> Result<FilesetExpression, CommandError> {
let mut diagnostics = FilesetDiagnostics::new();
let expressions: Vec<_> = file_args
.iter()
.map(|arg| fileset::parse_maybe_bare(arg, self.path_converter()))
.map(|arg| fileset::parse_maybe_bare(&mut diagnostics, arg, self.path_converter()))
.try_collect()?;
print_parse_diagnostics(ui, "In fileset expression", &diagnostics)?;
Ok(FilesetExpression::union_all(expressions))
}
pub fn auto_tracking_matcher(&self, _ui: &Ui) -> Result<Box<dyn Matcher>, CommandError> {
pub fn auto_tracking_matcher(&self, ui: &Ui) -> Result<Box<dyn Matcher>, CommandError> {
let mut diagnostics = FilesetDiagnostics::new();
let pattern = self.settings().config().get_string("snapshot.auto-track")?;
let expression = fileset::parse(
&mut diagnostics,
&pattern,
&RepoPathUiConverter::Fs {
cwd: "".into(),
base: "".into(),
},
)?;
print_parse_diagnostics(ui, "In `snapshot.auto-track`", &diagnostics)?;
Ok(expression.to_matcher())
}

18
cli/src/command_error.rs
View file

@ -22,6 +22,7 @@ use std::sync::Arc;
use itertools::Itertools as _;
use jj_lib::backend::BackendError;
use jj_lib::dsl_util::Diagnostics;
use jj_lib::fileset::FilePatternParseError;
use jj_lib::fileset::FilesetParseError;
use jj_lib::fileset::FilesetParseErrorKind;
@ -835,3 +836,20 @@ fn handle_clap_error(ui: &mut Ui, err: &clap::Error, hints: &[ErrorHint]) -> io:
print_error_hints(ui, hints)?;
Ok(ExitCode::from(2))
}
/// Prints diagnostic messages emitted during parsing.
pub fn print_parse_diagnostics<T: error::Error>(
ui: &Ui,
context_message: &str,
diagnostics: &Diagnostics<T>,
) -> io::Result<()> {
for diag in diagnostics {
writeln!(ui.warning_default(), "{context_message}")?;
for err in iter::successors(Some(diag as &dyn error::Error), |err| err.source()) {
writeln!(ui.stderr(), "{err}")?;
}
// If we add support for multiple error diagnostics, we might have to do
// find_source_parse_error_hint() and print it here.
}
Ok(())
}

6
cli/src/commands/debug/fileset.rs
View file

@ -16,8 +16,10 @@ use std::fmt::Debug;
use std::io::Write as _;
use jj_lib::fileset;
use jj_lib::fileset::FilesetDiagnostics;
use crate::cli_util::CommandHelper;
use crate::command_error::print_parse_diagnostics;
use crate::command_error::CommandError;
use crate::ui::Ui;
@ -36,7 +38,9 @@ pub fn cmd_debug_fileset(
let workspace_command = command.workspace_helper(ui)?;
let path_converter = workspace_command.path_converter();
let expression = fileset::parse_maybe_bare(&args.path, path_converter)?;
let mut diagnostics = FilesetDiagnostics::new();
let expression = fileset::parse_maybe_bare(&mut diagnostics, &args.path, path_converter)?;
print_parse_diagnostics(ui, "In fileset expression", &diagnostics)?;
writeln!(ui.stdout(), "-- Parsed:")?;
writeln!(ui.stdout(), "{expression:#?}")?;
writeln!(ui.stdout())?;

37
cli/src/commands/fix.rs
View file

@ -25,6 +25,7 @@ use jj_lib::backend::CommitId;
use jj_lib::backend::FileId;
use jj_lib::backend::TreeValue;
use jj_lib::fileset;
use jj_lib::fileset::FilesetDiagnostics;
use jj_lib::fileset::FilesetExpression;
use jj_lib::matchers::EverythingMatcher;
use jj_lib::matchers::Matcher;
@ -46,6 +47,7 @@ use tracing::instrument;
use crate::cli_util::CommandHelper;
use crate::cli_util::RevisionArg;
use crate::command_error::config_error;
use crate::command_error::print_parse_diagnostics;
use crate::command_error::CommandError;
use crate::config::to_toml_value;
use crate::config::CommandNameAndArgs;
@ -473,25 +475,28 @@ fn get_tools_config(ui: &mut Ui, config: &config::Config) -> Result<ToolsConfig,
let mut tools: Vec<ToolConfig> = tools_table
.into_iter()
.sorted_by(|a, b| a.0.cmp(&b.0))
.map(|(_name, value)| -> Result<ToolConfig, CommandError> {
.map(|(name, value)| -> Result<ToolConfig, CommandError> {
let mut diagnostics = FilesetDiagnostics::new();
let tool: RawToolConfig = value.try_deserialize()?;
let expression = FilesetExpression::union_all(
tool.patterns
.iter()
.map(|arg| {
fileset::parse(
&mut diagnostics,
arg,
&RepoPathUiConverter::Fs {
cwd: "".into(),
base: "".into(),
},
)
})
.try_collect()?,
);
print_parse_diagnostics(ui, &format!("In `fix.tools.{name}`"), &diagnostics)?;
Ok(ToolConfig {
command: tool.command,
matcher: FilesetExpression::union_all(
tool.patterns
.iter()
.map(|arg| {
fileset::parse(
arg,
&RepoPathUiConverter::Fs {
cwd: "".into(),
base: "".into(),
},
)
})
.try_collect()?,
)
.to_matcher(),
matcher: expression.to_matcher(),
})
})
.try_collect()?;

4
cli/src/commit_templater.rs
View file

@ -28,6 +28,7 @@ use jj_lib::copies::CopiesTreeDiffEntry;
use jj_lib::copies::CopyRecords;
use jj_lib::extensions_map::ExtensionsMap;
use jj_lib::fileset;
use jj_lib::fileset::FilesetDiagnostics;
use jj_lib::fileset::FilesetExpression;
use jj_lib::git;
use jj_lib::hex_util::to_reverse_hex;
@ -767,7 +768,8 @@ fn expect_fileset_literal(
path_converter: &RepoPathUiConverter,
) -> Result<FilesetExpression, TemplateParseError> {
template_parser::expect_string_literal_with(node, |text, span| {
fileset::parse(text, path_converter).map_err(|err| {
let mut inner_diagnostics = FilesetDiagnostics::new(); // TODO
fileset::parse(&mut inner_diagnostics, text, path_converter).map_err(|err| {
TemplateParseError::expression("In fileset expression", span).with_source(err)
})
})

42
lib/src/fileset.rs
View file

@ -28,6 +28,7 @@ use crate::fileset_parser;
use crate::fileset_parser::BinaryOp;
use crate::fileset_parser::ExpressionKind;
use crate::fileset_parser::ExpressionNode;
pub use crate::fileset_parser::FilesetDiagnostics;
pub use crate::fileset_parser::FilesetParseError;
pub use crate::fileset_parser::FilesetParseErrorKind;
pub use crate::fileset_parser::FilesetParseResult;
@ -378,18 +379,21 @@ fn union_all_matchers(matchers: &mut [Option<Box<dyn Matcher>>]) -> Box<dyn Matc
}
}
type FilesetFunction =
fn(&RepoPathUiConverter, &FunctionCallNode) -> FilesetParseResult<FilesetExpression>;
type FilesetFunction = fn(
&mut FilesetDiagnostics,
&RepoPathUiConverter,
&FunctionCallNode,
) -> FilesetParseResult<FilesetExpression>;
static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, FilesetFunction>> = 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, FilesetFunction> = HashMap::new();
map.insert("none", |_path_converter, function| {
map.insert("none", |_diagnostics, _path_converter, function| {
function.expect_no_arguments()?;
Ok(FilesetExpression::none())
});
map.insert("all", |_path_converter, function| {
map.insert("all", |_diagnostics, _path_converter, function| {
function.expect_no_arguments()?;
Ok(FilesetExpression::all())
});
@ -397,11 +401,12 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, FilesetFunction>> = Lazy
});
fn resolve_function(
diagnostics: &mut FilesetDiagnostics,
path_converter: &RepoPathUiConverter,
function: &FunctionCallNode,
) -> FilesetParseResult<FilesetExpression> {
if let Some(func) = BUILTIN_FUNCTION_MAP.get(function.name) {
func(path_converter, function)
func(diagnostics, path_converter, function)
} else {
Err(FilesetParseError::new(
FilesetParseErrorKind::NoSuchFunction {
@ -414,6 +419,7 @@ fn resolve_function(
}
fn resolve_expression(
diagnostics: &mut FilesetDiagnostics,
path_converter: &RepoPathUiConverter,
node: &ExpressionNode,
) -> FilesetParseResult<FilesetExpression> {
@ -436,14 +442,14 @@ fn resolve_expression(
Ok(FilesetExpression::pattern(pattern))
}
ExpressionKind::Unary(op, arg_node) => {
let arg = resolve_expression(path_converter, arg_node)?;
let arg = resolve_expression(diagnostics, path_converter, arg_node)?;
match op {
UnaryOp::Negate => Ok(FilesetExpression::all().difference(arg)),
}
}
ExpressionKind::Binary(op, lhs_node, rhs_node) => {
let lhs = resolve_expression(path_converter, lhs_node)?;
let rhs = resolve_expression(path_converter, rhs_node)?;
let lhs = resolve_expression(diagnostics, path_converter, lhs_node)?;
let rhs = resolve_expression(diagnostics, path_converter, rhs_node)?;
match op {
BinaryOp::Intersection => Ok(lhs.intersection(rhs)),
BinaryOp::Difference => Ok(lhs.difference(rhs)),
@ -452,22 +458,25 @@ fn resolve_expression(
ExpressionKind::UnionAll(nodes) => {
let expressions = nodes
.iter()
.map(|node| resolve_expression(path_converter, node))
.map(|node| resolve_expression(diagnostics, path_converter, node))
.try_collect()?;
Ok(FilesetExpression::union_all(expressions))
}
ExpressionKind::FunctionCall(function) => resolve_function(path_converter, function),
ExpressionKind::FunctionCall(function) => {
resolve_function(diagnostics, path_converter, function)
}
}
}
/// Parses text into `FilesetExpression` without bare string fallback.
pub fn parse(
diagnostics: &mut FilesetDiagnostics,
text: &str,
path_converter: &RepoPathUiConverter,
) -> FilesetParseResult<FilesetExpression> {
let node = fileset_parser::parse_program(text)?;
// TODO: add basic tree substitution pass to eliminate redundant expressions
resolve_expression(path_converter, &node)
resolve_expression(diagnostics, path_converter, &node)
}
/// Parses text into `FilesetExpression` with bare string fallback.
@ -475,12 +484,13 @@ pub fn parse(
/// If the text can't be parsed as a fileset expression, and if it doesn't
/// contain any operator-like characters, it will be parsed as a file path.
pub fn parse_maybe_bare(
diagnostics: &mut FilesetDiagnostics,
text: &str,
path_converter: &RepoPathUiConverter,
) -> FilesetParseResult<FilesetExpression> {
let node = fileset_parser::parse_program_or_bare_string(text)?;
// TODO: add basic tree substitution pass to eliminate redundant expressions
resolve_expression(path_converter, &node)
resolve_expression(diagnostics, path_converter, &node)
}
#[cfg(test)]
@ -515,7 +525,7 @@ mod tests {
cwd: PathBuf::from("/ws/cur"),
base: PathBuf::from("/ws"),
};
let parse = |text| parse_maybe_bare(text, &path_converter);
let parse = |text| parse_maybe_bare(&mut FilesetDiagnostics::new(), text, &path_converter);
// cwd-relative patterns
assert_eq!(
@ -567,7 +577,7 @@ mod tests {
cwd: PathBuf::from("/ws/cur*"),
base: PathBuf::from("/ws"),
};
let parse = |text| parse_maybe_bare(text, &path_converter);
let parse = |text| parse_maybe_bare(&mut FilesetDiagnostics::new(), text, &path_converter);
let glob_expr = |dir: &str, pattern: &str| {
FilesetExpression::pattern(FilePattern::FileGlob {
dir: repo_path_buf(dir),
@ -652,7 +662,7 @@ mod tests {
cwd: PathBuf::from("/ws/cur"),
base: PathBuf::from("/ws"),
};
let parse = |text| parse_maybe_bare(text, &path_converter);
let parse = |text| parse_maybe_bare(&mut FilesetDiagnostics::new(), text, &path_converter);
assert_eq!(parse("all()").unwrap(), FilesetExpression::all());
assert_eq!(parse("none()").unwrap(), FilesetExpression::none());
@ -680,7 +690,7 @@ mod tests {
cwd: PathBuf::from("/ws/cur"),
base: PathBuf::from("/ws"),
};
let parse = |text| parse_maybe_bare(text, &path_converter);
let parse = |text| parse_maybe_bare(&mut FilesetDiagnostics::new(), text, &path_converter);
insta::assert_debug_snapshot!(parse("~x").unwrap(), @r###"
Difference(

5
lib/src/fileset_parser.rs
View file

@ -27,6 +27,7 @@ use pest_derive::Parser;
use thiserror::Error;
use crate::dsl_util;
use crate::dsl_util::Diagnostics;
use crate::dsl_util::InvalidArguments;
use crate::dsl_util::StringLiteralParser;
@ -74,6 +75,10 @@ impl Rule {
}
}
/// Manages diagnostic messages emitted during fileset parsing and name
/// resolution.
pub type FilesetDiagnostics = Diagnostics<FilesetParseError>;
/// Result of fileset parsing and name resolution.
pub type FilesetParseResult<T> = Result<T, FilesetParseError>;

4
lib/src/revset.rs
View file

@ -36,6 +36,7 @@ use crate::dsl_util;
use crate::dsl_util::collect_similar;
use crate::dsl_util::AliasExpandError as _;
use crate::fileset;
use crate::fileset::FilesetDiagnostics;
use crate::fileset::FilesetExpression;
use crate::graph::GraphEdge;
use crate::hex_util::to_forward_hex;
@ -804,7 +805,8 @@ pub fn expect_fileset_expression(
// 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| {
fileset::parse(node.span.as_str(), path_converter).map_err(|err| {
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)
})
})