From eb3c2646a04fdcc2f9dab8d37d49b895233b5e3b Mon Sep 17 00:00:00 2001 From: elkowar <5300871+elkowar@users.noreply.github.com> Date: Mon, 2 Aug 2021 15:54:38 +0200 Subject: [PATCH] Add did you mean question to wrong variables --- Cargo.lock | 7 +++++ crates/simplexpr/Cargo.toml | 1 + crates/simplexpr/src/eval.rs | 18 +++++++++--- crates/yuck/Cargo.toml | 2 +- .../yuck/src/config/backend_window_options.rs | 4 +-- crates/yuck/src/config/config.rs | 25 ++++++++++------ .../yuck/src/config/script_var_definition.rs | 8 ++--- crates/yuck/src/config/var_definition.rs | 11 +++---- crates/yuck/src/config/widget_definition.rs | 4 +-- crates/yuck/src/config/window_definition.rs | 4 +-- crates/yuck/src/config/window_geometry.rs | 4 +-- crates/yuck/src/format_diagnostic.rs | 29 +++++++++++++------ crates/yuck/src/parser/from_ast.rs | 6 ++-- 13 files changed, 74 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cca7641..6e58f10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1114,6 +1114,12 @@ version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" +[[package]] +name = "levenshtein" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db13adb97ab515a3691f56e4dbab09283d0b86cb45abd991d8634a9d6f501760" + [[package]] name = "libc" version = "0.2.98" @@ -1890,6 +1896,7 @@ dependencies = [ "itertools 0.10.1", "lalrpop", "lalrpop-util", + "levenshtein", "logos", "maplit", "once_cell", diff --git a/crates/simplexpr/Cargo.toml b/crates/simplexpr/Cargo.toml index 97b032a..bf1785f 100644 --- a/crates/simplexpr/Cargo.toml +++ b/crates/simplexpr/Cargo.toml @@ -18,6 +18,7 @@ logos = "0.12" once_cell = "1.8.0" serde = {version = "1.0", features = ["derive"]} serde_json = "1.0" +levenshtein = "1.0" strum = { version = "0.21", features = ["derive"] } diff --git a/crates/simplexpr/src/eval.rs b/crates/simplexpr/src/eval.rs index cf508ca..5d02944 100644 --- a/crates/simplexpr/src/eval.rs +++ b/crates/simplexpr/src/eval.rs @@ -16,7 +16,7 @@ pub enum EvalError { InvalidRegex(#[from] regex::Error), #[error("Unknown variable {0}")] - UnknownVariable(VarName), + UnknownVariable(VarName, Vec), #[error(transparent)] ConversionError(#[from] ConversionError), @@ -108,7 +108,11 @@ impl SimplExpr { )), VarRef(span, ref name) => match variables.get(name) { Some(value) => Ok(Literal(value.clone())), - None => Err(EvalError::UnknownVariable(name.clone()).at(span)), + None => { + let similar_ish = + variables.keys().filter(|key| levenshtein::levenshtein(&key.0, &name.0) < 3).cloned().collect_vec(); + Err(EvalError::UnknownVariable(name.clone(), similar_ish).at(span)) + } }, } } @@ -138,7 +142,7 @@ impl SimplExpr { match self.eval(&HashMap::new()) { Ok(x) => Ok(x), Err(x) => Err(x.map_in_span(|err| match err { - EvalError::UnknownVariable(name) => EvalError::NoVariablesAllowed(name), + EvalError::UnknownVariable(name, _) => EvalError::NoVariablesAllowed(name), other => other, })), } @@ -149,7 +153,13 @@ impl SimplExpr { let value = match self { SimplExpr::Literal(x) => Ok(x.clone()), SimplExpr::VarRef(span, ref name) => { - Ok(values.get(name).cloned().ok_or_else(|| EvalError::UnknownVariable(name.clone()).at(*span))?.at(*span)) + let similar_ish = + values.keys().filter(|keys| levenshtein::levenshtein(&keys.0, &name.0) < 3).cloned().collect_vec(); + Ok(values + .get(name) + .cloned() + .ok_or_else(|| EvalError::UnknownVariable(name.clone(), similar_ish).at(*span))? + .at(*span)) } SimplExpr::BinOp(_, a, op, b) => { let a = a.eval(values)?; diff --git a/crates/yuck/Cargo.toml b/crates/yuck/Cargo.toml index ec53745..4e65d56 100644 --- a/crates/yuck/Cargo.toml +++ b/crates/yuck/Cargo.toml @@ -30,10 +30,10 @@ strum = { version = "0.21", features = ["derive"] } anyhow = "1" static_assertions = "1.1" - simplexpr = { path = "../simplexpr" } eww_shared_util = { path = "../eww_shared_util" } + [build-dependencies] lalrpop = "0.19.5" diff --git a/crates/yuck/src/config/backend_window_options.rs b/crates/yuck/src/config/backend_window_options.rs index d5ced8f..46eedfa 100644 --- a/crates/yuck/src/config/backend_window_options.rs +++ b/crates/yuck/src/config/backend_window_options.rs @@ -92,9 +92,7 @@ mod backend { } impl FromAstElementContent for StrutDefinition { - fn get_element_name() -> &'static str { - "struts" - } + const ELEMENT_NAME: &'static str = "struts"; fn from_tail>(span: Span, mut iter: AstIterator) -> AstResult { let mut attrs = iter.expect_key_values()?; diff --git a/crates/yuck/src/config/config.rs b/crates/yuck/src/config/config.rs index 9e94cb0..71d84bf 100644 --- a/crates/yuck/src/config/config.rs +++ b/crates/yuck/src/config/config.rs @@ -25,6 +25,15 @@ use crate::{ }; use eww_shared_util::{AttrName, Span, VarName}; +pub static TOP_LEVEL_DEFINITION_NAMES: &[&str] = &[ + WidgetDefinition::ELEMENT_NAME, + WindowDefinition::ELEMENT_NAME, + VarDefinition::ELEMENT_NAME, + ListenScriptVar::ELEMENT_NAME, + PollScriptVar::ELEMENT_NAME, + Include::ELEMENT_NAME, +]; + #[derive(Debug, PartialEq, Eq, Clone, serde::Serialize)] pub struct Include { pub path: String, @@ -32,9 +41,7 @@ pub struct Include { } impl FromAstElementContent for Include { - fn get_element_name() -> &'static str { - "include" - } + const ELEMENT_NAME: &'static str = "include"; fn from_tail>(span: Span, mut iter: AstIterator) -> AstResult { let (path_span, path) = iter.expect_literal()?; @@ -57,16 +64,16 @@ impl FromAst for TopLevel { let mut iter = e.try_ast_iter()?; let (sym_span, element_name) = iter.expect_symbol()?; Ok(match element_name.as_str() { - x if x == Include::get_element_name() => Self::Include(Include::from_tail(span, iter)?), - x if x == WidgetDefinition::get_element_name() => Self::WidgetDefinition(WidgetDefinition::from_tail(span, iter)?), - x if x == VarDefinition::get_element_name() => Self::VarDefinition(VarDefinition::from_tail(span, iter)?), - x if x == PollScriptVar::get_element_name() => { + x if x == Include::ELEMENT_NAME => Self::Include(Include::from_tail(span, iter)?), + x if x == WidgetDefinition::ELEMENT_NAME => Self::WidgetDefinition(WidgetDefinition::from_tail(span, iter)?), + x if x == VarDefinition::ELEMENT_NAME => Self::VarDefinition(VarDefinition::from_tail(span, iter)?), + x if x == PollScriptVar::ELEMENT_NAME => { Self::ScriptVarDefinition(ScriptVarDefinition::Poll(PollScriptVar::from_tail(span, iter)?)) } - x if x == ListenScriptVar::get_element_name() => { + x if x == ListenScriptVar::ELEMENT_NAME => { Self::ScriptVarDefinition(ScriptVarDefinition::Listen(ListenScriptVar::from_tail(span, iter)?)) } - x if x == WindowDefinition::get_element_name() => Self::WindowDefinition(WindowDefinition::from_tail(span, iter)?), + x if x == WindowDefinition::ELEMENT_NAME => Self::WindowDefinition(WindowDefinition::from_tail(span, iter)?), x => return Err(AstError::UnknownToplevel(sym_span, x.to_string())), }) } diff --git a/crates/yuck/src/config/script_var_definition.rs b/crates/yuck/src/config/script_var_definition.rs index d616dd3..bc48e51 100644 --- a/crates/yuck/src/config/script_var_definition.rs +++ b/crates/yuck/src/config/script_var_definition.rs @@ -61,9 +61,7 @@ pub struct PollScriptVar { } impl FromAstElementContent for PollScriptVar { - fn get_element_name() -> &'static str { - "defpoll" - } + const ELEMENT_NAME: &'static str = "defpoll"; fn from_tail>(span: Span, mut iter: AstIterator) -> AstResult { let result: AstResult<_> = try { @@ -91,9 +89,7 @@ pub struct ListenScriptVar { pub name_span: Span, } impl FromAstElementContent for ListenScriptVar { - fn get_element_name() -> &'static str { - "deflisten" - } + const ELEMENT_NAME: &'static str = "deflisten"; fn from_tail>(span: Span, mut iter: AstIterator) -> AstResult { let result: AstResult<_> = try { diff --git a/crates/yuck/src/config/var_definition.rs b/crates/yuck/src/config/var_definition.rs index 948d81a..77f6927 100644 --- a/crates/yuck/src/config/var_definition.rs +++ b/crates/yuck/src/config/var_definition.rs @@ -2,11 +2,14 @@ use std::collections::HashMap; use simplexpr::{dynval::DynVal, SimplExpr}; -use crate::{error::{AstResult, AstResultExt}, parser::{ +use crate::{ + error::{AstResult, AstResultExt}, + parser::{ ast::Ast, ast_iterator::AstIterator, from_ast::{FromAst, FromAstElementContent}, - }}; + }, +}; use eww_shared_util::{AttrName, Span, VarName}; #[derive(Debug, PartialEq, Eq, Clone, serde::Serialize)] @@ -17,9 +20,7 @@ pub struct VarDefinition { } impl FromAstElementContent for VarDefinition { - fn get_element_name() -> &'static str { - "defvar" - } + const ELEMENT_NAME: &'static str = "defvar"; fn from_tail>(span: Span, mut iter: AstIterator) -> AstResult { let result: AstResult<_> = try { diff --git a/crates/yuck/src/config/widget_definition.rs b/crates/yuck/src/config/widget_definition.rs index 02d6fdb..6dea6ee 100644 --- a/crates/yuck/src/config/widget_definition.rs +++ b/crates/yuck/src/config/widget_definition.rs @@ -23,9 +23,7 @@ pub struct WidgetDefinition { } impl FromAstElementContent for WidgetDefinition { - fn get_element_name() -> &'static str { - "defwidget" - } + const ELEMENT_NAME: &'static str = "defwidget"; fn from_tail>(span: Span, mut iter: AstIterator) -> AstResult { let (_, name) = iter.expect_symbol()?; diff --git a/crates/yuck/src/config/window_definition.rs b/crates/yuck/src/config/window_definition.rs index 58726ea..06b3578 100644 --- a/crates/yuck/src/config/window_definition.rs +++ b/crates/yuck/src/config/window_definition.rs @@ -27,9 +27,7 @@ pub struct WindowDefinition { } impl FromAstElementContent for WindowDefinition { - fn get_element_name() -> &'static str { - "defwindow" - } + const ELEMENT_NAME: &'static str = "defwindow"; fn from_tail>(span: Span, mut iter: AstIterator) -> AstResult { let (_, name) = iter.expect_symbol()?; diff --git a/crates/yuck/src/config/window_geometry.rs b/crates/yuck/src/config/window_geometry.rs index 49a5059..2d06eb4 100644 --- a/crates/yuck/src/config/window_geometry.rs +++ b/crates/yuck/src/config/window_geometry.rs @@ -117,9 +117,7 @@ pub struct WindowGeometry { } impl FromAstElementContent for WindowGeometry { - fn get_element_name() -> &'static str { - "geometry" - } + const ELEMENT_NAME: &'static str = "geometry"; fn from_tail>(span: Span, mut iter: AstIterator) -> AstResult { let mut attrs = iter.expect_key_values()?; diff --git a/crates/yuck/src/format_diagnostic.rs b/crates/yuck/src/format_diagnostic.rs index e7dbee0..c9b82dd 100644 --- a/crates/yuck/src/format_diagnostic.rs +++ b/crates/yuck/src/format_diagnostic.rs @@ -1,10 +1,12 @@ use codespan_reporting::{diagnostic, files}; +use config::TOP_LEVEL_DEFINITION_NAMES; +use itertools::Itertools; use simplexpr::dynval; use diagnostic::*; use crate::{ - config::{attributes::AttrError, validate::ValidationError}, + config::{attributes::AttrError, config, validate::ValidationError}, error::{get_parse_error_span, AstError}, }; @@ -75,12 +77,15 @@ impl ToDiagnostic for Diagnostic { impl ToDiagnostic for AstError { fn to_diagnostic(&self) -> Diagnostic { match self { - AstError::UnknownToplevel(span, name) => gen_diagnostic!(self, span), + AstError::UnknownToplevel(span, name) => gen_diagnostic! { + msg = self, + label = span, + note = format!("Must be one of: {}", TOP_LEVEL_DEFINITION_NAMES.iter().join(", ")) + }, AstError::MissingNode(span) => gen_diagnostic! { msg = "Expected another element", label = span => "Expected another element here", }, - AstError::WrongExprType(span, expected, actual) => gen_diagnostic! { msg = "Wrong type of expression", label = span => format!("Expected a `{}` here", expected), @@ -207,12 +212,18 @@ impl ToDiagnostic for simplexpr::eval::EvalError { use simplexpr::eval::EvalError::*; match self { NoVariablesAllowed(name) => gen_diagnostic!(self), - // 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 - UnknownVariable(name) => gen_diagnostic! { - msg = self, - note = format!("If you meant to use the literal value \"{}\", surround the value in quotes", name) - }, + UnknownVariable(name, similar) => { + let mut notes = Vec::new(); + if similar.len() == 1 { + notes.push(format!("Did you mean `{}`?", similar.first().unwrap())) + } else if similar.len() > 1 { + notes.push(format!("Did you mean one of: {}?", similar.iter().map(|x| format!("`{}`", x)).join(", "))) + } + // 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)); + gen_diagnostic!(self).with_notes(notes) + } Spanned(span, error) => error.as_ref().to_diagnostic().with_label(span_to_primary_label(*span)), _ => gen_diagnostic!(self, self.span()), } diff --git a/crates/yuck/src/parser/from_ast.rs b/crates/yuck/src/parser/from_ast.rs index 3aca5ea..09541ce 100644 --- a/crates/yuck/src/parser/from_ast.rs +++ b/crates/yuck/src/parser/from_ast.rs @@ -31,7 +31,7 @@ impl FromAst for String { /// A trait that allows creating a type from the tail of a list-node. /// I.e. to parse (foo [a b] (c d)), [from_tail] would just get [a b] (c d). pub trait FromAstElementContent: Sized { - fn get_element_name() -> &'static str; + const ELEMENT_NAME: &'static str; fn from_tail>(span: Span, iter: AstIterator) -> AstResult; } @@ -40,8 +40,8 @@ impl FromAst for T { let span = e.span(); let mut iter = e.try_ast_iter()?; let (element_name_span, element_name) = iter.expect_symbol()?; - if Self::get_element_name() != element_name { - return Err(AstError::MismatchedElementName(element_name_span, Self::get_element_name().to_string(), element_name)); + if Self::ELEMENT_NAME != element_name { + return Err(AstError::MismatchedElementName(element_name_span, Self::ELEMENT_NAME.to_string(), element_name)); } Self::from_tail(span, iter) }