changeset 46428:5893706af3de

rust: Simplify error type for reading hex node IDs

If a string is not valid hexadecimal it’s not that useful to track the precise reason.

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

--- a/rust/hg-core/src/revlog.rs	Mon Jan 25 12:00:23 2021 +0100
+++ b/rust/hg-core/src/revlog.rs	Mon Jan 25 12:28:39 2021 +0100
@@ -9,7 +9,7 @@
 pub mod nodemap;
 mod nodemap_docket;
 pub mod path_encode;
-pub use node::{Node, NodeError, NodePrefix, NodePrefixRef};
+pub use node::{FromHexError, Node, NodePrefix, NodePrefixRef};
 pub mod changelog;
 pub mod index;
 pub mod manifest;

--- a/rust/hg-core/src/revlog/node.rs	Mon Jan 25 12:00:23 2021 +0100
+++ b/rust/hg-core/src/revlog/node.rs	Mon Jan 25 12:28:39 2021 +0100
@@ -9,7 +9,7 @@
 //! of a revision.
 
 use bytes_cast::BytesCast;
-use hex::{self, FromHex, FromHexError};
+use hex::{self, FromHex};
 use std::convert::TryFrom;
 use std::fmt;
 
@@ -47,10 +47,9 @@
 /// if they need a loop boundary.
 ///
 /// All methods that create a `Node` either take a type that enforces
-/// the size or fail immediately at runtime with [`ExactLengthRequired`].
+/// the size or return an error at runtime.
 ///
 /// [`nybbles_len`]: #method.nybbles_len
-/// [`ExactLengthRequired`]: struct.NodeError#variant.ExactLengthRequired
 #[derive(Clone, Debug, PartialEq, BytesCast)]
 #[repr(transparent)]
 pub struct Node {
@@ -90,12 +89,8 @@
     }
 }
 
-#[derive(Debug, PartialEq)]
-pub enum NodeError {
-    ExactLengthRequired(usize, String),
-    PrefixTooLong(String),
-    HexError(FromHexError, String),
-}
+#[derive(Debug)]
+pub struct FromHexError;
 
 /// Low level utility function, also for prefixes
 fn get_nybble(s: &[u8], i: usize) -> u8 {
@@ -128,9 +123,9 @@
     ///
     /// To be used in FFI and I/O only, in order to facilitate future
     /// changes of hash format.
-    pub fn from_hex(hex: impl AsRef<[u8]>) -> Result<Node, NodeError> {
+    pub fn from_hex(hex: impl AsRef<[u8]>) -> Result<Node, FromHexError> {
         Ok(NodeData::from_hex(hex.as_ref())
-            .map_err(|e| NodeError::from((e, hex)))?
+            .map_err(|_| FromHexError)?
             .into())
     }
 
@@ -143,19 +138,6 @@
     }
 }
 
-impl<T: AsRef<[u8]>> From<(FromHexError, T)> for NodeError {
-    fn from(err_offender: (FromHexError, T)) -> Self {
-        let (err, offender) = err_offender;
-        let offender = String::from_utf8_lossy(offender.as_ref()).into_owned();
-        match err {
-            FromHexError::InvalidStringLength => {
-                NodeError::ExactLengthRequired(NODE_NYBBLES_LENGTH, offender)
-            }
-            _ => NodeError::HexError(err, offender),
-        }
-    }
-}
-
 /// The beginning of a binary revision SHA.
 ///
 /// Since it can potentially come from an hexadecimal representation with
@@ -175,31 +157,22 @@
     ///
     /// To be used in FFI and I/O only, in order to facilitate future
     /// changes of hash format.
-    pub fn from_hex(hex: impl AsRef<[u8]>) -> Result<Self, NodeError> {
+    pub fn from_hex(hex: impl AsRef<[u8]>) -> Result<Self, FromHexError> {
         let hex = hex.as_ref();
         let len = hex.len();
         if len > NODE_NYBBLES_LENGTH {
-            return Err(NodeError::PrefixTooLong(
-                String::from_utf8_lossy(hex).to_owned().to_string(),
-            ));
+            return Err(FromHexError);
         }
 
         let is_odd = len % 2 == 1;
         let even_part = if is_odd { &hex[..len - 1] } else { hex };
         let mut buf: Vec<u8> =
-            Vec::from_hex(&even_part).map_err(|e| (e, hex))?;
+            Vec::from_hex(&even_part).map_err(|_| FromHexError)?;
 
         if is_odd {
             let latest_char = char::from(hex[len - 1]);
-            let latest_nybble = latest_char.to_digit(16).ok_or_else(|| {
-                (
-                    FromHexError::InvalidHexCharacter {
-                        c: latest_char,
-                        index: len - 1,
-                    },
-                    hex,
-                )
-            })? as u8;
+            let latest_nybble =
+                latest_char.to_digit(16).ok_or_else(|| FromHexError)? as u8;
             buf.push(latest_nybble << 4);
         }
         Ok(NodePrefix { buf, is_odd })
@@ -329,24 +302,15 @@
 
     #[test]
     fn test_node_from_hex() {
-        assert_eq!(Node::from_hex(&sample_node_hex()), Ok(sample_node()));
+        assert_eq!(Node::from_hex(&sample_node_hex()).unwrap(), sample_node());
 
         let mut short = hex_pad_right("0123");
         short.pop();
         short.pop();
-        assert_eq!(
-            Node::from_hex(&short),
-            Err(NodeError::ExactLengthRequired(NODE_NYBBLES_LENGTH, short)),
-        );
+        assert!(Node::from_hex(&short).is_err());
 
         let not_hex = hex_pad_right("012... oops");
-        assert_eq!(
-            Node::from_hex(&not_hex),
-            Err(NodeError::HexError(
-                FromHexError::InvalidHexCharacter { c: '.', index: 3 },
-                not_hex,
-            )),
-        );
+        assert!(Node::from_hex(&not_hex).is_err(),);
     }
 
     #[test]
@@ -355,7 +319,7 @@
     }
 
     #[test]
-    fn test_prefix_from_hex() -> Result<(), NodeError> {
+    fn test_prefix_from_hex() -> Result<(), FromHexError> {
         assert_eq!(
             NodePrefix::from_hex("0e1")?,
             NodePrefix {
@@ -386,25 +350,14 @@
 
     #[test]
     fn test_prefix_from_hex_errors() {
-        assert_eq!(
-            NodePrefix::from_hex("testgr"),
-            Err(NodeError::HexError(
-                FromHexError::InvalidHexCharacter { c: 't', index: 0 },
-                "testgr".to_string()
-            ))
-        );
+        assert!(NodePrefix::from_hex("testgr").is_err());
         let mut long = format!("{:x}", NULL_NODE);
         long.push('c');
-        match NodePrefix::from_hex(&long)
-            .expect_err("should be refused as too long")
-        {
-            NodeError::PrefixTooLong(s) => assert_eq!(s, long),
-            err => panic!(format!("Should have been TooLong, got {:?}", err)),
-        }
+        assert!(NodePrefix::from_hex(&long).is_err())
     }
 
     #[test]
-    fn test_is_prefix_of() -> Result<(), NodeError> {
+    fn test_is_prefix_of() -> Result<(), FromHexError> {
         let mut node_data = [0; NODE_BYTES_LENGTH];
         node_data[0] = 0x12;
         node_data[1] = 0xca;
@@ -417,7 +370,7 @@
     }
 
     #[test]
-    fn test_get_nybble() -> Result<(), NodeError> {
+    fn test_get_nybble() -> Result<(), FromHexError> {
         let prefix = NodePrefix::from_hex("dead6789cafe")?;
         assert_eq!(prefix.borrow().get_nybble(0), 13);
         assert_eq!(prefix.borrow().get_nybble(7), 9);

--- a/rust/hg-core/src/revlog/nodemap.rs	Mon Jan 25 12:00:23 2021 +0100
+++ b/rust/hg-core/src/revlog/nodemap.rs	Mon Jan 25 12:28:39 2021 +0100
@@ -13,7 +13,7 @@
 //! is used in a more abstract context.
 
 use super::{
-    node::NULL_NODE, Node, NodeError, NodePrefix, NodePrefixRef, Revision,
+    node::NULL_NODE, FromHexError, Node, NodePrefix, NodePrefixRef, Revision,
     RevlogIndex, NULL_REVISION,
 };
 
@@ -27,14 +27,14 @@
 #[derive(Debug, PartialEq)]
 pub enum NodeMapError {
     MultipleResults,
-    InvalidNodePrefix(NodeError),
+    InvalidNodePrefix,
     /// A `Revision` stored in the nodemap could not be found in the index
     RevisionNotInIndex(Revision),
 }
 
-impl From<NodeError> for NodeMapError {
-    fn from(err: NodeError) -> Self {
-        NodeMapError::InvalidNodePrefix(err)
+impl From<FromHexError> for NodeMapError {
+    fn from(_: FromHexError) -> Self {
+        NodeMapError::InvalidNodePrefix
     }
 }
 

--- a/rust/hg-cpython/src/revlog.rs	Mon Jan 25 12:00:23 2021 +0100
+++ b/rust/hg-cpython/src/revlog.rs	Mon Jan 25 12:28:39 2021 +0100
@@ -18,7 +18,7 @@
 use hg::{
     nodemap::{Block, NodeMapError, NodeTree},
     revlog::{nodemap::NodeMap, RevlogIndex},
-    NodeError, Revision,
+    Revision,
 };
 use std::cell::RefCell;
 
@@ -468,17 +468,12 @@
     match err {
         NodeMapError::MultipleResults => revlog_error(py),
         NodeMapError::RevisionNotInIndex(r) => rev_not_in_index(py, r),
-        NodeMapError::InvalidNodePrefix(s) => invalid_node_prefix(py, &s),
+        NodeMapError::InvalidNodePrefix => {
+            PyErr::new::<ValueError, _>(py, "Invalid node or prefix")
+        }
     }
 }
 
-fn invalid_node_prefix(py: Python, ne: &NodeError) -> PyErr {
-    PyErr::new::<ValueError, _>(
-        py,
-        format!("Invalid node or prefix: {:?}", ne),
-    )
-}
-
 /// Create the module, with __package__ given from parent
 pub fn init_module(py: Python, package: &str) -> PyResult<PyModule> {
     let dotted_name = &format!("{}.revlog", package);