Merge pull request #6806 from jacinta-stacks/refactor/remove-ast-error

Refactor: remove ASTError from VmExecutionError and seperate parse_ast from vm calls

Author: jacinta-stacks

clarity/src/vm/analysis/type_checker/v2_1/tests/contracts.rs

@@ -1636,9 +1636,7 @@ fn clarity_trait_experiments_use_undefined(
     let err = db
         .execute(|db| load_versioned(db, "no-trait", version, epoch))
         .unwrap_err();
-    assert!(err.starts_with(
-        "ASTError(ParseError { err: TraitReferenceUnknown(\"trait-to-be-defined-later\")"
-    ));
+    assert!(err.starts_with("TraitReferenceUnknown(\"trait-to-be-defined-later\")"));
 }
 
 #[apply(test_clarity_versions)]
@@ -1656,7 +1654,7 @@ fn clarity_trait_experiments_circular(
             load_versioned(db, "circular-trait-2", version, epoch)
         })
         .unwrap_err();
-    assert!(err.starts_with("ASTError(ParseError { err: CircularReference([\"circular\"])"));
+    assert!(err.starts_with("CircularReference([\"circular\"])"));
 }
 
 #[apply(test_clarity_versions)]
@@ -1894,7 +1892,7 @@ fn clarity_trait_experiments_selfret_trait(
     let err = db
         .execute(|db| load_versioned(db, "selfret-trait", version, epoch))
         .unwrap_err();
-    assert!(err.starts_with("ASTError(ParseError { err: CircularReference([\"self-return\"])"));
+    assert!(err.starts_with("CircularReference([\"self-return\"])"));
 }
 
 #[apply(test_clarity_versions)]
@@ -2300,7 +2298,7 @@ fn clarity_trait_experiments_trait_data_1(
             load_versioned(db, "trait-data-1", version, epoch)
         })
         .unwrap_err();
-    assert!(err.starts_with("ASTError(ParseError { err: TraitReferenceNotAllowed"));
+    assert!(err.starts_with("TraitReferenceNotAllowed"));
 }
 
 #[apply(test_clarity_versions)]
@@ -2319,7 +2317,7 @@ fn clarity_trait_experiments_trait_data_2(
             load_versioned(db, "trait-data-2", version, epoch)
         })
         .unwrap_err();
-    assert!(err.starts_with("ASTError(ParseError { err: TraitReferenceNotAllowed"));
+    assert!(err.starts_with("TraitReferenceNotAllowed"));
 }
 
 #[apply(test_clarity_versions)]

clarity/src/vm/ast/mod.rs

@@ -50,7 +50,7 @@ pub fn parse(
     source_code: &str,
     version: ClarityVersion,
     epoch: StacksEpochId,
-) -> Result<Vec<crate::vm::representations::SymbolicExpression>, crate::vm::errors::VmExecutionError>
+) -> Result<Vec<crate::vm::representations::SymbolicExpression>, clarity_types::errors::ParseError>
 {
     let ast = build_ast(contract_identifier, source_code, &mut (), version, epoch)?;
     Ok(ast.expressions)

clarity/src/vm/clarity.rs

@@ -25,7 +25,7 @@ use crate::vm::ast::errors::{ParseError, ParseErrorKind};
 use crate::vm::contexts::{AssetMap, Environment, OwnedEnvironment};
 use crate::vm::costs::{ExecutionCost, LimitedCostTracker};
 use crate::vm::database::ClarityDatabase;
-use crate::vm::errors::VmExecutionError;
+use crate::vm::errors::{ClarityEvalError, VmExecutionError};
 use crate::vm::events::StacksTransactionEvent;
 use crate::vm::types::{BuffData, PrincipalData, QualifiedContractIdentifier};
 use crate::vm::{ClarityVersion, ContractContext, SymbolicExpression, Value, analysis, ast};
@@ -111,6 +111,15 @@ impl From<StaticCheckError> for ClarityError {
     }
 }
 
+impl From<ClarityEvalError> for ClarityError {
+    fn from(e: ClarityEvalError) -> Self {
+        match e {
+            ClarityEvalError::Parse(err) => ClarityError::from(err),
+            ClarityEvalError::Vm(err) => ClarityError::from(err),
+        }
+    }
+}
+
 /// Converts [`VmExecutionError`] to [`ClarityError`] for transaction execution contexts.
 ///
 /// This conversion is used in:
@@ -195,9 +204,9 @@ pub trait ClarityConnection {
         sponsor: Option<PrincipalData>,
         cost_track: LimitedCostTracker,
         to_do: F,
-    ) -> Result<R, VmExecutionError>
+    ) -> Result<R, ClarityEvalError>
     where
-        F: FnOnce(&mut Environment) -> Result<R, VmExecutionError>,
+        F: FnOnce(&mut Environment) -> Result<R, ClarityEvalError>,
     {
         let epoch_id = self.get_epoch();
         let clarity_version = ClarityVersion::default_for_epoch(epoch_id);

clarity/src/vm/contexts.rs

@@ -19,6 +19,7 @@ use std::fmt;
 use std::mem::replace;
 use std::time::{Duration, Instant};
 
+use clarity_types::errors::{ParseError, ParseErrorKind};
 use clarity_types::representations::ClarityName;
 use serde::Serialize;
 use serde_json::json;
@@ -36,7 +37,8 @@ use crate::vm::database::{
     NonFungibleTokenMetadata,
 };
 use crate::vm::errors::{
-    RuntimeCheckErrorKind, RuntimeError, StackTrace, VmExecutionError, VmInternalError,
+    ClarityEvalError, RuntimeCheckErrorKind, RuntimeError, StackTrace, VmExecutionError,
+    VmInternalError,
 };
 use crate::vm::events::*;
 use crate::vm::representations::SymbolicExpression;
@@ -722,7 +724,7 @@ impl<'a, 'hooks> OwnedEnvironment<'a, 'hooks> {
         contract_identifier: QualifiedContractIdentifier,
         contract_content: &str,
         sponsor: Option<PrincipalData>,
-    ) -> Result<((), AssetMap, Vec<StacksTransactionEvent>), VmExecutionError> {
+    ) -> Result<((), AssetMap, Vec<StacksTransactionEvent>), ClarityEvalError> {
         self.execute_in_env(
             contract_identifier.issuer.clone().into(),
             sponsor,
@@ -737,7 +739,7 @@ impl<'a, 'hooks> OwnedEnvironment<'a, 'hooks> {
         version: ClarityVersion,
         contract_content: &str,
         sponsor: Option<PrincipalData>,
-    ) -> Result<((), AssetMap, Vec<StacksTransactionEvent>), VmExecutionError> {
+    ) -> Result<((), AssetMap, Vec<StacksTransactionEvent>), ClarityEvalError> {
         self.execute_in_env(
             contract_identifier.issuer.clone().into(),
             sponsor,
@@ -831,7 +833,7 @@ impl<'a, 'hooks> OwnedEnvironment<'a, 'hooks> {
     pub fn eval_raw(
         &mut self,
         program: &str,
-    ) -> Result<(Value, AssetMap, Vec<StacksTransactionEvent>), VmExecutionError> {
+    ) -> Result<(Value, AssetMap, Vec<StacksTransactionEvent>), ClarityEvalError> {
         self.execute_in_env(
             QualifiedContractIdentifier::transient().issuer.into(),
             None,
@@ -844,7 +846,7 @@ impl<'a, 'hooks> OwnedEnvironment<'a, 'hooks> {
         &mut self,
         contract: &QualifiedContractIdentifier,
         program: &str,
-    ) -> Result<(Value, AssetMap, Vec<StacksTransactionEvent>), VmExecutionError> {
+    ) -> Result<(Value, AssetMap, Vec<StacksTransactionEvent>), ClarityEvalError> {
         self.execute_in_env(
             QualifiedContractIdentifier::transient().issuer.into(),
             None,
@@ -1014,31 +1016,8 @@ impl<'a, 'b, 'hooks> Environment<'a, 'b, 'hooks> {
         &mut self,
         contract_identifier: &QualifiedContractIdentifier,
         program: &str,
-    ) -> Result<Value, VmExecutionError> {
-        let clarity_version = self.contract_context.clarity_version;
-
-        let parsed = ast::build_ast(
-            contract_identifier,
-            program,
-            self,
-            clarity_version,
-            self.global_context.epoch_id,
-        )?
-        .expressions;
-
-        if parsed.is_empty() {
-            // `TypeParseFailure` is **unreachable** in standard Clarity VM execution.
-            // - `eval_read_only` parses a raw program string into an AST.
-            // - Any empty or invalid program would be rejected at publish/deploy time or earlier parsing stages.
-            // - Therefore, `parsed.is_empty()` cannot occur for programs originating from a valid contract
-            // or transaction.
-            // - Only malformed input fed directly to this internal method (e.g., in unit tests or
-            // artificial VM invocations) can trigger this error.
-            return Err(RuntimeError::TypeParseFailure(
-                "Expected a program of at least length 1".to_string(),
-            )
-            .into());
-        }
+    ) -> Result<Value, ClarityEvalError> {
+        let parsed = self.parse_nonempty_program(contract_identifier, program)?;
 
         self.global_context.begin();
 
@@ -1062,40 +1041,19 @@ impl<'a, 'b, 'hooks> Environment<'a, 'b, 'hooks> {
             );
             let local_context = LocalContext::new();
             eval(&parsed[0], &mut nested_env, &local_context)
-        };
+        }
+        .map_err(ClarityEvalError::from);
 
         self.global_context.roll_back()?;
 
         result
     }
 
-    pub fn eval_raw(&mut self, program: &str) -> Result<Value, VmExecutionError> {
-        let contract_id = QualifiedContractIdentifier::transient();
-        let clarity_version = self.contract_context.clarity_version;
-
-        let parsed = ast::build_ast(
-            &contract_id,
-            program,
-            self,
-            clarity_version,
-            self.global_context.epoch_id,
-        )?
-        .expressions;
-
-        if parsed.is_empty() {
-            // `TypeParseFailure` is **unreachable** in standard Clarity VM execution.
-            // - `eval_raw` parses a raw program string into an AST.
-            // - All programs deployed or called via the standard VM go through static parsing and validation first.
-            // - Any empty or invalid program would be rejected at publish/deploy time or earlier parsing stages.
-            // - Therefore, `parsed.is_empty()` cannot occur for a program that originates from a valid Clarity contract or transaction.
-            // Only malformed input directly fed to this internal method (e.g., in unit tests) can trigger this error.
-            return Err(RuntimeError::TypeParseFailure(
-                "Expected a program of at least length 1".to_string(),
-            )
-            .into());
-        }
+    pub fn eval_raw(&mut self, program: &str) -> Result<Value, ClarityEvalError> {
+        let parsed =
+            self.parse_nonempty_program(&QualifiedContractIdentifier::transient(), program)?;
         let local_context = LocalContext::new();
-        eval(&parsed[0], self, &local_context)
+        eval(&parsed[0], self, &local_context).map_err(ClarityEvalError::from)
     }
 
     /// Used only for contract-call! cost short-circuiting. Once the short-circuited cost
@@ -1115,6 +1073,41 @@ impl<'a, 'b, 'hooks> Environment<'a, 'b, 'hooks> {
         result
     }
 
+    /// Parse `program` into a **non-empty** list of `SymbolicExpression`s.
+    ///
+    /// This is a wrapper around `ast::build_ast(..)` that enforces the invariant
+    /// that a parsed program must contain at least one top-level expression.
+    ///
+    /// # Errors
+    /// - Returns `Err` if the program fails to parse/build an AST.
+    /// - Returns `Err(UnexpectedParserFailure)` if parsing succeeds but yields *zero* expressions.
+    ///
+    /// # Notes
+    /// The empty-expression case should be unreachable for normal VM execution because
+    /// published/deployed contract code and transaction programs are validated earlier.
+    /// It exists as a defensive check for malformed input in tests, fuzzing, or internal
+    /// callers that bypass normal validation paths.
+    fn parse_nonempty_program(
+        &mut self,
+        contract_identifier: &QualifiedContractIdentifier,
+        program: &str,
+    ) -> Result<Vec<SymbolicExpression>, ClarityEvalError> {
+        let expressions = ast::build_ast(
+            contract_identifier,
+            program,
+            self,
+            self.contract_context.clarity_version,
+            self.global_context.epoch_id,
+        )?
+        .expressions;
+
+        if expressions.is_empty() {
+            return Err(ParseError::from(ParseErrorKind::UnexpectedParserFailure).into());
+        }
+
+        Ok(expressions)
+    }
+
     /// This is the epoch of the block that this transaction is executing within.
     /// Note: in the current plans for 2.1, there is also a contract-specific **Clarity version**
     ///  which governs which native functions are available / defined. That is separate from this
@@ -1303,7 +1296,7 @@ impl<'a, 'b, 'hooks> Environment<'a, 'b, 'hooks> {
         &mut self,
         contract_identifier: QualifiedContractIdentifier,
         contract_content: &str,
-    ) -> Result<(), VmExecutionError> {
+    ) -> Result<(), ClarityEvalError> {
         let clarity_version = self.contract_context.clarity_version;
 
         let contract_ast = ast::build_ast(
@@ -1319,6 +1312,7 @@ impl<'a, 'b, 'hooks> Environment<'a, 'b, 'hooks> {
             &contract_ast,
             contract_content,
         )
+        .map_err(ClarityEvalError::from)
     }
 
     pub fn initialize_contract_from_ast(
@@ -1754,7 +1748,7 @@ impl<'a, 'hooks> GlobalContext<'a, 'hooks> {
         f: F,
     ) -> std::result::Result<A, E>
     where
-        E: From<VmExecutionError>,
+        E: From<ClarityEvalError>,
         F: FnOnce(&mut Environment) -> std::result::Result<A, E>,
     {
         self.begin();
@@ -1773,7 +1767,7 @@ impl<'a, 'hooks> GlobalContext<'a, 'hooks> {
             );
             f(&mut exec_env)
         };
-        self.roll_back()?;
+        self.roll_back().map_err(ClarityEvalError::from)?;
 
         match result {
             Ok(return_value) => Ok(return_value),
@@ -2422,13 +2416,9 @@ mod test {
         // Call eval_read_only with an empty program
         let program = ""; // empty program triggers parsed.is_empty()
         let err = env.eval_raw(program).unwrap_err();
-
-        assert!(
-            matches!(
-            err,
-            VmExecutionError::Runtime(RuntimeError::TypeParseFailure(msg), _) if msg.contains("Expected a program of at least length 1")),
-            "Expected a type parse failure"
-        );
+        let expected_err =
+            ClarityEvalError::from(ParseError::new(ParseErrorKind::UnexpectedParserFailure));
+        assert_eq!(err, expected_err, "Expected a type parse failure");
     }
 
     #[test]
@@ -2443,13 +2433,9 @@ mod test {
         // Call eval_read_only with an empty program
         let program = ""; // empty program triggers parsed.is_empty()
         let err = env.eval_read_only(&contract_id, program).unwrap_err();
-
-        assert!(
-            matches!(
-            err,
-            VmExecutionError::Runtime(RuntimeError::TypeParseFailure(msg), _) if msg.contains("Expected a program of at least length 1")),
-            "Expected a type parse failure"
-        );
+        let expected_err =
+            ClarityEvalError::from(ParseError::new(ParseErrorKind::UnexpectedParserFailure));
+        assert_eq!(err, expected_err, "Expected a type parse failure");
     }
 
     #[test]

clarity/src/vm/docs/contracts.rs

@@ -23,6 +23,7 @@ use crate::vm::contexts::GlobalContext;
 use crate::vm::costs::LimitedCostTracker;
 use crate::vm::database::MemoryBackingStore;
 use crate::vm::docs::{get_input_type_string, get_output_type_string, get_signature};
+use crate::vm::errors::ClarityEvalError;
 use crate::vm::types::{FunctionType, QualifiedContractIdentifier, Value};
 use crate::vm::version::ClarityVersion;
 use crate::vm::{self, ContractContext};
@@ -81,7 +82,7 @@ fn get_constant_value(var_name: &str, contract_content: &str) -> Value {
         .expect("BUG: failed to return constant value")
 }
 
-fn doc_execute(program: &str) -> Result<Option<Value>, vm::VmExecutionError> {
+fn doc_execute(program: &str) -> Result<Option<Value>, ClarityEvalError> {
     let contract_id = QualifiedContractIdentifier::transient();
     let mut contract_context = ContractContext::new(contract_id.clone(), ClarityVersion::Clarity2);
     let mut marf = MemoryBackingStore::new();
@@ -93,17 +94,17 @@ fn doc_execute(program: &str) -> Result<Option<Value>, vm::VmExecutionError> {
         LimitedCostTracker::new_free(),
         DOCS_GENERATION_EPOCH,
     );
-    global_context.execute(|g| {
-        let parsed = build_ast(
-            &contract_id,
-            program,
-            &mut (),
-            ClarityVersion::latest(),
-            StacksEpochId::latest(),
-        )?
-        .expressions;
-        vm::eval_all(&parsed, &mut contract_context, g, None)
-    })
+    let parsed = build_ast(
+        &contract_id,
+        program,
+        &mut (),
+        ClarityVersion::latest(),
+        StacksEpochId::latest(),
+    )?
+    .expressions;
+    global_context
+        .execute(|g| vm::eval_all(&parsed, &mut contract_context, g, None))
+        .map_err(ClarityEvalError::from)
 }
 
 #[allow(clippy::expect_used)]