Skip to content

Commit ed22100

Browse files
authored
Merge pull request #6343 from jcnelson/fix/bad-syntax-binding-error-variants
Fix/bad syntax binding error variants
2 parents 94a5a60 + e047935 commit ed22100

File tree

19 files changed

+508
-166
lines changed

19 files changed

+508
-166
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,14 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
1111

1212
- When determining a global transaction replay set, the state evaluator now uses a longest-common-prefix algorithm to find a replay set in the case where a single replay set has less than 70% of signer weight.
1313

14+
### Changed
15+
16+
- Clarity errors pertaining to syntax binding errors have been made more
17+
expressive (#6337)
18+
19+
1420
## [3.2.0.0.1]
21+
1522
### Added
1623

1724
- Adds node-config-docsgen to automatically create config documentation (#6227)

clarity/src/vm/analysis/errors.rs

Lines changed: 151 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,127 @@ use crate::vm::types::{TraitIdentifier, TupleTypeSignature, TypeSignature, Value
2323

2424
pub type CheckResult<T> = Result<T, CheckError>;
2525

26+
/// What kind of syntax binding was found to be in error?
27+
#[derive(Debug, PartialEq, Clone, Copy)]
28+
pub enum SyntaxBindingErrorType {
29+
Let,
30+
Eval,
31+
TupleCons,
32+
}
33+
34+
impl fmt::Display for SyntaxBindingErrorType {
35+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
36+
write!(f, "{}", &self.message())
37+
}
38+
}
39+
40+
impl DiagnosableError for SyntaxBindingErrorType {
41+
fn message(&self) -> String {
42+
match &self {
43+
Self::Let => "Let-binding".to_string(),
44+
Self::Eval => "Function argument definition".to_string(),
45+
Self::TupleCons => "Tuple constructor".to_string(),
46+
}
47+
}
48+
49+
fn suggestion(&self) -> Option<String> {
50+
None
51+
}
52+
}
53+
54+
/// Syntax binding error types
55+
#[derive(Debug, PartialEq)]
56+
pub enum SyntaxBindingError {
57+
/// binding list item is not a list
58+
NotList(SyntaxBindingErrorType, usize),
59+
/// binding list item has an invalid length (e.g. not 2)
60+
InvalidLength(SyntaxBindingErrorType, usize),
61+
/// binding name is not an atom
62+
NotAtom(SyntaxBindingErrorType, usize),
63+
}
64+
65+
impl fmt::Display for SyntaxBindingError {
66+
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
67+
write!(f, "{self:?}")
68+
}
69+
}
70+
71+
impl DiagnosableError for SyntaxBindingError {
72+
fn message(&self) -> String {
73+
match &self {
74+
Self::NotList(err_type, item_index) => {
75+
let item_no = item_index + 1;
76+
format!("{err_type} item #{item_no} is not a list",)
77+
}
78+
Self::InvalidLength(err_type, item_index) => {
79+
let item_no = item_index + 1;
80+
format!("{err_type} item #{item_no} is not a two-element list",)
81+
}
82+
Self::NotAtom(err_type, item_index) => {
83+
let item_no = item_index + 1;
84+
format!("{err_type} item #{item_no}'s name is not an atom",)
85+
}
86+
}
87+
}
88+
89+
fn suggestion(&self) -> Option<String> {
90+
None
91+
}
92+
}
93+
94+
impl SyntaxBindingError {
95+
/// Helper constructor for NotList(SyntaxBindingErrorType::Let, item_no)
96+
pub fn let_binding_not_list(item_no: usize) -> Self {
97+
Self::NotList(SyntaxBindingErrorType::Let, item_no)
98+
}
99+
100+
/// Helper constructor for InvalidLength(SyntaxBindingErrorType::Let, item_no)
101+
pub fn let_binding_invalid_length(item_no: usize) -> Self {
102+
Self::InvalidLength(SyntaxBindingErrorType::Let, item_no)
103+
}
104+
105+
/// Helper constructor for NotAtom(SyntaxBindingErrorType::Let, item_no)
106+
pub fn let_binding_not_atom(item_no: usize) -> Self {
107+
Self::NotAtom(SyntaxBindingErrorType::Let, item_no)
108+
}
109+
110+
/// Helper constructor for NotList(SyntaxBindingErrorType::Eval, item_no)
111+
pub fn eval_binding_not_list(item_no: usize) -> Self {
112+
Self::NotList(SyntaxBindingErrorType::Eval, item_no)
113+
}
114+
115+
/// Helper constructor for InvalidLength(SyntaxBindingErrorType::Eval, item_no)
116+
pub fn eval_binding_invalid_length(item_no: usize) -> Self {
117+
Self::InvalidLength(SyntaxBindingErrorType::Eval, item_no)
118+
}
119+
120+
/// Helper constructor for NotAtom(SyntaxBindingErrorType::Eval, item_no)
121+
pub fn eval_binding_not_atom(item_no: usize) -> Self {
122+
Self::NotAtom(SyntaxBindingErrorType::Eval, item_no)
123+
}
124+
125+
/// Helper constructor for NotList(SyntaxBindingErrorType::TupleCons, item_no)
126+
pub fn tuple_cons_not_list(item_no: usize) -> Self {
127+
Self::NotList(SyntaxBindingErrorType::TupleCons, item_no)
128+
}
129+
130+
/// Helper constructor for InvalidLength(SyntaxBindingErrorType::TupleCons, item_no)
131+
pub fn tuple_cons_invalid_length(item_no: usize) -> Self {
132+
Self::InvalidLength(SyntaxBindingErrorType::TupleCons, item_no)
133+
}
134+
135+
/// Helper constructor for NotAtom(SyntaxBindingErrorType::TupleCons, item_no)
136+
pub fn tuple_cons_not_atom(item_no: usize) -> Self {
137+
Self::NotAtom(SyntaxBindingErrorType::TupleCons, item_no)
138+
}
139+
}
140+
141+
impl From<SyntaxBindingError> for CheckErrors {
142+
fn from(e: SyntaxBindingError) -> Self {
143+
Self::BadSyntaxBinding(e)
144+
}
145+
}
146+
26147
#[derive(Debug, PartialEq)]
27148
pub enum CheckErrors {
28149
// cost checker errors
@@ -101,8 +222,7 @@ pub enum CheckErrors {
101222
ExpectedTuple(TypeSignature),
102223
NoSuchTupleField(String, TupleTypeSignature),
103224
EmptyTuplesNotAllowed,
104-
BadTupleConstruction,
105-
TupleExpectsPairs,
225+
BadTupleConstruction(String),
106226

107227
// variables
108228
NoSuchDataVariable(String),
@@ -152,8 +272,7 @@ pub enum CheckErrors {
152272
BadLetSyntax,
153273

154274
// generic binding syntax
155-
BadSyntaxBinding,
156-
BadSyntaxExpectedListOfPairs,
275+
BadSyntaxBinding(SyntaxBindingError),
157276

158277
MaxContextDepthReached,
159278
UndefinedFunction(String),
@@ -240,6 +359,32 @@ impl CheckError {
240359
self.diagnostic.spans = exprs.iter().map(|e| e.span().clone()).collect();
241360
self.expressions.replace(exprs.to_vec());
242361
}
362+
363+
pub fn with_expression(err: CheckErrors, expr: &SymbolicExpression) -> Self {
364+
let mut r = Self::new(err);
365+
r.set_expression(expr);
366+
r
367+
}
368+
}
369+
370+
impl From<(SyntaxBindingError, &SymbolicExpression)> for CheckError {
371+
fn from(e: (SyntaxBindingError, &SymbolicExpression)) -> Self {
372+
Self::with_expression(CheckErrors::BadSyntaxBinding(e.0), e.1)
373+
}
374+
}
375+
376+
impl From<(CheckErrors, &SymbolicExpression)> for CheckError {
377+
fn from(e: (CheckErrors, &SymbolicExpression)) -> Self {
378+
let mut ce = Self::new(e.0);
379+
ce.set_expression(e.1);
380+
ce
381+
}
382+
}
383+
384+
impl From<(CheckErrors, &SymbolicExpression)> for CheckErrors {
385+
fn from(e: (CheckErrors, &SymbolicExpression)) -> Self {
386+
e.0
387+
}
243388
}
244389

245390
impl fmt::Display for CheckErrors {
@@ -362,7 +507,6 @@ impl DiagnosableError for CheckErrors {
362507
CheckErrors::MemoryBalanceExceeded(a, b) => format!("contract execution cost exceeded memory budget: {a:?} > {b:?}"),
363508
CheckErrors::InvalidTypeDescription => "supplied type description is invalid".into(),
364509
CheckErrors::EmptyTuplesNotAllowed => "tuple types may not be empty".into(),
365-
CheckErrors::BadSyntaxExpectedListOfPairs => "bad syntax: function expects a list of pairs to bind names, e.g., ((name-0 a) (name-1 b) ...)".into(),
366510
CheckErrors::UnknownTypeName(name) => format!("failed to parse type: '{name}'"),
367511
CheckErrors::ValueTooLarge => "created a type which was greater than maximum allowed value size".into(),
368512
CheckErrors::ValueOutOfBounds => "created a type which value size was out of defined bounds".into(),
@@ -390,8 +534,7 @@ impl DiagnosableError for CheckErrors {
390534
CheckErrors::BadTupleFieldName => "invalid tuple field name".into(),
391535
CheckErrors::ExpectedTuple(type_signature) => format!("expecting tuple, found '{type_signature}'"),
392536
CheckErrors::NoSuchTupleField(field_name, tuple_signature) => format!("cannot find field '{field_name}' in tuple '{tuple_signature}'"),
393-
CheckErrors::BadTupleConstruction => "invalid tuple syntax, expecting list of pair".into(),
394-
CheckErrors::TupleExpectsPairs => "invalid tuple syntax, expecting pair".into(),
537+
CheckErrors::BadTupleConstruction(message) => format!("invalid tuple syntax: {message}"),
395538
CheckErrors::NoSuchDataVariable(var_name) => format!("use of unresolved persisted variable '{var_name}'"),
396539
CheckErrors::BadTransferSTXArguments => "STX transfer expects an int amount, from principal, to principal".into(),
397540
CheckErrors::BadTransferFTArguments => "transfer expects an int amount, from principal, to principal".into(),
@@ -428,7 +571,7 @@ impl DiagnosableError for CheckErrors {
428571
CheckErrors::MaxLengthOverflow => format!("expecting a value <= {}", u32::MAX),
429572
CheckErrors::BadLetSyntax => "invalid syntax of 'let'".into(),
430573
CheckErrors::CircularReference(references) => format!("detected circular reference: ({})", references.join(", ")),
431-
CheckErrors::BadSyntaxBinding => "invalid syntax binding".into(),
574+
CheckErrors::BadSyntaxBinding(binding_error) => format!("invalid syntax binding: {}", &binding_error.message()),
432575
CheckErrors::MaxContextDepthReached => "reached depth limit".into(),
433576
CheckErrors::UndefinedVariable(var_name) => format!("use of unresolved variable '{var_name}'"),
434577
CheckErrors::UndefinedFunction(var_name) => format!("use of unresolved function '{var_name}'"),
@@ -476,9 +619,6 @@ impl DiagnosableError for CheckErrors {
476619

477620
fn suggestion(&self) -> Option<String> {
478621
match &self {
479-
CheckErrors::BadSyntaxBinding => {
480-
Some("binding syntax example: ((supply int) (ttl int))".into())
481-
}
482622
CheckErrors::BadLetSyntax => Some(
483623
"'let' syntax example: (let ((supply 1000) (ttl 60)) <next-expression>)".into(),
484624
),

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use stacks_common::types::StacksEpochId;
1919

2020
pub use super::errors::{
2121
check_argument_count, check_arguments_at_least, CheckError, CheckErrors, CheckResult,
22+
SyntaxBindingError,
2223
};
2324
use super::AnalysisDatabase;
2425
use crate::vm::analysis::types::{AnalysisPass, ContractAnalysis};
@@ -324,10 +325,18 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> {
324325

325326
let binding_list = args[0].match_list().ok_or(CheckErrors::BadLetSyntax)?;
326327

327-
for pair in binding_list.iter() {
328-
let pair_expression = pair.match_list().ok_or(CheckErrors::BadSyntaxBinding)?;
328+
for (i, pair) in binding_list.iter().enumerate() {
329+
let pair_expression = pair.match_list().ok_or_else(|| {
330+
CheckError::with_expression(
331+
SyntaxBindingError::let_binding_not_list(i).into(),
332+
pair,
333+
)
334+
})?;
329335
if pair_expression.len() != 2 {
330-
return Err(CheckErrors::BadSyntaxBinding.into());
336+
return Err(CheckError::with_expression(
337+
SyntaxBindingError::let_binding_invalid_length(i).into(),
338+
pair,
339+
));
331340
}
332341

333342
if !self.check_read_only(&pair_expression[1])? {
@@ -364,11 +373,18 @@ impl<'a, 'b> ReadOnlyChecker<'a, 'b> {
364373
self.check_expression_application_is_read_only(args)
365374
}
366375
TupleCons => {
367-
for pair in args.iter() {
368-
let pair_expression =
369-
pair.match_list().ok_or(CheckErrors::TupleExpectsPairs)?;
376+
for (i, pair) in args.iter().enumerate() {
377+
let pair_expression = pair.match_list().ok_or_else(|| {
378+
CheckError::with_expression(
379+
SyntaxBindingError::tuple_cons_not_list(i).into(),
380+
pair,
381+
)
382+
})?;
370383
if pair_expression.len() != 2 {
371-
return Err(CheckErrors::TupleExpectsPairs.into());
384+
return Err(CheckError::with_expression(
385+
SyntaxBindingError::tuple_cons_invalid_length(i).into(),
386+
pair,
387+
));
372388
}
373389

374390
if !self.check_read_only(&pair_expression[1])? {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,16 @@ fn test_no_such_tuple_field() {
123123
fn test_bad_tuple_construction() {
124124
let snippet = "(tuple (key 1) (key 2))";
125125
let err = mem_type_check(snippet).unwrap_err();
126-
assert!(format!("{}", err.diagnostic).contains("invalid tuple syntax, expecting list of pair"));
126+
assert!(format!("{}", err.diagnostic)
127+
.contains("invalid tuple syntax: defining 'key' conflicts with previous value."));
127128
}
128129

129130
#[test]
130131
fn test_tuple_expects_pairs() {
131132
let snippet = "(tuple (key 1) (key-with-missing-value))";
132133
let err = mem_type_check(snippet).unwrap_err();
133-
assert!(format!("{}", err.diagnostic).contains("invalid tuple syntax, expecting pair"));
134+
assert!(format!("{}", err.diagnostic)
135+
.contains("invalid syntax binding: Tuple constructor item #2 is not a two-element list."));
134136
}
135137

136138
#[test]

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use super::contexts::{TypeMap, TypingContext};
2828
use super::ContractAnalysis;
2929
pub use crate::vm::analysis::errors::{
3030
check_argument_count, check_arguments_at_least, CheckError, CheckErrors, CheckResult,
31+
SyntaxBindingErrorType,
3132
};
3233
use crate::vm::analysis::AnalysisDatabase;
3334
use crate::vm::costs::cost_functions::ClarityCostFunction;
@@ -555,8 +556,12 @@ impl<'a, 'b> TypeChecker<'a, 'b> {
555556
let function_name = function_name
556557
.match_atom()
557558
.ok_or(CheckErrors::BadFunctionName)?;
558-
let args = parse_name_type_pairs::<()>(StacksEpochId::Epoch2_05, args, &mut ())
559-
.map_err(|_| CheckErrors::BadSyntaxBinding)?;
559+
let args = parse_name_type_pairs::<(), CheckError>(
560+
StacksEpochId::Epoch2_05,
561+
args,
562+
SyntaxBindingErrorType::Eval,
563+
&mut (),
564+
)?;
560565

561566
if self.function_return_tracker.is_some() {
562567
return Err(CheckErrors::Expects(

0 commit comments

Comments
 (0)