From 20475ab0bf4f28511064195b71890a3cf5d188a1 Mon Sep 17 00:00:00 2001 From: +merlan #flirora Date: Tue, 11 Jun 2024 05:08:30 -0400 Subject: [PATCH] Add hint when string is used in place of label (#4330) --- crates/typst-syntax/src/lexer.rs | 15 +++++++++++++-- crates/typst-syntax/src/lib.rs | 3 ++- crates/typst/src/foundations/cast.rs | 27 ++++++++++++++++++++------- crates/typst/src/foundations/value.rs | 6 +----- tests/suite/layout/length.typ | 3 ++- tests/suite/model/cite.typ | 12 ++++++++++++ 6 files changed, 50 insertions(+), 16 deletions(-) diff --git a/crates/typst-syntax/src/lexer.rs b/crates/typst-syntax/src/lexer.rs index 6719deafd..3f409b2d0 100644 --- a/crates/typst-syntax/src/lexer.rs +++ b/crates/typst-syntax/src/lexer.rs @@ -408,7 +408,7 @@ impl Lexer<'_> { } fn ref_marker(&mut self) -> SyntaxKind { - self.s.eat_while(|c| is_id_continue(c) || matches!(c, ':' | '.')); + self.s.eat_while(is_valid_in_label_literal); // Don't include the trailing characters likely to be part of text. while matches!(self.s.scout(-1), Some('.' | ':')) { @@ -419,7 +419,7 @@ impl Lexer<'_> { } fn label(&mut self) -> SyntaxKind { - let label = self.s.eat_while(|c| is_id_continue(c) || matches!(c, ':' | '.')); + let label = self.s.eat_while(is_valid_in_label_literal); if label.is_empty() { return self.error("label cannot be empty"); } @@ -918,3 +918,14 @@ fn is_math_id_start(c: char) -> bool { fn is_math_id_continue(c: char) -> bool { is_xid_continue(c) && c != '_' } + +/// Whether a character can be part of a label literal's name. +#[inline] +fn is_valid_in_label_literal(c: char) -> bool { + is_id_continue(c) || matches!(c, ':' | '.') +} + +/// Returns true if this string is valid in a label literal. +pub fn is_valid_label_literal_id(id: &str) -> bool { + !id.is_empty() && id.chars().all(is_valid_in_label_literal) +} diff --git a/crates/typst-syntax/src/lib.rs b/crates/typst-syntax/src/lib.rs index 6a6063d28..5e7b710fc 100644 --- a/crates/typst-syntax/src/lib.rs +++ b/crates/typst-syntax/src/lib.rs @@ -19,7 +19,8 @@ pub use self::file::FileId; pub use self::highlight::{highlight, highlight_html, Tag}; pub use self::kind::SyntaxKind; pub use self::lexer::{ - is_id_continue, is_id_start, is_ident, is_newline, link_prefix, split_newlines, + is_id_continue, is_id_start, is_ident, is_newline, is_valid_label_literal_id, + link_prefix, split_newlines, }; pub use self::node::{LinkedChildren, LinkedNode, Side, SyntaxError, SyntaxNode}; pub use self::parser::{parse, parse_code, parse_math}; diff --git a/crates/typst/src/foundations/cast.rs b/crates/typst/src/foundations/cast.rs index b53576c25..51e361289 100644 --- a/crates/typst/src/foundations/cast.rs +++ b/crates/typst/src/foundations/cast.rs @@ -3,7 +3,7 @@ use std::fmt::Write; use std::hash::Hash; use std::ops::Add; -use ecow::{eco_format, EcoString}; +use ecow::eco_format; use smallvec::SmallVec; use unicode_math_class::MathClass; @@ -42,7 +42,7 @@ pub trait Reflect { /// dynamic checks instead of optimized machine code for each type). fn castable(value: &Value) -> bool; - /// Produce an error message for an inacceptable value type. + /// Produce an error message for an unacceptable value type. /// /// ```ignore /// assert_eq!( @@ -51,7 +51,7 @@ pub trait Reflect { /// ); /// ``` fn error(found: &Value) -> HintedString { - Self::input().error(found).into() + Self::input().error(found) } } @@ -300,7 +300,7 @@ pub enum CastInfo { impl CastInfo { /// Produce an error message describing what was expected and what was /// found. - pub fn error(&self, found: &Value) -> EcoString { + pub fn error(&self, found: &Value) -> HintedString { let mut matching_type = false; let mut parts = vec![]; @@ -328,13 +328,26 @@ impl CastInfo { write!(msg, "{}", found.ty()).unwrap(); } + let mut msg: HintedString = msg.into(); + if let Value::Int(i) = found { - if parts.iter().any(|p| p == "length") && !matching_type { - write!(msg, ": a length needs a unit - did you mean {i}pt?").unwrap(); + if !matching_type && parts.iter().any(|p| p == "length") { + msg.hint(eco_format!("a length needs a unit - did you mean {i}pt?")); + } + } else if let Value::Str(s) = found { + if !matching_type && parts.iter().any(|p| p == "label") { + if typst_syntax::is_valid_label_literal_id(s) { + msg.hint(eco_format!( + "use `<{s}>` or `label({})` to create a label", + s.repr() + )); + } else { + msg.hint(eco_format!("use `label({})` to create a label", s.repr())); + } } } - msg.into() + msg } /// Walk all contained non-union infos. diff --git a/crates/typst/src/foundations/value.rs b/crates/typst/src/foundations/value.rs index d2190ae76..47ce5d5dc 100644 --- a/crates/typst/src/foundations/value.rs +++ b/crates/typst/src/foundations/value.rs @@ -610,11 +610,7 @@ macro_rules! primitive { match value { Value::$variant(v) => Ok(v), $(Value::$other$(($binding))? => Ok($out),)* - v => Err(eco_format!( - "expected {}, found {}", - Type::of::(), - v.ty(), - ).into()), + v => Err(::error(&v)), } } } diff --git a/tests/suite/layout/length.typ b/tests/suite/layout/length.typ index 687556197..34a4b9397 100644 --- a/tests/suite/layout/length.typ +++ b/tests/suite/layout/length.typ @@ -45,7 +45,8 @@ } --- length-unit-hint --- -// Error: 1:17-1:19 expected length, found integer: a length needs a unit - did you mean 12pt? +// Error: 17-19 expected length, found integer +// Hint: 17-19 a length needs a unit - did you mean 12pt? #set text(size: 12) --- length-ignore-em-pt-hint --- diff --git a/tests/suite/model/cite.typ b/tests/suite/model/cite.typ index 24d6ad047..750db38ba 100644 --- a/tests/suite/model/cite.typ +++ b/tests/suite/model/cite.typ @@ -114,3 +114,15 @@ B #cite() #cite(). @mcintosh_anxiety #show bibliography: none #bibliography("/assets/bib/works.bib", style: "chicago-author-date") + +--- cite-type-error-hint --- +// Test hint for cast error from str to label +// Error: 7-15 expected label, found string +// Hint: 7-15 use `` or `label("netwok")` to create a label +#cite("netwok") + +--- cite-type-error-hint-invalid-literal --- +// Test hint for cast error from str to label +// Error: 7-17 expected label, found string +// Hint: 7-17 use `label("%@&#*!\\")` to create a label +#cite("%@&#*!\\")