changeset 46637:bc08c2331f99

rust: Add a `ConfigValueParseError` variant to common errors

Configuration files are parsed into sections of key/value pairs when
they are read, but at that point values are still arbitrary bytes.
Only when a value is accessed by various parts of the code do we know
its expected type and syntax, so values are parsed at that point.

Let’s make a new error type for this latter kind of parsing error,
and add a variant to the common `HgError` so that most code can propagate it
without much boilerplate.

Differential Revision: https://phab.mercurial-scm.org/D10009

--- a/rust/hg-core/src/config.rs	Tue Feb 16 13:55:31 2021 +0100
+++ b/rust/hg-core/src/config.rs	Tue Feb 16 15:22:20 2021 +0100
@@ -11,5 +11,5 @@
 
 mod config;
 mod layer;
-pub use config::Config;
+pub use config::{Config, ConfigValueParseError};
 pub use layer::{ConfigError, ConfigParseError};

--- a/rust/hg-core/src/config/config.rs	Tue Feb 16 13:55:31 2021 +0100
+++ b/rust/hg-core/src/config/config.rs	Tue Feb 16 15:22:20 2021 +0100
@@ -9,7 +9,7 @@
 
 use super::layer;
 use crate::config::layer::{
-    ConfigError, ConfigLayer, ConfigParseError, ConfigValue,
+    ConfigError, ConfigLayer, ConfigOrigin, ConfigValue,
 };
 use crate::utils::files::get_bytes_from_os_str;
 use format_bytes::{write_bytes, DisplayBytes};
@@ -54,6 +54,16 @@
     Parsed(layer::ConfigLayer),
 }
 
+#[derive(Debug)]
+pub struct ConfigValueParseError {
+    pub origin: ConfigOrigin,
+    pub line: Option<usize>,
+    pub section: Vec<u8>,
+    pub item: Vec<u8>,
+    pub value: Vec<u8>,
+    pub expected_type: &'static str,
+}
+
 pub fn parse_bool(v: &[u8]) -> Option<bool> {
     match v.to_ascii_lowercase().as_slice() {
         b"1" | b"yes" | b"true" | b"on" | b"always" => Some(true),
@@ -262,15 +272,19 @@
         &'config self,
         section: &[u8],
         item: &[u8],
+        expected_type: &'static str,
         parse: impl Fn(&'config [u8]) -> Option<T>,
-    ) -> Result<Option<T>, ConfigParseError> {
+    ) -> Result<Option<T>, ConfigValueParseError> {
         match self.get_inner(&section, &item) {
             Some((layer, v)) => match parse(&v.bytes) {
                 Some(b) => Ok(Some(b)),
-                None => Err(ConfigParseError {
+                None => Err(ConfigValueParseError {
                     origin: layer.origin.to_owned(),
                     line: v.line,
-                    bytes: v.bytes.to_owned(),
+                    value: v.bytes.to_owned(),
+                    section: section.to_owned(),
+                    item: item.to_owned(),
+                    expected_type,
                 }),
             },
             None => Ok(None),
@@ -283,8 +297,10 @@
         &self,
         section: &[u8],
         item: &[u8],
-    ) -> Result<Option<&str>, ConfigParseError> {
-        self.get_parse(section, item, |value| str::from_utf8(value).ok())
+    ) -> Result<Option<&str>, ConfigValueParseError> {
+        self.get_parse(section, item, "ASCII or UTF-8 string", |value| {
+            str::from_utf8(value).ok()
+        })
     }
 
     /// Returns an `Err` if the first value found is not a valid unsigned
@@ -293,8 +309,8 @@
         &self,
         section: &[u8],
         item: &[u8],
-    ) -> Result<Option<u32>, ConfigParseError> {
-        self.get_parse(section, item, |value| {
+    ) -> Result<Option<u32>, ConfigValueParseError> {
+        self.get_parse(section, item, "valid integer", |value| {
             str::from_utf8(value).ok()?.parse().ok()
         })
     }
@@ -306,8 +322,8 @@
         &self,
         section: &[u8],
         item: &[u8],
-    ) -> Result<Option<u64>, ConfigParseError> {
-        self.get_parse(section, item, parse_byte_size)
+    ) -> Result<Option<u64>, ConfigValueParseError> {
+        self.get_parse(section, item, "byte quantity", parse_byte_size)
     }
 
     /// Returns an `Err` if the first value found is not a valid boolean.
@@ -317,8 +333,8 @@
         &self,
         section: &[u8],
         item: &[u8],
-    ) -> Result<Option<bool>, ConfigParseError> {
-        self.get_parse(section, item, parse_bool)
+    ) -> Result<Option<bool>, ConfigValueParseError> {
+        self.get_parse(section, item, "boolean", parse_bool)
     }
 
     /// Returns the corresponding boolean in the config. Returns `Ok(false)`
@@ -327,7 +343,7 @@
         &self,
         section: &[u8],
         item: &[u8],
-    ) -> Result<bool, ConfigError> {
+    ) -> Result<bool, ConfigValueParseError> {
         Ok(self.get_option(section, item)?.unwrap_or(false))
     }
 

--- a/rust/hg-core/src/errors.rs	Tue Feb 16 13:55:31 2021 +0100
+++ b/rust/hg-core/src/errors.rs	Tue Feb 16 15:22:20 2021 +0100
@@ -1,7 +1,8 @@
+use crate::config::ConfigValueParseError;
 use std::fmt;
 
 /// Common error cases that can happen in many different APIs
-#[derive(Debug)]
+#[derive(Debug, derive_more::From)]
 pub enum HgError {
     IoError {
         error: std::io::Error,
@@ -29,6 +30,14 @@
     /// The given string is a short explanation for users, not intended to be
     /// machine-readable.
     Abort(String),
+
+    /// A configuration value is not in the expected syntax.
+    ///
+    /// These errors can happen in many places in the code because values are
+    /// parsed lazily as the file-level parser does not know the expected type
+    /// and syntax of each value.
+    #[from]
+    ConfigValueParseError(ConfigValueParseError),
 }
 
 /// Details about where an I/O error happened
@@ -63,6 +72,7 @@
 impl fmt::Display for HgError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
+            HgError::Abort(explanation) => write!(f, "{}", explanation),
             HgError::IoError { error, context } => {
                 write!(f, "{}: {}", error, context)
             }
@@ -72,7 +82,25 @@
             HgError::UnsupportedFeature(explanation) => {
                 write!(f, "unsupported feature: {}", explanation)
             }
-            HgError::Abort(explanation) => explanation.fmt(f),
+            HgError::ConfigValueParseError(ConfigValueParseError {
+                origin: _,
+                line: _,
+                section,
+                item,
+                value,
+                expected_type,
+            }) => {
+                // TODO: add origin and line number information, here and in
+                // corresponding python code
+                write!(
+                    f,
+                    "config error: {}.{} is not a {} ('{}')",
+                    String::from_utf8_lossy(section),
+                    String::from_utf8_lossy(item),
+                    expected_type,
+                    String::from_utf8_lossy(value)
+                )
+            }
         }
     }
 }