chore: address PR feedback; have handle_binding_list() and parse_name_type_pairs() return Result<_, E> where E: for<'a> From<(CheckErrors, &'a SymbolicExpression)> so these functions can be used in both analysis and runtime contexts with the appropriate error types (this patch also adds conversions for them)

Author: jcnelson

clarity/src/vm/analysis/errors.rs

@@ -373,6 +373,20 @@ impl From<(SyntaxBindingError, &SymbolicExpression)> for CheckError {
     }
 }
 
+impl From<(CheckErrors, &SymbolicExpression)> for CheckError {
+    fn from(e: (CheckErrors, &SymbolicExpression)) -> Self {
+        let mut ce = Self::new(e.0);
+        ce.set_expression(e.1);
+        ce
+    }
+}
+
+impl From<(CheckErrors, &SymbolicExpression)> for CheckErrors {
+    fn from(e: (CheckErrors, &SymbolicExpression)) -> Self {
+        e.0
+    }
+}
+
 impl fmt::Display for CheckErrors {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "{self:?}")

clarity/src/vm/analysis/read_only_checker/mod.rs

@@ -335,7 +335,7 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> {
                     if pair_expression.len() != 2 {
                         return Err(CheckError::with_expression(
                             SyntaxBindingError::let_binding_invalid_length(i).into(),
-                            &SymbolicExpression::list(pair_expression.to_vec()),
+                            pair,
                         ));
                     }
 
@@ -383,7 +383,7 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> {
                     if pair_expression.len() != 2 {
                         return Err(CheckError::with_expression(
                             SyntaxBindingError::tuple_cons_invalid_length(i).into(),
-                            &SymbolicExpression::list(pair_expression.to_vec()),
+                            pair,
                         ));
                     }
 

clarity/src/vm/analysis/type_checker/v2_05/mod.rs

@@ -556,7 +556,7 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
         let function_name = function_name
             .match_atom()
             .ok_or(CheckErrors::BadFunctionName)?;
-        let args = parse_name_type_pairs::<()>(
+        let args = parse_name_type_pairs::<(), CheckError>(
             StacksEpochId::Epoch2_05,
             args,
             SyntaxBindingErrorType::Eval,

clarity/src/vm/analysis/type_checker/v2_1/mod.rs

@@ -1251,7 +1251,7 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
         let function_name = function_name
             .match_atom()
             .ok_or(CheckErrors::BadFunctionName)?;
-        let args = parse_name_type_pairs::<()>(
+        let args = parse_name_type_pairs::<(), CheckError>(
             StacksEpochId::Epoch21,
             args,
             SyntaxBindingErrorType::Eval,

clarity/src/vm/errors.rs

@@ -30,6 +30,7 @@ use crate::vm::ast::errors::ParseError;
 use crate::vm::contexts::StackTrace;
 use crate::vm::costs::CostErrors;
 use crate::vm::types::Value;
+use crate::vm::SymbolicExpression;
 
 #[derive(Debug)]
 pub struct IncomparableError<T> {
@@ -207,6 +208,12 @@ impl From<CheckErrors> for Error {
     }
 }
 
+impl From<(CheckErrors, &SymbolicExpression)> for Error {
+    fn from(err: (CheckErrors, &SymbolicExpression)) -> Self {
+        Error::Unchecked(err.0)
+    }
+}
+
 impl From<ShortReturnType> for Error {
     fn from(err: ShortReturnType) -> Self {
         Error::ShortReturn(err)

clarity/src/vm/functions/define.rs

@@ -142,8 +142,12 @@ fn handle_define_function(
 
     check_legal_define(function_name, env.contract_context)?;
 
-    let arguments =
-        parse_name_type_pairs(*env.epoch(), arg_symbols, SyntaxBindingErrorType::Eval, env)?;
+    let arguments = parse_name_type_pairs::<_, CheckErrors>(
+        *env.epoch(),
+        arg_symbols,
+        SyntaxBindingErrorType::Eval,
+        env,
+    )?;
 
     for (argument, _) in arguments.iter() {
         check_legal_define(argument, env.contract_context)?;

clarity/src/vm/functions/mod.rs

@@ -669,23 +669,27 @@ pub fn handle_binding_list<F, E>(
 ) -> std::result::Result<(), E>
 where
     F: FnMut(&ClarityName, &SymbolicExpression) -> std::result::Result<(), E>,
-    E: From<CheckErrors>,
+    E: for<'a> From<(CheckErrors, &'a SymbolicExpression)>,
 {
     for (i, binding) in bindings.iter().enumerate() {
         let binding_expression = binding.match_list().ok_or_else(|| {
-            CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotList(binding_error_type, i))
+            (
+                SyntaxBindingError::NotList(binding_error_type, i).into(),
+                binding,
+            )
         })?;
         if binding_expression.len() != 2 {
-            return Err(
-                CheckErrors::BadSyntaxBinding(SyntaxBindingError::InvalidLength(
-                    binding_error_type,
-                    i,
-                ))
-                .into(),
-            );
+            return Err((
+                SyntaxBindingError::InvalidLength(binding_error_type, i).into(),
+                binding,
+            )
+                .into());
         }
         let var_name = binding_expression[0].match_atom().ok_or_else(|| {
-            CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotAtom(binding_error_type, i))
+            (
+                SyntaxBindingError::NotAtom(binding_error_type, i).into(),
+                &binding_expression[0],
+            )
         })?;
         let var_sexp = &binding_expression[1];
 

clarity/src/vm/types/signatures.rs

@@ -1494,7 +1494,7 @@ impl TypeSignature {
         type_args: &[SymbolicExpression],
         accounting: &mut A,
     ) -> Result<TypeSignature> {
-        let mapped_key_types = parse_name_type_pairs(
+        let mapped_key_types = parse_name_type_pairs::<_, CheckErrors>(
             epoch,
             type_args,
             SyntaxBindingErrorType::TupleCons,
@@ -1919,57 +1919,71 @@ use crate::vm::ClarityVersion;
 /// Try to parse a list of (name_i, type_i) pairs into Vec<(ClarityName, TypeSignature)>.
 /// On failure, return both the type-check error as well as the index of the symbolic expression which caused
 /// the problem (for purposes of reporting the error).
-pub fn parse_name_type_pairs<A: CostTracker>(
+pub fn parse_name_type_pairs<A: CostTracker, E>(
     epoch: StacksEpochId,
     name_type_pairs: &[SymbolicExpression],
     binding_error_type: SyntaxBindingErrorType,
     accounting: &mut A,
-) -> std::result::Result<Vec<(ClarityName, TypeSignature)>, CheckErrors> {
+) -> std::result::Result<Vec<(ClarityName, TypeSignature)>, E>
+where
+    E: for<'a> From<(CheckErrors, &'a SymbolicExpression)>,
+{
     // this is a pretty deep nesting here, but what we're trying to do is pick out the values of
     // the form:
     // ((name1 type1) (name2 type2) (name3 type3) ...)
     // which is a list of 2-length lists of atoms.
     use crate::vm::representations::SymbolicExpressionType::List;
 
     // step 1: parse it into a vec of symbolicexpression pairs.
-    let as_pairs: std::result::Result<Vec<_>, CheckErrors> = name_type_pairs
+    let as_pairs: std::result::Result<Vec<_>, (CheckErrors, &SymbolicExpression)> = name_type_pairs
         .iter()
         .enumerate()
         .map(|(i, key_type_pair)| {
             if let List(ref as_vec) = key_type_pair.expr {
                 if as_vec.len() != 2 {
-                    Err(CheckErrors::BadSyntaxBinding(
-                        SyntaxBindingError::InvalidLength(binding_error_type, i),
+                    Err((
+                        CheckErrors::BadSyntaxBinding(SyntaxBindingError::InvalidLength(
+                            binding_error_type,
+                            i,
+                        )),
+                        key_type_pair,
                     ))
                 } else {
                     Ok((&as_vec[0], &as_vec[1]))
                 }
             } else {
-                Err(SyntaxBindingError::NotList(binding_error_type, i).into())
+                Err((
+                    SyntaxBindingError::NotList(binding_error_type, i).into(),
+                    key_type_pair,
+                ))
             }
         })
         .collect();
 
     // step 2: turn into a vec of (name, typesignature) pairs.
-    let key_types: std::result::Result<Vec<_>, CheckErrors> = (as_pairs?)
+    let key_types: std::result::Result<Vec<_>, (CheckErrors, &SymbolicExpression)> = (as_pairs?)
         .iter()
         .enumerate()
         .map(|(i, (name_symbol, type_symbol))| {
             let name = name_symbol
                 .match_atom()
                 .ok_or_else(|| {
-                    CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotAtom(
-                        binding_error_type,
-                        i,
-                    ))
+                    (
+                        CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotAtom(
+                            binding_error_type,
+                            i,
+                        )),
+                        *name_symbol,
+                    )
                 })?
                 .clone();
-            let type_info = TypeSignature::parse_type_repr(epoch, type_symbol, accounting)?;
+            let type_info = TypeSignature::parse_type_repr(epoch, type_symbol, accounting)
+                .map_err(|e| (e, *type_symbol))?;
             Ok((name, type_info))
         })
         .collect();
 
-    key_types
+    Ok(key_types?)
 }
 
 impl fmt::Display for TupleTypeSignature {