Add config validation step

crates/eww/src/config/eww_config.rs

@@ -38,6 +38,10 @@ impl EwwConfig {
             bail!("The configuration file `{}` does not exist", path.as_ref().display());
         }
         let config = Config::generate_from_main_file(files, path)?;
+
+        // run some validations on the configuration
+        yuck::config::validate::validate(&config, super::inbuilt::get_inbuilt_vars().keys().cloned().collect())?;
+
         let Config { widget_definitions, window_definitions, var_definitions, mut script_vars } = config;
         script_vars.extend(crate::config::inbuilt::get_inbuilt_vars());
         Ok(EwwConfig {

crates/eww/src/error_handling_ctx.rs

@@ -10,7 +10,12 @@ use codespan_reporting::{
 use eww_shared_util::Span;
 use once_cell::sync::Lazy;
 use simplexpr::{dynval::ConversionError, eval::EvalError};
-use yuck::{config::file_provider::YuckFiles, error::AstError, format_diagnostic::ToDiagnostic, gen_diagnostic};
+use yuck::{
+    config::{file_provider::YuckFiles, validate::ValidationError},
+    error::AstError,
+    format_diagnostic::ToDiagnostic,
+    gen_diagnostic,
+};
 
 use crate::error::DiagError;
 
@@ -46,6 +51,8 @@ pub fn anyhow_err_to_diagnostic(err: &anyhow::Error) -> Diagnostic<usize> {
         err.to_diagnostic()
     } else if let Some(err) = err.downcast_ref::<ConversionError>() {
         err.to_diagnostic()
+    } else if let Some(err) = err.downcast_ref::<ValidationError>() {
+        err.to_diagnostic()
     } else if let Some(err) = err.downcast_ref::<EvalError>() {
         err.to_diagnostic()
     } else {

crates/yuck/examples/validation.rs

@@ -1,40 +0,0 @@
-use yuck::{
-    config::{widget_definition::WidgetDefinition, widget_use::WidgetUse, *},
-    error::AstError,
-    format_diagnostic::ToDiagnostic,
-    parser::from_ast::FromAst,
-};
-
-fn main() {
-    let mut files = codespan_reporting::files::SimpleFiles::new();
-
-    let input_use = r#"
-        (foo :something 12
-             :bla "bruh"
-          "some text")
-    "#;
-    let input_def = r#"
-        (defwidget foo [something bla] "foo")
-    "#;
-
-    let file_id_use = files.add("use.eww", input_use);
-    let file_id_def = files.add("def.eww", input_def);
-    let parsed_use = WidgetUse::from_ast(yuck::parser::parse_string(file_id_use, input_use).unwrap()).unwrap();
-    let parsed_def = WidgetDefinition::from_ast(yuck::parser::parse_string(file_id_def, input_def).unwrap()).unwrap();
-    let defs = maplit::hashmap! {
-        "foo".to_string() => parsed_def,
-    };
-    match validate::validate(&defs, &parsed_use) {
-        Ok(ast) => {
-            println!("{:?}", ast);
-        }
-        Err(err) => {
-            let err = AstError::ValidationError(err);
-            let diag = err.to_diagnostic();
-            use codespan_reporting::term;
-            let config = term::Config::default();
-            let mut writer = term::termcolor::StandardStream::stderr(term::termcolor::ColorChoice::Always);
-            term::emit(&mut writer, &config, &files, &diag).unwrap();
-        }
-    }
-}

crates/yuck/src/config/validate.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
 use simplexpr::SimplExpr;
 
@@ -7,7 +7,7 @@ use crate::{
     parser::{ast::Ast, ast_iterator::AstIterator, from_ast::FromAst},
 };
 
-use super::{widget_definition::WidgetDefinition, widget_use::WidgetUse};
+use super::{widget_definition::WidgetDefinition, widget_use::WidgetUse, Config};
 use eww_shared_util::{AttrName, Span, Spanned, VarName};
 
 #[derive(Debug, thiserror::Error)]
@@ -17,6 +17,14 @@ pub enum ValidationError {
 
     #[error("Missing attribute `{arg_name}` in use of widget `{widget_name}`")]
     MissingAttr { widget_name: String, arg_name: AttrName, arg_list_span: Option<Span>, use_span: Span },
+
+    #[error("No variable named `{name}` in scope")]
+    UnknownVariable {
+        span: Span,
+        name: VarName,
+        /// True if the error occurred inside a widget definition, false if it occurred in a window definition
+        in_definition: bool,
+    },
 }
 
 impl Spanned for ValidationError {
@@ -24,24 +32,70 @@ impl Spanned for ValidationError {
         match self {
             ValidationError::UnknownWidget(span, _) => *span,
             ValidationError::MissingAttr { use_span, .. } => *use_span,
+            ValidationError::UnknownVariable { span, .. } => *span,
         }
     }
 }
 
-pub fn validate(defs: &HashMap<String, WidgetDefinition>, content: &WidgetUse) -> Result<(), ValidationError> {
+pub fn validate(config: &Config, additional_globals: Vec<VarName>) -> Result<(), ValidationError> {
-    if let Some(def) = defs.get(&content.name) {
+    let var_names = std::iter::empty()
-        for expected in def.expected_args.iter() {
+        .chain(additional_globals.iter().cloned())
-            if !content.attrs.attrs.contains_key(expected) {
+        .chain(config.script_vars.keys().cloned())
-                return Err(ValidationError::MissingAttr {
+        .chain(config.var_definitions.keys().cloned())
-                    widget_name: def.name.to_string(),
+        .collect();
-                    arg_name: expected.clone(),
+    for window in config.window_definitions.values() {
-                    arg_list_span: Some(def.args_span),
+        validate_variables_in_widget_use(&config.widget_definitions, &var_names, &window.widget, false)?;
-                    use_span: content.span,
+    }
-                });
+    for def in config.widget_definitions.values() {
-            }
+        validate_widget_definition(&config.widget_definitions, &var_names, &def)?;
-        }
-    } else {
-        return Err(ValidationError::UnknownWidget(content.span, content.name.to_string()));
     }
     Ok(())
 }
+
+pub fn validate_widget_definition(
+    other_defs: &HashMap<String, WidgetDefinition>,
+    globals: &HashSet<VarName>,
+    def: &WidgetDefinition,
+) -> Result<(), ValidationError> {
+    let mut variables_in_scope = globals.clone();
+    for arg in def.expected_args.iter() {
+        variables_in_scope.insert(VarName(arg.0.to_string()));
+    }
+
+    validate_variables_in_widget_use(other_defs, &variables_in_scope, &def.widget, true)
+}
+
+pub fn validate_variables_in_widget_use(
+    defs: &HashMap<String, WidgetDefinition>,
+    variables: &HashSet<VarName>,
+    widget: &WidgetUse,
+    is_in_definition: bool,
+) -> Result<(), ValidationError> {
+    let matching_definition = defs.get(&widget.name);
+    if let Some(matching_def) = matching_definition {
+        let missing_arg = matching_def.expected_args.iter().find(|expected| !widget.attrs.attrs.contains_key(*expected));
+        if let Some(missing_arg) = missing_arg {
+            return Err(ValidationError::MissingAttr {
+                widget_name: widget.name.clone(),
+                arg_name: missing_arg.clone(),
+                arg_list_span: Some(matching_def.args_span),
+                use_span: widget.attrs.span,
+            });
+        }
+    }
+
+    let values = widget.attrs.attrs.values();
+    let unknown_var = values.filter_map(|value| value.value.as_simplexpr().ok()).find_map(|expr: SimplExpr| {
+        let span = expr.span();
+        expr.var_refs().iter().map(move |&x| (span, x.clone())).find(|(span, var_ref)| !variables.contains(var_ref))
+    });
+    if let Some((span, var)) = unknown_var {
+        return Err(ValidationError::UnknownVariable { span, name: var.clone(), in_definition: is_in_definition });
+    }
+
+    for child in widget.children.iter() {
+        let _ = validate_variables_in_widget_use(defs, variables, child, is_in_definition)?;
+    }
+
+    Ok(())
+}

crates/yuck/src/format_diagnostic.rs

@@ -162,13 +162,33 @@ impl ToDiagnostic for ValidationError {
                 label = span => "Used here",
             },
             ValidationError::MissingAttr { widget_name, arg_name, arg_list_span, use_span } => {
-                let mut diag =
+                let mut diag = Diagnostic::error()
-                    gen_diagnostic!(self).with_label(span_to_secondary_label(*use_span).with_message("Argument missing here"));
+                    .with_message(self.to_string())
+                    .with_label(span_to_secondary_label(*use_span).with_message("Argument missing here"))
+                    .with_notes(vec![format!(
+                        "Hint: pass the attribute like so: `({} :{} your-value ...`",
+                        widget_name, arg_name
+                    )]);
                 if let Some(arg_list_span) = arg_list_span {
                     diag = diag.with_label(span_to_secondary_label(*arg_list_span).with_message("But is required here"));
                 }
                 diag
             }
+            ValidationError::UnknownVariable { span, name, in_definition } => {
+                let diag = gen_diagnostic! {
+                    msg = self,
+                    label = span => "Used here",
+                    note = if *in_definition {
+                        "Hint: Either define it as a global variable, or add it to the argument-list of your `defwidget` and pass it as an argument"
+                    } else {
+                        "Hint: Define it as a global variable"
+                    }
+                };
+                diag.with_notes(vec![format!(
+                    "Hint: If you meant to use the literal value \"{}\", surround the value in quotes",
+                    name
+                )])
+            }
         }
     }
 }
@@ -226,7 +246,7 @@ impl ToDiagnostic for simplexpr::eval::EvalError {
                 }
                 // TODO the note here is confusing when it's an unknown variable being used _within_ a string literal / simplexpr
                 // it only really makes sense on top-level symbols
-                notes.push(format!("If you meant to use the literal value \"{}\", surround the value in quotes", name));
+                notes.push(format!("Hint: If you meant to use the literal value \"{}\", surround the value in quotes", name));
                 gen_diagnostic!(self).with_notes(notes)
             }
             Spanned(span, error) => error.as_ref().to_diagnostic().with_label(span_to_primary_label(*span)),

crates/yuck/src/parser/ast.rs

@@ -91,12 +91,12 @@ impl Ast {
         }
     }
 
-    pub fn as_simplexpr(self) -> AstResult<SimplExpr> {
+    pub fn as_simplexpr(&self) -> AstResult<SimplExpr> {
         match self {
             // TODO do I do this?
             // Ast::Array(span, elements) => todo!()
-            Ast::Symbol(span, x) => Ok(SimplExpr::VarRef(span, VarName(x))),
+            Ast::Symbol(span, x) => Ok(SimplExpr::VarRef(*span, VarName(x.clone()))),
-            Ast::SimplExpr(span, x) => Ok(x),
+            Ast::SimplExpr(span, x) => Ok(x.clone()),
             _ => Err(AstError::WrongExprType(self.span(), AstType::IntoPrimitive, self.expr_type())),
         }
     }