From 1750132ed81ac4720c698283fc23eab973505197 Mon Sep 17 00:00:00 2001 From: Jeff Date: Mon, 18 Mar 2024 03:24:41 -0400 Subject: [PATCH] Begin improving errors --- src/abstract_tree/assignment.rs | 38 +++--- src/abstract_tree/function_call.rs | 18 ++- src/abstract_tree/identifier.rs | 15 ++- src/abstract_tree/list_index.rs | 14 ++- src/abstract_tree/map_index.rs | 9 +- src/abstract_tree/mod.rs | 10 -- src/error.rs | 195 ++++++++++++++++++----------- src/main.rs | 71 +++++++++-- src/parser.rs | 6 +- 9 files changed, 253 insertions(+), 123 deletions(-) diff --git a/src/abstract_tree/assignment.rs b/src/abstract_tree/assignment.rs index e966783..aac360b 100644 --- a/src/abstract_tree/assignment.rs +++ b/src/abstract_tree/assignment.rs @@ -7,7 +7,7 @@ use super::{AbstractTree, Action, Identifier, Statement, Type, WithPosition}; #[derive(Clone, Debug, Eq, PartialEq, PartialOrd, Ord)] pub struct Assignment { - identifier: Identifier, + identifier: WithPosition, r#type: Option>, operator: AssignmentOperator, statement: Box>, @@ -22,7 +22,7 @@ pub enum AssignmentOperator { impl Assignment { pub fn new( - identifier: Identifier, + identifier: WithPosition, r#type: Option>, operator: AssignmentOperator, statement: WithPosition, @@ -57,12 +57,12 @@ impl AbstractTree for Assignment { } })?; - context.set_type(self.identifier.clone(), expected_type.clone())?; + context.set_type(self.identifier.node.clone(), expected_type.clone())?; } else { - context.set_type(self.identifier.clone(), statement_type)?; + context.set_type(self.identifier.node.clone(), statement_type)?; } - self.identifier.validate(context)?; + self.identifier.node.validate(context)?; self.statement.node.validate(context)?; Ok(()) @@ -77,27 +77,33 @@ impl AbstractTree for Assignment { match self.operator { AssignmentOperator::Assign => { - context.set_value(self.identifier, value)?; + context.set_value(self.identifier.node, value)?; } AssignmentOperator::AddAssign => { - if let Some(previous_value) = context.get_value(&self.identifier)? { + if let Some(previous_value) = context.get_value(&self.identifier.node)? { let new_value = previous_value.add(&value)?; - context.set_value(self.identifier, new_value)?; + context.set_value(self.identifier.node, new_value)?; } else { return Err(RuntimeError::ValidationFailure( - ValidationError::VariableNotFound(self.identifier), + ValidationError::VariableNotFound { + identifier: self.identifier.node, + position: self.identifier.position, + }, )); } } AssignmentOperator::SubAssign => { - if let Some(previous_value) = context.get_value(&self.identifier)? { + if let Some(previous_value) = context.get_value(&self.identifier.node)? { let new_value = previous_value.subtract(&value)?; - context.set_value(self.identifier, new_value)?; + context.set_value(self.identifier.node, new_value)?; } else { return Err(RuntimeError::ValidationFailure( - ValidationError::VariableNotFound(self.identifier), + ValidationError::VariableNotFound { + identifier: self.identifier.node, + position: self.identifier.position, + }, )); } } @@ -122,7 +128,7 @@ mod tests { let context = Context::new(); Assignment::new( - Identifier::new("foobar"), + Identifier::new("foobar").with_position((0, 0)), None, AssignmentOperator::Assign, Statement::Expression(Expression::Value(ValueNode::Integer(42))).with_position((0, 0)), @@ -145,7 +151,7 @@ mod tests { .unwrap(); Assignment::new( - Identifier::new("foobar"), + Identifier::new("foobar").with_position((0, 0)), None, AssignmentOperator::AddAssign, Statement::Expression(Expression::Value(ValueNode::Integer(41))).with_position((0, 0)), @@ -168,7 +174,7 @@ mod tests { .unwrap(); Assignment::new( - Identifier::new("foobar"), + Identifier::new("foobar").with_position((0, 0)), None, AssignmentOperator::SubAssign, Statement::Expression(Expression::Value(ValueNode::Integer(1))).with_position((0, 0)), @@ -185,7 +191,7 @@ mod tests { #[test] fn type_check() { let validation = Assignment::new( - Identifier::new("foobar"), + Identifier::new("foobar").with_position((0, 0)), Some(WithPosition { node: Type::Boolean, position: (0, 0).into(), diff --git a/src/abstract_tree/function_call.rs b/src/abstract_tree/function_call.rs index c1be10b..fb22762 100644 --- a/src/abstract_tree/function_call.rs +++ b/src/abstract_tree/function_call.rs @@ -52,7 +52,14 @@ impl AbstractTree for FunctionCall { } fn run(self, context: &Context) -> Result { - let value = self.function.node.run(context)?.as_return_value()?; + let action = self.function.node.run(context)?; + let value = if let Action::Return(value) = action { + value + } else { + return Err(RuntimeError::ValidationFailure( + ValidationError::InterpreterExpectedReturn(self.function.position), + )); + }; let function = if let ValueInner::Function(function) = value.inner().as_ref() { function } else { @@ -66,7 +73,14 @@ impl AbstractTree for FunctionCall { let mut arguments = Vec::with_capacity(self.arguments.len()); for expression in self.arguments { - let value = expression.node.run(context)?.as_return_value()?; + let action = expression.node.run(context)?; + let value = if let Action::Return(value) = action { + value + } else { + return Err(RuntimeError::ValidationFailure( + ValidationError::InterpreterExpectedReturn(expression.position), + )); + }; arguments.push(value); } diff --git a/src/abstract_tree/identifier.rs b/src/abstract_tree/identifier.rs index aaaa9be..78aa1d8 100644 --- a/src/abstract_tree/identifier.rs +++ b/src/abstract_tree/identifier.rs @@ -28,7 +28,10 @@ impl AbstractTree for Identifier { if let Some(r#type) = context.get_type(self)? { Ok(r#type) } else { - Err(ValidationError::VariableNotFound(self.clone())) + Err(ValidationError::VariableNotFound { + identifier: todo!(), + position: todo!(), + }) } } @@ -36,7 +39,10 @@ impl AbstractTree for Identifier { if context.contains(self)? { Ok(()) } else { - Err(ValidationError::VariableNotFound(self.clone())) + Err(ValidationError::VariableNotFound { + identifier: todo!(), + position: todo!(), + }) } } @@ -47,7 +53,10 @@ impl AbstractTree for Identifier { Ok(action) } else { Err(RuntimeError::ValidationFailure( - ValidationError::VariableNotFound(self.clone()), + ValidationError::VariableNotFound { + identifier: todo!(), + position: todo!(), + }, )) } } diff --git a/src/abstract_tree/list_index.rs b/src/abstract_tree/list_index.rs index 19f21dc..001129e 100644 --- a/src/abstract_tree/list_index.rs +++ b/src/abstract_tree/list_index.rs @@ -54,9 +54,10 @@ impl AbstractTree for ListIndex { Ok(()) } else { Err(ValidationError::CannotIndexWith { - collection_type: todo!(), - index_type: todo!(), - position: todo!(), + collection_type: left_type, + collection_position: self.left.position, + index_type: right_type, + index_position: self.right.position, }) } } @@ -82,9 +83,10 @@ impl AbstractTree for ListIndex { } else { Err(RuntimeError::ValidationFailure( ValidationError::CannotIndexWith { - collection_type: todo!(), - index_type: todo!(), - position: todo!(), + collection_type: left_value.r#type(), + collection_position: self.left.position, + index_type: right_value.r#type(), + index_position: self.right.position, }, )) } diff --git a/src/abstract_tree/map_index.rs b/src/abstract_tree/map_index.rs index 0d5ce51..8de559e 100644 --- a/src/abstract_tree/map_index.rs +++ b/src/abstract_tree/map_index.rs @@ -72,8 +72,9 @@ impl AbstractTree for MapIndex { Err(ValidationError::CannotIndexWith { collection_type: left_type, + collection_position: todo!(), index_type: self.right.node.expected_type(_context)?, - position: self.right.position, + index_position: self.right.position, }) } @@ -91,8 +92,9 @@ impl AbstractTree for MapIndex { } else { Err(ValidationError::CannotIndexWith { collection_type: left_type, + collection_position: self.left.position, index_type: self.right.node.expected_type(context)?, - position: self.right.position, + index_position: self.right.position, }) } } @@ -113,8 +115,9 @@ impl AbstractTree for MapIndex { Err(RuntimeError::ValidationFailure( ValidationError::CannotIndexWith { collection_type: collection.r#type(), + collection_position: todo!(), index_type: self.right.node.expected_type(_context)?, - position: self.right.position, + index_position: self.right.position, }, )) } diff --git a/src/abstract_tree/mod.rs b/src/abstract_tree/mod.rs index 7fb6a25..ec99013 100644 --- a/src/abstract_tree/mod.rs +++ b/src/abstract_tree/mod.rs @@ -80,13 +80,3 @@ pub enum Action { Break, None, } - -impl Action { - pub fn as_return_value(self) -> Result { - if let Action::Return(value) = self { - Ok(value) - } else { - Err(ValidationError::InterpreterExpectedReturn) - } - } -} diff --git a/src/error.rs b/src/error.rs index f41512a..c0935eb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,6 +1,6 @@ use std::{io, ops::Range, sync::PoisonError}; -use ariadne::{Color, Fmt, Label, ReportBuilder}; +use ariadne::{Color, Fmt, Label, Report, ReportBuilder, ReportKind}; use chumsky::{prelude::Rich, span::Span}; use crate::{ @@ -29,61 +29,93 @@ pub enum Error { } impl Error { - pub fn build_report( - self, - mut builder: ReportBuilder<'_, Range>, - ) -> ReportBuilder<'_, Range> { - let type_color = Color::Green; - - match self { + pub fn build_report<'a>(self) -> ReportBuilder<'a, (&'a str, Range)> { + let (mut builder, validation_error) = match &self { Error::Parse { expected, span } => { - let message = match expected.as_str() { - "" => "Invalid character.".to_string(), - expected => format!("Expected {expected}."), + let message = if expected.is_empty() { + "Invalid token.".to_string() + } else { + format!("Expected {expected}.") }; - builder = builder.with_note("Parsing error."); - - builder.add_label(Label::new(span.0..span.1).with_message(message)); + ( + Report::build( + ReportKind::Custom("Parsing Error", Color::White), + "input", + span.1, + ) + .with_label( + Label::new(("input", span.0..span.1)) + .with_message(message) + .with_color(Color::Red), + ), + None, + ) } Error::Lex { expected, span } => { - let message = match expected.as_str() { - "" => "Invalid character.".to_string(), - expected => format!("Expected {expected}."), + let message = if expected.is_empty() { + "Invalid token.".to_string() + } else { + format!("Expected {expected}.") }; - builder = builder.with_note("Lexing error."); - - builder.add_label(Label::new(span.0..span.1).with_message(message)); + ( + Report::build( + ReportKind::Custom("Dust Error", Color::White), + "input", + span.1, + ) + .with_label( + Label::new(("input", span.0..span.1)) + .with_message(message) + .with_color(Color::Red), + ), + None, + ) } - Error::Runtime { error, position } => match error { - RuntimeError::RwLockPoison(_) => todo!(), - RuntimeError::ValidationFailure(validation_error) => { - builder = - Error::Validation { - error: validation_error, - position, - } - .build_report(builder.with_note( - "The interpreter failed to catch this error during validation.", - )); - } - RuntimeError::Io(_) => todo!(), - }, - Error::Validation { error, position } => match error { + Error::Runtime { error, position } => ( + Report::build( + ReportKind::Custom("Dust Error", Color::White), + "input", + position.1, + ), + if let RuntimeError::ValidationFailure(validation_error) = error { + Some(validation_error) + } else { + None + }, + ), + Error::Validation { error, position } => ( + Report::build( + ReportKind::Custom("Dust Error", Color::White), + "input", + position.1, + ), + Some(error), + ), + }; + + let type_color = Color::Green; + + if let Some(validation_error) = validation_error { + match validation_error { ValidationError::ExpectedBoolean { actual, position } => { - builder.add_label(Label::new(position.0..position.1).with_message(format!( - "Expected {} but got {}.", - "boolean".fg(type_color), - actual.fg(type_color) - ))); + builder.add_label(Label::new(("input", position.0..position.1)).with_message( + format!( + "Expected {} but got {}.", + "boolean".fg(type_color), + actual.fg(type_color) + ), + )); } - ValidationError::ExpectedIntegerOrFloat => { - builder.add_label(Label::new(position.0..position.1).with_message(format!( - "Expected {} or {}.", - "integer".fg(type_color), - "float".fg(type_color) - ))); + ValidationError::ExpectedIntegerOrFloat(position) => { + builder.add_label(Label::new(("input", position.0..position.1)).with_message( + format!( + "Expected {} or {}.", + "integer".fg(type_color), + "float".fg(type_color) + ), + )); } ValidationError::RwLockPoison(_) => todo!(), ValidationError::TypeCheck { @@ -94,39 +126,54 @@ impl Error { let TypeConflict { actual, expected } = conflict; builder.add_labels([ - Label::new(expected_postion.0..expected_postion.1).with_message(format!( - "Type {} established here.", - expected.fg(type_color) - )), - Label::new(actual_position.0..actual_position.1) + Label::new(("input", expected_postion.0..expected_postion.1)).with_message( + format!("Type {} established here.", expected.fg(type_color)), + ), + Label::new(("input", actual_position.0..actual_position.1)) .with_message(format!("Got type {} here.", actual.fg(type_color))), ]); } - ValidationError::VariableNotFound(identifier) => { + ValidationError::VariableNotFound { + identifier, + position, + } => { builder.add_label( - Label::new(position.0..position.1) + Label::new(("input", position.0..position.1)) .with_message(format!("The variable {identifier} does not exist.")) .with_priority(1), ); } ValidationError::CannotIndex { r#type, position } => builder.add_label( - Label::new(position.0..position.1) + Label::new(("input", position.0..position.1)) .with_message(format!("Cannot index into a {}.", r#type.fg(type_color))), ), ValidationError::CannotIndexWith { collection_type, + collection_position, index_type, - position, - } => builder.add_label(Label::new(position.0..position.1).with_message(format!( - "Cannot index into a {} with a {}.", - collection_type.fg(type_color), - index_type.fg(type_color) - ))), - ValidationError::InterpreterExpectedReturn => todo!(), + index_position, + } => { + builder = builder.with_message(format!( + "Cannot index into {} with {}.", + collection_type.clone().fg(type_color), + index_type.clone().fg(type_color) + )); + + builder.add_labels([ + Label::new(("input", collection_position.0..collection_position.1)) + .with_message(format!( + "This has type {}.", + collection_type.fg(type_color), + )), + Label::new(("input", index_position.0..index_position.1)) + .with_message(format!("This has type {}.", index_type.fg(type_color),)), + ]) + } + ValidationError::InterpreterExpectedReturn(_) => todo!(), ValidationError::ExpectedFunction { .. } => todo!(), - ValidationError::ExpectedValue => todo!(), - ValidationError::PropertyNotFound(_) => todo!(), - }, + ValidationError::ExpectedValue(_) => todo!(), + ValidationError::PropertyNotFound { .. } => todo!(), + } } builder @@ -197,8 +244,9 @@ pub enum ValidationError { }, CannotIndexWith { collection_type: Type, + collection_position: SourcePosition, index_type: Type, - position: SourcePosition, + index_position: SourcePosition, }, ExpectedBoolean { actual: Type, @@ -208,9 +256,9 @@ pub enum ValidationError { actual: Type, position: SourcePosition, }, - ExpectedIntegerOrFloat, - ExpectedValue, - InterpreterExpectedReturn, + ExpectedIntegerOrFloat(SourcePosition), + ExpectedValue(SourcePosition), + InterpreterExpectedReturn(SourcePosition), RwLockPoison(RwLockPoisonError), TypeCheck { /// The mismatch that caused the error. @@ -222,8 +270,15 @@ pub enum ValidationError { /// The position of the item that gave the "expected" type. expected_position: SourcePosition, }, - VariableNotFound(Identifier), - PropertyNotFound(Identifier), + VariableNotFound { + identifier: Identifier, + position: SourcePosition, + }, + + PropertyNotFound { + identifier: Identifier, + position: SourcePosition, + }, } impl From for ValidationError { diff --git a/src/main.rs b/src/main.rs index df416d5..c734fb9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,12 +1,13 @@ //! Command line interface for the dust programming language. -use ariadne::{Color, Report, ReportKind, Source}; +use ariadne::{sources, Color, Label, Report, ReportKind, Source}; +use chumsky::span::SimpleSpan; use clap::Parser; use colored::Colorize; -use std::{fs::read_to_string, io::Write}; +use std::{fs::read_to_string, io::Write, ops::Range}; -use dust_lang::{context::Context, Interpreter}; +use dust_lang::{context::Context, error::Error, Interpreter}; /// Command-line arguments to be parsed. #[derive(Parser, Debug)] @@ -53,17 +54,63 @@ fn main() { } } Err(errors) => { - let mut report_builder = - Report::build(ReportKind::Custom("Dust Error", Color::White), (), 5); - for error in errors { - report_builder = error.build_report(report_builder); - } + let mut report_builder = match &error { + Error::Parse { expected, span } => { + let message = if expected.is_empty() { + "Invalid token.".to_string() + } else { + format!("Expected {expected}.") + }; - report_builder - .finish() - .eprint(Source::from(source)) - .unwrap() + Report::build( + ReportKind::Custom("Parsing Error", Color::White), + "input", + span.1, + ) + .with_label( + Label::new(("input", span.0..span.1)) + .with_message(message) + .with_color(Color::Red), + ) + } + Error::Lex { expected, span } => { + let message = if expected.is_empty() { + "Invalid token.".to_string() + } else { + format!("Expected {expected}.") + }; + + Report::build( + ReportKind::Custom("Dust Error", Color::White), + "input", + span.1, + ) + .with_label( + Label::new(("input", span.0..span.1)) + .with_message(message) + .with_color(Color::Red), + ) + } + Error::Runtime { error, position } => Report::build( + ReportKind::Custom("Dust Error", Color::White), + "input", + position.1, + ), + Error::Validation { error, position } => Report::build( + ReportKind::Custom("Dust Error", Color::White), + "input", + position.1, + ), + }; + + report_builder = error.build_report(report_builder); + + report_builder + .finish() + .eprint(sources([("input", &source)])) + .unwrap() + } } } } diff --git a/src/parser.rs b/src/parser.rs index 0ee413a..66ef5c3 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -47,6 +47,10 @@ pub fn parser<'src>() -> DustParser<'src> { } }; + let positioned_identifier = identifier + .clone() + .map_with(|identifier, state| identifier.with_position(state.span())); + let basic_value = select! { Token::Boolean(boolean) => ValueNode::Boolean(boolean), Token::Float(float) => ValueNode::Float(float), @@ -360,7 +364,7 @@ pub fn parser<'src>() -> DustParser<'src> { let r#break = just(Token::Keyword("break")) .map_with(|_, state| Statement::Break.with_position(state.span())); - let assignment = identifier + let assignment = positioned_identifier .clone() .then(type_specification.clone().or_not()) .then(choice((