Refactor attribute parsing

src/config/attributes.rs

@@ -0,0 +1,109 @@
+use std::{
+    collections::HashMap,
+    convert::{TryFrom, TryInto},
+};
+
+use simplexpr::{dynval::DynVal, eval::EvalError, SimplExpr};
+
+use crate::{
+    parser::{
+        ast::{Ast, Span},
+        from_ast::FromAst,
+    },
+    value::AttrName,
+};
+
+#[derive(Debug, thiserror::Error)]
+pub enum AttrError {
+    #[error("Missing required attribute {0}")]
+    MissingRequiredAttr(Span, AttrName),
+
+    #[error("Failed to parse attribute value {0} in this context")]
+    AttrTypeError(Span, AttrName),
+
+    #[error("{1}")]
+    EvaluationError(Span, EvalError),
+}
+
+impl AttrError {
+    pub fn span(&self) -> Span {
+        match self {
+            AttrError::MissingRequiredAttr(span, _) => *span,
+            AttrError::AttrTypeError(span, _) => *span,
+            AttrError::EvaluationError(span, _) => *span,
+        }
+    }
+}
+
+#[derive(Debug)]
+pub struct UnusedAttrs {
+    definition_span: Span,
+    attrs: Vec<(Span, AttrName)>,
+}
+
+#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize)]
+pub struct AttrEntry {
+    pub key_span: Span,
+    pub value: SimplExpr,
+}
+
+impl AttrEntry {
+    pub fn new(key_span: Span, value: SimplExpr) -> AttrEntry {
+        AttrEntry { key_span, value }
+    }
+}
+
+#[derive(Debug, Clone, Eq, PartialEq, serde::Serialize)]
+pub struct Attributes {
+    pub span: Span,
+    pub attrs: HashMap<AttrName, AttrEntry>,
+}
+
+impl Attributes {
+    pub fn new(span: Span, attrs: HashMap<AttrName, AttrEntry>) -> Self {
+        Attributes { span, attrs }
+    }
+
+    pub fn eval_required<T: TryFrom<DynVal>>(&mut self, key: &str) -> Result<T, AttrError> {
+        let key = AttrName(key.to_string());
+        match self.attrs.remove(&key) {
+            Some(AttrEntry { key_span, value }) => {
+                let value_span = value.span();
+                let dynval = value.eval_no_vars().map_err(|err| AttrError::EvaluationError(value_span.into(), err))?;
+                T::try_from(dynval).map_err(|_| AttrError::AttrTypeError(value_span.into(), key.clone()))
+            }
+            None => Err(AttrError::MissingRequiredAttr(self.span, key.clone())),
+        }
+    }
+
+    // pub fn parse_required<T: TryFrom<SimplExpr>>(&mut self, key: &str) -> Result<T, AttrError> {
+    // let key = AttrName(key.to_string());
+    // match self.attrs.remove(&key) {
+    // Some(value) => match value.value.try_into() {
+    // Ok(value) => Ok(value),
+    // Err(_) => Err(AttrError::AttrTypeError(value.value.span().into(), key.clone())),
+    // },
+    // None => Err(AttrError::MissingRequiredAttr(self.span, key.clone())),
+    // }
+    // }
+    //
+    // pub fn parse_optional<T: TryFrom<SimplExpr>>(&mut self, key: &str) -> Result<Option<T>, AttrError> {
+    // let key = AttrName(key.to_string());
+    // match self.attrs.remove(&key) {
+    // Some(value) => match value.value.try_into() {
+    // Ok(value) => Ok(Some(value)),
+    // Err(_) => Err(AttrError::AttrTypeError(value.value.span().into(), key.clone())),
+    // },
+    // None => Ok(None),
+    // }
+    // }
+
+    /// Consumes the attributes to return a list of unused attributes which may be used to emit a warning.
+    /// TODO actually use this and implement warnings,... lol
+    pub fn get_unused(self, definition_span: Span) -> UnusedAttrs {
+        UnusedAttrs {
+            definition_span,
+            attrs: self.attrs.into_iter().map(|(k, v)| (v.key_span.to(v.value.span().into()), k)).collect(),
+        }
+    }
+}

src/config/config.rs

@@ -27,8 +27,7 @@ impl FromAst for TopLevel {
     fn from_ast(e: Ast) -> AstResult<Self> {
         let span = e.span();
         spanned!(e.span(), {
-            let list = e.as_list()?;
+            let mut iter = e.try_ast_iter()?;
-            let mut iter = AstIterator::new(list.into_iter());
             let (sym_span, element_name) = iter.expect_symbol()?;
             match element_name.as_str() {
                 x if x == WidgetDefinition::get_element_name() => {

src/config/mod.rs

@@ -1,3 +1,4 @@
+pub mod attributes;
 mod config;
 pub mod config_parse_error;
 pub mod script_var_definition;

src/config/script_var_definition.rs

@@ -48,9 +48,9 @@ impl FromAstElementContent for PollScriptVar {
 
     fn from_tail<I: Iterator<Item = Ast>>(span: Span, mut iter: AstIterator<I>) -> AstResult<Self> {
         let (_, name) = iter.expect_symbol()?;
-        let attrs: HashMap<String, String> = iter.expect_key_values()?;
+        let mut attrs = iter.expect_key_values()?;
-        let interval = attrs.get("interval").unwrap();
+        let interval: String = attrs.eval_required("interval")?;
-        let interval = crate::util::parse_duration(interval).map_err(|e| AstError::Other(Some(span), e.into()))?;
+        let interval = crate::util::parse_duration(&interval).map_err(|e| AstError::Other(Some(span), e.into()))?;
         let (_, script) = iter.expect_literal()?;
         Ok(Self { name: VarName(name), command: VarSource::Shell(script.to_string()), interval })
     }

src/config/validate.rs

@@ -26,7 +26,7 @@ pub enum ValidationError {
 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() {
-            if !content.attrs.contains_key(expected) {
+            if !content.attrs.attrs.contains_key(expected) {
                 return Err(ValidationError::MissingAttr {
                     widget_name: def.name.to_string(),
                     arg_name: expected.clone(),

src/config/widget_use.rs

@@ -3,6 +3,7 @@ use std::collections::HashMap;
 use simplexpr::SimplExpr;
 
 use crate::{
+    config::attributes::AttrEntry,
     error::AstResult,
     parser::{
         ast::{Ast, AstIterator, Span},
@@ -11,10 +12,13 @@ use crate::{
     spanned,
     value::AttrName,
 };
+
+use super::attributes::Attributes;
+
 #[derive(Debug, PartialEq, Eq, Clone, serde::Serialize)]
 pub struct WidgetUse {
     pub name: String,
-    pub attrs: HashMap<AttrName, SimplExpr>,
+    pub attrs: Attributes,
     pub children: Vec<WidgetUse>,
     pub span: Span,
 }
@@ -26,15 +30,22 @@ impl FromAst for WidgetUse {
             if let Ok(text) = e.as_literal_ref() {
                 Self {
                     name: "text".to_string(),
-                    attrs: maplit::hashmap! { AttrName("text".to_string()) => SimplExpr::Literal(span.into(), text.clone()) },
+                    attrs: Attributes::new(
+                        span.into(),
+                        maplit::hashmap! {
+                            AttrName("text".to_string()) => AttrEntry::new(
+                                span.into(),
+                                SimplExpr::Literal(span.into(), text.clone())
+                            )
+                        },
+                    ),
                     children: Vec::new(),
                     span,
                 }
             } else {
-                let list = e.as_list()?;
+                let mut iter = e.try_ast_iter()?;
-                let mut iter = AstIterator::new(list.into_iter());
                 let (_, name) = iter.expect_symbol()?;
-                let attrs = iter.expect_key_values()?.into_iter().map(|(k, v)| (AttrName(k), v)).collect();
+                let attrs = iter.expect_key_values()?;
                 let children = iter.map(WidgetUse::from_ast).collect::<AstResult<Vec<_>>>()?;
                 Self { name, attrs, children, span }
             }

src/error.rs

@@ -1,5 +1,5 @@
 use crate::{
-    config::validate::ValidationError,
+    config::{attributes::AttrError, validate::ValidationError},
     parser::{
         ast::{Ast, AstType, Span},
         lexer, parse_error,
@@ -22,9 +22,20 @@ pub enum AstError {
     NotAValue(Option<Span>, AstType),
     #[error("Expected element {1}, but read {2}")]
     MismatchedElementName(Option<Span>, String, String),
+
     #[error("{1}")]
     Other(Option<Span>, Box<dyn std::error::Error>),
 
+    #[error(transparent)]
+    AttrError(#[from] AttrError),
+
+    //#[error("{msg}: {source}")]
+    // Context {
+    // span: Option<Span>,
+    //#[source]
+    // source: Box<dyn std::error::Error>,
+    // msg: String,
+    //},
     #[error(transparent)]
     ValidationError(#[from] ValidationError),
 
@@ -40,7 +51,9 @@ impl AstError {
             AstError::WrongExprType(span, ..) => *span,
             AstError::NotAValue(span, ..) => *span,
             AstError::MismatchedElementName(span, ..) => *span,
+            AstError::AttrError(err) => Some(err.span()),
             AstError::Other(span, ..) => *span,
+            // AstError::Context { span, .. } => *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)),
         }
@@ -80,6 +93,7 @@ pub fn spanned(span: Span, err: impl Into<AstError>) -> AstError {
         MissingNode(None) => MissingNode(Some(span)),
         NotAValue(None, x) => NotAValue(Some(span), x),
         MismatchedElementName(None, x, y) => MismatchedElementName(Some(span), x, y),
+        // Context { span: None, source, msg } => Context { span: Some(span), source, msg },
         Other(None, x) => Other(Some(span), x),
         x => x,
     }
@@ -98,12 +112,21 @@ pub trait AstResultExt<T> {
     fn at(self, span: Span) -> Result<T, AstError>;
 }
 
+pub trait Context<T> {
+    fn context(self, span: Span, msg: String) -> Result<T, AstError>;
+}
+
 impl<T, E: Into<AstError>> AstResultExt<T> for Result<T, E> {
     fn at(self, span: Span) -> Result<T, AstError> {
         self.map_err(|err| spanned(span, err))
     }
 }
 
+// impl<T, E: std::error::Error + 'static> Context<T> for Result<T, E> {
+// fn context(self, span: Span, msg: String) -> Result<T, AstError> {
+// self.map_err(|x| AstError::Context { msg, span: Some(span), source: Box::new(x) })
+//}
+
 #[macro_export]
 macro_rules! spanned {
     ($span:expr, $block:expr) => {{

src/parser/ast.rs

@@ -5,16 +5,45 @@ use std::collections::HashMap;
 use std::fmt::Display;
 
 use super::from_ast::FromAst;
-use crate::error::{AstError, AstResult, OptionAstErrorExt};
+use crate::{
+    config::attributes::{AttrEntry, Attributes},
+    error::{AstError, AstResult, OptionAstErrorExt},
+    value::AttrName,
+};
 
 #[derive(Eq, PartialEq, Clone, Copy, serde::Serialize)]
 pub struct Span(pub usize, pub usize, pub usize);
 
+impl Span {
+    /// 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 {
+        assert!(other.2 == self.2);
+        self.1 = other.1;
+        self
+    }
+
+    pub fn ending_at(mut self, end: usize) -> Self {
+        self.1 = end;
+        self
+    }
+
+    pub fn with_length(mut self, end: usize) -> Self {
+        self.1 = self.0;
+        self
+    }
+}
+
 impl Into<simplexpr::Span> for Span {
     fn into(self) -> simplexpr::Span {
         simplexpr::Span(self.0, self.1, self.2)
     }
 }
+impl From<simplexpr::Span> for Span {
+    fn from(x: simplexpr::Span) -> Span {
+        Span(x.0, x.1, x.2)
+    }
+}
 
 impl std::fmt::Display for Span {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@@ -47,7 +76,7 @@ impl Display for AstType {
     }
 }
 
-#[derive(PartialEq, Eq, Clone)]
+#[derive(PartialEq, Eq, Clone, serde::Serialize)]
 pub enum Ast {
     List(Span, Vec<Ast>),
     Array(Span, Vec<Ast>),
@@ -119,6 +148,12 @@ impl Ast {
             _ => Err(AstError::WrongExprType(Some(self.span()), AstType::IntoPrimitive, self.expr_type())),
         }
     }
+
+    pub fn try_ast_iter(self) -> AstResult<AstIterator<impl Iterator<Item = Ast>>> {
+        let span = self.span();
+        let list = self.as_list()?;
+        Ok(AstIterator::new(span, list.into_iter()))
+    }
 }
 
 impl std::fmt::Display for Ast {
@@ -152,6 +187,7 @@ impl std::fmt::Debug for Ast {
 }
 
 pub struct AstIterator<I: Iterator<Item = Ast>> {
+    remaining_span: Span,
     iter: itertools::PutBack<I>,
 }
 
@@ -160,7 +196,11 @@ macro_rules! return_or_put_back {
         pub fn $name(&mut self) -> AstResult<$t> {
             let expr_type = $expr_type;
             match self.next() {
-                Some($p) => Ok($ret),
+                Some($p) => {
+                    let (span, value) = $ret;
+                    self.remaining_span.1 = span.1;
+                    Ok((span, value))
+                }
                 Some(other) => {
                     let span = other.span();
                     let actual_type = other.expr_type();
@@ -182,16 +222,16 @@ impl<I: Iterator<Item = Ast>> AstIterator<I> {
 
     return_or_put_back!(expect_array, AstType::Array, (Span, Vec<Ast>) = Ast::Array(span, x) => (span, x));
 
-    pub fn new(iter: I) -> Self {
+    pub fn new(span: Span, iter: I) -> Self {
-        AstIterator { iter: itertools::put_back(iter) }
+        AstIterator { remaining_span: span, iter: itertools::put_back(iter) }
     }
 
     pub fn expect_any<T: FromAst>(&mut self) -> AstResult<T> {
         self.iter.next().or_missing().and_then(T::from_ast)
     }
 
-    pub fn expect_key_values<T: FromAst>(&mut self) -> AstResult<HashMap<String, T>> {
+    pub fn expect_key_values(&mut self) -> AstResult<Attributes> {
-        parse_key_values(&mut self.iter)
+        parse_key_values(self)
     }
 }
 
@@ -203,25 +243,31 @@ impl<I: Iterator<Item = Ast>> Iterator for AstIterator<I> {
     }
 }
 
-/// Parse consecutive `:keyword value` pairs from an expression iterator into a HashMap. Transforms the keys using the FromExpr trait.
+/// Parse consecutive `:keyword value` pairs from an expression iterator into an [Attributes].
-fn parse_key_values<T: FromAst, I: Iterator<Item = Ast>>(iter: &mut itertools::PutBack<I>) -> AstResult<HashMap<String, T>> {
+fn parse_key_values(iter: &mut AstIterator<impl Iterator<Item = Ast>>) -> AstResult<Attributes> {
     let mut data = HashMap::new();
+    let mut attrs_span = Span(iter.remaining_span.0, iter.remaining_span.0, iter.remaining_span.1);
     loop {
         match iter.next() {
-            Some(Ast::Keyword(span, kw)) => match iter.next() {
+            Some(Ast::Keyword(key_span, kw)) => match iter.next() {
                 Some(value) => {
-                    data.insert(kw, T::from_ast(value)?);
+                    attrs_span.1 = iter.remaining_span.0;
+                    let attr_value = AttrEntry { key_span, value: value.as_simplexpr()? };
+                    data.insert(AttrName(kw), attr_value);
                 }
                 None => {
-                    iter.put_back(Ast::Keyword(span, kw));
+                    iter.iter.put_back(Ast::Keyword(key_span, kw));
-                    return Ok(data);
+                    attrs_span.1 = iter.remaining_span.0;
+                    return Ok(Attributes::new(attrs_span, data));
                 }
             },
-            Some(expr) => {
+            next => {
-                iter.put_back(expr);
+                if let Some(expr) = next {
-                return Ok(data);
+                    iter.iter.put_back(expr);
+                }
+                attrs_span.1 = iter.remaining_span.0;
+                return Ok(Attributes::new(attrs_span, data));
             }
-            None => return Ok(data),
         }
     }
 }

src/parser/from_ast.rs

@@ -35,8 +35,7 @@ impl<T: FromAstElementContent> FromAst for T {
     fn from_ast(e: Ast) -> AstResult<Self> {
         let span = e.span();
         spanned!(e.span(), {
-            let list = e.as_list()?;
+            let mut iter = e.try_ast_iter()?;
-            let mut iter = AstIterator::new(list.into_iter());
             let (_, element_name) = iter.expect_symbol()?;
             if Self::get_element_name() != element_name {
                 return Err(AstError::MismatchedElementName(Some(span), Self::get_element_name().to_string(), element_name));