From ed73938af121942d113a3b5184fbe5ca8dbfa7d0 Mon Sep 17 00:00:00 2001 From: elkowar <5300871+elkowar@users.noreply.github.com> Date: Sat, 24 Jul 2021 17:56:44 +0200 Subject: [PATCH] implement ToDiagnostic for more things --- crates/eww/src/error_handling_ctx.rs | 19 ++-- crates/eww/src/main.rs | 1 + crates/eww_shared_util/src/span.rs | 8 ++ crates/simplexpr/src/error.rs | 2 +- crates/simplexpr/src/parser/lexer.rs | 1 + crates/yuck/src/config/validate.rs | 9 ++ crates/yuck/src/error.rs | 18 ++-- crates/yuck/src/format_diagnostic.rs | 143 ++++++++++++++++---------- crates/yuck/src/parser/parse_error.rs | 14 ++- crates/yuck/src/parser/parser.lalrpop | 2 +- 10 files changed, 134 insertions(+), 83 deletions(-) diff --git a/crates/eww/src/error_handling_ctx.rs b/crates/eww/src/error_handling_ctx.rs index c451b4a..86f8ad4 100644 --- a/crates/eww/src/error_handling_ctx.rs +++ b/crates/eww/src/error_handling_ctx.rs @@ -1,14 +1,9 @@ use std::sync::{Arc, Mutex}; use codespan_reporting::{diagnostic::Diagnostic, term, term::Chars}; -use eww_shared_util::DUMMY_SPAN; +use eww_shared_util::Span; use simplexpr::eval::EvalError; -use yuck::{ - config::file_provider::YuckFiles, - error::AstError, - format_diagnostic::{eval_error_to_diagnostic, ToDiagnostic}, - gen_diagnostic, -}; +use yuck::{config::file_provider::YuckFiles, error::AstError, format_diagnostic::ToDiagnostic, gen_diagnostic}; use crate::error::DiagError; @@ -26,14 +21,14 @@ pub fn anyhow_err_to_diagnostic(err: &anyhow::Error) -> Diagnostic { } else if let Some(err) = err.downcast_ref::() { err.to_diagnostic() } else if let Some(err) = err.downcast_ref::() { - eval_error_to_diagnostic(err, err.span().unwrap_or(DUMMY_SPAN)) + err.to_diagnostic() } else { gen_diagnostic!(err) } } pub fn print_error(err: anyhow::Error) { - match stringify_diagnostic(&anyhow_err_to_diagnostic(&err)) { + match stringify_diagnostic(anyhow_err_to_diagnostic(&err)) { Ok(diag) => { eprintln!("{:?}\n{}", err, diag); } @@ -48,12 +43,14 @@ pub fn format_error(err: &anyhow::Error) -> String { format!("chain: {}", err); } match err.downcast_ref::() { - Some(err) => stringify_diagnostic(&err.to_diagnostic()).unwrap_or_else(|_| format!("{:?}", err)), + Some(err) => stringify_diagnostic(err.to_diagnostic()).unwrap_or_else(|_| format!("{:?}", err)), None => format!("{:?}", err), } } -pub fn stringify_diagnostic(diagnostic: &codespan_reporting::diagnostic::Diagnostic) -> anyhow::Result { +pub fn stringify_diagnostic(mut diagnostic: codespan_reporting::diagnostic::Diagnostic) -> anyhow::Result { + diagnostic.labels.drain_filter(|label| Span(label.range.start, label.range.end, label.file_id).is_dummy()); + let mut config = term::Config::default(); let mut chars = Chars::box_drawing(); chars.single_primary_caret = '─'; diff --git a/crates/eww/src/main.rs b/crates/eww/src/main.rs index 5ccb48a..2b35cab 100644 --- a/crates/eww/src/main.rs +++ b/crates/eww/src/main.rs @@ -1,4 +1,5 @@ #![feature(trace_macros)] +#![feature(drain_filter)] #![feature(box_syntax)] #![feature(box_patterns)] #![feature(slice_concat_trait)] diff --git a/crates/eww_shared_util/src/span.rs b/crates/eww_shared_util/src/span.rs index 1e0a693..7bde1fb 100644 --- a/crates/eww_shared_util/src/span.rs +++ b/crates/eww_shared_util/src/span.rs @@ -3,6 +3,10 @@ pub struct Span(pub usize, pub usize, pub usize); pub static DUMMY_SPAN: Span = Span(usize::MAX, usize::MAX, usize::MAX); impl Span { + pub fn point(loc: usize, file_id: usize) -> Self { + Span(loc, loc, file_id) + } + /// Get the span that includes this and the other span completely. /// Will panic if the spans are from different file_ids. pub fn to(mut self, other: Span) -> Self { @@ -32,6 +36,10 @@ impl Span { self.1 = isize::max(0, self.0 as isize + n) as usize; self } + + pub fn is_dummy(&self) -> bool { + *self == DUMMY_SPAN + } } impl std::fmt::Display for Span { diff --git a/crates/simplexpr/src/error.rs b/crates/simplexpr/src/error.rs index 76bbbbc..62300c1 100644 --- a/crates/simplexpr/src/error.rs +++ b/crates/simplexpr/src/error.rs @@ -14,7 +14,7 @@ pub enum Error { ConversionError(#[from] dynval::ConversionError), #[error("{1}")] - Spanned(Span, Box), + Spanned(Span, Box), #[error(transparent)] Eval(#[from] crate::eval::EvalError), diff --git a/crates/simplexpr/src/parser/lexer.rs b/crates/simplexpr/src/parser/lexer.rs index 27bbff4..f3e5891 100644 --- a/crates/simplexpr/src/parser/lexer.rs +++ b/crates/simplexpr/src/parser/lexer.rs @@ -48,6 +48,7 @@ pub enum Token { Error, } +// TODO can i include the fileid here already? #[derive(Debug, Eq, PartialEq, Copy, Clone)] pub struct LexicalError(pub usize, pub usize); diff --git a/crates/yuck/src/config/validate.rs b/crates/yuck/src/config/validate.rs index 9356dea..fc7c420 100644 --- a/crates/yuck/src/config/validate.rs +++ b/crates/yuck/src/config/validate.rs @@ -19,6 +19,15 @@ pub enum ValidationError { MissingAttr { widget_name: String, arg_name: AttrName, arg_list_span: Option, use_span: Span }, } +impl ValidationError { + pub fn span(&self) -> Span { + match self { + ValidationError::UnknownWidget(span, _) => *span, + ValidationError::MissingAttr { use_span, .. } => *use_span, + } + } +} + pub fn validate(defs: &HashMap, content: &WidgetUse) -> Result<(), ValidationError> { if let Some(def) = defs.get(&content.name) { for expected in def.expected_args.iter() { diff --git a/crates/yuck/src/error.rs b/crates/yuck/src/error.rs index bbeefab..3b20877 100644 --- a/crates/yuck/src/error.rs +++ b/crates/yuck/src/error.rs @@ -50,7 +50,7 @@ pub enum AstError { ValidationError(#[from] ValidationError), #[error("Parse error: {source}")] - ParseError { file_id: Option, source: lalrpop_util::ParseError }, + ParseError { file_id: usize, source: lalrpop_util::ParseError }, } static_assertions::assert_impl_all!(AstError: Send, Sync); @@ -79,8 +79,8 @@ impl AstError { AstError::IncludedFileNotFound(include) => Some(include.path_span), AstError::TooManyNodes(span, ..) => Some(*span), AstError::ErrorContext { label_span, .. } => Some(*label_span), - AstError::ValidationError(error) => None, // TODO none here is stupid - AstError::ParseError { file_id, source } => file_id.and_then(|id| get_parse_error_span(id, source)), + AstError::ValidationError(error) => Some(error.span()), + AstError::ParseError { file_id, source } => get_parse_error_span(*file_id, source, |err| err.span()), AstError::ErrorNote(_, err) => err.get_span(), } } @@ -89,23 +89,21 @@ impl AstError { file_id: usize, err: lalrpop_util::ParseError, ) -> AstError { - AstError::ParseError { file_id: Some(file_id), source: err } + AstError::ParseError { file_id, source: err } } } -fn get_parse_error_span( +pub fn get_parse_error_span( file_id: usize, - err: &lalrpop_util::ParseError, + err: &lalrpop_util::ParseError, + handle_user: impl FnOnce(&E) -> Option, ) -> Option { match err { lalrpop_util::ParseError::InvalidToken { location } => Some(Span(*location, *location, file_id)), lalrpop_util::ParseError::UnrecognizedEOF { location, expected } => Some(Span(*location, *location, file_id)), lalrpop_util::ParseError::UnrecognizedToken { token, expected } => Some(Span(token.0, token.2, file_id)), lalrpop_util::ParseError::ExtraToken { token } => Some(Span(token.0, token.2, file_id)), - lalrpop_util::ParseError::User { error } => match error { - parse_error::ParseError::SimplExpr(span, error) => *span, - parse_error::ParseError::LexicalError(span) => Some(*span), - }, + lalrpop_util::ParseError::User { error } => handle_user(error), } } diff --git a/crates/yuck/src/format_diagnostic.rs b/crates/yuck/src/format_diagnostic.rs index 86de768..a2f3cd0 100644 --- a/crates/yuck/src/format_diagnostic.rs +++ b/crates/yuck/src/format_diagnostic.rs @@ -5,7 +5,7 @@ use diagnostic::*; use crate::{ config::{attributes::AttrError, validate::ValidationError}, - error::AstError, + error::{get_parse_error_span, AstError}, }; use super::parser::parse_error; @@ -68,30 +68,27 @@ impl ToDiagnostic for AstError { // TODO this if let should be unnecessary if let AstError::ValidationError(error) = self { error.to_diagnostic() - } else if let Some(span) = self.get_span() { + } else { match self { - AstError::UnknownToplevel(_, name) => gen_diagnostic!(format!("{}", self), span), - AstError::MissingNode(_) => gen_diagnostic! { + AstError::UnknownToplevel(span, name) => gen_diagnostic!(self, span), + AstError::MissingNode(span) => gen_diagnostic! { msg = "Expected another element", label = span => "Expected another element here", }, - AstError::WrongExprType(_, expected, actual) => gen_diagnostic! { + AstError::WrongExprType(span, expected, actual) => gen_diagnostic! { msg = "Wrong type of expression", label = span => format!("Expected a `{}` here", expected), note = format!("Expected: {}\nGot: {}", expected, actual), }, - AstError::NotAValue(_, actual) => gen_diagnostic! { + AstError::NotAValue(span, actual) => gen_diagnostic! { msg = format!("Expected value, but got `{}`", actual), label = span => "Expected some value here", note = format!("Got: {}", actual), }, - AstError::ParseError { file_id, source } => lalrpop_error_to_diagnostic(source, span, |error| match error { - parse_error::ParseError::SimplExpr(_, error) => simplexpr_error_to_diagnostic(error, span), - parse_error::ParseError::LexicalError(span) => lexical_error_to_diagnostic(*span), - }), - AstError::MismatchedElementName(_, expected, got) => gen_diagnostic! { + AstError::ParseError { file_id, source } => lalrpop_error_to_diagnostic(source, *file_id, |error| error.to_diagnostic()), + AstError::MismatchedElementName(span, expected, got) => gen_diagnostic! { msg = format!("Expected element `{}`, but found `{}`", expected, got), label = span => format!("Expected `{}` here", expected), note = format!("Expected: {}\nGot: {}", expected, got), @@ -100,9 +97,10 @@ impl ToDiagnostic for AstError { main_err.to_diagnostic().with_opt_label(Some(span_to_secondary_label(*label_span).with_message(context))) } - AstError::ConversionError(err) => conversion_error_to_diagnostic(err, span), - AstError::Other(_, source) => gen_diagnostic!(source, span), - AstError::AttrError(source) => gen_diagnostic!(source, span), + AstError::ConversionError(source) => source.to_diagnostic(), + AstError::Other(Some(span), source) => gen_diagnostic!(source, span), + AstError::Other(None, source) => gen_diagnostic!(source), + AstError::AttrError(source) => source.to_diagnostic(), AstError::IncludedFileNotFound(include) => gen_diagnostic!( msg = format!("Included file `{}` not found", include.path), label = include.path_span => "Included here", @@ -116,8 +114,15 @@ impl ToDiagnostic for AstError { AstError::ErrorNote(note, source) => source.to_diagnostic().with_notes(vec![note.to_string()]), AstError::ValidationError(source) => source.to_diagnostic(), } - } else { - Diagnostic::error().with_message(format!("{}", self)) + } + } +} + +impl ToDiagnostic for parse_error::ParseError { + fn to_diagnostic(&self) -> Diagnostic { + match self { + parse_error::ParseError::SimplExpr(error) => error.to_diagnostic(), + parse_error::ParseError::LexicalError(span) => lexical_error_diagnostic(*span), } } } @@ -128,7 +133,7 @@ impl ToDiagnostic for AttrError { AttrError::MissingRequiredAttr(span, attr_name) => { gen_diagnostic!(format!("Missing attribute `{}`", attr_name), span) } - AttrError::EvaluationError(span, source) => eval_error_to_diagnostic(source, *span), + AttrError::EvaluationError(span, source) => source.to_diagnostic(), AttrError::Other(span, source) => gen_diagnostic!(source, span), } } @@ -148,53 +153,77 @@ impl ToDiagnostic for ValidationError { } } -fn lalrpop_error_to_diagnostic( +fn lalrpop_error_to_diagnostic( error: &lalrpop_util::ParseError, - span: Span, + file_id: usize, handle_user_error: impl FnOnce(&E) -> Diagnostic, ) -> Diagnostic { use lalrpop_util::ParseError::*; - match error { - InvalidToken { location } => gen_diagnostic! { msg = "Invalid token", label = span }, - UnrecognizedEOF { location, expected } => gen_diagnostic! { - msg = "Input ended unexpectedly. Check if you have any unclosed delimiters", - label = span - }, - UnrecognizedToken { token, expected } => gen_diagnostic! { - msg = format!("Unexpected token `{}` encountered", token.1), - label = span => "Token unexpected", - }, - ExtraToken { token } => gen_diagnostic!(format!("Extra token encountered: `{}`", token.1)), - User { error } => handle_user_error(error), - } -} - -// TODO this needs a lot of improvement -pub fn simplexpr_error_to_diagnostic(error: &simplexpr::error::Error, span: Span) -> Diagnostic { - use simplexpr::error::Error::*; - match error { - ParseError { source, .. } => lalrpop_error_to_diagnostic(source, span, move |error| lexical_error_to_diagnostic(span)), - ConversionError(error) => conversion_error_to_diagnostic(error, span), - Eval(error) => eval_error_to_diagnostic(error, span), - Other(error) => gen_diagnostic!(error, span), - Spanned(span, error) => gen_diagnostic!(error, span), - } -} - -// TODO this needs a lot of improvement -pub fn eval_error_to_diagnostic(error: &simplexpr::eval::EvalError, span: Span) -> Diagnostic { - gen_diagnostic!(error, error.span().unwrap_or(span)) -} - -fn conversion_error_to_diagnostic(error: &dynval::ConversionError, span: Span) -> Diagnostic { - let diag = gen_diagnostic! { - msg = error, - label = span => format!("{} is not of type `{}`", error.value, error.target_type), + // None is okay here, as the case that would be affected by it (User { error }) is manually handled here anyways + let span = get_parse_error_span(file_id, error, |e| None); + let res: Option<_> = try { + match error { + InvalidToken { location } => gen_diagnostic!("Invalid token", span?), + UnrecognizedEOF { location, expected } => gen_diagnostic! { + "Input ended unexpectedly. Check if you have any unclosed delimiters", + span? + }, + UnrecognizedToken { token, expected } => gen_diagnostic! { + msg = format!("Unexpected token `{}` encountered", token.1), + label = span? => "Token unexpected", + }, + ExtraToken { token } => gen_diagnostic!(format!("Extra token encountered: `{}`", token.1)), + User { error } => handle_user_error(error), + } }; - diag.with_notes(error.source.as_ref().map(|x| vec![format!("{}", x)]).unwrap_or_default()) + res.unwrap_or_else(|| gen_diagnostic!(error)) } -fn lexical_error_to_diagnostic(span: Span) -> Diagnostic { +impl ToDiagnostic for simplexpr::error::Error { + // TODO this needs a lot of improvement + fn to_diagnostic(&self) -> Diagnostic { + use simplexpr::error::Error::*; + let res: Option<_> = try { + match self { + ParseError { source, file_id } => { + let span = get_parse_error_span(*file_id, source, |e| Some(Span(e.0, e.1, *file_id)))?; + lalrpop_error_to_diagnostic(source, *file_id, move |error| lexical_error_diagnostic(span)) + } + ConversionError(error) => error.to_diagnostic(), + Eval(error) => error.to_diagnostic(), + Other(error) => gen_diagnostic!(error), + Spanned(span, error) => gen_diagnostic!(error, span), + } + }; + res.unwrap_or_else(|| gen_diagnostic!(self)) + } +} + +impl ToDiagnostic for simplexpr::eval::EvalError { + // TODO this needs a lot of improvement + fn to_diagnostic(&self) -> Diagnostic { + match self.span() { + Some(span) => gen_diagnostic!(self, span), + None => gen_diagnostic!(self), + } + } +} + +impl ToDiagnostic for dynval::ConversionError { + fn to_diagnostic(&self) -> Diagnostic { + let diag = match self.span() { + Some(span) => gen_diagnostic! { + msg = self, + label = span => format!("`{}` is not of type `{}`", self.value, self.target_type), + }, + None => gen_diagnostic!(self), + }; + diag.with_notes(self.source.as_ref().map(|x| vec![format!("{}", x)]).unwrap_or_default()) + } +} + +/// Generate a simple diagnostic indicating a lexical error +fn lexical_error_diagnostic(span: Span) -> Diagnostic { gen_diagnostic! { msg = "Invalid token", label = span => "Invalid token" diff --git a/crates/yuck/src/parser/parse_error.rs b/crates/yuck/src/parser/parse_error.rs index b5f0816..c2a6007 100644 --- a/crates/yuck/src/parser/parse_error.rs +++ b/crates/yuck/src/parser/parse_error.rs @@ -2,9 +2,17 @@ use eww_shared_util::{AttrName, Span, VarName}; #[derive(Debug, thiserror::Error)] pub enum ParseError { - #[error("{1}")] - SimplExpr(Option, simplexpr::error::Error), - + #[error("{0}")] + SimplExpr(simplexpr::error::Error), #[error("Unknown token")] LexicalError(Span), } + +impl ParseError { + pub fn span(&self) -> Option { + match self { + ParseError::SimplExpr(err) => err.get_span(), + ParseError::LexicalError(span) => Some(*span), + } + } +} diff --git a/crates/yuck/src/parser/parser.lalrpop b/crates/yuck/src/parser/parser.lalrpop index 848120d..a1d30d5 100644 --- a/crates/yuck/src/parser/parser.lalrpop +++ b/crates/yuck/src/parser/parser.lalrpop @@ -60,7 +60,7 @@ SimplExpr: SimplExpr = { =>? { let expr = x[1..x.len() - 1].to_string(); simplexpr::parse_string(l + 1, file_id, &expr).map_err(|e| { - ParseError::User { error: parse_error::ParseError::SimplExpr(e.get_span(), e) }}) + ParseError::User { error: parse_error::ParseError::SimplExpr(e) }}) } }