revset, templater: simplify parse error impls by using thiserror

This patch moves all "source" errors to the source field to conform to
thiserror API. It will probably help to keep ErrorKind enums comparable.

cli/src/template_parser.rs

@@ -13,7 +13,6 @@
 // limitations under the License.
 
 use std::collections::HashMap;
-use std::num::ParseIntError;
 use std::{error, fmt, mem};
 
 use itertools::Itertools as _;
@@ -64,7 +63,8 @@ impl Rule {
 
 pub type TemplateParseResult<T> = Result<T, TemplateParseError>;
 
-#[derive(Debug)]
+#[derive(Debug, Error)]
+#[error("{pest_error}")]
 pub struct TemplateParseError {
     kind: TemplateParseErrorKind,
     pest_error: Box<pest::error::Error<Rule>>,
@@ -76,7 +76,7 @@ pub enum TemplateParseErrorKind {
     #[error("Syntax error")]
     SyntaxError,
     #[error("Invalid integer literal")]
-    ParseIntError(#[source] ParseIntError),
+    ParseIntError,
     #[error(r#"Keyword "{name}" doesn't exist"#)]
     NoSuchKeyword {
         name: String,
@@ -252,24 +252,6 @@ impl From<pest::error::Error<Rule>> for TemplateParseError {
     }
 }
 
-impl fmt::Display for TemplateParseError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.pest_error.fmt(f)
-    }
-}
-
-impl error::Error for TemplateParseError {
-    fn source(&self) -> Option<&(dyn error::Error + 'static)> {
-        if let Some(e) = self.source.as_deref() {
-            Some(e)
-        } else {
-            // SyntaxError originates from self.pest_error, but
-            // pest::error::Error doesn't have a source anyway.
-            self.kind.source()
-        }
-    }
-}
-
 fn rename_rules_in_pest_error(err: pest::error::Error<Rule>) -> pest::error::Error<Rule> {
     err.renamed_rules(|rule| {
         rule.to_symbol()
@@ -453,7 +435,11 @@ fn parse_term_node(pair: Pair<Rule>) -> TemplateParseResult<ExpressionNode> {
         }
         Rule::integer_literal => {
             let value = expr.as_str().parse().map_err(|err| {
-                TemplateParseError::with_span(TemplateParseErrorKind::ParseIntError(err), span)
+                TemplateParseError::with_span_and_source(
+                    TemplateParseErrorKind::ParseIntError,
+                    span,
+                    err,
+                )
             })?;
             ExpressionNode::new(ExpressionKind::Integer(value), span)
         }
@@ -1276,7 +1262,7 @@ mod tests {
         );
         assert_matches!(
             parse_into_kind(&format!("{}", (i64::MAX as u64) + 1)),
-            Err(TemplateParseErrorKind::ParseIntError(_))
+            Err(TemplateParseErrorKind::ParseIntError)
         );
     }
 

lib/src/revset.rs

@@ -38,7 +38,7 @@ use crate::hex_util::to_forward_hex;
 use crate::object_id::{HexPrefix, PrefixResolution};
 use crate::op_store::WorkspaceId;
 use crate::repo::Repo;
-use crate::repo_path::{FsPathParseError, RepoPathBuf};
+use crate::repo_path::RepoPathBuf;
 use crate::revset_graph::RevsetGraphEdge;
 use crate::store::Store;
 use crate::str_util::StringPattern;
@@ -141,7 +141,8 @@ impl Rule {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Error)]
+#[error("{pest_error}")]
 pub struct RevsetParseError {
     kind: RevsetParseErrorKind,
     pest_error: Box<pest::error::Error<Rule>>,
@@ -178,7 +179,7 @@ pub enum RevsetParseErrorKind {
     #[error("Invalid arguments to revset function \"{name}\": {message}")]
     InvalidFunctionArguments { name: String, message: String },
     #[error("Invalid file pattern")]
-    FsPathParseError(#[source] FsPathParseError),
+    FsPathParseError,
     #[error("Cannot resolve file pattern without workspace")]
     FsPathWithoutWorkspace,
     #[error(r#"Cannot resolve "@" without workspace"#)]
@@ -242,24 +243,6 @@ impl From<pest::error::Error<Rule>> for RevsetParseError {
     }
 }
 
-impl fmt::Display for RevsetParseError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        self.pest_error.fmt(f)
-    }
-}
-
-impl error::Error for RevsetParseError {
-    fn source(&self) -> Option<&(dyn error::Error + 'static)> {
-        if let Some(e) = self.source.as_deref() {
-            Some(e)
-        } else {
-            // SyntaxError originates from self.pest_error, but
-            // pest::error::Error doesn't have a source anyway.
-            self.kind.source()
-        }
-    }
-}
-
 fn rename_rules_in_pest_error(mut err: pest::error::Error<Rule>) -> pest::error::Error<Rule> {
     let pest::error::ErrorVariant::ParsingError {
         positives,
@@ -1288,9 +1271,10 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
                     let needle = parse_function_argument_to_string(name, arg, state)?;
                     let path = RepoPathBuf::parse_fs_path(ctx.cwd, ctx.workspace_root, needle)
                         .map_err(|e| {
-                            RevsetParseError::with_span(
-                                RevsetParseErrorKind::FsPathParseError(e),
+                            RevsetParseError::with_span_and_source(
+                                RevsetParseErrorKind::FsPathParseError,
                                 span,
+                                e,
                             )
                         })?;
                     Ok(path)