Merge branch 'aac-client-breaking' of https://github.com/stacks-network/stacks-core into chore/aac-rename-check-errors

Author: jferrant

CONTRIBUTING.md

@@ -501,7 +501,7 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> {
     /// - Returns CheckErrorKind::WriteAttemptedInReadOnly if there is a read-only
     ///   violation, i.e. if some function marked read-only attempts to modify
     ///   the chainstate.
-    pub fn run(&mut self, contract_analysis: &ContractAnalysis) -> Result<(), CheckError>
+    pub fn run(&mut self, contract_analysis: &ContractAnalysis) -> Result<(), StaticCheckError>
 ```
 
 This comment is considered positive because it explains the contract of the function in pseudo-code. Someone who understands the constructs mentioned could, e.g., write a test for this method from this description.

clarity-types/src/errors/analysis.rs

@@ -485,9 +485,15 @@ pub enum CheckErrorKind {
 }
 
 #[derive(Debug, PartialEq)]
-pub struct CheckError {
+/// Represents an error encountered during Clarity's type-checking and semantic analysis phase.
+/// Wraps a `CheckErrorKind` variant, optionally includes the expressions causing the error,
+/// and provides diagnostic information for debugging.
+pub struct StaticCheckError {
+    /// The specific type-checking or semantic error that occurred.
     pub err: Box<CheckErrorKind>,
+    /// Optional vector of expressions related to the error, if available.
     pub expressions: Option<Vec<SymbolicExpression>>,
+    /// Diagnostic details (e.g., line/column numbers, error message, suggestions) around the error.
     pub diagnostic: Diagnostic,
 }
 
@@ -502,10 +508,10 @@ impl CheckErrorKind {
     }
 }
 
-impl CheckError {
-    pub fn new(err: CheckErrorKind) -> CheckError {
+impl StaticCheckError {
+    pub fn new(err: CheckErrorKind) -> StaticCheckError {
         let diagnostic = Diagnostic::err(&err);
-        CheckError {
+        StaticCheckError {
             err: Box::new(err),
             expressions: None,
             diagnostic,
@@ -533,13 +539,13 @@ impl CheckError {
     }
 }
 
-impl From<(SyntaxBindingError, &SymbolicExpression)> for CheckError {
+impl From<(SyntaxBindingError, &SymbolicExpression)> for StaticCheckError {
     fn from(e: (SyntaxBindingError, &SymbolicExpression)) -> Self {
         Self::with_expression(CheckErrorKind::BadSyntaxBinding(e.0), e.1)
     }
 }
 
-impl From<(CheckErrorKind, &SymbolicExpression)> for CheckError {
+impl From<(CheckErrorKind, &SymbolicExpression)> for StaticCheckError {
     fn from(e: (CheckErrorKind, &SymbolicExpression)) -> Self {
         let mut ce = Self::new(e.0);
         ce.set_expression(e.1);
@@ -559,7 +565,7 @@ impl fmt::Display for CheckErrorKind {
     }
 }
 
-impl fmt::Display for CheckError {
+impl fmt::Display for StaticCheckError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "{}", self.err)?;
 
@@ -571,9 +577,9 @@ impl fmt::Display for CheckError {
     }
 }
 
-impl From<CostErrors> for CheckError {
+impl From<CostErrors> for StaticCheckError {
     fn from(err: CostErrors) -> Self {
-        CheckError::from(CheckErrorKind::from(err))
+        StaticCheckError::from(CheckErrorKind::from(err))
     }
 }
 
@@ -596,7 +602,7 @@ impl From<CostErrors> for CheckErrorKind {
     }
 }
 
-impl error::Error for CheckError {
+impl error::Error for StaticCheckError {
     fn source(&self) -> Option<&(dyn error::Error + 'static)> {
         None
     }
@@ -608,9 +614,9 @@ impl error::Error for CheckErrorKind {
     }
 }
 
-impl From<CheckErrorKind> for CheckError {
+impl From<CheckErrorKind> for StaticCheckError {
     fn from(err: CheckErrorKind) -> Self {
-        CheckError::new(err)
+        StaticCheckError::new(err)
     }
 }
 

clarity-types/src/errors/mod.rs

@@ -20,7 +20,7 @@ pub mod lexer;
 
 use std::{error, fmt};
 
-pub use analysis::{CheckError, CheckErrorKind};
+pub use analysis::{CheckErrorKind, StaticCheckError};
 pub use ast::{ParseError, ParseErrors, ParseResult};
 pub use cost::CostErrors;
 pub use lexer::LexerError;
@@ -46,7 +46,7 @@ pub enum Error {
     Unchecked(CheckErrorKind),
     Interpreter(InterpreterError),
     Runtime(RuntimeErrorType, Option<StackTrace>),
-    ShortReturn(ShortReturnType),
+    EarlyReturn(EarlyReturnError),
 }
 
 /// InterpreterErrors are errors that *should never* occur.
@@ -104,8 +104,15 @@ pub enum RuntimeErrorType {
 }
 
 #[derive(Debug, PartialEq)]
-pub enum ShortReturnType {
-    ExpectedValue(Box<Value>),
+/// Errors triggered during Clarity contract evaluation that cause early termination.
+/// These errors halt evaluation and fail the transaction.
+pub enum EarlyReturnError {
+    /// Failed to unwrap an `Optional` (`none`) or `Response` (`err` or `ok`) Clarity value.
+    /// The `Box<Value>` holds the original or thrown value. Triggered by `try!`, `unwrap-or`, or
+    /// `unwrap-err-or`.
+    UnwrapFailed(Box<Value>),
+    /// An 'asserts!' expression evaluated to false.
+    /// The `Box<Value>` holds the value provided as the second argument to `asserts!`.
     AssertionFailed(Box<Value>),
 }
 
@@ -122,7 +129,7 @@ impl PartialEq<Error> for Error {
         match (self, other) {
             (Error::Runtime(x, _), Error::Runtime(y, _)) => x == y,
             (Error::Unchecked(x), Error::Unchecked(y)) => x == y,
-            (Error::ShortReturn(x), Error::ShortReturn(y)) => x == y,
+            (Error::EarlyReturn(x), Error::EarlyReturn(y)) => x == y,
             (Error::Interpreter(x), Error::Interpreter(y)) => x == y,
             _ => false,
         }
@@ -208,9 +215,9 @@ impl From<(CheckErrorKind, &SymbolicExpression)> for Error {
     }
 }
 
-impl From<ShortReturnType> for Error {
-    fn from(err: ShortReturnType) -> Self {
-        Error::ShortReturn(err)
+impl From<EarlyReturnError> for Error {
+    fn from(err: EarlyReturnError) -> Self {
+        Error::EarlyReturn(err)
     }
 }
 
@@ -225,11 +232,11 @@ impl From<Error> for () {
     fn from(_err: Error) -> Self {}
 }
 
-impl From<ShortReturnType> for Value {
-    fn from(val: ShortReturnType) -> Self {
+impl From<EarlyReturnError> for Value {
+    fn from(val: EarlyReturnError) -> Self {
         match val {
-            ShortReturnType::ExpectedValue(v) => *v,
-            ShortReturnType::AssertionFailed(v) => *v,
+            EarlyReturnError::UnwrapFailed(v) => *v,
+            EarlyReturnError::AssertionFailed(v) => *v,
         }
     }
 }
@@ -241,15 +248,15 @@ mod test {
     #[test]
     fn equality() {
         assert_eq!(
-            Error::ShortReturn(ShortReturnType::ExpectedValue(Box::new(Value::Bool(true)))),
-            Error::ShortReturn(ShortReturnType::ExpectedValue(Box::new(Value::Bool(true))))
+            Error::EarlyReturn(EarlyReturnError::UnwrapFailed(Box::new(Value::Bool(true)))),
+            Error::EarlyReturn(EarlyReturnError::UnwrapFailed(Box::new(Value::Bool(true))))
         );
         assert_eq!(
             Error::Interpreter(InterpreterError::InterpreterError("".to_string())),
             Error::Interpreter(InterpreterError::InterpreterError("".to_string()))
         );
         assert!(
-            Error::ShortReturn(ShortReturnType::ExpectedValue(Box::new(Value::Bool(true))))
+            Error::EarlyReturn(EarlyReturnError::UnwrapFailed(Box::new(Value::Bool(true))))
                 != Error::Interpreter(InterpreterError::InterpreterError("".to_string()))
         );
     }

clarity-types/src/types/serialization.rs

@@ -33,7 +33,7 @@ use crate::types::{
 
 /// Errors that may occur in serialization or deserialization
 /// If deserialization failed because the described type is a bad type and
-///   a CheckError is thrown, it gets wrapped in BadTypeError.
+///   a CheckErrorKind is thrown, it gets wrapped in BadTypeError.
 /// Any IOErrrors from the supplied buffer will manifest as IOError variants,
 ///   except for EOF -- if the deserialization code experiences an EOF, it is caught
 ///   and rethrown as DeserializationError
@@ -493,7 +493,7 @@ impl TypeSignature {
                     Err(CheckErrorKind::CouldNotDetermineSerializationType) => {
                         if no_ok_type {
                             // if both the ok type and the error type are NoType,
-                            //  throw a CheckError. This should not be possible, but the check
+                            //  throw a CheckErrorKind. This should not be possible, but the check
                             //  is done out of caution.
                             return Err(CheckErrorKind::CouldNotDetermineSerializationType);
                         } else {

clarity/src/vm/analysis/analysis_db.rs

@@ -20,7 +20,7 @@ use clarity_types::representations::ClarityName;
 use clarity_types::types::{QualifiedContractIdentifier, TraitIdentifier};
 use stacks_common::types::StacksEpochId;
 
-use crate::vm::analysis::errors::{CheckError, CheckErrorKind};
+use crate::vm::analysis::errors::{CheckErrorKind, StaticCheckError};
 use crate::vm::analysis::type_checker::ContractAnalysis;
 use crate::vm::database::{
     ClarityBackingStore, ClarityDeserializable, ClaritySerializable, RollbackWrapper,
@@ -63,13 +63,13 @@ impl<'a> AnalysisDatabase<'a> {
         self.store.nest();
     }
 
-    pub fn commit(&mut self) -> Result<(), CheckError> {
+    pub fn commit(&mut self) -> Result<(), StaticCheckError> {
         self.store
             .commit()
             .map_err(|e| CheckErrorKind::Expects(format!("{e:?}")).into())
     }
 
-    pub fn roll_back(&mut self) -> Result<(), CheckError> {
+    pub fn roll_back(&mut self) -> Result<(), StaticCheckError> {
         self.store
             .rollback()
             .map_err(|e| CheckErrorKind::Expects(format!("{e:?}")).into())
@@ -99,11 +99,11 @@ impl<'a> AnalysisDatabase<'a> {
     pub fn load_contract_non_canonical(
         &mut self,
         contract_identifier: &QualifiedContractIdentifier,
-    ) -> Result<Option<ContractAnalysis>, CheckError> {
+    ) -> Result<Option<ContractAnalysis>, StaticCheckError> {
         self.store
             .get_metadata(contract_identifier, AnalysisDatabase::storage_key())
             // treat NoSuchContract error thrown by get_metadata as an Option::None --
-            //    the analysis will propagate that as a CheckError anyways.
+            //    the analysis will propagate that as a StaticCheckError anyways.
             .ok()
             .flatten()
             .map(|x| {
@@ -118,12 +118,12 @@ impl<'a> AnalysisDatabase<'a> {
         &mut self,
         contract_identifier: &QualifiedContractIdentifier,
         epoch: &StacksEpochId,
-    ) -> Result<Option<ContractAnalysis>, CheckError> {
+    ) -> Result<Option<ContractAnalysis>, StaticCheckError> {
         Ok(self
             .store
             .get_metadata(contract_identifier, AnalysisDatabase::storage_key())
             // treat NoSuchContract error thrown by get_metadata as an Option::None --
-            //    the analysis will propagate that as a CheckError anyways.
+            //    the analysis will propagate that as a StaticCheckError anyways.
             .ok()
             .flatten()
             .map(|x| {
@@ -141,7 +141,7 @@ impl<'a> AnalysisDatabase<'a> {
         &mut self,
         contract_identifier: &QualifiedContractIdentifier,
         contract: &ContractAnalysis,
-    ) -> Result<(), CheckError> {
+    ) -> Result<(), StaticCheckError> {
         let key = AnalysisDatabase::storage_key();
         if self.store.has_metadata_entry(contract_identifier, key) {
             return Err(
@@ -158,7 +158,7 @@ impl<'a> AnalysisDatabase<'a> {
     pub fn get_clarity_version(
         &mut self,
         contract_identifier: &QualifiedContractIdentifier,
-    ) -> Result<ClarityVersion, CheckError> {
+    ) -> Result<ClarityVersion, StaticCheckError> {
         // TODO: this function loads the whole contract to obtain the function type.
         //         but it doesn't need to -- rather this information can just be
         //         stored as its own entry. the analysis cost tracking currently only
@@ -176,7 +176,7 @@ impl<'a> AnalysisDatabase<'a> {
         contract_identifier: &QualifiedContractIdentifier,
         function_name: &str,
         epoch: &StacksEpochId,
-    ) -> Result<Option<FunctionType>, CheckError> {
+    ) -> Result<Option<FunctionType>, StaticCheckError> {
         // TODO: this function loads the whole contract to obtain the function type.
         //         but it doesn't need to -- rather this information can just be
         //         stored as its own entry. the analysis cost tracking currently only
@@ -196,7 +196,7 @@ impl<'a> AnalysisDatabase<'a> {
         contract_identifier: &QualifiedContractIdentifier,
         function_name: &str,
         epoch: &StacksEpochId,
-    ) -> Result<Option<FunctionType>, CheckError> {
+    ) -> Result<Option<FunctionType>, StaticCheckError> {
         // TODO: this function loads the whole contract to obtain the function type.
         //         but it doesn't need to -- rather this information can just be
         //         stored as its own entry. the analysis cost tracking currently only
@@ -216,7 +216,7 @@ impl<'a> AnalysisDatabase<'a> {
         contract_identifier: &QualifiedContractIdentifier,
         trait_name: &str,
         epoch: &StacksEpochId,
-    ) -> Result<Option<BTreeMap<ClarityName, FunctionSignature>>, CheckError> {
+    ) -> Result<Option<BTreeMap<ClarityName, FunctionSignature>>, StaticCheckError> {
         // TODO: this function loads the whole contract to obtain the function type.
         //         but it doesn't need to -- rather this information can just be
         //         stored as its own entry. the analysis cost tracking currently only
@@ -237,7 +237,7 @@ impl<'a> AnalysisDatabase<'a> {
     pub fn get_implemented_traits(
         &mut self,
         contract_identifier: &QualifiedContractIdentifier,
-    ) -> Result<BTreeSet<TraitIdentifier>, CheckError> {
+    ) -> Result<BTreeSet<TraitIdentifier>, StaticCheckError> {
         let contract = self
             .load_contract_non_canonical(contract_identifier)?
             .ok_or(CheckErrorKind::NoSuchContract(

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

@@ -17,7 +17,7 @@
 use clarity_types::representations::ClarityName;
 
 pub use super::errors::{
-    check_argument_count, check_arguments_at_least, CheckError, CheckErrorKind,
+    check_argument_count, check_arguments_at_least, CheckErrorKind, StaticCheckError,
 };
 use crate::vm::analysis::types::ContractAnalysis;
 use crate::vm::functions::define::{DefineFunctions, DefineFunctionsParsed};

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

@@ -19,7 +19,7 @@ use std::collections::{BTreeMap, BTreeSet};
 use stacks_common::types::StacksEpochId;
 
 use crate::vm::analysis::types::ContractAnalysis;
-use crate::vm::analysis::CheckError;
+use crate::vm::analysis::StaticCheckError;
 use crate::vm::types::signatures::CallableSubtype;
 use crate::vm::types::{
     FixedFunction, FunctionArg, FunctionType, TupleTypeSignature, TypeSignature,
@@ -28,7 +28,7 @@ use crate::vm::{CheckErrorKind, ClarityName, ClarityVersion};
 
 pub fn build_contract_interface(
     contract_analysis: &ContractAnalysis,
-) -> Result<ContractInterface, CheckError> {
+) -> Result<ContractInterface, StaticCheckError> {
     let mut contract_interface =
         ContractInterface::new(contract_analysis.epoch, contract_analysis.clarity_version);
 
@@ -267,7 +267,7 @@ impl ContractInterfaceFunction {
     fn from_map(
         map: &BTreeMap<ClarityName, FunctionType>,
         access: ContractInterfaceFunctionAccess,
-    ) -> Result<Vec<ContractInterfaceFunction>, CheckError> {
+    ) -> Result<Vec<ContractInterfaceFunction>, StaticCheckError> {
         map.iter()
             .map(|(name, function_type)| {
                 Ok(ContractInterfaceFunction {
@@ -400,7 +400,7 @@ impl ContractInterface {
         }
     }
 
-    pub fn serialize(&self) -> Result<String, CheckError> {
+    pub fn serialize(&self) -> Result<String, StaticCheckError> {
         serde_json::to_string(self).map_err(|_| {
             CheckErrorKind::Expects("Failed to serialize contract interface".into()).into()
         })

clarity/src/vm/analysis/errors.rs

@@ -15,6 +15,6 @@
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 pub use clarity_types::errors::analysis::{
-    check_argument_count, check_arguments_at_least, check_arguments_at_most, CheckError,
-    CheckErrorKind, SyntaxBindingError, SyntaxBindingErrorType,
+    check_argument_count, check_arguments_at_least, check_arguments_at_most, CheckErrorKind,
+    StaticCheckError, SyntaxBindingError, SyntaxBindingErrorType,
 };