Migrate remaining errors

Closes ethereum#432

analyzer/src/errors.rs

@@ -1,101 +1,41 @@
 //! Semantic errors.
 
-use ansi_term::Color::Red;
 use fe_common::diagnostics::Diagnostic;
-use fe_parser::node::Span;
 
 /// Error to be returned from APIs that should reject duplicate definitions
 #[derive(Debug)]
 pub struct AlreadyDefined;
+
+/// Error to be returned when otherwise no meaningful information can be returned
+#[derive(Debug)]
+pub struct FatalError;
+
 /// Error indicating that a value can not move between memory and storage
 #[derive(Debug)]
 pub struct CannotMove;
 
+/// Error indicating that a value has the wrong type
 #[derive(Debug)]
-pub struct AnalyzerError {
-    pub diagnostics: Vec<Diagnostic>,
-    pub classic: Option<SemanticError>,
-}
+pub struct TypeError;
 
-/// Errors for things that may arise in a valid Fe AST.
+/// Errors that can result from indexing
 #[derive(Debug, PartialEq)]
-pub enum ErrorKind {
+pub enum IndexingError {
+    WrongIndexType,
     NotSubscriptable,
-    SignedExponentNotAllowed,
-    TypeError,
-    Fatal,
 }
 
+/// Errors that can result from a binary operation
 #[derive(Debug, PartialEq)]
-pub struct SemanticError {
-    pub kind: ErrorKind,
-    /// A sequence of nested spans containing the error's origin in the source
-    /// code.
-    pub context: Vec<Span>,
+pub enum BinaryOperationError {
+    TypesNotEqual,
+    TypesNotNumeric,
+    RightTooLarge,
+    RightIsSigned,
+    NotEqualAndUnsigned,
 }
 
-impl SemanticError {
-    pub fn fatal() -> Self {
-        SemanticError {
-            kind: ErrorKind::Fatal,
-            context: vec![],
-        }
-    }
-
-    /// Create a new error with kind `NotSubscriptable`
-    pub fn not_subscriptable() -> Self {
-        SemanticError {
-            kind: ErrorKind::NotSubscriptable,
-            context: vec![],
-        }
-    }
-
-    /// Create a new error with kind `SignedExponentNotAllowed`
-    pub fn signed_exponent_not_allowed() -> Self {
-        SemanticError {
-            kind: ErrorKind::SignedExponentNotAllowed,
-            context: vec![],
-        }
-    }
-
-    /// Create a new error with kind `TypeError`
-    pub fn type_error() -> Self {
-        SemanticError {
-            kind: ErrorKind::TypeError,
-            context: vec![],
-        }
-    }
-
-    /// Maps the error to a new error that contains the given span in its
-    /// context.
-    pub fn with_context(mut self, span: Span) -> Self {
-        self.context.push(span);
-        self
-    }
-
-    /// Formats the error using the source code.
-    ///
-    /// The string will contain the error kind, line number, and surrounding
-    /// code.
-    pub fn format_user(&self, src: &str) -> String {
-        let line = if let Some(span) = self.context.first() {
-            src[..span.start].lines().count()
-        } else {
-            0
-        };
-
-        let context = match (self.context.get(0), self.context.get(1)) {
-            (Some(inner), Some(outer)) => {
-                let first_part = src[outer.start..inner.start].to_string();
-                let middle_part = Red.paint(&src[inner.start..inner.end]).to_string();
-                let last_part = src[inner.end..outer.end].to_string();
-
-                format!("{}{}{}", first_part, middle_part, last_part)
-            }
-            (Some(span), None) => src[span.start..span.end].to_string(),
-            (_, _) => "no error context available".to_string(),
-        };
-
-        format!("{:?} on line {}\n{}", self.kind, line, context)
-    }
+#[derive(Debug)]
+pub struct AnalyzerError {
+    pub diagnostics: Vec<Diagnostic>,
 }

analyzer/src/lib.rs

@@ -12,7 +12,7 @@ pub mod namespace;
 mod operations;
 mod traversal;
 
-use crate::errors::{AnalyzerError, ErrorKind};
+use crate::errors::{AnalyzerError, FatalError};
 use context::Context;
 use fe_common::files::SourceFileId;
 use fe_parser::ast as fe;
@@ -30,18 +30,17 @@ pub fn analyze(module: &fe::Module, file_id: SourceFileId) -> Result<Context, An
             } else {
                 Err(AnalyzerError {
                     diagnostics: context.diagnostics,
-                    classic: None,
                 })
             }
         }
-        Err(err) => Err(AnalyzerError {
-            diagnostics: context.diagnostics,
-            classic: if err.kind == ErrorKind::Fatal {
-                None
-            } else {
-                Some(err)
-            },
-        }),
+        Err(FatalError) => {
+            if context.diagnostics.is_empty() {
+                panic!("Expected at least one error")
+            }
+            Err(AnalyzerError {
+                diagnostics: context.diagnostics,
+            })
+        }
     }
 }
 

analyzer/src/namespace/scopes.rs

@@ -1,4 +1,4 @@
-use crate::errors::{AlreadyDefined, SemanticError};
+use crate::errors::AlreadyDefined;
 use crate::namespace::events::EventDef;
 use crate::namespace::types::{Array, FixedSize, Tuple, Type};
 use std::cell::RefCell;
@@ -201,9 +201,8 @@ impl ContractScope {
     }
 
     /// Add a static string definition to the scope.
-    pub fn add_string(&mut self, value: &str) -> Result<(), SemanticError> {
+    pub fn add_string(&mut self, value: &str) {
         self.string_defs.insert(value.to_owned());
-        Ok(())
     }
 
     /// Add the name of another contract that has been created within this

analyzer/src/namespace/types.rs

@@ -1,4 +1,4 @@
-use crate::errors::{AlreadyDefined, SemanticError};
+use crate::errors::{AlreadyDefined, TypeError};
 use std::convert::TryFrom;
 use std::fmt;
 
@@ -380,16 +380,16 @@ impl From<Base> for FixedSize {
 }
 
 impl TryFrom<Type> for FixedSize {
-    type Error = SemanticError;
+    type Error = TypeError;
 
-    fn try_from(value: Type) -> Result<Self, SemanticError> {
+    fn try_from(value: Type) -> Result<Self, TypeError> {
         match value {
             Type::Array(array) => Ok(FixedSize::Array(array)),
             Type::Base(base) => Ok(FixedSize::Base(base)),
             Type::Tuple(tuple) => Ok(FixedSize::Tuple(tuple)),
             Type::String(string) => Ok(FixedSize::String(string)),
             Type::Struct(val) => Ok(FixedSize::Struct(val)),
-            Type::Map(_) => Err(SemanticError::type_error()),
+            Type::Map(_) => Err(TypeError),
             Type::Contract(contract) => Ok(FixedSize::Contract(contract)),
         }
     }

analyzer/src/operations.rs

@@ -1,41 +1,41 @@
-use crate::errors::SemanticError;
+use crate::errors::{BinaryOperationError, IndexingError};
 use crate::namespace::types::{Array, Base, Map, Type, U256};
 
 use fe_parser::ast as fe;
 
 /// Finds the type of an index operation and checks types.
 ///
 /// e.g. `foo[42]`
-pub fn index(value: Type, index: Type) -> Result<Type, SemanticError> {
+pub fn index(value: Type, index: Type) -> Result<Type, IndexingError> {
     match value {
         Type::Array(array) => index_array(array, index),
         Type::Map(map) => index_map(map, index),
-        Type::Base(_) => Err(SemanticError::not_subscriptable()),
-        Type::Tuple(_) => Err(SemanticError::not_subscriptable()),
-        Type::String(_) => Err(SemanticError::not_subscriptable()),
-        Type::Contract(_) => Err(SemanticError::not_subscriptable()),
-        Type::Struct(_) => Err(SemanticError::not_subscriptable()),
+        Type::Base(_) => Err(IndexingError::NotSubscriptable),
+        Type::Tuple(_) => Err(IndexingError::NotSubscriptable),
+        Type::String(_) => Err(IndexingError::NotSubscriptable),
+        Type::Contract(_) => Err(IndexingError::NotSubscriptable),
+        Type::Struct(_) => Err(IndexingError::NotSubscriptable),
     }
 }
 
-fn index_array(array: Array, index: Type) -> Result<Type, SemanticError> {
+fn index_array(array: Array, index: Type) -> Result<Type, IndexingError> {
     if index != Type::Base(U256) {
-        return Err(SemanticError::type_error());
+        return Err(IndexingError::WrongIndexType);
     }
 
     Ok(Type::Base(array.inner))
 }
 
-fn index_map(map: Map, index: Type) -> Result<Type, SemanticError> {
+fn index_map(map: Map, index: Type) -> Result<Type, IndexingError> {
     if index != Type::Base(map.key) {
-        return Err(SemanticError::type_error());
+        return Err(IndexingError::WrongIndexType);
     }
 
     Ok(*map.value)
 }
 
 /// Finds the type of a binary operation and checks types.
-pub fn bin(left: &Type, op: &fe::BinOperator, right: &Type) -> Result<Type, SemanticError> {
+pub fn bin(left: &Type, op: &fe::BinOperator, right: &Type) -> Result<Type, BinaryOperationError> {
     match op {
         fe::BinOperator::Add
         | fe::BinOperator::Sub
@@ -50,7 +50,7 @@ pub fn bin(left: &Type, op: &fe::BinOperator, right: &Type) -> Result<Type, Sema
     }
 }
 
-fn bin_arithmetic(left: &Type, right: &Type) -> Result<Type, SemanticError> {
+fn bin_arithmetic(left: &Type, right: &Type) -> Result<Type, BinaryOperationError> {
     if let (Type::Base(Base::Numeric(left)), Type::Base(Base::Numeric(right))) = (left, right) {
         if left == right {
             // For now, we require that the types be numeric and equal. In the future, we
@@ -64,14 +64,14 @@ fn bin_arithmetic(left: &Type, right: &Type) -> Result<Type, SemanticError> {
             Ok(Type::Base(Base::Numeric(left.to_owned())))
         } else {
             // The types are not equal. Again, there is no need to be this strict.
-            Err(SemanticError::type_error())
+            Err(BinaryOperationError::TypesNotEqual)
         }
     } else {
-        Err(SemanticError::type_error())
+        Err(BinaryOperationError::TypesNotNumeric)
     }
 }
 
-fn bin_pow(left: &Type, right: &Type) -> Result<Type, SemanticError> {
+fn bin_pow(left: &Type, right: &Type) -> Result<Type, BinaryOperationError> {
     if let (Type::Base(Base::Numeric(left)), Type::Base(Base::Numeric(right))) = (left, right) {
         // The exponent is not allowed to be a signed integer. To allow calculations
         // such as -2 ** 3 we allow the right hand side to be an unsigned integer
@@ -80,46 +80,46 @@ fn bin_pow(left: &Type, right: &Type) -> Result<Type, SemanticError> {
         // but not i16 ** u32). The type of the result will be the type of the left
         // side and under/overflow checks are based on that type.
         if right.is_signed() {
-            Err(SemanticError::signed_exponent_not_allowed())
+            Err(BinaryOperationError::RightIsSigned)
         } else if left.can_hold(&right) {
             Ok(Type::Base(Base::Numeric(left.to_owned())))
         } else {
-            Err(SemanticError::type_error())
+            Err(BinaryOperationError::RightTooLarge)
         }
     } else {
-        Err(SemanticError::type_error())
+        Err(BinaryOperationError::TypesNotNumeric)
     }
 }
 
-fn bin_bit_shift(left: &Type, right: &Type) -> Result<Type, SemanticError> {
+fn bin_bit_shift(left: &Type, right: &Type) -> Result<Type, BinaryOperationError> {
     if let (Type::Base(Base::Numeric(left)), Type::Base(Base::Numeric(right))) = (left, right) {
         // The right side must be unsigned.
         if !right.is_signed() {
             Ok(Type::Base(Base::Numeric(left.to_owned())))
         } else {
-            Err(SemanticError::type_error())
+            Err(BinaryOperationError::RightIsSigned)
         }
     } else {
-        Err(SemanticError::type_error())
+        Err(BinaryOperationError::TypesNotNumeric)
     }
 }
 
-fn bin_bit(left: &Type, right: &Type) -> Result<Type, SemanticError> {
+fn bin_bit(left: &Type, right: &Type) -> Result<Type, BinaryOperationError> {
     if let (Type::Base(Base::Numeric(left)), Type::Base(Base::Numeric(right))) = (left, right) {
         // We require that both numbers be unsigned and equal in size.
         if !left.is_signed() && left == right {
             Ok(Type::Base(Base::Numeric(left.to_owned())))
         } else {
-            Err(SemanticError::type_error())
+            Err(BinaryOperationError::NotEqualAndUnsigned)
         }
     } else {
-        Err(SemanticError::type_error())
+        Err(BinaryOperationError::TypesNotNumeric)
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use crate::errors::ErrorKind;
+    use crate::errors::IndexingError;
     use crate::namespace::types::{Array, Base, Map, Type, U256};
     use crate::operations;
     use rstest::rstest;
@@ -159,6 +159,6 @@ mod tests {
     )]
     fn type_error_index(value: Type, index: Type) {
         let actual = operations::index(value, index).expect_err("didn't fail");
-        assert_eq!(actual.kind, ErrorKind::TypeError)
+        assert_eq!(actual, IndexingError::WrongIndexType)
     }
 }