implement ToDiagnostic for more things

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::{
+use yuck::{config::file_provider::YuckFiles, error::AstError, format_diagnostic::ToDiagnostic, gen_diagnostic};
-    config::file_provider::YuckFiles,
-    error::AstError,
-    format_diagnostic::{eval_error_to_diagnostic, ToDiagnostic},
-    gen_diagnostic,
-};
 
 use crate::error::DiagError;
 
@@ -26,14 +21,14 @@ pub fn anyhow_err_to_diagnostic(err: &anyhow::Error) -> Diagnostic<usize> {
     } else if let Some(err) = err.downcast_ref::<AstError>() {
         err.to_diagnostic()
     } else if let Some(err) = err.downcast_ref::<EvalError>() {
-        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::<AstError>() {
-        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<usize>) -> anyhow::Result<String> {
+pub fn stringify_diagnostic(mut diagnostic: codespan_reporting::diagnostic::Diagnostic<usize>) -> anyhow::Result<String> {
+    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 = '─';

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)]

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 {

crates/simplexpr/src/error.rs

@@ -14,7 +14,7 @@ pub enum Error {
     ConversionError(#[from] dynval::ConversionError),
 
     #[error("{1}")]
-    Spanned(Span, Box<dyn std::error::Error + Send + Sync>),
+    Spanned(Span, Box<Error>),
 
     #[error(transparent)]
     Eval(#[from] crate::eval::EvalError),

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);
 

crates/yuck/src/config/validate.rs

@@ -19,6 +19,15 @@ pub enum ValidationError {
     MissingAttr { widget_name: String, arg_name: AttrName, arg_list_span: Option<Span>, 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<String, WidgetDefinition>, content: &WidgetUse) -> Result<(), ValidationError> {
     if let Some(def) = defs.get(&content.name) {
         for expected in def.expected_args.iter() {

crates/yuck/src/error.rs

@@ -50,7 +50,7 @@ pub enum AstError {
     ValidationError(#[from] ValidationError),
 
     #[error("Parse error: {source}")]
-    ParseError { file_id: Option<usize>, source: lalrpop_util::ParseError<usize, lexer::Token, parse_error::ParseError> },
+    ParseError { file_id: usize, source: lalrpop_util::ParseError<usize, lexer::Token, parse_error::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::ValidationError(error) => Some(error.span()),
-            AstError::ParseError { file_id, source } => file_id.and_then(|id| get_parse_error_span(id, source)),
+            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<usize, lexer::Token, parse_error::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<T, E>(
     file_id: usize,
-    err: &lalrpop_util::ParseError<usize, lexer::Token, parse_error::ParseError>,
+    err: &lalrpop_util::ParseError<usize, T, E>,
+    handle_user: impl FnOnce(&E) -> Option<Span>,
 ) -> Option<Span> {
     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 {
+        lalrpop_util::ParseError::User { error } => handle_user(error),
-            parse_error::ParseError::SimplExpr(span, error) => *span,
-            parse_error::ParseError::LexicalError(span) => Some(*span),
-        },
     }
 }
 

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::UnknownToplevel(span, name) => gen_diagnostic!(self, span),
-                AstError::MissingNode(_) => gen_diagnostic! {
+                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 {
+                AstError::ParseError { file_id, source } => lalrpop_error_to_diagnostic(source, *file_id, |error| error.to_diagnostic()),
-                    parse_error::ParseError::SimplExpr(_, error) => simplexpr_error_to_diagnostic(error, span),
+                AstError::MismatchedElementName(span, expected, got) => gen_diagnostic! {
-                    parse_error::ParseError::LexicalError(span) => lexical_error_to_diagnostic(*span),
-                }),
-                AstError::MismatchedElementName(_, 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::ConversionError(source) => source.to_diagnostic(),
-                AstError::Other(_, source) => gen_diagnostic!(source, span),
+                AstError::Other(Some(span), source) => gen_diagnostic!(source, span),
-                AstError::AttrError(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<usize> {
+        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<T: std::fmt::Display, E>(
+fn lalrpop_error_to_diagnostic<T: std::fmt::Display, E: std::fmt::Display>(
     error: &lalrpop_util::ParseError<usize, T, E>,
-    span: Span,
+    file_id: usize,
     handle_user_error: impl FnOnce(&E) -> Diagnostic<usize>,
 ) -> Diagnostic<usize> {
     use lalrpop_util::ParseError::*;
-    match error {
+    // None is okay here, as the case that would be affected by it (User { error }) is manually handled here anyways
-        InvalidToken { location } => gen_diagnostic! { msg = "Invalid token", label = span },
+    let span = get_parse_error_span(file_id, error, |e| None);
-        UnrecognizedEOF { location, expected } => gen_diagnostic! {
+    let res: Option<_> = try {
-            msg = "Input ended unexpectedly. Check if you have any unclosed delimiters",
+        match error {
-            label = span
+            InvalidToken { location } => gen_diagnostic!("Invalid token", span?),
-        },
+            UnrecognizedEOF { location, expected } => gen_diagnostic! {
-        UnrecognizedToken { token, expected } => gen_diagnostic! {
+                "Input ended unexpectedly. Check if you have any unclosed delimiters",
-            msg = format!("Unexpected token `{}` encountered", token.1),
+                span?
-            label = span => "Token unexpected",
+            },
-        },
+            UnrecognizedToken { token, expected } => gen_diagnostic! {
-        ExtraToken { token } => gen_diagnostic!(format!("Extra token encountered: `{}`", token.1)),
+                msg = format!("Unexpected token `{}` encountered", token.1),
-        User { error } => handle_user_error(error),
+                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<usize> {
-    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<usize> {
-    gen_diagnostic!(error, error.span().unwrap_or(span))
-}
-
-fn conversion_error_to_diagnostic(error: &dynval::ConversionError, span: Span) -> Diagnostic<usize> {
-    let diag = gen_diagnostic! {
-        msg = error,
-        label = span => format!("{} is not of type `{}`", error.value, error.target_type),
     };
-    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<usize> {
+impl ToDiagnostic for simplexpr::error::Error {
+    // TODO this needs a lot of improvement
+    fn to_diagnostic(&self) -> Diagnostic<usize> {
+        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<usize> {
+        match self.span() {
+            Some(span) => gen_diagnostic!(self, span),
+            None => gen_diagnostic!(self),
+        }
+    }
+}
+
+impl ToDiagnostic for dynval::ConversionError {
+    fn to_diagnostic(&self) -> Diagnostic<usize> {
+        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<usize> {
     gen_diagnostic! {
         msg = "Invalid token",
         label = span => "Invalid token"

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}")]
+    #[error("{0}")]
-    SimplExpr(Option<Span>, simplexpr::error::Error),
+    SimplExpr(simplexpr::error::Error),
-
     #[error("Unknown token")]
     LexicalError(Span),
 }
+
+impl ParseError {
+    pub fn span(&self) -> Option<Span> {
+        match self {
+            ParseError::SimplExpr(err) => err.get_span(),
+            ParseError::LexicalError(span) => Some(*span),
+        }
+    }
+}

crates/yuck/src/parser/parser.lalrpop

@@ -60,7 +60,7 @@ SimplExpr: SimplExpr = {
    <l:@L> <x:"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) }})
    }
 }