Add hints for invalid identifier errors (#2583)

6 files changed, 51 insertions, 23 deletions

diff --git a/crates/typst-syntax/src/parser.rs b/crates/typst-syntax/src/parser.rs
index f1c798c6..9397952f 100644
--- a/crates/typst-syntax/src/parser.rs
+++ b/crates/typst-syntax/src/parser.rs
@@ -1685,7 +1685,14 @@ impl<'s> Parser<'s> {
         if at {
             self.eat();
         } else if kind == SyntaxKind::Ident && self.current.is_keyword() {
-            self.expected_found(kind.name(), self.current.name());
+            let found_text = self.current_text();
+            let found = self.current.name();
+            self.expected_found(kind.name(), found);
+            self.hint(eco_format!(
+                "{} is not allowed as an identifier; try `{}_` instead",
+                found,
+                found_text
+            ));
         } else {
             self.balanced &= !kind.is_grouping();
             self.expected(kind.name());
diff --git a/crates/typst/src/diag.rs b/crates/typst/src/diag.rs
index d5589678..830237e3 100644
--- a/crates/typst/src/diag.rs
+++ b/crates/typst/src/diag.rs
@@ -225,7 +225,8 @@ impl<T> Trace<T> for SourceResult<T> {
 /// A result type with a string error message.
 pub type StrResult<T> = Result<T, EcoString>;
 
-/// Convert a [`StrResult`] to a [`SourceResult`] by adding span information.
+/// Convert a [`StrResult`] or [`HintedStrResult`] to a [`SourceResult`] by
+/// adding span information.
 pub trait At<T> {
     /// Add the span information.
     fn at(self, span: Span) -> SourceResult<T>;
@@ -261,6 +262,12 @@ pub struct HintedString {
     pub hints: Vec<EcoString>,
 }
 
+impl From<EcoString> for HintedString {
+    fn from(value: EcoString) -> Self {
+        Self { message: value, hints: vec![] }
+    }
+}
+
 impl<T> At<T> for Result<T, HintedString> {
     fn at(self, span: Span) -> SourceResult<T> {
         self.map_err(|diags| {
diff --git a/crates/typst/src/eval/func.rs b/crates/typst/src/eval/func.rs
index 8660abac..2311b2cf 100644
--- a/crates/typst/src/eval/func.rs
+++ b/crates/typst/src/eval/func.rs
@@ -9,7 +9,7 @@ use super::{
     cast, scope, ty, Args, CastInfo, Eval, FlowEvent, IntoValue, Route, Scope, Scopes,
     Tracer, Type, Value, Vm,
 };
-use crate::diag::{bail, SourceResult, StrResult};
+use crate::diag::{bail, HintedStrResult, SourceResult, StrResult};
 use crate::model::{
     Content, DelayedErrors, Element, Introspector, Locator, Selector, Vt,
 };
@@ -779,7 +779,7 @@ impl<'a> CapturesVisitor<'a> {
     fn capture(
         &mut self,
         ident: &str,
-        getter: impl FnOnce(&'a Scopes<'a>, &str) -> StrResult<&'a Value>,
+        getter: impl FnOnce(&'a Scopes<'a>, &str) -> HintedStrResult<&'a Value>,
     ) {
         if self.internal.get(ident).is_err() {
             let Some(value) = self
diff --git a/crates/typst/src/eval/scope.rs b/crates/typst/src/eval/scope.rs
index 85bef53a..71e29fde 100644
--- a/crates/typst/src/eval/scope.rs
+++ b/crates/typst/src/eval/scope.rs
@@ -7,7 +7,7 @@ use indexmap::IndexMap;
 use super::{
     Func, IntoValue, Library, Module, NativeFunc, NativeFuncData, NativeType, Type, Value,
 };
-use crate::diag::{bail, StrResult};
+use crate::diag::{bail, HintedStrResult, HintedString, StrResult};
 use crate::model::{Element, NativeElement};
 
 /// A stack of scopes.
@@ -40,7 +40,7 @@ impl<'a> Scopes<'a> {
     }
 
     /// Try to access a variable immutably.
-    pub fn get(&self, var: &str) -> StrResult<&Value> {
+    pub fn get(&self, var: &str) -> HintedStrResult<&Value> {
         std::iter::once(&self.top)
             .chain(self.scopes.iter().rev())
             .chain(self.base.map(|base| base.global.scope()))
@@ -49,22 +49,22 @@ impl<'a> Scopes<'a> {
     }
 
     /// Try to access a variable immutably in math.
-    pub fn get_in_math(&self, var: &str) -> StrResult<&Value> {
+    pub fn get_in_math(&self, var: &str) -> HintedStrResult<&Value> {
         std::iter::once(&self.top)
             .chain(self.scopes.iter().rev())
             .chain(self.base.map(|base| base.math.scope()))
             .find_map(|scope| scope.get(var))
-            .ok_or_else(|| eco_format!("unknown variable: {}", var))
+            .ok_or_else(|| unknown_variable(var))
     }
 
     /// Try to access a variable mutably.
-    pub fn get_mut(&mut self, var: &str) -> StrResult<&mut Value> {
+    pub fn get_mut(&mut self, var: &str) -> HintedStrResult<&mut Value> {
         std::iter::once(&mut self.top)
             .chain(&mut self.scopes.iter_mut().rev())
             .find_map(|scope| scope.get_mut(var))
             .ok_or_else(|| {
                 match self.base.and_then(|base| base.global.scope().get(var)) {
-                    Some(_) => eco_format!("cannot mutate a constant: {}", var),
+                    Some(_) => eco_format!("cannot mutate a constant: {}", var).into(),
                     _ => unknown_variable(var),
                 }
             })?
@@ -73,16 +73,23 @@ impl<'a> Scopes<'a> {
 
 /// The error message when a variable is not found.
 #[cold]
-fn unknown_variable(var: &str) -> EcoString {
-    if var.contains('-') {
-        eco_format!(
-            "unknown variable: {} - if you meant to use subtraction, \
-             try adding spaces around the minus sign.",
-            var
-        )
-    } else {
-        eco_format!("unknown variable: {}", var)
+fn unknown_variable(var: &str) -> HintedString {
+    let mut res = HintedString {
+        message: eco_format!("unknown variable: {}", var),
+        hints: vec![],
+    };
+
+    if matches!(var, "none" | "auto" | "false" | "true") {
+        res.hints.push(eco_format!(
+            "if you meant to use a literal, try adding a hash before it"
+        ));
+    } else if var.contains('-') {
+        res.hints.push(eco_format!(
+            "if you meant to use subtraction, try adding spaces around the minus sign",
+        ));
     }
+
+    res
 }
 
 /// A map from binding names to values.
@@ -170,8 +177,11 @@ impl Scope {
     }
 
     /// Try to access a variable mutably.
-    pub fn get_mut(&mut self, var: &str) -> Option<StrResult<&mut Value>> {
-        self.map.get_mut(var).map(Slot::write)
+    pub fn get_mut(&mut self, var: &str) -> Option<HintedStrResult<&mut Value>> {
+        self.map
+            .get_mut(var)
+            .map(Slot::write)
+            .map(|res| res.map_err(HintedString::from))
     }
 
     /// Get the category of a definition.
diff --git a/tests/typ/compiler/embedded-expr.typ b/tests/typ/compiler/embedded-expr.typ
index ecdd86a9..c95184e4 100644
--- a/tests/typ/compiler/embedded-expr.typ
+++ b/tests/typ/compiler/embedded-expr.typ
@@ -3,11 +3,13 @@
 
 ---
 // Error: 6-8 expected identifier, found keyword `as`
+// Hint: 6-8 keyword `as` is not allowed as an identifier; try `as_` instead
 #let as = 1 + 2
 
 ---
 #{
   // Error: 7-9 expected identifier, found keyword `as`
+  // Hint: 7-9 keyword `as` is not allowed as an identifier; try `as_` instead
   let as = 10
 }
 
diff --git a/tests/typ/compiler/hint.typ b/tests/typ/compiler/hint.typ
index 6b60eafb..1a5efcaa 100644
--- a/tests/typ/compiler/hint.typ
+++ b/tests/typ/compiler/hint.typ
@@ -11,13 +11,15 @@
   a = 1-a
   a = a -1
 
-  // Error: 7-10 unknown variable: a-1 - if you meant to use subtraction, try adding spaces around the minus sign.
+  // Error: 7-10 unknown variable: a-1
+  // Hint: 7-10 if you meant to use subtraction, try adding spaces around the minus sign
   a = a-1
 }
 
 ---
 #{
-  // Error: 3-6 unknown variable: a-1 - if you meant to use subtraction, try adding spaces around the minus sign.
+  // Error: 3-6 unknown variable: a-1
+  // Hint: 3-6 if you meant to use subtraction, try adding spaces around the minus sign
   a-1 = 2
 }