Repository: ergoplatform/sigma-rust (language: rust)

Issue:
Switch AST casting and parsing API from Option to Result 
To bubble up error earlier in AST layer and simplify AST -> HIR lowering. 


Discussion / hints:

Fix patch:
diff --git a/ergoscript-compiler/src/ast.rs b/ergoscript-compiler/src/ast.rs
index 923b2e707..8396aab4d 100644
--- a/ergoscript-compiler/src/ast.rs
+++ b/ergoscript-compiler/src/ast.rs
@@ -1,40 +1,25 @@
+use crate::error::pretty_error_desc;
+
 use super::syntax::{SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken};
 use text_size::TextRange;
 
-// #[derive(Debug)]
-// pub struct Root(SyntaxNode);
-
-// impl Root {
-//     pub fn cast(node: SyntaxNode) -> Option<Self> {
-//         if node.kind() == SyntaxKind::Root {
-//             Some(Self(node))
-//         } else {
-//             None
-//         }
-//     }
-
-//     pub fn expr(&self) -> impl Iterator<Item = Stmt> {
-//         self.0.children().filter_map(Stmt::cast)
-//     }
-// }
-
-// #[derive(Debug)]
-// pub enum Stmt {
-//     VariableDef(VariableDef),
-//     Expr(Expr),
-// }
+#[derive(Debug, PartialEq)]
+pub struct AstError {
+    pub msg: String,
+    pub span: TextRange,
+}
 
-// impl Stmt {
-//     pub fn cast(node: SyntaxNode) -> Option<Self> {
-//         let result = match node.kind() {
-//             SyntaxKind::VariableDef => Self::VariableDef(VariableDef(node)),
-//             _ => Self::Expr(Expr::cast(node)?),
-//         };
+impl AstError {
+    pub fn new(msg: String, span: TextRange) -> Self {
+        AstError { msg, span }
+    }
 
-//         Some(result)
-//     }
-// }
+    pub fn pretty_desc(&self, source: &str) -> String {
+        pretty_error_desc(&source, self.span, &self.msg)
+    }
+}
 
+#[derive(Debug)]
 pub struct Root(SyntaxNode);
 
 impl Root {
@@ -49,26 +34,27 @@ impl Root {
     pub fn children(&self) -> impl Iterator<Item = Expr> {
         self.0.children().filter_map(Expr::cast)
     }
+
+    pub fn span(&self) -> TextRange {
+        self.0.text_range()
+    }
 }
 
 #[derive(Debug)]
 pub struct Ident(SyntaxNode);
 
 impl Ident {
-    pub fn name(&self) -> Option<SyntaxToken> {
+    pub fn name(&self) -> Result<SyntaxToken, AstError> {
         self.0
             .children_with_tokens()
             .filter_map(SyntaxElement::into_token)
             .find(|token| token.kind() == SyntaxKind::Ident)
+            .ok_or_else(|| AstError::new(format!("Empty Ident.name in: {:?}", self.0), self.span()))
     }
 
     pub fn span(&self) -> TextRange {
         self.0.text_range()
     }
-
-    // pub fn value(&self) -> Option<Expr> {
-    //     self.0.children().find_map(Expr::cast)
-    // }
 }
 
 #[derive(Debug)]
@@ -107,15 +93,29 @@ impl Expr {
 pub struct BinaryExpr(SyntaxNode);
 
 impl BinaryExpr {
-    pub fn lhs(&self) -> Option<Expr> {
-        self.0.children().find_map(Expr::cast)
+    pub fn lhs(&self) -> Result<Expr, AstError> {
+        self.0.children().find_map(Expr::cast).ok_or_else(|| {
+            AstError::new(
+                format!("Cannot find lhs in {:?}", self.0.children()),
+                self.0.text_range(),
+            )
+        })
     }
 
-    pub fn rhs(&self) -> Option<Expr> {
-        self.0.children().filter_map(Expr::cast).nth(1)
+    pub fn rhs(&self) -> Result<Expr, AstError> {
+        self.0
+            .children()
+            .filter_map(Expr::cast)
+            .nth(1)
+            .ok_or_else(|| {
+                AstError::new(
+                    format!("Cannot find rhs in {:?}", self.0.children()),
+                    self.0.text_range(),
+                )
+            })
     }
 
-    pub fn op(&self) -> Option<SyntaxToken> {
+    pub fn op(&self) -> Result<SyntaxToken, AstError> {
         self.0
             .children_with_tokens()
             .filter_map(SyntaxElement::into_token)
@@ -129,6 +129,12 @@ impl BinaryExpr {
                         | SyntaxKind::And,
                 )
             })
+            .ok_or_else(|| {
+                AstError::new(
+                    format!("Cannot find bin op in {:?}", self.0),
+                    self.0.text_range(),
+                )
+            })
     }
 
     pub fn span(&self) -> TextRange {
@@ -146,15 +152,7 @@ pub enum LiteralValue {
 pub struct Literal(SyntaxNode);
 
 impl Literal {
-    // pub fn cast(node: SyntaxNode) -> Option<Self> {
-    //     if node.kind() == SyntaxKind::Literal {
-    //         Some(Self(node))
-    //     } else {
-    //         None
-    //     }
-    // }
-
-    pub fn parse(&self) -> Option<LiteralValue> {
+    pub fn parse(&self) -> Result<LiteralValue, AstError> {
         let text = self.0.first_token().unwrap().text().to_string();
         if text.ends_with('L') {
             text.strip_suffix("L")
@@ -165,6 +163,12 @@ impl Literal {
         } else {
             text.parse().ok().map(LiteralValue::Int)
         }
+        .ok_or_else(|| {
+            AstError::new(
+                format!("Failed to parse Literal from: {:?}", self.0),
+                self.span(),
+            )
+        })
     }
 
     pub fn span(&self) -> TextRange {
diff --git a/ergoscript-compiler/src/hir.rs b/ergoscript-compiler/src/hir.rs
index cc2f82ff1..5577ee82a 100644
--- a/ergoscript-compiler/src/hir.rs
+++ b/ergoscript-compiler/src/hir.rs
@@ -7,6 +7,7 @@ use ergotree_ir::types::stype::SType;
 pub use rewrite::rewrite;
 
 use super::ast;
+use crate::ast::AstError;
 use crate::error::pretty_error_desc;
 use crate::syntax::SyntaxKind;
 use text_size::TextRange;
@@ -15,8 +16,16 @@ extern crate derive_more;
 use derive_more::From;
 
 pub fn lower(ast: ast::Root) -> Result<Expr, HirLoweringError> {
-    // TODO: return error if more than one expr is found
-    let first_expr = ast.children().next().unwrap();
+    let exprs: Vec<ast::Expr> = ast.children().collect();
+    if exprs.len() > 1 {
+        return Err(HirLoweringError::new(
+            format!("More than one root expr found: {:?}", exprs),
+            ast.span(),
+        ));
+    }
+    let first_expr = exprs
+        .first()
+        .ok_or_else(|| AstError::new(format!("Cannot parse empty root: {:?}", ast), ast.span()))?;
     Expr::lower(&first_expr)
 }
 
@@ -43,6 +52,12 @@ impl HirLoweringError {
     }
 }
 
+impl From<AstError> for HirLoweringError {
+    fn from(ast: AstError) -> Self {
+        HirLoweringError::new(format!("AST error: {0}", ast.msg), ast.span)
+    }
+}
+
 impl Expr {
     pub fn lower(expr: &ast::Expr) -> Result<Expr, HirLoweringError> {
         match expr {
@@ -51,23 +66,16 @@ impl Expr {
                 span: ast.span(),
                 tpe: None,
             }),
-            ast::Expr::Ident(ast) => ast
-                .name()
-                .map(|node| Expr {
-                    kind: ExprKind::Ident(node.text().to_string()),
+            ast::Expr::Ident(ast) => {
+                let name = ast.name()?;
+                Ok(Expr {
+                    kind: ExprKind::Ident(name.text().to_string()),
                     span: ast.span(),
                     tpe: None,
                 })
-                .ok_or_else(|| {
-                    HirLoweringError::new(format!("Empty Ident.name: {:?}", ast), ast.span())
-                }),
+            }
             ast::Expr::Literal(ast) => {
-                let v = ast.parse().ok_or_else(|| {
-                    HirLoweringError::new(
-                        format!("Failed to parse Literal from: {:?}", ast),
-                        ast.span(),
-                    )
-                })?;
+                let v = ast.parse()?;
                 let expr = match v {
                     ast::LiteralValue::Int(v) => Expr {
                         kind: Literal::Int(v).into(),
@@ -106,8 +114,8 @@ pub struct Binary {
 
 impl Binary {
     fn lower(ast: &ast::BinaryExpr) -> Result<Binary, HirLoweringError> {
-        // TODO: unwraps -> errors
-        let op = match ast.op().unwrap().kind() {
+        let syntax_token = ast.op()?;
+        let op = match syntax_token.kind() {
             SyntaxKind::Plus => BinaryOp::Plus,
             SyntaxKind::Minus => BinaryOp::Minus,
             SyntaxKind::Star => BinaryOp::Multiply,
@@ -115,18 +123,18 @@ impl Binary {
             _ => {
                 return Err(HirLoweringError::new(
                     format!("unknown binary operator: {:?}", ast.op()),
-                    ast.op().unwrap().text_range(),
+                    syntax_token.text_range(),
                 ))
             }
         };
 
-        let lhs = Expr::lower(&ast.lhs().unwrap());
-        let rhs = Expr::lower(&ast.rhs().unwrap());
+        let lhs = Expr::lower(&ast.lhs()?);
+        let rhs = Expr::lower(&ast.rhs()?);
 
         Ok(Binary {
             op: Spanned {
                 node: op,
-                span: ast.op().unwrap().text_range(),
+                span: syntax_token.text_range(),
             },
             lhs: Box::new(lhs?),
             rhs: Box::new(rhs?),
diff --git a/ergoscript-compiler/src/lib.rs b/ergoscript-compiler/src/lib.rs
index 83b69e900..f09d1ac08 100644
--- a/ergoscript-compiler/src/lib.rs
+++ b/ergoscript-compiler/src/lib.rs
@@ -6,8 +6,7 @@
 #![deny(non_camel_case_types)]
 #![deny(non_snake_case)]
 #![deny(unused_mut)]
-// TODO: restore
-// #![deny(dead_code)]
+#![deny(dead_code)]
 #![deny(unused_imports)]
 #![deny(missing_docs)]
 // Clippy exclusions


Test patch: