Move span directly into diagnostics 🚚

src/diag.rs

@@ -1,58 +1,64 @@
-//! Diagnostics and decorations for source code.
+//! Diagnostics for source code.
 //!
 //! Errors are never fatal, the document will always compile and yield a layout.
 
-use crate::syntax::Spanned;
+use std::collections::BTreeSet;
 use std::fmt::{self, Display, Formatter};
 
-/// The result of some pass: Some output `T` and [`Feedback`] data.
+use crate::syntax::Span;
+
+/// The result of some pass: Some output `T` and diagnostics.
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct Pass<T> {
     /// The output of this compilation pass.
     pub output: T,
-    /// User feedback data accumulated in this pass.
+    /// User diagnostics accumulated in this pass.
-    pub feedback: Feedback,
+    pub diags: DiagSet,
 }
 
 impl<T> Pass<T> {
-    /// Create a new pass from output and feedback data.
+    /// Create a new pass from output and diagnostics.
-    pub fn new(output: T, feedback: Feedback) -> Self {
+    pub fn new(output: T, diags: DiagSet) -> Self {
-        Self { output, feedback }
+        Self { output, diags }
     }
 }
 
-/// Diagnostics and semantic syntax highlighting information.
+/// A set of diagnostics.
-#[derive(Debug, Default, Clone, Eq, PartialEq)]
+///
-pub struct Feedback {
+/// Since this is a [`BTreeSet`], there cannot be two equal (up to span)
-    /// Diagnostics about the source code.
+/// diagnostics and you can quickly iterate diagnostics in source location
-    pub diags: Vec<Spanned<Diag>>,
+/// order.
-    /// Decorations of the source code for semantic syntax highlighting.
+pub type DiagSet = BTreeSet<Diag>;
-    pub decos: Vec<Spanned<Deco>>,
-}
-
-impl Feedback {
-    /// Create a new feedback instance without errors and decos.
-    pub fn new() -> Self {
-        Self { diags: vec![], decos: vec![] }
-    }
-
-    /// Add other feedback data to this feedback.
-    pub fn extend(&mut self, more: Self) {
-        self.diags.extend(more.diags);
-        self.decos.extend(more.decos);
-    }
-}
 
 /// A diagnostic with severity level and message.
 #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize))]
 pub struct Diag {
+    /// The source code location.
+    pub span: Span,
     /// How severe / important the diagnostic is.
     pub level: Level,
     /// A message describing the diagnostic.
     pub message: String,
 }
 
+impl Diag {
+    /// Create a new diagnostic from message and level.
+    pub fn new(span: impl Into<Span>, level: Level, message: impl Into<String>) -> Self {
+        Self {
+            span: span.into(),
+            level,
+            message: message.into(),
+        }
+    }
+}
+
+impl Display for Diag {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+        write!(f, "{}: {}", self.level, self.message)
+    }
+}
+
 /// How severe / important a diagnostic is.
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize))]
@@ -62,13 +68,6 @@ pub enum Level {
     Error,
 }
 
-impl Diag {
-    /// Create a new diagnostic from message and level.
-    pub fn new(level: Level, message: impl Into<String>) -> Self {
-        Self { level, message: message.into() }
-    }
-}
-
 impl Display for Level {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         f.pad(match self {
@@ -78,17 +77,6 @@ impl Display for Level {
     }
 }
 
-/// Decorations for semantic syntax highlighting.
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize))]
-#[cfg_attr(feature = "serde", serde(rename_all = "camelCase"))]
-pub enum Deco {
-    /// Strong text.
-    Strong,
-    /// Emphasized text.
-    Emph,
-}
-
 /// Construct a diagnostic with [`Error`](Level::Error) level.
 ///
 /// ```
@@ -96,16 +84,16 @@ pub enum Deco {
 /// # use typst::syntax::Span;
 /// # let span = Span::ZERO;
 /// # let name = "";
-/// // Create formatted error values.
+/// let error = error!(span, "there is an error with {}", name);
-/// let error = error!("expected {}", name);
-///
-/// // Create spanned errors.
-/// let spanned = error!(span, "there is an error here");
 /// ```
 #[macro_export]
 macro_rules! error {
-    ($($tts:tt)*) => {
+    ($span:expr, $($tts:tt)*) => {
-        $crate::__impl_diagnostic!($crate::diag::Level::Error; $($tts)*)
+        $crate::diag::Diag::new(
+            $span,
+            $crate::diag::Level::Error,
+            format!($($tts)*),
+        )
     };
 }
 
@@ -115,23 +103,11 @@ macro_rules! error {
 /// information.
 #[macro_export]
 macro_rules! warning {
-    ($($tts:tt)*) => {
+    ($span:expr, $($tts:tt)*) => {
-        $crate::__impl_diagnostic!($crate::diag::Level::Warning; $($tts)*)
+        $crate::diag::Diag::new(
-    };
-}
-
-/// Backs the `error!` and `warning!` macros.
-#[macro_export]
-#[doc(hidden)]
-macro_rules! __impl_diagnostic {
-    ($level:expr; $fmt:literal $($tts:tt)*) => {
-        $crate::diag::Diag::new($level, format!($fmt $($tts)*))
-    };
-
-    ($level:expr; $span:expr, $fmt:literal $($tts:tt)*) => {
-        $crate::syntax::Spanned::new(
-            $crate::__impl_diagnostic!($level; $fmt $($tts)*),
             $span,
+            $crate::diag::Level::Warning,
+            format!($($tts)*),
         )
     };
 }

src/eval/mod.rs

@@ -15,7 +15,7 @@ use std::rc::Rc;
 
 use super::*;
 use crate::color::Color;
-use crate::diag::{Diag, Feedback};
+use crate::diag::{Diag, DiagSet};
 use crate::geom::{Angle, Length, Relative};
 use crate::syntax::visit::Visit;
 use crate::syntax::*;
@@ -27,14 +27,14 @@ use crate::syntax::*;
 pub fn eval(env: &mut Env, tree: &Tree, scope: &Scope) -> Pass<ExprMap> {
     let mut ctx = EvalContext::new(env, scope);
     let map = tree.eval(&mut ctx);
-    Pass::new(map, ctx.feedback)
+    Pass::new(map, ctx.diags)
 }
 
 /// A map from expression to values to evaluated to.
 ///
-/// The raw pointers point into the expressions contained in `tree`. Since
+/// The raw pointers point into the expressions contained in some [tree](Tree).
-/// the lifetime is erased, `tree` could go out of scope while the hash map
+/// Since the lifetime is erased, the tree could go out of scope while the hash
-/// still lives. While this could lead to lookup panics, it is not unsafe
+/// map still lives. Though this could lead to lookup panics, it is not unsafe
 /// since the pointers are never dereferenced.
 pub type ExprMap = HashMap<*const Expr, Value>;
 
@@ -45,8 +45,8 @@ pub struct EvalContext<'a> {
     pub env: &'a mut Env,
     /// The active scopes.
     pub scopes: Scopes<'a>,
-    /// The accumulated feedback.
+    /// Evaluation diagnostics.
-    feedback: Feedback,
+    pub diags: DiagSet,
 }
 
 impl<'a> EvalContext<'a> {
@@ -55,13 +55,13 @@ impl<'a> EvalContext<'a> {
         Self {
             env,
             scopes: Scopes::with_base(scope),
-            feedback: Feedback::new(),
+            diags: DiagSet::new(),
         }
     }
 
-    /// Add a diagnostic to the feedback.
+    /// Add a diagnostic.
-    pub fn diag(&mut self, diag: Spanned<Diag>) {
+    pub fn diag(&mut self, diag: Diag) {
-        self.feedback.diags.push(diag);
+        self.diags.insert(diag);
     }
 }
 

src/exec/context.rs

@@ -4,8 +4,7 @@ use std::rc::Rc;
 use fontdock::FontStyle;
 
 use super::*;
-use crate::diag::Diag;
+use crate::diag::{Diag, DiagSet};
-use crate::diag::{Deco, Feedback, Pass};
 use crate::geom::{ChildAlign, Dir, Gen, LayoutDirs, Length, Linear, Sides, Size};
 use crate::layout::{
     Expansion, Node, NodePad, NodePages, NodePar, NodeSpacing, NodeStack, NodeText, Tree,
@@ -18,8 +17,8 @@ pub struct ExecContext<'a> {
     pub env: &'a mut Env,
     /// The active execution state.
     pub state: State,
-    /// The accumulated feedback.
+    /// Execution diagnostics.
-    feedback: Feedback,
+    pub diags: DiagSet,
     /// The finished page runs.
     runs: Vec<NodePages>,
     /// The stack of logical groups (paragraphs and such).
@@ -39,27 +38,22 @@ impl<'a> ExecContext<'a> {
         Self {
             env,
             state,
+            diags: DiagSet::new(),
+            runs: vec![],
             groups: vec![],
             inner: vec![],
-            runs: vec![],
-            feedback: Feedback::new(),
         }
     }
 
     /// Finish execution and return the created layout tree.
     pub fn finish(self) -> Pass<Tree> {
         assert!(self.groups.is_empty(), "unfinished group");
-        Pass::new(Tree { runs: self.runs }, self.feedback)
+        Pass::new(Tree { runs: self.runs }, self.diags)
     }
 
-    /// Add a diagnostic to the feedback.
+    /// Add a diagnostic.
-    pub fn diag(&mut self, diag: Spanned<Diag>) {
+    pub fn diag(&mut self, diag: Diag) {
-        self.feedback.diags.push(diag);
+        self.diags.insert(diag);
-    }
-
-    /// Add a decoration to the feedback.
-    pub fn deco(&mut self, deco: Spanned<Deco>) {
-        self.feedback.decos.push(deco);
     }
 
     /// Push a layout node to the active group.

src/lib.rs

@@ -65,9 +65,9 @@ pub fn typeset(
     let executed = exec::exec(env, &parsed.output, &evaluated.output, state);
     let frames = layout::layout(env, &executed.output);
 
-    let mut feedback = parsed.feedback;
+    let mut diags = parsed.diags;
-    feedback.extend(evaluated.feedback);
+    diags.extend(evaluated.diags);
-    feedback.extend(executed.feedback);
+    diags.extend(executed.diags);
 
-    Pass::new(frames, feedback)
+    Pass::new(frames, diags)
 }

src/main.rs

@@ -4,7 +4,7 @@ use std::path::{Path, PathBuf};
 use anyhow::{anyhow, bail, Context};
 use fontdock::fs::FsIndex;
 
-use typst::diag::{Feedback, Pass};
+use typst::diag::Pass;
 use typst::env::{Env, ResourceLoader};
 use typst::exec::State;
 use typst::export::pdf;
@@ -48,26 +48,19 @@ fn main() -> anyhow::Result<()> {
     let scope = library::new();
     let state = State::default();
 
-    let Pass {
+    let Pass { output: frames, diags } = typeset(&mut env, &src, &scope, state);
-        output: frames,
-        feedback: Feedback { mut diags, .. },
-    } = typeset(&mut env, &src, &scope, state);
-
     if !diags.is_empty() {
-        diags.sort();
-
         let map = LineMap::new(&src);
         for diag in diags {
-            let span = diag.span;
+            let start = map.location(diag.span.start).unwrap();
-            let start = map.location(span.start).unwrap();
+            let end = map.location(diag.span.end).unwrap();
-            let end = map.location(span.end).unwrap();
             println!(
                 "{}: {}:{}-{}: {}",
-                diag.v.level,
+                diag.level,
                 src_path.display(),
                 start,
                 end,
-                diag.v.message,
+                diag.message,
             );
         }
     }

src/parse/mod.rs

@@ -22,7 +22,7 @@ use collection::{args, parenthesized};
 /// Parse a string of source code.
 pub fn parse(src: &str) -> Pass<Tree> {
     let mut p = Parser::new(src);
-    Pass::new(tree(&mut p), p.finish())
+    Pass::new(tree(&mut p), p.diags)
 }
 
 /// Parse a syntax tree.

src/parse/parser.rs

@@ -1,12 +1,13 @@
 use std::fmt::{self, Debug, Formatter};
 
 use super::{Scanner, TokenMode, Tokens};
-use crate::diag::Diag;
+use crate::diag::{Diag, DiagSet};
-use crate::diag::{Deco, Feedback};
+use crate::syntax::{Pos, Span, Token};
-use crate::syntax::{Pos, Span, Spanned, Token};
 
 /// A convenient token-based parser.
 pub struct Parser<'s> {
+    /// Parsing diagnostics.
+    pub diags: DiagSet,
     /// An iterator over the source tokens.
     tokens: Tokens<'s>,
     /// The next token.
@@ -20,8 +21,6 @@ pub struct Parser<'s> {
     last_end: Pos,
     /// The stack of open groups.
     groups: Vec<GroupEntry>,
-    /// Accumulated feedback.
-    feedback: Feedback,
 }
 
 /// A logical group of tokens, e.g. `[...]`.
@@ -67,18 +66,13 @@ impl<'s> Parser<'s> {
             next_start: Pos::ZERO,
             last_end: Pos::ZERO,
             groups: vec![],
-            feedback: Feedback::new(),
+            diags: DiagSet::new(),
         }
     }
 
-    /// Finish parsing and return the accumulated feedback.
+    /// Add a diagnostic.
-    pub fn finish(self) -> Feedback {
+    pub fn diag(&mut self, diag: Diag) {
-        self.feedback
+        self.diags.insert(diag);
-    }
-
-    /// Add a diagnostic to the feedback.
-    pub fn diag(&mut self, diag: Spanned<Diag>) {
-        self.feedback.diags.push(diag);
     }
 
     /// Eat the next token and add a diagnostic that it is not the expected
@@ -112,11 +106,6 @@ impl<'s> Parser<'s> {
         }
     }
 
-    /// Add a decoration to the feedback.
-    pub fn deco(&mut self, deco: Spanned<Deco>) {
-        self.feedback.decos.push(deco);
-    }
-
     /// Continue parsing in a group.
     ///
     /// When the end delimiter of the group is reached, all subsequent calls to

src/prelude.rs

@@ -1,6 +1,6 @@
 //! A prelude for building custom functions.
 
-pub use crate::diag::{Feedback, Pass};
+pub use crate::diag::{Diag, Pass};
 #[doc(no_inline)]
 pub use crate::eval::{
     CastResult, Eval, EvalContext, TemplateAny, TemplateNode, Value, ValueAny, ValueArgs,

src/pretty.rs

@@ -1,6 +1,6 @@
 //! Pretty printing.
 
-use std::fmt::{Arguments, Result, Write};
+use std::fmt::{self, Arguments, Write};
 
 use crate::color::{Color, RgbaColor};
 use crate::eval::*;
@@ -70,7 +70,7 @@ impl Printer {
     }
 
     /// Write formatted items into the buffer.
-    pub fn write_fmt(&mut self, fmt: Arguments<'_>) -> Result {
+    pub fn write_fmt(&mut self, fmt: Arguments<'_>) -> fmt::Result {
         Write::write_fmt(self, fmt)
     }
 
@@ -97,7 +97,7 @@ impl Printer {
 }
 
 impl Write for Printer {
-    fn write_str(&mut self, s: &str) -> Result {
+    fn write_str(&mut self, s: &str) -> fmt::Result {
         self.push_str(s);
         Ok(())
     }
@@ -484,6 +484,7 @@ impl Pretty for Value {
             Value::Relative(v) => v.pretty(p),
             Value::Linear(v) => v.pretty(p),
             Value::Color(v) => v.pretty(p),
+            // TODO: Handle like text when directly in template.
             Value::Str(v) => v.pretty(p),
             Value::Array(v) => v.pretty(p),
             Value::Dict(v) => v.pretty(p),

tests/typeset.rs

@@ -14,7 +14,7 @@ use tiny_skia::{
 use ttf_parser::OutlineBuilder;
 use walkdir::WalkDir;
 
-use typst::diag::{Diag, Feedback, Level, Pass};
+use typst::diag::{Diag, DiagSet, Level, Pass};
 use typst::env::{Env, ImageResource, ResourceLoader};
 use typst::eval::{EvalContext, Scope, Value, ValueArgs, ValueFunc};
 use typst::exec::State;
@@ -25,7 +25,7 @@ use typst::layout::{Element, Expansion, Fill, Frame, Geometry, Image, Shape};
 use typst::library;
 use typst::parse::{LineMap, Scanner};
 use typst::shaping::Shaped;
-use typst::syntax::{Location, Pos, Spanned};
+use typst::syntax::{Location, Pos};
 use typst::typeset;
 
 const TYP_DIR: &str = "typ";
@@ -219,17 +219,11 @@ fn test_part(
     state.page.expand = Spec::new(Expansion::Fill, Expansion::Fit);
     state.page.margins = Sides::uniform(Some(Length::pt(10.0).into()));
 
-    let Pass {
+    let Pass { output: mut frames, diags } = typeset(env, &src, &scope, state);
-        output: mut frames,
-        feedback: Feedback { mut diags, .. },
-    } = typeset(env, &src, &scope, state);
-
     if !compare_ref {
         frames.clear();
     }
 
-    diags.sort_by_key(|d| d.span);
-
     let mut ok = true;
 
     for panic in &*panics.borrow() {
@@ -266,8 +260,8 @@ fn test_part(
     (ok, frames)
 }
 
-fn parse_metadata(src: &str, map: &LineMap) -> (Option<bool>, Vec<Spanned<Diag>>) {
+fn parse_metadata(src: &str, map: &LineMap) -> (Option<bool>, DiagSet) {
-    let mut diags = vec![];
+    let mut diags = DiagSet::new();
     let mut compare_ref = None;
 
     for (i, line) in src.lines().enumerate() {
@@ -303,13 +297,9 @@ fn parse_metadata(src: &str, map: &LineMap) -> (Option<bool>, Vec<Spanned<Diag>>
 
         let mut s = Scanner::new(rest);
         let (start, _, end) = (pos(&mut s), s.eat_assert('-'), pos(&mut s));
-
+        diags.insert(Diag::new(start .. end, level, s.rest().trim()));
-        let diag = Diag::new(level, s.rest().trim());
-        diags.push(Spanned::new(diag, start .. end));
     }
 
-    diags.sort_by_key(|d| d.span);
-
     (compare_ref, diags)
 }
 
@@ -341,12 +331,12 @@ fn register_helpers(scope: &mut Scope, panics: Rc<RefCell<Vec<Panic>>>) {
     scope.def_const("test", ValueFunc::new("test", test));
 }
 
-fn print_diag(diag: &Spanned<Diag>, map: &LineMap, lines: u32) {
+fn print_diag(diag: &Diag, map: &LineMap, lines: u32) {
     let mut start = map.location(diag.span.start).unwrap();
     let mut end = map.location(diag.span.end).unwrap();
     start.line += lines;
     end.line += lines;
-    println!("{}: {}-{}: {}", diag.v.level, start, end, diag.v.message);
+    println!("{}: {}-{}: {}", diag.level, start, end, diag.message);
 }
 
 fn draw(frames: &[Frame], env: &Env, pixel_per_pt: f32) -> Canvas {