From fb56db8835307ffc1582b3d98f6dc499b928a317 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:46:42 -0400 Subject: [PATCH 01/21] Add new SyntaxBindingError and SyntaxBindingErrorType variants (incl. formatting code) to specialize CheckErrors::BadSyntaxBinding. Also, remove tuple-specific bad syntax binding errors since they are now captured by BadSyntaxBinding --- clarity/src/vm/analysis/errors.rs | 155 +++++++++++++++++++++++++++--- 1 file changed, 144 insertions(+), 11 deletions(-) diff --git a/clarity/src/vm/analysis/errors.rs b/clarity/src/vm/analysis/errors.rs index 29a5081626..18377e67d9 100644 --- a/clarity/src/vm/analysis/errors.rs +++ b/clarity/src/vm/analysis/errors.rs @@ -23,6 +23,141 @@ use crate::vm::types::{TraitIdentifier, TupleTypeSignature, TypeSignature, Value pub type CheckResult = Result; +/// What kind of syntax binding was found to be in error? +#[derive(Debug, PartialEq, Clone, Copy)] +pub enum SyntaxBindingErrorType { + Let, + Eval, + TupleCons, + TypeDefinition, +} + +impl fmt::Display for SyntaxBindingErrorType { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", &self.message()) + } +} + +impl DiagnosableError for SyntaxBindingErrorType { + fn message(&self) -> String { + match &self { + Self::Let => "Let-binding".to_string(), + Self::Eval => "Function argument definition".to_string(), + Self::TupleCons => "Tuple constructor".to_string(), + Self::TypeDefinition => "Type definition".to_string(), + } + } + + fn suggestion(&self) -> Option { + None + } +} + +/// Syntax binding error types +#[derive(Debug, PartialEq)] +pub enum SyntaxBindingError { + /// binding list item is not a list + NotList(SyntaxBindingErrorType, usize, SymbolicExpression), + /// binding list item has an invalid length (e.g. not 2) + InvalidLength(SyntaxBindingErrorType, usize, SymbolicExpression), + /// binding name is not an atom + NotAtom(SyntaxBindingErrorType, usize, SymbolicExpression), + /// second binding item is a type signature, and the type signature itself is bad. + /// NOTE: type signature parsing returns CheckErrors, so we cannot include a CheckErrors here + /// directly without creating a recursive type. Instead, we just report the `Display` + /// representation of the error here as the third item. + BadTypeSignature(usize, SymbolicExpression, String), +} + +impl fmt::Display for SyntaxBindingError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{self:?}") + } +} + +impl DiagnosableError for SyntaxBindingError { + fn message(&self) -> String { + match &self { + Self::NotList(err_type, item_index, item) => { + let item_no = item_index + 1; + format!( + "{err_type} item #{item_no} is not a list: {}", + item.as_error_string() + ) + } + Self::InvalidLength(err_type, item_index, item) => { + let item_no = item_index + 1; + format!( + "{err_type} item #{item_no} is not a two-element list: {}", + item.as_error_string() + ) + } + Self::NotAtom(err_type, item_index, item) => { + let item_no = item_index + 1; + format!( + "{err_type} item #{item_no}'s name is not an atom: {}", + item.as_error_string() + ) + } + Self::BadTypeSignature(item_index, item, error_message) => { + let item_no = item_index + 1; + format!("Type definition item #{item_no} has an invalid type signature: {} (reason: {error_message})", item.as_error_string()) + } + } + } + + fn suggestion(&self) -> Option { + None + } +} + +impl SyntaxBindingError { + /// Helper constructor for NotList(SyntaxBindingErrorType::Let, item_no, item) + pub fn let_binding_not_list(item_no: usize, item: SymbolicExpression) -> Self { + Self::NotList(SyntaxBindingErrorType::Let, item_no, item) + } + + /// Helper constructor for InvalidLength(SyntaxBindingErrorType::Let, item_no, item) + pub fn let_binding_invalid_length(item_no: usize, item: SymbolicExpression) -> Self { + Self::InvalidLength(SyntaxBindingErrorType::Let, item_no, item) + } + + /// Helper constructor for NotAtom(SyntaxBindingErrorType::Let, item_no, item) + pub fn let_binding_not_atom(item_no: usize, item: SymbolicExpression) -> Self { + Self::NotAtom(SyntaxBindingErrorType::Let, item_no, item) + } + + /// Helper constructor for NotList(SyntaxBindingErrorType::Eval, item_no, item) + pub fn eval_binding_not_list(item_no: usize, item: SymbolicExpression) -> Self { + Self::NotList(SyntaxBindingErrorType::Eval, item_no, item) + } + + /// Helper constructor for InvalidLength(SyntaxBindingErrorType::Eval, item_no, item) + pub fn eval_binding_invalid_length(item_no: usize, item: SymbolicExpression) -> Self { + Self::InvalidLength(SyntaxBindingErrorType::Eval, item_no, item) + } + + /// Helper constructor for NotAtom(SyntaxBindingErrorType::Eval, item_no, item) + pub fn eval_binding_not_atom(item_no: usize, item: SymbolicExpression) -> Self { + Self::NotAtom(SyntaxBindingErrorType::Eval, item_no, item) + } + + /// Helper constructor for NotList(SyntaxBindingErrorType::TupleCons, item_no, item) + pub fn tuple_cons_not_list(item_no: usize, item: SymbolicExpression) -> Self { + Self::NotList(SyntaxBindingErrorType::TupleCons, item_no, item) + } + + /// Helper constructor for InvalidLength(SyntaxBindingErrorType::TupleCons, item_no, item) + pub fn tuple_cons_invalid_length(item_no: usize, item: SymbolicExpression) -> Self { + Self::InvalidLength(SyntaxBindingErrorType::TupleCons, item_no, item) + } + + /// Helper constructor for NotAtom(SyntaxBindingErrorType::TupleCons, item_no, item) + pub fn tuple_cons_not_atom(item_no: usize, item: SymbolicExpression) -> Self { + Self::NotAtom(SyntaxBindingErrorType::TupleCons, item_no, item) + } +} + #[derive(Debug, PartialEq)] pub enum CheckErrors { // cost checker errors @@ -101,8 +236,7 @@ pub enum CheckErrors { ExpectedTuple(TypeSignature), NoSuchTupleField(String, TupleTypeSignature), EmptyTuplesNotAllowed, - BadTupleConstruction, - TupleExpectsPairs, + BadTupleConstruction(String), // variables NoSuchDataVariable(String), @@ -152,8 +286,7 @@ pub enum CheckErrors { BadLetSyntax, // generic binding syntax - BadSyntaxBinding, - BadSyntaxExpectedListOfPairs, + BadSyntaxBinding(SyntaxBindingError), MaxContextDepthReached, UndefinedFunction(String), @@ -215,6 +348,11 @@ impl CheckErrors { CheckErrors::SupertypeTooLarge | CheckErrors::Expects(_) ) } + + /// Is the given error message due to a BadSyntaxBinding? + pub fn has_nested_bad_syntax_binding_message(msg: &str) -> bool { + msg.contains("invalid syntax binding: ") + } } impl CheckError { @@ -362,7 +500,6 @@ impl DiagnosableError for CheckErrors { CheckErrors::MemoryBalanceExceeded(a, b) => format!("contract execution cost exceeded memory budget: {a:?} > {b:?}"), CheckErrors::InvalidTypeDescription => "supplied type description is invalid".into(), CheckErrors::EmptyTuplesNotAllowed => "tuple types may not be empty".into(), - CheckErrors::BadSyntaxExpectedListOfPairs => "bad syntax: function expects a list of pairs to bind names, e.g., ((name-0 a) (name-1 b) ...)".into(), CheckErrors::UnknownTypeName(name) => format!("failed to parse type: '{name}'"), CheckErrors::ValueTooLarge => "created a type which was greater than maximum allowed value size".into(), CheckErrors::ValueOutOfBounds => "created a type which value size was out of defined bounds".into(), @@ -390,8 +527,7 @@ impl DiagnosableError for CheckErrors { CheckErrors::BadTupleFieldName => "invalid tuple field name".into(), CheckErrors::ExpectedTuple(type_signature) => format!("expecting tuple, found '{type_signature}'"), CheckErrors::NoSuchTupleField(field_name, tuple_signature) => format!("cannot find field '{field_name}' in tuple '{tuple_signature}'"), - CheckErrors::BadTupleConstruction => "invalid tuple syntax, expecting list of pair".into(), - CheckErrors::TupleExpectsPairs => "invalid tuple syntax, expecting pair".into(), + CheckErrors::BadTupleConstruction(message) => format!("invalid tuple syntax: {message}"), CheckErrors::NoSuchDataVariable(var_name) => format!("use of unresolved persisted variable '{var_name}'"), CheckErrors::BadTransferSTXArguments => "STX transfer expects an int amount, from principal, to principal".into(), CheckErrors::BadTransferFTArguments => "transfer expects an int amount, from principal, to principal".into(), @@ -428,7 +564,7 @@ impl DiagnosableError for CheckErrors { CheckErrors::MaxLengthOverflow => format!("expecting a value <= {}", u32::MAX), CheckErrors::BadLetSyntax => "invalid syntax of 'let'".into(), CheckErrors::CircularReference(references) => format!("detected circular reference: ({})", references.join(", ")), - CheckErrors::BadSyntaxBinding => "invalid syntax binding".into(), + CheckErrors::BadSyntaxBinding(binding_error) => format!("invalid syntax binding: {}", &binding_error.message()), CheckErrors::MaxContextDepthReached => "reached depth limit".into(), CheckErrors::UndefinedVariable(var_name) => format!("use of unresolved variable '{var_name}'"), CheckErrors::UndefinedFunction(var_name) => format!("use of unresolved function '{var_name}'"), @@ -476,9 +612,6 @@ impl DiagnosableError for CheckErrors { fn suggestion(&self) -> Option { match &self { - CheckErrors::BadSyntaxBinding => { - Some("binding syntax example: ((supply int) (ttl int))".into()) - } CheckErrors::BadLetSyntax => Some( "'let' syntax example: (let ((supply 1000) (ttl 60)) )".into(), ), From 01452b773255c2e2c7780a36b9cbf49cbb5b3d05 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:48:04 -0400 Subject: [PATCH 02/21] feat: when encountering an error while checking let-bindings and tuple-bindings, report the specific kind of error instead of the generic BadSyntaxBinding --- .../src/vm/analysis/read_only_checker/mod.rs | 36 +++++++++++++++---- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/clarity/src/vm/analysis/read_only_checker/mod.rs b/clarity/src/vm/analysis/read_only_checker/mod.rs index a244bf7101..8eacbc5e86 100644 --- a/clarity/src/vm/analysis/read_only_checker/mod.rs +++ b/clarity/src/vm/analysis/read_only_checker/mod.rs @@ -19,6 +19,7 @@ use stacks_common::types::StacksEpochId; pub use super::errors::{ check_argument_count, check_arguments_at_least, CheckError, CheckErrors, CheckResult, + SyntaxBindingError, }; use super::AnalysisDatabase; use crate::vm::analysis::types::{AnalysisPass, ContractAnalysis}; @@ -324,10 +325,21 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> { let binding_list = args[0].match_list().ok_or(CheckErrors::BadLetSyntax)?; - for pair in binding_list.iter() { - let pair_expression = pair.match_list().ok_or(CheckErrors::BadSyntaxBinding)?; + for (i, pair) in binding_list.iter().enumerate() { + let pair_expression = pair.match_list().ok_or_else(|| { + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_list( + i, + pair.clone(), + )) + })?; if pair_expression.len() != 2 { - return Err(CheckErrors::BadSyntaxBinding.into()); + return Err(CheckErrors::BadSyntaxBinding( + SyntaxBindingError::let_binding_invalid_length( + i, + SymbolicExpression::list(pair_expression.to_vec()), + ), + ) + .into()); } if !self.check_read_only(&pair_expression[1])? { @@ -364,11 +376,21 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> { self.check_expression_application_is_read_only(args) } TupleCons => { - for pair in args.iter() { - let pair_expression = - pair.match_list().ok_or(CheckErrors::TupleExpectsPairs)?; + for (i, pair) in args.iter().enumerate() { + let pair_expression = pair.match_list().ok_or_else(|| { + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_list( + i, + pair.clone(), + )) + })?; if pair_expression.len() != 2 { - return Err(CheckErrors::TupleExpectsPairs.into()); + return Err(CheckErrors::BadSyntaxBinding( + SyntaxBindingError::tuple_cons_invalid_length( + i, + SymbolicExpression::list(pair_expression.to_vec()), + ), + ) + .into()); } if !self.check_read_only(&pair_expression[1])? { From 810330f712260c296d28e288858364a8b50248eb Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:48:57 -0400 Subject: [PATCH 03/21] chore: update tuple construction and pair tests with the new error message text --- clarity/src/vm/analysis/tests/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clarity/src/vm/analysis/tests/mod.rs b/clarity/src/vm/analysis/tests/mod.rs index 01d5e98136..f434e636cf 100644 --- a/clarity/src/vm/analysis/tests/mod.rs +++ b/clarity/src/vm/analysis/tests/mod.rs @@ -123,14 +123,15 @@ fn test_no_such_tuple_field() { fn test_bad_tuple_construction() { let snippet = "(tuple (key 1) (key 2))"; let err = mem_type_check(snippet).unwrap_err(); - assert!(format!("{}", err.diagnostic).contains("invalid tuple syntax, expecting list of pair")); + assert!(format!("{}", err.diagnostic) + .contains("invalid tuple syntax: defining 'key' conflicts with previous value.")); } #[test] fn test_tuple_expects_pairs() { let snippet = "(tuple (key 1) (key-with-missing-value))"; let err = mem_type_check(snippet).unwrap_err(); - assert!(format!("{}", err.diagnostic).contains("invalid tuple syntax, expecting pair")); + assert!(format!("{}", err.diagnostic).contains("invalid syntax binding: Tuple constructor item #2 is not a two-element list: ( key-with-missing-value ).")); } #[test] From 7b5dc8e5c0b57787b1129bb3b7e4f81214190694 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:49:32 -0400 Subject: [PATCH 04/21] chore: pass BadSyntaxErrorType to use in the event the code fails to type-check a list of (name, value) pairs --- clarity/src/vm/analysis/type_checker/v2_05/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/clarity/src/vm/analysis/type_checker/v2_05/mod.rs b/clarity/src/vm/analysis/type_checker/v2_05/mod.rs index 40bc742334..2638e6ae4e 100644 --- a/clarity/src/vm/analysis/type_checker/v2_05/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_05/mod.rs @@ -28,6 +28,7 @@ use super::contexts::{TypeMap, TypingContext}; use super::ContractAnalysis; pub use crate::vm::analysis::errors::{ check_argument_count, check_arguments_at_least, CheckError, CheckErrors, CheckResult, + SyntaxBindingErrorType, }; use crate::vm::analysis::AnalysisDatabase; use crate::vm::costs::cost_functions::ClarityCostFunction; @@ -555,8 +556,12 @@ impl<'a, 'b> TypeChecker<'a, 'b> { let function_name = function_name .match_atom() .ok_or(CheckErrors::BadFunctionName)?; - let args = parse_name_type_pairs::<()>(StacksEpochId::Epoch2_05, args, &mut ()) - .map_err(|_| CheckErrors::BadSyntaxBinding)?; + let args = parse_name_type_pairs::<()>( + StacksEpochId::Epoch2_05, + args, + SyntaxBindingErrorType::Eval, + &mut (), + )?; if self.function_return_tracker.is_some() { return Err(CheckErrors::Expects( From 22a0a78470fe42f8ad823c24b16e6f38eebdb5b5 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:50:26 -0400 Subject: [PATCH 05/21] chore: update calls to handle_binding_list() to include the specific kind of binding being checked so an error will result in a meaningful message --- .../type_checker/v2_05/natives/mod.rs | 71 +++++++++++-------- 1 file changed, 40 insertions(+), 31 deletions(-) diff --git a/clarity/src/vm/analysis/type_checker/v2_05/natives/mod.rs b/clarity/src/vm/analysis/type_checker/v2_05/natives/mod.rs index c0dfef5350..840a03c2dc 100644 --- a/clarity/src/vm/analysis/type_checker/v2_05/natives/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_05/natives/mod.rs @@ -19,9 +19,10 @@ use stacks_common::types::StacksEpochId; use super::{ check_argument_count, check_arguments_at_least, no_type, TypeChecker, TypeResult, TypingContext, }; -use crate::vm::analysis::errors::{CheckError, CheckErrors}; +use crate::vm::analysis::errors::{CheckError, CheckErrors, SyntaxBindingErrorType}; use crate::vm::costs::cost_functions::ClarityCostFunction; use crate::vm::costs::{analysis_typecheck_cost, runtime_cost}; +use crate::vm::diagnostic::DiagnosableError; use crate::vm::functions::{handle_binding_list, NativeFunctions}; use crate::vm::types::{ BlockInfoProperty, FixedFunction, FunctionArg, FunctionSignature, FunctionType, PrincipalData, @@ -188,20 +189,24 @@ pub fn check_special_tuple_cons( let mut tuple_type_data = Vec::with_capacity(len); - handle_binding_list(args, |var_name, var_sexp| { - checker.type_check(var_sexp, context).and_then(|var_type| { - runtime_cost( - ClarityCostFunction::AnalysisTupleItemsCheck, - checker, - var_type.type_size()?, - )?; - tuple_type_data.push((var_name.clone(), var_type)); - Ok(()) - }) - })?; + handle_binding_list( + args, + SyntaxBindingErrorType::TupleCons, + |var_name, var_sexp| { + checker.type_check(var_sexp, context).and_then(|var_type| { + runtime_cost( + ClarityCostFunction::AnalysisTupleItemsCheck, + checker, + var_type.type_size()?, + )?; + tuple_type_data.push((var_name.clone(), var_type)); + Ok(()) + }) + }, + )?; let tuple_signature = TupleTypeSignature::try_from(tuple_type_data) - .map_err(|_e| CheckErrors::BadTupleConstruction)?; + .map_err(|e| CheckErrors::BadTupleConstruction(e.message()))?; Ok(TypeSignature::TupleType(tuple_signature)) } @@ -221,26 +226,30 @@ fn check_special_let( runtime_cost(ClarityCostFunction::AnalysisCheckLet, checker, args.len())?; - handle_binding_list(binding_list, |var_name, var_sexp| { - checker.contract_context.check_name_used(var_name)?; - if out_context.lookup_variable_type(var_name).is_some() { - return Err(CheckError::new(CheckErrors::NameAlreadyUsed( - var_name.to_string(), - ))); - } + handle_binding_list( + binding_list, + SyntaxBindingErrorType::Let, + |var_name, var_sexp| { + checker.contract_context.check_name_used(var_name)?; + if out_context.lookup_variable_type(var_name).is_some() { + return Err(CheckError::new(CheckErrors::NameAlreadyUsed( + var_name.to_string(), + ))); + } - let typed_result = checker.type_check(var_sexp, &out_context)?; + let typed_result = checker.type_check(var_sexp, &out_context)?; - runtime_cost( - ClarityCostFunction::AnalysisBindName, - checker, - typed_result.type_size()?, - )?; - out_context - .variable_types - .insert(var_name.clone(), typed_result); - Ok(()) - })?; + runtime_cost( + ClarityCostFunction::AnalysisBindName, + checker, + typed_result.type_size()?, + )?; + out_context + .variable_types + .insert(var_name.clone(), typed_result); + Ok(()) + }, + )?; checker.type_check_consecutive_statements(&args[1..args.len()], &out_context) } From 24fbf5da0fbe6bf0eb08463fe2ae79e62c2c2ae4 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:51:13 -0400 Subject: [PATCH 06/21] chore: update tests to check the specific variant of CheckErrors::BadSyntaxBinding(..) --- .../analysis/type_checker/v2_05/tests/mod.rs | 64 ++++++++++++++++--- 1 file changed, 55 insertions(+), 9 deletions(-) diff --git a/clarity/src/vm/analysis/type_checker/v2_05/tests/mod.rs b/clarity/src/vm/analysis/type_checker/v2_05/tests/mod.rs index 68c7bec3b6..da7942f609 100644 --- a/clarity/src/vm/analysis/type_checker/v2_05/tests/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_05/tests/mod.rs @@ -16,19 +16,19 @@ use stacks_common::types::StacksEpochId; -use crate::vm::analysis::errors::CheckErrors; +use crate::vm::analysis::errors::{CheckErrors, SyntaxBindingError}; use crate::vm::analysis::mem_type_check; use crate::vm::analysis::type_checker::v2_05::TypeResult; use crate::vm::ast::build_ast; use crate::vm::ast::errors::ParseErrors; +use crate::vm::diagnostic::DiagnosableError; use crate::vm::types::SequenceSubtype::*; use crate::vm::types::StringSubtype::*; use crate::vm::types::TypeSignature::{BoolType, IntType, PrincipalType, UIntType}; use crate::vm::types::{ FixedFunction, FunctionType, QualifiedContractIdentifier, TypeSignature, BUFF_32, BUFF_64, }; -use crate::vm::ClarityVersion; - +use crate::vm::{ClarityVersion, SymbolicExpression, Value}; mod assets; mod contracts; @@ -647,8 +647,16 @@ fn test_simple_lets() { ]; let bad_expected = [ - CheckErrors::BadSyntaxBinding, - CheckErrors::BadSyntaxBinding, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::literal_value(Value::Int(1)).with_id(5) + ]), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom( + 0, + SymbolicExpression::literal_value(Value::Int(1)).with_id(5), + )), CheckErrors::TypeError(TypeSignature::IntType, TypeSignature::UIntType), ]; @@ -1237,7 +1245,12 @@ fn test_empty_tuple_should_fail() { ) .unwrap_err() .err, - CheckErrors::BadSyntaxBinding + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![SymbolicExpression::atom("tuple".into()).with_id(8)]) + .with_id(7), + CheckErrors::EmptyTuplesNotAllowed.message() + )) ); } @@ -2467,7 +2480,18 @@ fn test_buff_negative_len() { StacksEpochId::Epoch2_05, ) .unwrap_err(); - assert!(matches!(res.err, CheckErrors::BadSyntaxBinding)); + assert_eq!( + res.err, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("buff".into()).with_id(8), + SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), + ]) + .with_id(7), + CheckErrors::ValueOutOfBounds.message() + )) + ); } #[test] @@ -2481,7 +2505,18 @@ fn test_string_ascii_negative_len() { StacksEpochId::Epoch2_05, ) .unwrap_err(); - assert!(matches!(res.err, CheckErrors::BadSyntaxBinding)); + assert_eq!( + res.err, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("string-ascii".into()).with_id(8), + SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), + ]) + .with_id(7), + CheckErrors::ValueOutOfBounds.message() + )) + ); } #[test] @@ -2495,5 +2530,16 @@ fn test_string_utf8_negative_len() { StacksEpochId::Epoch2_05, ) .unwrap_err(); - assert!(matches!(res.err, CheckErrors::BadSyntaxBinding)); + assert_eq!( + res.err, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("string-utf8".into()).with_id(8), + SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), + ]) + .with_id(7), + CheckErrors::ValueOutOfBounds.message() + )) + ); } From 8fed34602f87291762204d6fe57a41b53496ab10 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:51:48 -0400 Subject: [PATCH 07/21] chore: pass the specific kind of type-checking when parsing (name,value) pairs --- clarity/src/vm/analysis/type_checker/v2_1/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/clarity/src/vm/analysis/type_checker/v2_1/mod.rs b/clarity/src/vm/analysis/type_checker/v2_1/mod.rs index 17ee17f615..25da96eb9b 100644 --- a/clarity/src/vm/analysis/type_checker/v2_1/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_1/mod.rs @@ -27,7 +27,7 @@ use super::contexts::{TypeMap, TypingContext}; use super::ContractAnalysis; pub use crate::vm::analysis::errors::{ check_argument_count, check_arguments_at_least, check_arguments_at_most, CheckError, - CheckErrors, CheckResult, + CheckErrors, CheckResult, SyntaxBindingErrorType, }; use crate::vm::analysis::AnalysisDatabase; use crate::vm::costs::cost_functions::ClarityCostFunction; @@ -1251,8 +1251,12 @@ impl<'a, 'b> TypeChecker<'a, 'b> { let function_name = function_name .match_atom() .ok_or(CheckErrors::BadFunctionName)?; - let args = parse_name_type_pairs::<()>(StacksEpochId::Epoch21, args, &mut ()) - .map_err(|_| CheckErrors::BadSyntaxBinding)?; + let args = parse_name_type_pairs::<()>( + StacksEpochId::Epoch21, + args, + SyntaxBindingErrorType::Eval, + &mut (), + )?; if self.function_return_tracker.is_some() { return Err(CheckErrors::Expects( From 664b6485f15549b404e5774976a26fb3d450c70e Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:53:07 -0400 Subject: [PATCH 08/21] chore: update calls to handle_binding_list() to include the kind of type-check being performed so error messages can reflect it --- .../analysis/type_checker/v2_1/natives/mod.rs | 105 ++++++++++-------- 1 file changed, 58 insertions(+), 47 deletions(-) diff --git a/clarity/src/vm/analysis/type_checker/v2_1/natives/mod.rs b/clarity/src/vm/analysis/type_checker/v2_1/natives/mod.rs index 500632f0dd..65ba214359 100644 --- a/clarity/src/vm/analysis/type_checker/v2_1/natives/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_1/natives/mod.rs @@ -20,9 +20,10 @@ use super::{ check_argument_count, check_arguments_at_least, check_arguments_at_most, compute_typecheck_cost, no_type, TypeChecker, TypeResult, TypingContext, }; -use crate::vm::analysis::errors::{CheckError, CheckErrors}; +use crate::vm::analysis::errors::{CheckError, CheckErrors, SyntaxBindingErrorType}; use crate::vm::costs::cost_functions::ClarityCostFunction; use crate::vm::costs::{analysis_typecheck_cost, runtime_cost, CostErrors, CostTracker}; +use crate::vm::diagnostic::DiagnosableError; use crate::vm::functions::{handle_binding_list, NativeFunctions}; use crate::vm::types::signatures::{ CallableSubtype, FunctionArgSignature, FunctionReturnsSignature, SequenceSubtype, ASCII_40, @@ -231,30 +232,36 @@ pub fn check_special_tuple_cons( let mut type_size = 0u32; let mut cons_error = Ok(()); - handle_binding_list(args, |var_name, var_sexp| { - checker.type_check(var_sexp, context).and_then(|var_type| { - runtime_cost( - ClarityCostFunction::AnalysisTupleItemsCheck, - checker, - var_type.type_size()?, - )?; - if type_size < MAX_VALUE_SIZE { - type_size = type_size - .saturating_add(var_name.len() as u32) - .saturating_add(var_name.len() as u32) - .saturating_add(var_type.type_size()?) - .saturating_add(var_type.size()?); - tuple_type_data.push((var_name.clone(), var_type)); - } else { - cons_error = Err(CheckErrors::BadTupleConstruction); - } - Ok(()) - }) - })?; + handle_binding_list( + args, + SyntaxBindingErrorType::TupleCons, + |var_name, var_sexp| { + checker.type_check(var_sexp, context).and_then(|var_type| { + runtime_cost( + ClarityCostFunction::AnalysisTupleItemsCheck, + checker, + var_type.type_size()?, + )?; + if type_size < MAX_VALUE_SIZE { + type_size = type_size + .saturating_add(var_name.len() as u32) + .saturating_add(var_name.len() as u32) + .saturating_add(var_type.type_size()?) + .saturating_add(var_type.size()?); + tuple_type_data.push((var_name.clone(), var_type)); + } else { + cons_error = Err(CheckErrors::BadTupleConstruction(format!( + "type size of {type_size} bytes exceeds maximum of {MAX_VALUE_SIZE} bytes" + ))); + } + Ok(()) + }) + }, + )?; cons_error?; let tuple_signature = TupleTypeSignature::try_from(tuple_type_data) - .map_err(|_e| CheckErrors::BadTupleConstruction)?; + .map_err(|e| CheckErrors::BadTupleConstruction(e.message()))?; Ok(TypeSignature::TupleType(tuple_signature)) } @@ -276,33 +283,37 @@ fn check_special_let( runtime_cost(ClarityCostFunction::AnalysisCheckLet, checker, args.len())?; let mut added_memory = 0u64; - handle_binding_list(binding_list, |var_name, var_sexp| { - checker.contract_context.check_name_used(var_name)?; - if out_context.lookup_variable_type(var_name).is_some() { - return Err(CheckError::new(CheckErrors::NameAlreadyUsed( - var_name.to_string(), - ))); - } + handle_binding_list( + binding_list, + SyntaxBindingErrorType::Let, + |var_name, var_sexp| { + checker.contract_context.check_name_used(var_name)?; + if out_context.lookup_variable_type(var_name).is_some() { + return Err(CheckError::new(CheckErrors::NameAlreadyUsed( + var_name.to_string(), + ))); + } - let typed_result = checker.type_check(var_sexp, &out_context)?; + let typed_result = checker.type_check(var_sexp, &out_context)?; - runtime_cost( - ClarityCostFunction::AnalysisBindName, - checker, - typed_result.type_size()?, - )?; - if checker.epoch.analysis_memory() { - let memory_use = u64::from(var_name.len()) - .checked_add(u64::from(typed_result.type_size()?)) - .ok_or_else(|| CostErrors::CostOverflow)?; - added_memory = added_memory - .checked_add(memory_use) - .ok_or_else(|| CostErrors::CostOverflow)?; - checker.add_memory(memory_use)?; - } - out_context.add_variable_type(var_name.clone(), typed_result, checker.clarity_version); - Ok(()) - })?; + runtime_cost( + ClarityCostFunction::AnalysisBindName, + checker, + typed_result.type_size()?, + )?; + if checker.epoch.analysis_memory() { + let memory_use = u64::from(var_name.len()) + .checked_add(u64::from(typed_result.type_size()?)) + .ok_or_else(|| CostErrors::CostOverflow)?; + added_memory = added_memory + .checked_add(memory_use) + .ok_or_else(|| CostErrors::CostOverflow)?; + checker.add_memory(memory_use)?; + } + out_context.add_variable_type(var_name.clone(), typed_result, checker.clarity_version); + Ok(()) + }, + )?; let res = checker.type_check_consecutive_statements(&args[1..args.len()], &out_context); if checker.epoch.analysis_memory() { From 418f7899941335123be05126f1217de0b1d2782b Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:53:40 -0400 Subject: [PATCH 09/21] chore: update all tests which check for CheckErrors::BadSyntaxBinding to now check for the specific variant of BadSyntaxBinding. Also, add comprehensive checks for each variant of BadSyntaxBinding, as well as test to verify that error messages do not grow unbound with the type's nesting depth --- .../analysis/type_checker/v2_1/tests/mod.rs | 218 +++++++++++++++++- 1 file changed, 210 insertions(+), 8 deletions(-) diff --git a/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs b/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs index bb3e86a6ec..b576b865a5 100644 --- a/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs @@ -21,12 +21,13 @@ use rstest_reuse::{self, *}; use stacks_common::types::StacksEpochId; use super::CheckResult; -use crate::vm::analysis::errors::CheckErrors; +use crate::vm::analysis::errors::{CheckErrors, SyntaxBindingError}; use crate::vm::analysis::mem_type_check as mem_run_analysis; use crate::vm::analysis::type_checker::v2_1::TypeResult; use crate::vm::analysis::types::ContractAnalysis; use crate::vm::ast::build_ast; use crate::vm::ast::errors::ParseErrors; +use crate::vm::diagnostic::DiagnosableError; use crate::vm::tests::test_clarity_versions; use crate::vm::types::signatures::TypeSignature::OptionalType; use crate::vm::types::signatures::{ListTypeData, StringUTF8Length}; @@ -37,7 +38,7 @@ use crate::vm::types::{ BufferLength, FixedFunction, FunctionType, QualifiedContractIdentifier, TraitIdentifier, TypeSignature, BUFF_1, BUFF_20, BUFF_21, BUFF_32, BUFF_64, }; -use crate::vm::{execute_v2, ClarityName, ClarityVersion}; +use crate::vm::{execute_v2, ClarityName, ClarityVersion, SymbolicExpression, Value}; mod assets; pub mod contracts; @@ -979,8 +980,16 @@ fn test_simple_lets() { ]; let bad_expected = [ - CheckErrors::BadSyntaxBinding, - CheckErrors::BadSyntaxBinding, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::literal_value(Value::Int(1)).with_id(5) + ]), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom( + 0, + SymbolicExpression::literal_value(Value::Int(1)).with_id(5), + )), CheckErrors::TypeError(TypeSignature::IntType, TypeSignature::UIntType), ]; @@ -1948,7 +1957,12 @@ fn test_empty_tuple_should_fail() { assert_eq!( mem_type_check(contract_src).unwrap_err().err, - CheckErrors::BadSyntaxBinding + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![SymbolicExpression::atom("tuple".into()).with_id(8)]) + .with_id(7), + CheckErrors::EmptyTuplesNotAllowed.message() + )) ); } @@ -3101,7 +3115,18 @@ fn test_buff_negative_len() { (func 0x00)"; let res = mem_type_check(contract_src).unwrap_err(); - assert!(matches!(res.err, CheckErrors::BadSyntaxBinding)); + assert_eq!( + res.err, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("buff".into()).with_id(8), + SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), + ]) + .with_id(7), + CheckErrors::ValueOutOfBounds.message() + )) + ); } #[test] @@ -3110,7 +3135,18 @@ fn test_string_ascii_negative_len() { (func \"\")"; let res = mem_type_check(contract_src).unwrap_err(); - assert!(matches!(res.err, CheckErrors::BadSyntaxBinding)); + assert_eq!( + res.err, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("string-ascii".into()).with_id(8), + SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), + ]) + .with_id(7), + CheckErrors::ValueOutOfBounds.message() + )) + ); } #[test] @@ -3119,7 +3155,18 @@ fn test_string_utf8_negative_len() { (func u\"\")"; let res = mem_type_check(contract_src).unwrap_err(); - assert!(matches!(res.err, CheckErrors::BadSyntaxBinding)); + assert_eq!( + res.err, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("string-utf8".into()).with_id(8), + SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), + ]) + .with_id(7), + CheckErrors::ValueOutOfBounds.message() + )) + ); } #[test] @@ -3654,3 +3701,158 @@ fn test_principal_admits() { assert!(res.is_err()); } } + +/// Comprehensive test of all of the bad syntax binding error variants we can detect. +/// Only concerns itself with simple errors (no nesting). +#[test] +fn test_simple_bad_syntax_bindings() { + let bad = vec![ + // bad let-binding -- binding item is not a list + "(let (oops (bar u1)) (ok true))", + // bad let-binding -- binding item is not a 2-element list + "(let ((oops u1 u2) (bar u1)) (ok true))", + // bad let-binding -- binding item name is not an atom + "(let ((1 2) (bar u1)) (ok true))", + // bad eval-binding -- binding item is not a list + "(define-private (foo oops (bar uint)) (ok true))", + // bad eval-binding -- binding item is not a 2-element list + "(define-private (foo (oops uint uint) (bar uint)) (ok true))", + // bad eval-binding -- binding item name is not an atom + "(define-private (foo (u1 uint) (bar uint)) (ok true))", + // bad tuple binding -- binding item is not a list + "(tuple oops (bar u1))", + // bad tuple binding -- binding item is not a 2-element list + "(tuple (oops u1 u2) (bar u1))", + // bad tuple binding -- binding item name is not an atom + "(tuple (u1 u2) (bar u1))", + // bad type binding -- bad type signature + "(define-private (foo (bar (string-ascii -12))) (ok true))", + ]; + let expected = vec![ + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_list( + 0, + SymbolicExpression::atom("oops".into()).with_id(4), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("oops".into()).with_id(5), + SymbolicExpression::literal_value(Value::UInt(1)).with_id(6), + SymbolicExpression::literal_value(Value::UInt(2)).with_id(7), + ]), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom( + 0, + SymbolicExpression::literal_value(Value::Int(1)).with_id(5), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_not_list( + 0, + SymbolicExpression::atom("oops".into()).with_id(5), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_invalid_length( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("oops".into()).with_id(6), + SymbolicExpression::atom("uint".into()).with_id(7), + SymbolicExpression::atom("uint".into()).with_id(8), + ]) + .with_id(5), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_not_atom( + 0, + SymbolicExpression::literal_value(Value::UInt(1)).with_id(6), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_list( + 0, + SymbolicExpression::atom("oops".into()).with_id(3), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_invalid_length( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("oops".into()).with_id(4), + SymbolicExpression::literal_value(Value::UInt(1)).with_id(5), + SymbolicExpression::literal_value(Value::UInt(2)).with_id(6), + ]), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_atom( + 0, + SymbolicExpression::literal_value(Value::UInt(1)).with_id(4), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("string-ascii".into()).with_id(8), + SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), + ]) + .with_id(7), + CheckErrors::ValueOutOfBounds.message(), + )), + ]; + + for (bad_code, expected_err) in bad.iter().zip(expected.iter()) { + debug!("test simple bad syntax binding: '{}'", bad_code); + assert_eq!(expected_err, &type_check_helper(bad_code).unwrap_err().err); + } +} + +/// Nested type signature binding errors. +/// Make sure the reported message only reports one level of nesting of bad syntax binding errors. +#[test] +fn test_nested_bad_type_signature_syntax_bindings() { + let bad = vec![ + // bad tuple type signature within a tower of tuples + "(define-public (foo (bar { a: { b: { c: (string-ascii -19) } } })) (ok true))", + ]; + + let expected = vec![CheckErrors::BadSyntaxBinding( + SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("tuple".into()).with_id(8), + SymbolicExpression::list(vec![ + SymbolicExpression::atom("a".into()).with_id(10), + SymbolicExpression::list(vec![ + SymbolicExpression::atom("tuple".into()).with_id(12), + SymbolicExpression::list(vec![ + SymbolicExpression::atom("b".into()).with_id(14), + SymbolicExpression::list(vec![ + SymbolicExpression::atom("tuple".into()).with_id(16), + SymbolicExpression::list(vec![ + SymbolicExpression::atom("c".into()).with_id(18), + SymbolicExpression::list(vec![ + SymbolicExpression::atom("string-ascii".into()).with_id(20), + SymbolicExpression::literal_value(Value::Int(-19)) + .with_id(21), + ]) + .with_id(19), + ]) + .with_id(17), + ]) + .with_id(15), + ]) + .with_id(13), + ]) + .with_id(11), + ]) + .with_id(9), + ]) + .with_id(7), + // only one level of nesting -- only the innermost bad syntax binding error is reported + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("string-ascii".into()).with_id(20), + SymbolicExpression::literal_value(Value::Int(-19)).with_id(21), + ]) + .with_id(19), + CheckErrors::ValueOutOfBounds.message(), + )) + .message(), + ), + )]; + + for (bad_code, expected_err) in bad.iter().zip(expected.iter()) { + debug!("test nested bad syntax binding: '{}'", bad_code); + assert_eq!(expected_err, &type_check_helper(bad_code).unwrap_err().err); + } +} From 2730aee0accd9e150187b327d5733ee52abd9444 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:54:34 -0400 Subject: [PATCH 10/21] chore: export SyntaxBindingError and SyntaxBindingErrorType --- clarity/src/vm/errors.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/clarity/src/vm/errors.rs b/clarity/src/vm/errors.rs index d46f7a9ca1..852551e2ce 100644 --- a/clarity/src/vm/errors.rs +++ b/clarity/src/vm/errors.rs @@ -24,6 +24,7 @@ use stacks_common::types::chainstate::BlockHeaderHash; use super::ast::errors::ParseErrors; pub use crate::vm::analysis::errors::{ check_argument_count, check_arguments_at_least, check_arguments_at_most, CheckErrors, + SyntaxBindingError, SyntaxBindingErrorType, }; use crate::vm::ast::errors::ParseError; use crate::vm::contexts::StackTrace; From 9707f784996d4d851647d8ec7c1ef680c6a68551 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:54:54 -0400 Subject: [PATCH 11/21] chore: report syntax binding errors as originating from function checks --- clarity/src/vm/functions/define.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clarity/src/vm/functions/define.rs b/clarity/src/vm/functions/define.rs index 1e11ff76e9..1e7907a708 100644 --- a/clarity/src/vm/functions/define.rs +++ b/clarity/src/vm/functions/define.rs @@ -20,6 +20,7 @@ use crate::vm::callables::{DefineType, DefinedFunction}; use crate::vm::contexts::{ContractContext, Environment, LocalContext}; use crate::vm::errors::{ check_argument_count, check_arguments_at_least, CheckErrors, InterpreterResult as Result, + SyntaxBindingErrorType, }; use crate::vm::eval; use crate::vm::representations::SymbolicExpressionType::Field; @@ -141,7 +142,8 @@ fn handle_define_function( check_legal_define(function_name, env.contract_context)?; - let arguments = parse_name_type_pairs(*env.epoch(), arg_symbols, env)?; + let arguments = + parse_name_type_pairs(*env.epoch(), arg_symbols, SyntaxBindingErrorType::Eval, env)?; for (argument, _) in arguments.iter() { check_legal_define(argument, env.contract_context)?; From c75c8d65f84e88dc44e8028055279c44a142b00e Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:55:55 -0400 Subject: [PATCH 12/21] chore: update handle_binding_list() to report the specific kind of binding error and the specific context in which it occurs --- clarity/src/vm/functions/mod.rs | 37 +++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/clarity/src/vm/functions/mod.rs b/clarity/src/vm/functions/mod.rs index 7c92d9a929..6a6777e17a 100644 --- a/clarity/src/vm/functions/mod.rs +++ b/clarity/src/vm/functions/mod.rs @@ -21,7 +21,7 @@ use crate::vm::costs::cost_functions::ClarityCostFunction; use crate::vm::costs::{constants as cost_constants, runtime_cost, CostTracker, MemoryConsumer}; use crate::vm::errors::{ check_argument_count, check_arguments_at_least, CheckErrors, Error, - InterpreterResult as Result, ShortReturnType, + InterpreterResult as Result, ShortReturnType, SyntaxBindingError, SyntaxBindingErrorType, }; pub use crate::vm::functions::assets::stx_transfer_consolidated; use crate::vm::representations::{ClarityName, SymbolicExpression, SymbolicExpressionType}; @@ -664,20 +664,38 @@ fn special_asserts( pub fn handle_binding_list( bindings: &[SymbolicExpression], + binding_error_type: SyntaxBindingErrorType, mut handler: F, ) -> std::result::Result<(), E> where F: FnMut(&ClarityName, &SymbolicExpression) -> std::result::Result<(), E>, E: From, { - for binding in bindings.iter() { - let binding_expression = binding.match_list().ok_or(CheckErrors::BadSyntaxBinding)?; + for (i, binding) in bindings.iter().enumerate() { + let binding_expression = binding.match_list().ok_or_else(|| { + CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotList( + binding_error_type, + i, + binding.clone(), + )) + })?; if binding_expression.len() != 2 { - return Err(CheckErrors::BadSyntaxBinding.into()); + return Err( + CheckErrors::BadSyntaxBinding(SyntaxBindingError::InvalidLength( + binding_error_type, + i, + SymbolicExpression::list(binding_expression.to_vec()), + )) + .into(), + ); } - let var_name = binding_expression[0] - .match_atom() - .ok_or(CheckErrors::BadSyntaxBinding)?; + let var_name = binding_expression[0].match_atom().ok_or_else(|| { + CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotAtom( + binding_error_type, + i, + binding_expression[0].clone(), + )) + })?; let var_sexp = &binding_expression[1]; handler(var_name, var_sexp)?; @@ -687,11 +705,12 @@ where pub fn parse_eval_bindings( bindings: &[SymbolicExpression], + binding_error_type: SyntaxBindingErrorType, env: &mut Environment, context: &LocalContext, ) -> Result> { let mut result = Vec::with_capacity(bindings.len()); - handle_binding_list(bindings, |var_name, var_sexp| { + handle_binding_list(bindings, binding_error_type, |var_name, var_sexp| { eval(var_sexp, env, context).map(|value| result.push((var_name.clone(), value))) })?; @@ -719,7 +738,7 @@ fn special_let( let mut memory_use = 0; finally_drop_memory!( env, memory_use; { - handle_binding_list::<_, Error>(bindings, |binding_name, var_sexp| { + handle_binding_list::<_, Error>(bindings, SyntaxBindingErrorType::Let, |binding_name, var_sexp| { if is_reserved(binding_name, env.contract_context.get_clarity_version()) || env.contract_context.lookup_function(binding_name).is_some() || inner_context.lookup_variable(binding_name).is_some() { From af57eeee0026853b0912614abef45f54305be527 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:56:37 -0400 Subject: [PATCH 13/21] chore: update call to parse_eval_bindings() to report that it was a tuple being checked --- clarity/src/vm/functions/tuples.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clarity/src/vm/functions/tuples.rs b/clarity/src/vm/functions/tuples.rs index 44519f1320..5aefe70e60 100644 --- a/clarity/src/vm/functions/tuples.rs +++ b/clarity/src/vm/functions/tuples.rs @@ -17,7 +17,7 @@ use crate::vm::costs::cost_functions::ClarityCostFunction; use crate::vm::costs::runtime_cost; use crate::vm::errors::{ check_argument_count, check_arguments_at_least, CheckErrors, InterpreterError, - InterpreterResult as Result, + InterpreterResult as Result, SyntaxBindingErrorType, }; use crate::vm::representations::SymbolicExpression; use crate::vm::types::{TupleData, TypeSignature, Value}; @@ -34,7 +34,7 @@ pub fn tuple_cons( check_arguments_at_least(1, args)?; - let bindings = parse_eval_bindings(args, env, context)?; + let bindings = parse_eval_bindings(args, SyntaxBindingErrorType::TupleCons, env, context)?; runtime_cost(ClarityCostFunction::TupleCons, env, bindings.len())?; TupleData::from_data(bindings).map(Value::from) From 93854b5a16340ee1b5096064dc7d4085010a1123 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 16:57:35 -0400 Subject: [PATCH 14/21] feat: add as_error_string() to SymbolicExpression for readable error reporting, and consolidate Display implementation for SymbolicExpression and SymbolicExpressionType --- clarity/src/vm/representations.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/clarity/src/vm/representations.rs b/clarity/src/vm/representations.rs index 08ddedf069..94134cb595 100644 --- a/clarity/src/vm/representations.rs +++ b/clarity/src/vm/representations.rs @@ -622,11 +622,25 @@ impl SymbolicExpression { None } } + + /// Encode this SymbolicExpression as a String suitable for logging an error (such as in + /// CheckErrors). The `developer-mode` feature includes the `span`. + #[cfg(feature = "developer-mode")] + pub fn as_error_string(&self) -> String { + format!("{} at {:?}", &self.expr, &self.span) + } + + /// Encode this SymbolicExpression as a String suitable for logging an error (such as in + /// CheckErrors). + #[cfg(not(feature = "developer-mode"))] + pub fn as_error_string(&self) -> String { + format!("{}", &self.expr) + } } -impl fmt::Display for SymbolicExpression { +impl fmt::Display for SymbolicExpressionType { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self.expr { + match self { SymbolicExpressionType::List(ref list) => { write!(f, "(")?; for item in list.iter() { @@ -647,12 +661,17 @@ impl fmt::Display for SymbolicExpression { SymbolicExpressionType::Field(ref value) => { write!(f, "<{value}>")?; } - }; - + } Ok(()) } } +impl fmt::Display for SymbolicExpression { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", &self.expr) + } +} + #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Default)] pub struct Span { pub start_line: u32, From 38529ae7b439db09b873bac13b5b56083e89ece3 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 17:03:25 -0400 Subject: [PATCH 15/21] chore: update tests to use the specific variant of BadSyntaxBinding, and update tests to expect when BadSyntaxBinding wraps another CheckErrors variant --- clarity/src/vm/tests/datamaps.rs | 39 +++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/clarity/src/vm/tests/datamaps.rs b/clarity/src/vm/tests/datamaps.rs index 90d9e2a642..4348932b07 100644 --- a/clarity/src/vm/tests/datamaps.rs +++ b/clarity/src/vm/tests/datamaps.rs @@ -17,10 +17,11 @@ use crate::vm::errors::Error; use crate::vm::types::{TupleData, Value}; #[cfg(test)] use crate::vm::{ - errors::{CheckErrors, ShortReturnType}, + diagnostic::DiagnosableError, + errors::{CheckErrors, ShortReturnType, SyntaxBindingError}, types::{ListData, SequenceData, TupleTypeSignature, TypeSignature}, }; -use crate::vm::{execute, ClarityName}; +use crate::vm::{execute, ClarityName, SymbolicExpression}; fn assert_executes(expected: Result, input: &str) { assert_eq!(expected.unwrap(), execute(input).unwrap().unwrap()); @@ -645,11 +646,31 @@ fn bad_define_maps() { "(define-map lists { name: int } { contents: (list 5 0 int) })", ]; let expected: Vec = vec![ - CheckErrors::BadSyntaxExpectedListOfPairs.into(), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_invalid_length( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("contents".into()).with_id(12), + SymbolicExpression::atom("int".into()).with_id(13), + SymbolicExpression::atom("bool".into()).with_id(14), + ]) + .with_id(11), + )) + .into(), CheckErrors::UnknownTypeName("contents".to_string()).into(), CheckErrors::ExpectedName.into(), CheckErrors::IncorrectArgumentCount(3, 4).into(), - CheckErrors::InvalidTypeDescription.into(), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("list".into()).with_id(14), + SymbolicExpression::literal_value(Value::Int(5)).with_id(15), + SymbolicExpression::literal_value(Value::Int(0)).with_id(16), + SymbolicExpression::atom("int".into()).with_id(17), + ]) + .with_id(13), + CheckErrors::InvalidTypeDescription.message(), + )) + .into(), ]; for (test, expected_err) in tests.iter().zip(expected.into_iter()) { @@ -670,8 +691,14 @@ fn bad_tuples() { ]; let expected = vec![ CheckErrors::NameAlreadyUsed("name".into()), - CheckErrors::BadSyntaxBinding, - CheckErrors::BadSyntaxBinding, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_list( + 0, + SymbolicExpression::atom("name".into()).with_id(3), + )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_invalid_length( + 1, + SymbolicExpression::list(vec![SymbolicExpression::atom("blame".into()).with_id(7)]), + )), CheckErrors::NoSuchTupleField( "value".into(), TupleTypeSignature::try_from(vec![("name".into(), TypeSignature::IntType)]).unwrap(), From e1ddf255dc9fe1c80fdfb9ccf431c284bdc17334 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 17:04:13 -0400 Subject: [PATCH 16/21] chore: update tests to use the new BadSyntaxBinding variant, including expecting it to wrap an inner CheckErrors variant which lead to a bad binding --- clarity/src/vm/tests/defines.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/clarity/src/vm/tests/defines.rs b/clarity/src/vm/tests/defines.rs index fea98b7298..9a9bab5cc3 100644 --- a/clarity/src/vm/tests/defines.rs +++ b/clarity/src/vm/tests/defines.rs @@ -21,8 +21,11 @@ use rstest_reuse::{self, *}; #[cfg(test)] use stacks_common::types::StacksEpochId; +use crate::vm::analysis::errors::SyntaxBindingError; +use crate::vm::diagnostic::DiagnosableError; use crate::vm::errors::{CheckErrors, Error}; use crate::vm::tests::test_clarity_versions; +use crate::vm::SymbolicExpression; #[cfg(test)] use crate::vm::{ ast::{ @@ -86,7 +89,17 @@ fn test_accept_options(#[case] version: ClarityVersion, #[case] epoch: StacksEpo let bad_defun = "(define-private (f (b (optional int int))) (* 10 (default-to 0 b)))"; assert_eq!( execute(bad_defun).unwrap_err(), - CheckErrors::InvalidTypeDescription.into() + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + 0, + SymbolicExpression::list(vec![ + SymbolicExpression::atom("optional".into()).with_id(8), + SymbolicExpression::atom("int".into()).with_id(9), + SymbolicExpression::atom("int".into()).with_id(10), + ]) + .with_id(7), + CheckErrors::InvalidTypeDescription.message() + )) + .into() ); } @@ -279,7 +292,10 @@ fn test_define_parse_panic() { fn test_define_parse_panic_2() { let tests = "(define-private (a b (d)) 1)"; assert_eq_err( - CheckErrors::BadSyntaxExpectedListOfPairs, + CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_not_list( + 0, + SymbolicExpression::atom("b".into()).with_id(5), + )), execute(tests).unwrap_err(), ); } From ce8bcc5263175409b76eaa248917d9dfec2ff1e3 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 17:04:42 -0400 Subject: [PATCH 17/21] chore: add with_id() test constructor method to SynmbolicExpression --- clarity/src/vm/tests/mod.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/clarity/src/vm/tests/mod.rs b/clarity/src/vm/tests/mod.rs index 7d58fb515e..4e04c7b3e9 100644 --- a/clarity/src/vm/tests/mod.rs +++ b/clarity/src/vm/tests/mod.rs @@ -23,6 +23,7 @@ use super::ClarityVersion; use crate::vm::contexts::OwnedEnvironment; pub use crate::vm::database::BurnStateDB; use crate::vm::database::MemoryBackingStore; +use crate::vm::SymbolicExpression; #[cfg(test)] use crate::{vm::errors::Error, vm::types::Value}; @@ -223,3 +224,10 @@ pub fn test_only_mainnet_to_chain_id(mainnet: bool) -> u32 { CHAIN_ID_TESTNET } } + +impl SymbolicExpression { + pub fn with_id(mut self, id: u64) -> Self { + self.id = id; + self + } +} From 5bc3312d57b2a6cf603151817d1b07b5f5f83bfb Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 17:05:26 -0400 Subject: [PATCH 18/21] feat: update parse_name_type_pairs() to report BadSyntaxBinding(SyntaxBindingError::BadTypeSignature(..) ..), instead of BadSyntaxExpectedListOfPairs, so the caller has a better idea about what is structurally deficient about the binding or type signature --- clarity/src/vm/types/signatures.rs | 87 +++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 24 deletions(-) diff --git a/clarity/src/vm/types/signatures.rs b/clarity/src/vm/types/signatures.rs index a7fa8434f0..e51ddc4151 100644 --- a/clarity/src/vm/types/signatures.rs +++ b/clarity/src/vm/types/signatures.rs @@ -26,7 +26,8 @@ use lazy_static::lazy_static; use stacks_common::types::StacksEpochId; use crate::vm::costs::{runtime_cost, CostOverflowingMath}; -use crate::vm::errors::CheckErrors; +use crate::vm::diagnostic::DiagnosableError; +use crate::vm::errors::{CheckErrors, SyntaxBindingError, SyntaxBindingErrorType}; use crate::vm::representations::{ ClarityName, ContractName, SymbolicExpression, SymbolicExpressionType, TraitDefinition, CONTRACT_MAX_NAME_LENGTH, @@ -950,19 +951,6 @@ impl TupleTypeSignature { Ok(true) } - pub fn parse_name_type_pair_list( - epoch: StacksEpochId, - type_def: &SymbolicExpression, - accounting: &mut A, - ) -> Result { - if let SymbolicExpressionType::List(ref name_type_pairs) = type_def.expr { - let mapped_key_types = parse_name_type_pairs(epoch, name_type_pairs, accounting)?; - TupleTypeSignature::try_from(mapped_key_types) - } else { - Err(CheckErrors::BadSyntaxExpectedListOfPairs) - } - } - pub fn shallow_merge(&mut self, update: &mut TupleTypeSignature) { Arc::make_mut(&mut self.type_map).append(Arc::make_mut(&mut update.type_map)); } @@ -1507,7 +1495,12 @@ impl TypeSignature { type_args: &[SymbolicExpression], accounting: &mut A, ) -> Result { - let mapped_key_types = parse_name_type_pairs(epoch, type_args, accounting)?; + let mapped_key_types = parse_name_type_pairs( + epoch, + type_args, + SyntaxBindingErrorType::TupleCons, + accounting, + )?; let tuple_type_signature = TupleTypeSignature::try_from(mapped_key_types)?; Ok(TypeSignature::from(tuple_type_signature)) } @@ -1924,11 +1917,15 @@ use crate::vm::costs::cost_functions::ClarityCostFunction; use crate::vm::costs::CostTracker; 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( epoch: StacksEpochId, name_type_pairs: &[SymbolicExpression], + binding_error_type: SyntaxBindingErrorType, accounting: &mut A, -) -> Result> { +) -> std::result::Result, CheckErrors> { // 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) ...) @@ -1936,30 +1933,72 @@ pub fn parse_name_type_pairs( use crate::vm::representations::SymbolicExpressionType::List; // step 1: parse it into a vec of symbolicexpression pairs. - let as_pairs: Result> = name_type_pairs + let as_pairs: std::result::Result, _> = name_type_pairs .iter() - .map(|key_type_pair| { + .enumerate() + .map(|(i, key_type_pair)| { if let List(ref as_vec) = key_type_pair.expr { if as_vec.len() != 2 { - Err(CheckErrors::BadSyntaxExpectedListOfPairs) + Err(CheckErrors::BadSyntaxBinding( + SyntaxBindingError::InvalidLength( + binding_error_type, + i, + key_type_pair.clone(), + ), + )) } else { Ok((&as_vec[0], &as_vec[1])) } } else { - Err(CheckErrors::BadSyntaxExpectedListOfPairs) + Err(CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotList( + binding_error_type, + i, + key_type_pair.clone(), + ))) } }) .collect(); // step 2: turn into a vec of (name, typesignature) pairs. - let key_types: Result> = (as_pairs?) + let key_types: std::result::Result, _> = (as_pairs?) .iter() - .map(|(name_symbol, type_symbol)| { + .enumerate() + .map(|(i, (name_symbol, type_symbol))| { let name = name_symbol .match_atom() - .ok_or(CheckErrors::BadSyntaxExpectedListOfPairs)? + .ok_or_else(|| { + CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotAtom( + binding_error_type, + i, + (*name_symbol).clone(), + )) + })? .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| { + CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( + i, + (*type_symbol).clone(), + // if the inner error is itself a BadTypeSignature error, and it's + // `message` came from a BadSyntaxBinding, then just use its + // message directly so we don't get a tower of nested BadTypeSignature + // messages. We only want one level of nesting, so something like + // `(string-ascii -19)` gets reported instead of `-19` (so the caller gets + // some context, but not an unreasonably large amount) + if let CheckErrors::BadSyntaxBinding( + SyntaxBindingError::BadTypeSignature(_, _, message), + ) = &e + { + if CheckErrors::has_nested_bad_syntax_binding_message(message) { + message.clone() + } else { + e.message() + } + } else { + e.message() + }, + )) + })?; Ok((name, type_info)) }) .collect(); From 1396f49a7c1c385237b27ec9bb9ac1f3b12cce78 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Thu, 31 Jul 2025 17:37:34 -0400 Subject: [PATCH 19/21] chore: update changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 738439f18c..51e8ca33a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to the versioning scheme outlined in the [README.md](README.md). +## [unreleased] + +### Changed + +- Clarity errors pertaining to syntax binding errors have been made more + expressive (#6337) + ## [3.2.0.0.0] ### Added From 1a555309abb117e9674471f6513f27458109a953 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Fri, 15 Aug 2025 19:22:03 -0400 Subject: [PATCH 20/21] chore: address PR feedback --- clarity/src/vm/analysis/errors.rs | 115 +++++------ .../src/vm/analysis/read_only_checker/mod.rs | 38 ++-- clarity/src/vm/analysis/tests/mod.rs | 3 +- .../analysis/type_checker/v2_05/tests/mod.rs | 61 +----- .../analysis/type_checker/v2_1/tests/mod.rs | 192 +++--------------- clarity/src/vm/functions/mod.rs | 13 +- clarity/src/vm/tests/datamaps.rs | 37 +--- clarity/src/vm/tests/defines.rs | 21 +- clarity/src/vm/types/signatures.rs | 44 +--- 9 files changed, 126 insertions(+), 398 deletions(-) diff --git a/clarity/src/vm/analysis/errors.rs b/clarity/src/vm/analysis/errors.rs index 18377e67d9..692639467c 100644 --- a/clarity/src/vm/analysis/errors.rs +++ b/clarity/src/vm/analysis/errors.rs @@ -29,7 +29,6 @@ pub enum SyntaxBindingErrorType { Let, Eval, TupleCons, - TypeDefinition, } impl fmt::Display for SyntaxBindingErrorType { @@ -44,7 +43,6 @@ impl DiagnosableError for SyntaxBindingErrorType { Self::Let => "Let-binding".to_string(), Self::Eval => "Function argument definition".to_string(), Self::TupleCons => "Tuple constructor".to_string(), - Self::TypeDefinition => "Type definition".to_string(), } } @@ -57,16 +55,11 @@ impl DiagnosableError for SyntaxBindingErrorType { #[derive(Debug, PartialEq)] pub enum SyntaxBindingError { /// binding list item is not a list - NotList(SyntaxBindingErrorType, usize, SymbolicExpression), + NotList(SyntaxBindingErrorType, usize), /// binding list item has an invalid length (e.g. not 2) - InvalidLength(SyntaxBindingErrorType, usize, SymbolicExpression), + InvalidLength(SyntaxBindingErrorType, usize), /// binding name is not an atom - NotAtom(SyntaxBindingErrorType, usize, SymbolicExpression), - /// second binding item is a type signature, and the type signature itself is bad. - /// NOTE: type signature parsing returns CheckErrors, so we cannot include a CheckErrors here - /// directly without creating a recursive type. Instead, we just report the `Display` - /// representation of the error here as the third item. - BadTypeSignature(usize, SymbolicExpression, String), + NotAtom(SyntaxBindingErrorType, usize), } impl fmt::Display for SyntaxBindingError { @@ -78,30 +71,17 @@ impl fmt::Display for SyntaxBindingError { impl DiagnosableError for SyntaxBindingError { fn message(&self) -> String { match &self { - Self::NotList(err_type, item_index, item) => { + Self::NotList(err_type, item_index) => { let item_no = item_index + 1; - format!( - "{err_type} item #{item_no} is not a list: {}", - item.as_error_string() - ) + format!("{err_type} item #{item_no} is not a list",) } - Self::InvalidLength(err_type, item_index, item) => { + Self::InvalidLength(err_type, item_index) => { let item_no = item_index + 1; - format!( - "{err_type} item #{item_no} is not a two-element list: {}", - item.as_error_string() - ) + format!("{err_type} item #{item_no} is not a two-element list",) } - Self::NotAtom(err_type, item_index, item) => { + Self::NotAtom(err_type, item_index) => { let item_no = item_index + 1; - format!( - "{err_type} item #{item_no}'s name is not an atom: {}", - item.as_error_string() - ) - } - Self::BadTypeSignature(item_index, item, error_message) => { - let item_no = item_index + 1; - format!("Type definition item #{item_no} has an invalid type signature: {} (reason: {error_message})", item.as_error_string()) + format!("{err_type} item #{item_no}'s name is not an atom",) } } } @@ -112,49 +92,55 @@ impl DiagnosableError for SyntaxBindingError { } impl SyntaxBindingError { - /// Helper constructor for NotList(SyntaxBindingErrorType::Let, item_no, item) - pub fn let_binding_not_list(item_no: usize, item: SymbolicExpression) -> Self { - Self::NotList(SyntaxBindingErrorType::Let, item_no, item) + /// Helper constructor for NotList(SyntaxBindingErrorType::Let, item_no) + pub fn let_binding_not_list(item_no: usize) -> Self { + Self::NotList(SyntaxBindingErrorType::Let, item_no) + } + + /// Helper constructor for InvalidLength(SyntaxBindingErrorType::Let, item_no) + pub fn let_binding_invalid_length(item_no: usize) -> Self { + Self::InvalidLength(SyntaxBindingErrorType::Let, item_no) } - /// Helper constructor for InvalidLength(SyntaxBindingErrorType::Let, item_no, item) - pub fn let_binding_invalid_length(item_no: usize, item: SymbolicExpression) -> Self { - Self::InvalidLength(SyntaxBindingErrorType::Let, item_no, item) + /// Helper constructor for NotAtom(SyntaxBindingErrorType::Let, item_no) + pub fn let_binding_not_atom(item_no: usize) -> Self { + Self::NotAtom(SyntaxBindingErrorType::Let, item_no) } - /// Helper constructor for NotAtom(SyntaxBindingErrorType::Let, item_no, item) - pub fn let_binding_not_atom(item_no: usize, item: SymbolicExpression) -> Self { - Self::NotAtom(SyntaxBindingErrorType::Let, item_no, item) + /// Helper constructor for NotList(SyntaxBindingErrorType::Eval, item_no) + pub fn eval_binding_not_list(item_no: usize) -> Self { + Self::NotList(SyntaxBindingErrorType::Eval, item_no) } - /// Helper constructor for NotList(SyntaxBindingErrorType::Eval, item_no, item) - pub fn eval_binding_not_list(item_no: usize, item: SymbolicExpression) -> Self { - Self::NotList(SyntaxBindingErrorType::Eval, item_no, item) + /// Helper constructor for InvalidLength(SyntaxBindingErrorType::Eval, item_no) + pub fn eval_binding_invalid_length(item_no: usize) -> Self { + Self::InvalidLength(SyntaxBindingErrorType::Eval, item_no) } - /// Helper constructor for InvalidLength(SyntaxBindingErrorType::Eval, item_no, item) - pub fn eval_binding_invalid_length(item_no: usize, item: SymbolicExpression) -> Self { - Self::InvalidLength(SyntaxBindingErrorType::Eval, item_no, item) + /// Helper constructor for NotAtom(SyntaxBindingErrorType::Eval, item_no) + pub fn eval_binding_not_atom(item_no: usize) -> Self { + Self::NotAtom(SyntaxBindingErrorType::Eval, item_no) } - /// Helper constructor for NotAtom(SyntaxBindingErrorType::Eval, item_no, item) - pub fn eval_binding_not_atom(item_no: usize, item: SymbolicExpression) -> Self { - Self::NotAtom(SyntaxBindingErrorType::Eval, item_no, item) + /// Helper constructor for NotList(SyntaxBindingErrorType::TupleCons, item_no) + pub fn tuple_cons_not_list(item_no: usize) -> Self { + Self::NotList(SyntaxBindingErrorType::TupleCons, item_no) } - /// Helper constructor for NotList(SyntaxBindingErrorType::TupleCons, item_no, item) - pub fn tuple_cons_not_list(item_no: usize, item: SymbolicExpression) -> Self { - Self::NotList(SyntaxBindingErrorType::TupleCons, item_no, item) + /// Helper constructor for InvalidLength(SyntaxBindingErrorType::TupleCons, item_no) + pub fn tuple_cons_invalid_length(item_no: usize) -> Self { + Self::InvalidLength(SyntaxBindingErrorType::TupleCons, item_no) } - /// Helper constructor for InvalidLength(SyntaxBindingErrorType::TupleCons, item_no, item) - pub fn tuple_cons_invalid_length(item_no: usize, item: SymbolicExpression) -> Self { - Self::InvalidLength(SyntaxBindingErrorType::TupleCons, item_no, item) + /// Helper constructor for NotAtom(SyntaxBindingErrorType::TupleCons, item_no) + pub fn tuple_cons_not_atom(item_no: usize) -> Self { + Self::NotAtom(SyntaxBindingErrorType::TupleCons, item_no) } +} - /// Helper constructor for NotAtom(SyntaxBindingErrorType::TupleCons, item_no, item) - pub fn tuple_cons_not_atom(item_no: usize, item: SymbolicExpression) -> Self { - Self::NotAtom(SyntaxBindingErrorType::TupleCons, item_no, item) +impl From for CheckErrors { + fn from(e: SyntaxBindingError) -> Self { + Self::BadSyntaxBinding(e) } } @@ -348,11 +334,6 @@ impl CheckErrors { CheckErrors::SupertypeTooLarge | CheckErrors::Expects(_) ) } - - /// Is the given error message due to a BadSyntaxBinding? - pub fn has_nested_bad_syntax_binding_message(msg: &str) -> bool { - msg.contains("invalid syntax binding: ") - } } impl CheckError { @@ -378,6 +359,18 @@ impl CheckError { self.diagnostic.spans = exprs.iter().map(|e| e.span().clone()).collect(); self.expressions.replace(exprs.to_vec()); } + + pub fn with_expression(err: CheckErrors, expr: &SymbolicExpression) -> Self { + let mut r = Self::new(err); + r.set_expression(expr); + r + } +} + +impl From<(SyntaxBindingError, &SymbolicExpression)> for CheckError { + fn from(e: (SyntaxBindingError, &SymbolicExpression)) -> Self { + Self::with_expression(CheckErrors::BadSyntaxBinding(e.0), e.1) + } } impl fmt::Display for CheckErrors { diff --git a/clarity/src/vm/analysis/read_only_checker/mod.rs b/clarity/src/vm/analysis/read_only_checker/mod.rs index 8eacbc5e86..97de5bd42c 100644 --- a/clarity/src/vm/analysis/read_only_checker/mod.rs +++ b/clarity/src/vm/analysis/read_only_checker/mod.rs @@ -327,19 +327,16 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> { for (i, pair) in binding_list.iter().enumerate() { let pair_expression = pair.match_list().ok_or_else(|| { - CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_list( - i, - pair.clone(), - )) + CheckError::with_expression( + SyntaxBindingError::let_binding_not_list(i).into(), + pair, + ) })?; if pair_expression.len() != 2 { - return Err(CheckErrors::BadSyntaxBinding( - SyntaxBindingError::let_binding_invalid_length( - i, - SymbolicExpression::list(pair_expression.to_vec()), - ), - ) - .into()); + return Err(CheckError::with_expression( + SyntaxBindingError::let_binding_invalid_length(i).into(), + &SymbolicExpression::list(pair_expression.to_vec()), + )); } if !self.check_read_only(&pair_expression[1])? { @@ -378,19 +375,16 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> { TupleCons => { for (i, pair) in args.iter().enumerate() { let pair_expression = pair.match_list().ok_or_else(|| { - CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_list( - i, - pair.clone(), - )) + CheckError::with_expression( + SyntaxBindingError::tuple_cons_not_list(i).into(), + pair, + ) })?; if pair_expression.len() != 2 { - return Err(CheckErrors::BadSyntaxBinding( - SyntaxBindingError::tuple_cons_invalid_length( - i, - SymbolicExpression::list(pair_expression.to_vec()), - ), - ) - .into()); + return Err(CheckError::with_expression( + SyntaxBindingError::tuple_cons_invalid_length(i).into(), + &SymbolicExpression::list(pair_expression.to_vec()), + )); } if !self.check_read_only(&pair_expression[1])? { diff --git a/clarity/src/vm/analysis/tests/mod.rs b/clarity/src/vm/analysis/tests/mod.rs index f434e636cf..8fe286ce30 100644 --- a/clarity/src/vm/analysis/tests/mod.rs +++ b/clarity/src/vm/analysis/tests/mod.rs @@ -131,7 +131,8 @@ fn test_bad_tuple_construction() { fn test_tuple_expects_pairs() { let snippet = "(tuple (key 1) (key-with-missing-value))"; let err = mem_type_check(snippet).unwrap_err(); - assert!(format!("{}", err.diagnostic).contains("invalid syntax binding: Tuple constructor item #2 is not a two-element list: ( key-with-missing-value ).")); + assert!(format!("{}", err.diagnostic) + .contains("invalid syntax binding: Tuple constructor item #2 is not a two-element list.")); } #[test] diff --git a/clarity/src/vm/analysis/type_checker/v2_05/tests/mod.rs b/clarity/src/vm/analysis/type_checker/v2_05/tests/mod.rs index da7942f609..8316931eed 100644 --- a/clarity/src/vm/analysis/type_checker/v2_05/tests/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_05/tests/mod.rs @@ -21,14 +21,13 @@ use crate::vm::analysis::mem_type_check; use crate::vm::analysis::type_checker::v2_05::TypeResult; use crate::vm::ast::build_ast; use crate::vm::ast::errors::ParseErrors; -use crate::vm::diagnostic::DiagnosableError; use crate::vm::types::SequenceSubtype::*; use crate::vm::types::StringSubtype::*; use crate::vm::types::TypeSignature::{BoolType, IntType, PrincipalType, UIntType}; use crate::vm::types::{ FixedFunction, FunctionType, QualifiedContractIdentifier, TypeSignature, BUFF_32, BUFF_64, }; -use crate::vm::{ClarityVersion, SymbolicExpression, Value}; +use crate::vm::ClarityVersion; mod assets; mod contracts; @@ -647,16 +646,8 @@ fn test_simple_lets() { ]; let bad_expected = [ - CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::literal_value(Value::Int(1)).with_id(5) - ]), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom( - 0, - SymbolicExpression::literal_value(Value::Int(1)).with_id(5), - )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom(0)), CheckErrors::TypeError(TypeSignature::IntType, TypeSignature::UIntType), ]; @@ -1245,12 +1236,7 @@ fn test_empty_tuple_should_fail() { ) .unwrap_err() .err, - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![SymbolicExpression::atom("tuple".into()).with_id(8)]) - .with_id(7), - CheckErrors::EmptyTuplesNotAllowed.message() - )) + CheckErrors::EmptyTuplesNotAllowed ); } @@ -2480,18 +2466,7 @@ fn test_buff_negative_len() { StacksEpochId::Epoch2_05, ) .unwrap_err(); - assert_eq!( - res.err, - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("buff".into()).with_id(8), - SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), - ]) - .with_id(7), - CheckErrors::ValueOutOfBounds.message() - )) - ); + assert_eq!(res.err, CheckErrors::ValueOutOfBounds); } #[test] @@ -2505,18 +2480,7 @@ fn test_string_ascii_negative_len() { StacksEpochId::Epoch2_05, ) .unwrap_err(); - assert_eq!( - res.err, - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("string-ascii".into()).with_id(8), - SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), - ]) - .with_id(7), - CheckErrors::ValueOutOfBounds.message() - )) - ); + assert_eq!(res.err, CheckErrors::ValueOutOfBounds); } #[test] @@ -2530,16 +2494,5 @@ fn test_string_utf8_negative_len() { StacksEpochId::Epoch2_05, ) .unwrap_err(); - assert_eq!( - res.err, - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("string-utf8".into()).with_id(8), - SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), - ]) - .with_id(7), - CheckErrors::ValueOutOfBounds.message() - )) - ); + assert_eq!(res.err, CheckErrors::ValueOutOfBounds); } diff --git a/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs b/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs index b576b865a5..23ed30f723 100644 --- a/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs @@ -27,7 +27,6 @@ use crate::vm::analysis::type_checker::v2_1::TypeResult; use crate::vm::analysis::types::ContractAnalysis; use crate::vm::ast::build_ast; use crate::vm::ast::errors::ParseErrors; -use crate::vm::diagnostic::DiagnosableError; use crate::vm::tests::test_clarity_versions; use crate::vm::types::signatures::TypeSignature::OptionalType; use crate::vm::types::signatures::{ListTypeData, StringUTF8Length}; @@ -38,7 +37,7 @@ use crate::vm::types::{ BufferLength, FixedFunction, FunctionType, QualifiedContractIdentifier, TraitIdentifier, TypeSignature, BUFF_1, BUFF_20, BUFF_21, BUFF_32, BUFF_64, }; -use crate::vm::{execute_v2, ClarityName, ClarityVersion, SymbolicExpression, Value}; +use crate::vm::{execute_v2, ClarityName, ClarityVersion}; mod assets; pub mod contracts; @@ -980,16 +979,8 @@ fn test_simple_lets() { ]; let bad_expected = [ - CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::literal_value(Value::Int(1)).with_id(5) - ]), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom( - 0, - SymbolicExpression::literal_value(Value::Int(1)).with_id(5), - )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom(0)), CheckErrors::TypeError(TypeSignature::IntType, TypeSignature::UIntType), ]; @@ -1957,12 +1948,7 @@ fn test_empty_tuple_should_fail() { assert_eq!( mem_type_check(contract_src).unwrap_err().err, - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![SymbolicExpression::atom("tuple".into()).with_id(8)]) - .with_id(7), - CheckErrors::EmptyTuplesNotAllowed.message() - )) + CheckErrors::EmptyTuplesNotAllowed, ); } @@ -3115,18 +3101,7 @@ fn test_buff_negative_len() { (func 0x00)"; let res = mem_type_check(contract_src).unwrap_err(); - assert_eq!( - res.err, - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("buff".into()).with_id(8), - SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), - ]) - .with_id(7), - CheckErrors::ValueOutOfBounds.message() - )) - ); + assert_eq!(res.err, CheckErrors::ValueOutOfBounds); } #[test] @@ -3135,18 +3110,7 @@ fn test_string_ascii_negative_len() { (func \"\")"; let res = mem_type_check(contract_src).unwrap_err(); - assert_eq!( - res.err, - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("string-ascii".into()).with_id(8), - SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), - ]) - .with_id(7), - CheckErrors::ValueOutOfBounds.message() - )) - ); + assert_eq!(res.err, CheckErrors::ValueOutOfBounds); } #[test] @@ -3155,18 +3119,7 @@ fn test_string_utf8_negative_len() { (func u\"\")"; let res = mem_type_check(contract_src).unwrap_err(); - assert_eq!( - res.err, - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("string-utf8".into()).with_id(8), - SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), - ]) - .with_id(7), - CheckErrors::ValueOutOfBounds.message() - )) - ); + assert_eq!(res.err, CheckErrors::ValueOutOfBounds); } #[test] @@ -3725,68 +3678,23 @@ fn test_simple_bad_syntax_bindings() { "(tuple (oops u1 u2) (bar u1))", // bad tuple binding -- binding item name is not an atom "(tuple (u1 u2) (bar u1))", - // bad type binding -- bad type signature + // bad type signature (no longer a bad syntax binding error) "(define-private (foo (bar (string-ascii -12))) (ok true))", + // bad type signature (no longer a bad syntax binding error) + "(from-consensus-buff? (tuple (a (string-ascii -12))) 0x00)", ]; let expected = vec![ - CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_list( - 0, - SymbolicExpression::atom("oops".into()).with_id(4), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("oops".into()).with_id(5), - SymbolicExpression::literal_value(Value::UInt(1)).with_id(6), - SymbolicExpression::literal_value(Value::UInt(2)).with_id(7), - ]), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom( - 0, - SymbolicExpression::literal_value(Value::Int(1)).with_id(5), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_not_list( - 0, - SymbolicExpression::atom("oops".into()).with_id(5), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_invalid_length( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("oops".into()).with_id(6), - SymbolicExpression::atom("uint".into()).with_id(7), - SymbolicExpression::atom("uint".into()).with_id(8), - ]) - .with_id(5), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_not_atom( - 0, - SymbolicExpression::literal_value(Value::UInt(1)).with_id(6), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_list( - 0, - SymbolicExpression::atom("oops".into()).with_id(3), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_invalid_length( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("oops".into()).with_id(4), - SymbolicExpression::literal_value(Value::UInt(1)).with_id(5), - SymbolicExpression::literal_value(Value::UInt(2)).with_id(6), - ]), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_atom( - 0, - SymbolicExpression::literal_value(Value::UInt(1)).with_id(4), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("string-ascii".into()).with_id(8), - SymbolicExpression::literal_value(Value::Int(-12)).with_id(9), - ]) - .with_id(7), - CheckErrors::ValueOutOfBounds.message(), - )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_list(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_not_list(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_invalid_length(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_not_atom(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_list(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_invalid_length(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_atom(0)), + CheckErrors::ValueOutOfBounds, + CheckErrors::ValueOutOfBounds, ]; for (bad_code, expected_err) in bad.iter().zip(expected.iter()) { @@ -3796,60 +3704,24 @@ fn test_simple_bad_syntax_bindings() { } /// Nested type signature binding errors. -/// Make sure the reported message only reports one level of nesting of bad syntax binding errors. +/// These are no longer BadSyntaxBinding errors, but are instead reported as their innermost +/// type-check error. #[test] fn test_nested_bad_type_signature_syntax_bindings() { let bad = vec![ // bad tuple type signature within a tower of tuples "(define-public (foo (bar { a: { b: { c: (string-ascii -19) } } })) (ok true))", + // bad type signature within a tower of lists + "(define-public (foo (bar (list (list 10 (string-ascii -19))))) (ok true))", + // bad type signature within a tower of tuples + "(from-consensus-buff? { a : { b: { c: (string-ascii -19) } } } 0x00)", ]; - let expected = vec![CheckErrors::BadSyntaxBinding( - SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("tuple".into()).with_id(8), - SymbolicExpression::list(vec![ - SymbolicExpression::atom("a".into()).with_id(10), - SymbolicExpression::list(vec![ - SymbolicExpression::atom("tuple".into()).with_id(12), - SymbolicExpression::list(vec![ - SymbolicExpression::atom("b".into()).with_id(14), - SymbolicExpression::list(vec![ - SymbolicExpression::atom("tuple".into()).with_id(16), - SymbolicExpression::list(vec![ - SymbolicExpression::atom("c".into()).with_id(18), - SymbolicExpression::list(vec![ - SymbolicExpression::atom("string-ascii".into()).with_id(20), - SymbolicExpression::literal_value(Value::Int(-19)) - .with_id(21), - ]) - .with_id(19), - ]) - .with_id(17), - ]) - .with_id(15), - ]) - .with_id(13), - ]) - .with_id(11), - ]) - .with_id(9), - ]) - .with_id(7), - // only one level of nesting -- only the innermost bad syntax binding error is reported - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("string-ascii".into()).with_id(20), - SymbolicExpression::literal_value(Value::Int(-19)).with_id(21), - ]) - .with_id(19), - CheckErrors::ValueOutOfBounds.message(), - )) - .message(), - ), - )]; + let expected = vec![ + CheckErrors::ValueOutOfBounds, + CheckErrors::InvalidTypeDescription, + CheckErrors::ValueOutOfBounds, + ]; for (bad_code, expected_err) in bad.iter().zip(expected.iter()) { debug!("test nested bad syntax binding: '{}'", bad_code); diff --git a/clarity/src/vm/functions/mod.rs b/clarity/src/vm/functions/mod.rs index 6a6777e17a..f28594920f 100644 --- a/clarity/src/vm/functions/mod.rs +++ b/clarity/src/vm/functions/mod.rs @@ -673,28 +673,19 @@ where { for (i, binding) in bindings.iter().enumerate() { let binding_expression = binding.match_list().ok_or_else(|| { - CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotList( - binding_error_type, - i, - binding.clone(), - )) + CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotList(binding_error_type, i)) })?; if binding_expression.len() != 2 { return Err( CheckErrors::BadSyntaxBinding(SyntaxBindingError::InvalidLength( binding_error_type, i, - SymbolicExpression::list(binding_expression.to_vec()), )) .into(), ); } let var_name = binding_expression[0].match_atom().ok_or_else(|| { - CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotAtom( - binding_error_type, - i, - binding_expression[0].clone(), - )) + CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotAtom(binding_error_type, i)) })?; let var_sexp = &binding_expression[1]; diff --git a/clarity/src/vm/tests/datamaps.rs b/clarity/src/vm/tests/datamaps.rs index 4348932b07..74072b5062 100644 --- a/clarity/src/vm/tests/datamaps.rs +++ b/clarity/src/vm/tests/datamaps.rs @@ -17,11 +17,10 @@ use crate::vm::errors::Error; use crate::vm::types::{TupleData, Value}; #[cfg(test)] use crate::vm::{ - diagnostic::DiagnosableError, errors::{CheckErrors, ShortReturnType, SyntaxBindingError}, types::{ListData, SequenceData, TupleTypeSignature, TypeSignature}, }; -use crate::vm::{execute, ClarityName, SymbolicExpression}; +use crate::vm::{execute, ClarityName}; fn assert_executes(expected: Result, input: &str) { assert_eq!(expected.unwrap(), execute(input).unwrap().unwrap()); @@ -646,31 +645,11 @@ fn bad_define_maps() { "(define-map lists { name: int } { contents: (list 5 0 int) })", ]; let expected: Vec = vec![ - CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_invalid_length( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("contents".into()).with_id(12), - SymbolicExpression::atom("int".into()).with_id(13), - SymbolicExpression::atom("bool".into()).with_id(14), - ]) - .with_id(11), - )) - .into(), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_invalid_length(0)).into(), CheckErrors::UnknownTypeName("contents".to_string()).into(), CheckErrors::ExpectedName.into(), CheckErrors::IncorrectArgumentCount(3, 4).into(), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("list".into()).with_id(14), - SymbolicExpression::literal_value(Value::Int(5)).with_id(15), - SymbolicExpression::literal_value(Value::Int(0)).with_id(16), - SymbolicExpression::atom("int".into()).with_id(17), - ]) - .with_id(13), - CheckErrors::InvalidTypeDescription.message(), - )) - .into(), + CheckErrors::InvalidTypeDescription.into(), ]; for (test, expected_err) in tests.iter().zip(expected.into_iter()) { @@ -691,14 +670,8 @@ fn bad_tuples() { ]; let expected = vec![ CheckErrors::NameAlreadyUsed("name".into()), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_list( - 0, - SymbolicExpression::atom("name".into()).with_id(3), - )), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_invalid_length( - 1, - SymbolicExpression::list(vec![SymbolicExpression::atom("blame".into()).with_id(7)]), - )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_not_list(0)), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::tuple_cons_invalid_length(1)), CheckErrors::NoSuchTupleField( "value".into(), TupleTypeSignature::try_from(vec![("name".into(), TypeSignature::IntType)]).unwrap(), diff --git a/clarity/src/vm/tests/defines.rs b/clarity/src/vm/tests/defines.rs index 9a9bab5cc3..235d1af885 100644 --- a/clarity/src/vm/tests/defines.rs +++ b/clarity/src/vm/tests/defines.rs @@ -21,13 +21,11 @@ use rstest_reuse::{self, *}; #[cfg(test)] use stacks_common::types::StacksEpochId; -use crate::vm::analysis::errors::SyntaxBindingError; -use crate::vm::diagnostic::DiagnosableError; use crate::vm::errors::{CheckErrors, Error}; use crate::vm::tests::test_clarity_versions; -use crate::vm::SymbolicExpression; #[cfg(test)] use crate::vm::{ + analysis::errors::SyntaxBindingError, ast::{ build_ast, errors::{ParseError, ParseErrors}, @@ -89,17 +87,7 @@ fn test_accept_options(#[case] version: ClarityVersion, #[case] epoch: StacksEpo let bad_defun = "(define-private (f (b (optional int int))) (* 10 (default-to 0 b)))"; assert_eq!( execute(bad_defun).unwrap_err(), - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - 0, - SymbolicExpression::list(vec![ - SymbolicExpression::atom("optional".into()).with_id(8), - SymbolicExpression::atom("int".into()).with_id(9), - SymbolicExpression::atom("int".into()).with_id(10), - ]) - .with_id(7), - CheckErrors::InvalidTypeDescription.message() - )) - .into() + CheckErrors::InvalidTypeDescription.into() ); } @@ -292,10 +280,7 @@ fn test_define_parse_panic() { fn test_define_parse_panic_2() { let tests = "(define-private (a b (d)) 1)"; assert_eq_err( - CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_not_list( - 0, - SymbolicExpression::atom("b".into()).with_id(5), - )), + CheckErrors::BadSyntaxBinding(SyntaxBindingError::eval_binding_not_list(0)), execute(tests).unwrap_err(), ); } diff --git a/clarity/src/vm/types/signatures.rs b/clarity/src/vm/types/signatures.rs index e51ddc4151..bacd9f1d6d 100644 --- a/clarity/src/vm/types/signatures.rs +++ b/clarity/src/vm/types/signatures.rs @@ -26,7 +26,6 @@ use lazy_static::lazy_static; use stacks_common::types::StacksEpochId; use crate::vm::costs::{runtime_cost, CostOverflowingMath}; -use crate::vm::diagnostic::DiagnosableError; use crate::vm::errors::{CheckErrors, SyntaxBindingError, SyntaxBindingErrorType}; use crate::vm::representations::{ ClarityName, ContractName, SymbolicExpression, SymbolicExpressionType, TraitDefinition, @@ -1933,34 +1932,26 @@ pub fn parse_name_type_pairs( use crate::vm::representations::SymbolicExpressionType::List; // step 1: parse it into a vec of symbolicexpression pairs. - let as_pairs: std::result::Result, _> = name_type_pairs + let as_pairs: std::result::Result, CheckErrors> = 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, - key_type_pair.clone(), - ), + SyntaxBindingError::InvalidLength(binding_error_type, i), )) } else { Ok((&as_vec[0], &as_vec[1])) } } else { - Err(CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotList( - binding_error_type, - i, - key_type_pair.clone(), - ))) + Err(SyntaxBindingError::NotList(binding_error_type, i).into()) } }) .collect(); // step 2: turn into a vec of (name, typesignature) pairs. - let key_types: std::result::Result, _> = (as_pairs?) + let key_types: std::result::Result, CheckErrors> = (as_pairs?) .iter() .enumerate() .map(|(i, (name_symbol, type_symbol))| { @@ -1970,35 +1961,10 @@ pub fn parse_name_type_pairs( CheckErrors::BadSyntaxBinding(SyntaxBindingError::NotAtom( binding_error_type, i, - (*name_symbol).clone(), )) })? .clone(); - let type_info = TypeSignature::parse_type_repr(epoch, type_symbol, accounting) - .map_err(|e| { - CheckErrors::BadSyntaxBinding(SyntaxBindingError::BadTypeSignature( - i, - (*type_symbol).clone(), - // if the inner error is itself a BadTypeSignature error, and it's - // `message` came from a BadSyntaxBinding, then just use its - // message directly so we don't get a tower of nested BadTypeSignature - // messages. We only want one level of nesting, so something like - // `(string-ascii -19)` gets reported instead of `-19` (so the caller gets - // some context, but not an unreasonably large amount) - if let CheckErrors::BadSyntaxBinding( - SyntaxBindingError::BadTypeSignature(_, _, message), - ) = &e - { - if CheckErrors::has_nested_bad_syntax_binding_message(message) { - message.clone() - } else { - e.message() - } - } else { - e.message() - }, - )) - })?; + let type_info = TypeSignature::parse_type_repr(epoch, type_symbol, accounting)?; Ok((name, type_info)) }) .collect(); From 61daffea10a2d31e15e8f8f51176843a520ec63b Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Fri, 15 Aug 2025 22:38:48 -0400 Subject: [PATCH 21/21] chore: clippy --- clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs b/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs index 23ed30f723..65fe442461 100644 --- a/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs +++ b/clarity/src/vm/analysis/type_checker/v2_1/tests/mod.rs @@ -3659,7 +3659,7 @@ fn test_principal_admits() { /// Only concerns itself with simple errors (no nesting). #[test] fn test_simple_bad_syntax_bindings() { - let bad = vec![ + let bad = [ // bad let-binding -- binding item is not a list "(let (oops (bar u1)) (ok true))", // bad let-binding -- binding item is not a 2-element list @@ -3683,7 +3683,7 @@ fn test_simple_bad_syntax_bindings() { // bad type signature (no longer a bad syntax binding error) "(from-consensus-buff? (tuple (a (string-ascii -12))) 0x00)", ]; - let expected = vec![ + let expected = [ CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_list(0)), CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_invalid_length(0)), CheckErrors::BadSyntaxBinding(SyntaxBindingError::let_binding_not_atom(0)), @@ -3708,7 +3708,7 @@ fn test_simple_bad_syntax_bindings() { /// type-check error. #[test] fn test_nested_bad_type_signature_syntax_bindings() { - let bad = vec![ + let bad = [ // bad tuple type signature within a tower of tuples "(define-public (foo (bar { a: { b: { c: (string-ascii -19) } } })) (ok true))", // bad type signature within a tower of lists @@ -3717,7 +3717,7 @@ fn test_nested_bad_type_signature_syntax_bindings() { "(from-consensus-buff? { a : { b: { c: (string-ascii -19) } } } 0x00)", ]; - let expected = vec![ + let expected = [ CheckErrors::ValueOutOfBounds, CheckErrors::InvalidTypeDescription, CheckErrors::ValueOutOfBounds,