From a71a2057f286677b5bf2e064fea05024aeca0dd2 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Tue, 29 Aug 2023 17:35:35 +0200 Subject: [PATCH] More type safety for spans --- Cargo.lock | 1 + crates/typst-cli/src/compile.rs | 20 +- crates/typst-library/src/compute/construct.rs | 2 +- crates/typst-library/src/compute/data.rs | 12 +- crates/typst-library/src/meta/bibliography.rs | 2 +- crates/typst-library/src/text/raw.rs | 4 +- crates/typst-library/src/visualize/image.rs | 2 +- crates/typst-syntax/src/file.rs | 44 +--- crates/typst-syntax/src/node.rs | 6 +- crates/typst-syntax/src/reparser.rs | 4 +- crates/typst-syntax/src/source.rs | 24 +-- crates/typst-syntax/src/span.rs | 75 +++---- crates/typst/src/diag.rs | 16 +- crates/typst/src/eval/func.rs | 11 +- crates/typst/src/eval/mod.rs | 43 ++-- crates/typst/src/eval/tracer.rs | 2 +- crates/typst/src/ide/analyze.rs | 2 +- crates/typst/src/ide/highlight.rs | 6 +- crates/typst/src/ide/jump.rs | 14 +- crates/typst/src/lib.rs | 16 +- crates/typst/src/model/mod.rs | 8 +- tests/Cargo.toml | 1 + tests/src/tests.rs | 198 +++++++++--------- tests/typ/meta/state.typ | 10 +- 24 files changed, 244 insertions(+), 279 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 80a99eeec..00bf3e41f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2990,6 +2990,7 @@ version = "0.7.0" dependencies = [ "clap", "comemo", + "ecow", "iai", "once_cell", "oxipng", diff --git a/crates/typst-cli/src/compile.rs b/crates/typst-cli/src/compile.rs index 4c4c24be5..5ae569ba3 100644 --- a/crates/typst-cli/src/compile.rs +++ b/crates/typst-cli/src/compile.rs @@ -8,8 +8,8 @@ use typst::diag::{bail, Severity, SourceDiagnostic, StrResult}; use typst::doc::Document; use typst::eval::{eco_format, Tracer}; use typst::geom::Color; -use typst::syntax::{FileId, Source}; -use typst::World; +use typst::syntax::{FileId, Source, Span}; +use typst::{World, WorldExt}; use crate::args::{CompileCommand, DiagnosticFormat, OutputFormat}; use crate::watch::Status; @@ -231,19 +231,16 @@ pub fn print_diagnostics( .map(|e| (eco_format!("hint: {e}")).into()) .collect(), ) - .with_labels(vec![Label::primary( - diagnostic.span.id(), - world.range(diagnostic.span), - )]); + .with_labels(label(world, diagnostic.span).into_iter().collect()); term::emit(&mut w, &config, world, &diag)?; // Stacktrace-like helper diagnostics. for point in &diagnostic.trace { let message = point.v.to_string(); - let help = Diagnostic::help().with_message(message).with_labels(vec![ - Label::primary(point.span.id(), world.range(point.span)), - ]); + let help = Diagnostic::help() + .with_message(message) + .with_labels(label(world, point.span).into_iter().collect()); term::emit(&mut w, &config, world, &help)?; } @@ -252,6 +249,11 @@ pub fn print_diagnostics( Ok(()) } +/// Create a label for a span. +fn label(world: &SystemWorld, span: Span) -> Option> { + Some(Label::primary(span.id()?, world.range(span)?)) +} + impl<'a> codespan_reporting::files::Files<'a> for SystemWorld { type FileId = FileId; type Name = String; diff --git a/crates/typst-library/src/compute/construct.rs b/crates/typst-library/src/compute/construct.rs index 59a867a79..23f0c225d 100644 --- a/crates/typst-library/src/compute/construct.rs +++ b/crates/typst-library/src/compute/construct.rs @@ -935,7 +935,7 @@ pub fn plugin( vm: &mut Vm, ) -> SourceResult { let Spanned { v: path, span } = path; - let id = vm.location().join(&path).at(span)?; + let id = vm.resolve_path(&path).at(span)?; let data = vm.world().file(id).at(span)?; Plugin::new(data).at(span) } diff --git a/crates/typst-library/src/compute/data.rs b/crates/typst-library/src/compute/data.rs index 944baff36..cd28d61e0 100644 --- a/crates/typst-library/src/compute/data.rs +++ b/crates/typst-library/src/compute/data.rs @@ -37,7 +37,7 @@ pub fn read( vm: &mut Vm, ) -> SourceResult { let Spanned { v: path, span } = path; - let id = vm.location().join(&path).at(span)?; + let id = vm.resolve_path(&path).at(span)?; let data = vm.world().file(id).at(span)?; Ok(match encoding { None => Readable::Bytes(data), @@ -130,7 +130,7 @@ pub fn csv( vm: &mut Vm, ) -> SourceResult { let Spanned { v: path, span } = path; - let id = vm.location().join(&path).at(span)?; + let id = vm.resolve_path(&path).at(span)?; let data = vm.world().file(id).at(span)?; csv_decode(Spanned::new(Readable::Bytes(data), span), delimiter) } @@ -262,7 +262,7 @@ pub fn json( vm: &mut Vm, ) -> SourceResult { let Spanned { v: path, span } = path; - let id = vm.location().join(&path).at(span)?; + let id = vm.resolve_path(&path).at(span)?; let data = vm.world().file(id).at(span)?; json_decode(Spanned::new(Readable::Bytes(data), span)) } @@ -350,7 +350,7 @@ pub fn toml( vm: &mut Vm, ) -> SourceResult { let Spanned { v: path, span } = path; - let id = vm.location().join(&path).at(span)?; + let id = vm.resolve_path(&path).at(span)?; let data = vm.world().file(id).at(span)?; toml_decode(Spanned::new(Readable::Bytes(data), span)) @@ -462,7 +462,7 @@ pub fn yaml( vm: &mut Vm, ) -> SourceResult { let Spanned { v: path, span } = path; - let id = vm.location().join(&path).at(span)?; + let id = vm.resolve_path(&path).at(span)?; let data = vm.world().file(id).at(span)?; yaml_decode(Spanned::new(Readable::Bytes(data), span)) } @@ -568,7 +568,7 @@ pub fn xml( vm: &mut Vm, ) -> SourceResult { let Spanned { v: path, span } = path; - let id = vm.location().join(&path).at(span)?; + let id = vm.resolve_path(&path).at(span)?; let data = vm.world().file(id).at(span)?; xml_decode(Spanned::new(Readable::Bytes(data), span)) } diff --git a/crates/typst-library/src/meta/bibliography.rs b/crates/typst-library/src/meta/bibliography.rs index ed51058d9..164b1c11f 100644 --- a/crates/typst-library/src/meta/bibliography.rs +++ b/crates/typst-library/src/meta/bibliography.rs @@ -58,7 +58,7 @@ pub struct BibliographyElem { let data = paths.0 .iter() .map(|path| { - let id = vm.location().join(path).at(span)?; + let id = vm.resolve_path(path).at(span)?; vm.world().file(id).at(span) }) .collect::>>()?; diff --git a/crates/typst-library/src/text/raw.rs b/crates/typst-library/src/text/raw.rs index 063461a40..1b4a42236 100644 --- a/crates/typst-library/src/text/raw.rs +++ b/crates/typst-library/src/text/raw.rs @@ -490,7 +490,7 @@ fn parse_syntaxes( .0 .iter() .map(|path| { - let id = vm.location().join(path).at(span)?; + let id = vm.resolve_path(path).at(span)?; vm.world().file(id).at(span) }) .collect::>>()?; @@ -522,7 +522,7 @@ fn parse_theme( }; // Load theme file. - let id = vm.location().join(&path).at(span)?; + let id = vm.resolve_path(&path).at(span)?; let data = vm.world().file(id).at(span)?; // Check that parsing works. diff --git a/crates/typst-library/src/visualize/image.rs b/crates/typst-library/src/visualize/image.rs index 6ee930a74..5e6ac2d24 100644 --- a/crates/typst-library/src/visualize/image.rs +++ b/crates/typst-library/src/visualize/image.rs @@ -44,7 +44,7 @@ pub struct ImageElem { #[parse( let Spanned { v: path, span } = args.expect::>("path to image file")?; - let id = vm.location().join(&path).at(span)?; + let id = vm.resolve_path(&path).at(span)?; let data = vm.world().file(id).at(span)?; path )] diff --git a/crates/typst-syntax/src/file.rs b/crates/typst-syntax/src/file.rs index 6b3117cfb..f5fa493b2 100644 --- a/crates/typst-syntax/src/file.rs +++ b/crates/typst-syntax/src/file.rs @@ -16,9 +16,6 @@ use super::is_ident; static INTERNER: Lazy> = Lazy::new(|| RwLock::new(Interner { to_id: HashMap::new(), from_id: Vec::new() })); -/// The path that we use for detached file ids. -static DETACHED_PATH: Lazy = Lazy::new(|| VirtualPath::new("/unknown")); - /// A package-path interner. struct Interner { to_id: HashMap, @@ -48,66 +45,41 @@ impl FileId { } let mut interner = INTERNER.write().unwrap(); - let len = interner.from_id.len(); - if len >= usize::from(u16::MAX) { - panic!("too many file specifications"); - } + let num = interner.from_id.len().try_into().expect("out of file ids"); // Create a new entry forever by leaking the pair. We can't leak more // than 2^16 pair (and typically will leak a lot less), so its not a // big deal. - let id = FileId(len as u16); + let id = FileId(num); let leaked = Box::leak(Box::new(pair)); interner.to_id.insert(leaked, id); interner.from_id.push(leaked); id } - /// Get an id that does not identify any real file. - pub const fn detached() -> Self { - Self(u16::MAX) - } - - /// Whether the id is the detached. - pub const fn is_detached(self) -> bool { - self.0 == Self::detached().0 - } - /// The package the file resides in, if any. pub fn package(&self) -> Option<&'static PackageSpec> { - if self.is_detached() { - None - } else { - self.pair().0.as_ref() - } + self.pair().0.as_ref() } /// The absolute and normalized path to the file _within_ the project or /// package. pub fn vpath(&self) -> &'static VirtualPath { - if self.is_detached() { - &DETACHED_PATH - } else { - &self.pair().1 - } + &self.pair().1 } /// Resolve a file location relative to this file. - pub fn join(self, path: &str) -> Result { - if self.is_detached() { - Err("cannot access file system from here")?; - } - - Ok(Self::new(self.package().cloned(), self.vpath().join(path))) + pub fn join(self, path: &str) -> Self { + Self::new(self.package().cloned(), self.vpath().join(path)) } /// Construct from a raw number. - pub(crate) const fn from_u16(v: u16) -> Self { + pub(crate) const fn from_raw(v: u16) -> Self { Self(v) } /// Extract the raw underlying number. - pub(crate) const fn as_u16(self) -> u16 { + pub(crate) const fn into_raw(self) -> u16 { self.0 } diff --git a/crates/typst-syntax/src/node.rs b/crates/typst-syntax/src/node.rs index 8e4e056ec..77c73f585 100644 --- a/crates/typst-syntax/src/node.rs +++ b/crates/typst-syntax/src/node.rs @@ -202,7 +202,7 @@ impl SyntaxNode { return Err(Unnumberable); } - let mid = Span::new(id, (within.start + within.end) / 2); + let mid = Span::new(id, (within.start + within.end) / 2).unwrap(); match &mut self.0 { Repr::Leaf(leaf) => leaf.span = mid, Repr::Inner(inner) => Arc::make_mut(inner).numberize(id, None, within)?, @@ -424,7 +424,7 @@ impl InnerNode { let mut start = within.start; if range.is_none() { let end = start + stride; - self.span = Span::new(id, (start + end) / 2); + self.span = Span::new(id, (start + end) / 2).unwrap(); self.upper = within.end; start = end; } @@ -448,6 +448,7 @@ impl InnerNode { mut range: Range, replacement: Vec, ) -> NumberingResult { + let Some(id) = self.span.id() else { return Err(Unnumberable) }; let superseded = &self.children[range.clone()]; // Compute the new byte length. @@ -505,7 +506,6 @@ impl InnerNode { // Try to renumber. let within = start_number..end_number; - let id = self.span.id(); if self.numberize(id, Some(renumber), within).is_ok() { return Ok(()); } diff --git a/crates/typst-syntax/src/reparser.rs b/crates/typst-syntax/src/reparser.rs index a4186fa73..e03e1619c 100644 --- a/crates/typst-syntax/src/reparser.rs +++ b/crates/typst-syntax/src/reparser.rs @@ -21,7 +21,9 @@ pub fn reparse( try_reparse(text, replaced, replacement_len, None, root, 0).unwrap_or_else(|| { let id = root.span().id(); *root = parse(text); - root.numberize(id, Span::FULL).unwrap(); + if let Some(id) = id { + root.numberize(id, Span::FULL).unwrap(); + } 0..text.len() }) } diff --git a/crates/typst-syntax/src/source.rs b/crates/typst-syntax/src/source.rs index 036499aba..56b271955 100644 --- a/crates/typst-syntax/src/source.rs +++ b/crates/typst-syntax/src/source.rs @@ -9,6 +9,7 @@ use comemo::Prehashed; use super::reparser::reparse; use super::{is_newline, parse, FileId, LinkedNode, Span, SyntaxNode}; +use crate::VirtualPath; /// A source file. /// @@ -44,19 +45,7 @@ impl Source { /// Create a source file without a real id and path, usually for testing. pub fn detached(text: impl Into) -> Self { - Self::new(FileId::detached(), text.into()) - } - - /// Create a source file with the same synthetic span for all nodes. - pub fn synthesized(text: String, span: Span) -> Self { - let mut root = parse(&text); - root.synthesize(span); - Self(Arc::new(Repr { - id: FileId::detached(), - lines: lines(&text), - text: Prehashed::new(text), - root: Prehashed::new(root), - })) + Self::new(FileId::new(None, VirtualPath::new("main.typ")), text.into()) } /// The root node of the file's untyped syntax tree. @@ -151,12 +140,9 @@ impl Source { /// Get the byte range for the given span in this file. /// - /// Panics if the span does not point into this source file. - #[track_caller] - pub fn range(&self, span: Span) -> Range { - self.find(span) - .expect("span does not point into this source file") - .range() + /// Returns `None` if the span does not point into this source file. + pub fn range(&self, span: Span) -> Option> { + Some(self.find(span)?.range()) } /// Return the index of the UTF-16 code unit at the byte index. diff --git a/crates/typst-syntax/src/span.rs b/crates/typst-syntax/src/span.rs index 8715e476b..d715af1c9 100644 --- a/crates/typst-syntax/src/span.rs +++ b/crates/typst-syntax/src/span.rs @@ -28,54 +28,57 @@ pub struct Span(NonZeroU64); impl Span { /// The full range of numbers available for span numbering. - pub const FULL: Range = 2..(1 << Self::BITS); + pub(super) const FULL: Range = 2..(1 << Self::BITS); + + /// The value reserved for the detached span. const DETACHED: u64 = 1; - // Data layout: - // | 16 bits source id | 48 bits number | + /// Data layout: + /// | 16 bits source id | 48 bits number | const BITS: usize = 48; /// Create a new span from a source id and a unique number. /// - /// Panics if the `number` is not contained in `FULL`. - #[track_caller] - pub const fn new(id: FileId, number: u64) -> Self { - assert!( - Self::FULL.start <= number && number < Self::FULL.end, - "span number outside valid range" - ); + /// Returns `None` if `number` is not contained in `FULL`. + pub(super) const fn new(id: FileId, number: u64) -> Option { + if number < Self::FULL.start || number >= Self::FULL.end { + return None; + } - Self::pack(id, number) - } - - /// A span that does not point into any source file. - pub const fn detached() -> Self { - Self::pack(FileId::detached(), Self::DETACHED) - } - - /// Pack the components into a span. - #[track_caller] - const fn pack(id: FileId, number: u64) -> Span { - let bits = ((id.as_u16() as u64) << Self::BITS) | number; + let bits = ((id.into_raw() as u64) << Self::BITS) | number; match NonZeroU64::new(bits) { - Some(v) => Self(v), - None => panic!("span encoding is zero"), + Some(v) => Some(Self(v)), + None => unreachable!(), } } - /// The id of the source file the span points into. - pub const fn id(self) -> FileId { - FileId::from_u16((self.0.get() >> Self::BITS) as u16) - } - - /// The unique number of the span within its source file. - pub const fn number(self) -> u64 { - self.0.get() & ((1 << Self::BITS) - 1) + /// Create a span that does not point into any source file. + pub const fn detached() -> Self { + match NonZeroU64::new(Self::DETACHED) { + Some(v) => Self(v), + None => unreachable!(), + } } /// Whether the span is detached. pub const fn is_detached(self) -> bool { - self.id().is_detached() + self.0.get() == Self::DETACHED + } + + /// The id of the source file the span points into. + /// + /// Returns `None` if the span is detached. + pub const fn id(self) -> Option { + if self.is_detached() { + return None; + } + let bits = (self.0.get() >> Self::BITS) as u16; + Some(FileId::from_raw(bits)) + } + + /// The unique number of the span within its [`Source`](super::Source). + pub const fn number(self) -> u64 { + self.0.get() & ((1 << Self::BITS) - 1) } } @@ -120,9 +123,9 @@ mod tests { #[test] fn test_span_encoding() { - let id = FileId::from_u16(5); - let span = Span::new(id, 10); - assert_eq!(span.id(), id); + let id = FileId::from_raw(5); + let span = Span::new(id, 10).unwrap(); + assert_eq!(span.id(), Some(id)); assert_eq!(span.number(), 10); } } diff --git a/crates/typst/src/diag.rs b/crates/typst/src/diag.rs index 99c616086..7c298ec03 100644 --- a/crates/typst/src/diag.rs +++ b/crates/typst/src/diag.rs @@ -9,7 +9,7 @@ use std::string::FromUtf8Error; use comemo::Tracked; use crate::syntax::{PackageSpec, Span, Spanned, SyntaxError}; -use crate::World; +use crate::{World, WorldExt}; /// Early-return with a [`StrResult`] or [`SourceResult`]. /// @@ -206,16 +206,12 @@ impl Trace for SourceResult { F: Fn() -> Tracepoint, { self.map_err(|mut errors| { - if span.is_detached() { - return errors; - } - - let trace_range = world.range(span); - for error in errors.iter_mut().filter(|e| !e.span.is_detached()) { + let Some(trace_range) = world.range(span) else { return errors }; + for error in errors.iter_mut() { // Skip traces that surround the error. - if error.span.id() == span.id() { - let error_range = world.range(error.span); - if trace_range.start <= error_range.start + if let Some(error_range) = world.range(error.span) { + if error.span.id() == span.id() + && trace_range.start <= error_range.start && trace_range.end >= error_range.end { continue; diff --git a/crates/typst/src/eval/func.rs b/crates/typst/src/eval/func.rs index d81171593..d185b3194 100644 --- a/crates/typst/src/eval/func.rs +++ b/crates/typst/src/eval/func.rs @@ -94,9 +94,8 @@ impl Func { } Repr::Closure(closure) => { // Determine the route inside the closure. - let fresh = Route::new(closure.location); - let route = - if vm.location.is_detached() { fresh.track() } else { vm.route }; + let fresh = Route::new(closure.file); + let route = if vm.file.is_none() { fresh.track() } else { vm.route }; Closure::call( self, @@ -134,7 +133,7 @@ impl Func { delayed: TrackedMut::reborrow_mut(&mut vt.delayed), tracer: TrackedMut::reborrow_mut(&mut vt.tracer), }; - let mut vm = Vm::new(vt, route.track(), FileId::detached(), scopes); + let mut vm = Vm::new(vt, route.track(), None, scopes); let args = Args::new(self.span(), args); self.call_vm(&mut vm, args) } @@ -298,7 +297,7 @@ pub(super) struct Closure { /// The closure's syntax node. Must be castable to `ast::Closure`. pub node: SyntaxNode, /// The source file where the closure was defined. - pub location: FileId, + pub file: Option, /// Default values of named parameters. pub defaults: Vec, /// Captured values from outer scopes. @@ -351,7 +350,7 @@ impl Closure { }; // Prepare VM. - let mut vm = Vm::new(vt, route, this.location, scopes); + let mut vm = Vm::new(vt, route, this.file, scopes); vm.depth = depth; // Provide the closure itself for recursive calls. diff --git a/crates/typst/src/eval/mod.rs b/crates/typst/src/eval/mod.rs index 4eb8d1229..b71669928 100644 --- a/crates/typst/src/eval/mod.rs +++ b/crates/typst/src/eval/mod.rs @@ -122,7 +122,7 @@ pub fn eval( // Prepare VM. let route = Route::insert(route, id); let scopes = Scopes::new(Some(library)); - let mut vm = Vm::new(vt, route.track(), id, scopes); + let mut vm = Vm::new(vt, route.track(), Some(id), scopes); let root = source.root(); let errors = root.errors(); @@ -189,9 +189,8 @@ pub fn eval_string( // Prepare VM. let route = Route::default(); - let id = FileId::detached(); let scopes = Scopes::new(Some(world.library())); - let mut vm = Vm::new(vt, route.track(), id, scopes); + let mut vm = Vm::new(vt, route.track(), None, scopes); vm.scopes.scopes.push(scope); // Evaluate the code. @@ -235,8 +234,8 @@ pub struct Vm<'a> { items: LangItems, /// The route of source ids the VM took to reach its current location. route: Tracked<'a, Route<'a>>, - /// The current location. - location: FileId, + /// The id of the currently evaluated file. + file: Option, /// A control flow event that is currently happening. flow: Option, /// The stack of scopes. @@ -252,16 +251,16 @@ impl<'a> Vm<'a> { fn new( vt: Vt<'a>, route: Tracked<'a, Route>, - location: FileId, + file: Option, scopes: Scopes<'a>, ) -> Self { - let traced = vt.tracer.span(location); + let traced = file.and_then(|id| vt.tracer.span(id)); let items = vt.world.library().items.clone(); Self { vt, items, route, - location, + file, flow: None, scopes, depth: 0, @@ -274,9 +273,21 @@ impl<'a> Vm<'a> { self.vt.world } - /// The location to which paths are relative currently. - pub fn location(&self) -> FileId { - self.location + /// The id of the currently evaluated file. + /// + /// Returns `None` if the VM is in a detached context, e.g. when evaluating + /// a user-provided string. + pub fn file(&self) -> Option { + self.file + } + + /// Resolve a path relative to the currently evaluated file. + pub fn resolve_path(&self, path: &str) -> StrResult { + let Some(file) = self.file else { + bail!("cannot access file system from here"); + }; + + Ok(file.join(path)) } /// Define a variable in the current scope. @@ -331,8 +342,8 @@ pub struct Route<'a> { impl<'a> Route<'a> { /// Create a new route with just one entry. - pub fn new(id: FileId) -> Self { - Self { id: Some(id), outer: None } + pub fn new(id: Option) -> Self { + Self { id, outer: None } } /// Insert a new id into the route. @@ -1301,7 +1312,7 @@ impl Eval for ast::Closure<'_> { // Define the closure. let closure = Closure { node: self.to_untyped().clone(), - location: vm.location, + file: vm.file, defaults, captured, }; @@ -1809,7 +1820,7 @@ fn import_package(vm: &mut Vm, spec: PackageSpec, span: Span) -> SourceResult SourceResult SourceResult { // Load the source file. let world = vm.world(); - let id = vm.location().join(path).at(span)?; + let id = vm.resolve_path(path).at(span)?; let source = world.source(id).at(span)?; // Prevent cyclic importing. diff --git a/crates/typst/src/eval/tracer.rs b/crates/typst/src/eval/tracer.rs index 0be6c1896..22e67fb1d 100644 --- a/crates/typst/src/eval/tracer.rs +++ b/crates/typst/src/eval/tracer.rs @@ -45,7 +45,7 @@ impl Tracer { impl Tracer { /// The traced span if it is part of the given source file. pub fn span(&self, id: FileId) -> Option { - if self.span.map(Span::id) == Some(id) { + if self.span.and_then(Span::id) == Some(id) { self.span } else { None diff --git a/crates/typst/src/ide/analyze.rs b/crates/typst/src/ide/analyze.rs index c143720af..2e64069ef 100644 --- a/crates/typst/src/ide/analyze.rs +++ b/crates/typst/src/ide/analyze.rs @@ -46,7 +46,7 @@ pub fn analyze_expr(world: &dyn World, node: &LinkedNode) -> EcoVec { pub fn analyze_import(world: &dyn World, source: &Source, path: &str) -> Option { let route = Route::default(); let mut tracer = Tracer::default(); - let id = source.id().join(path).ok()?; + let id = source.id().join(path); let source = world.source(id).ok()?; eval(world.track(), route.track(), tracer.track_mut(), &source).ok() } diff --git a/crates/typst/src/ide/highlight.rs b/crates/typst/src/ide/highlight.rs index c9748e922..037106e41 100644 --- a/crates/typst/src/ide/highlight.rs +++ b/crates/typst/src/ide/highlight.rs @@ -379,7 +379,7 @@ mod tests { use std::ops::Range; use super::*; - use crate::syntax::Source; + use crate::syntax::parse; #[test] fn test_highlighting() { @@ -388,8 +388,8 @@ mod tests { #[track_caller] fn test(text: &str, goal: &[(Range, Tag)]) { let mut vec = vec![]; - let source = Source::detached(text); - highlight_tree(&mut vec, &LinkedNode::new(source.root())); + let root = parse(text); + highlight_tree(&mut vec, &LinkedNode::new(&root)); assert_eq!(vec, goal); } diff --git a/crates/typst/src/ide/jump.rs b/crates/typst/src/ide/jump.rs index b2aeab9d9..34d51c070 100644 --- a/crates/typst/src/ide/jump.rs +++ b/crates/typst/src/ide/jump.rs @@ -21,9 +21,10 @@ pub enum Jump { impl Jump { fn from_span(world: &dyn World, span: Span) -> Option { - let source = world.source(span.id()).ok()?; + let id = span.id()?; + let source = world.source(id).ok()?; let node = source.find(span)?; - Some(Self::Source(span.id(), node.offset())) + Some(Self::Source(id, node.offset())) } } @@ -67,18 +68,15 @@ pub fn jump_from_click( FrameItem::Text(text) => { for glyph in &text.glyphs { - let (span, span_offset) = glyph.span; - if span.is_detached() { - continue; - } - let width = glyph.x_advance.at(text.size); if is_in_rect( Point::new(pos.x, pos.y - text.size), Size::new(width, text.size), click, ) { - let source = world.source(span.id()).ok()?; + let (span, span_offset) = glyph.span; + let Some(id) = span.id() else { continue }; + let source = world.source(id).ok()?; let node = source.find(span)?; let pos = if node.kind() == SyntaxKind::Text { let range = node.range(); diff --git a/crates/typst/src/lib.rs b/crates/typst/src/lib.rs index 514aa25e4..8d7393a08 100644 --- a/crates/typst/src/lib.rs +++ b/crates/typst/src/lib.rs @@ -142,12 +142,18 @@ pub trait World { fn packages(&self) -> &[(PackageSpec, Option)] { &[] } +} +/// Helper methods on [`World`] implementations. +pub trait WorldExt { /// Get the byte range for a span. - #[track_caller] - fn range(&self, span: Span) -> Range { - self.source(span.id()) - .expect("span does not point into any source file") - .range(span) + /// + /// Returns `None` if the `Span` does not point into any source file. + fn range(&self, span: Span) -> Option>; +} + +impl WorldExt for T { + fn range(&self, span: Span) -> Option> { + self.source(span.id()?).ok()?.range(span) } } diff --git a/crates/typst/src/model/mod.rs b/crates/typst/src/model/mod.rs index 69dc6a6bf..9a0917844 100644 --- a/crates/typst/src/model/mod.rs +++ b/crates/typst/src/model/mod.rs @@ -31,6 +31,7 @@ use comemo::{Track, Tracked, TrackedMut, Validate}; use crate::diag::{warning, SourceDiagnostic, SourceResult}; use crate::doc::Document; use crate::eval::Tracer; +use crate::syntax::Span; use crate::World; /// Typeset content into a fully layouted document. @@ -88,11 +89,8 @@ pub fn typeset( if iter >= 5 { tracer.warn( - warning!( - world.main().root().span(), - "layout did not converge within 5 attempts", - ) - .with_hint("check if any states or queries are updating themselves"), + warning!(Span::detached(), "layout did not converge within 5 attempts",) + .with_hint("check if any states or queries are updating themselves"), ); break; } diff --git a/tests/Cargo.toml b/tests/Cargo.toml index 46ada9b14..960583eb7 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -10,6 +10,7 @@ publish = false typst = { path = "../crates/typst" } typst-library = { path = "../crates/typst-library" } comemo = "0.3" +ecow = { version = "0.1.1", features = ["serde"] } iai = { git = "https://github.com/reknih/iai" } once_cell = "1" oxipng = { version = "8.0.0", default-features = false, features = ["filetime", "parallel", "zopfli"] } diff --git a/tests/src/tests.rs b/tests/src/tests.rs index 66c779008..d5fe3b591 100644 --- a/tests/src/tests.rs +++ b/tests/src/tests.rs @@ -4,15 +4,15 @@ use std::cell::{RefCell, RefMut}; use std::collections::{HashMap, HashSet}; use std::env; use std::ffi::OsStr; -use std::fmt::Write as FmtWrite; +use std::fmt::{self, Display, Formatter, Write as _}; use std::fs; use std::io::{self, Write}; -use std::iter; use std::ops::Range; use std::path::{Path, PathBuf}; use clap::Parser; use comemo::{Prehashed, Track}; +use ecow::EcoString; use oxipng::{InFile, Options, OutFile}; use rayon::iter::{ParallelBridge, ParallelIterator}; use std::cell::OnceCell; @@ -26,7 +26,7 @@ use typst::eval::{eco_format, func, Bytes, Datetime, Library, NoneValue, Tracer, use typst::font::{Font, FontBook}; use typst::geom::{Abs, Color, RgbaColor, Smart}; use typst::syntax::{FileId, Source, Span, SyntaxNode, VirtualPath}; -use typst::World; +use typst::{World, WorldExt}; use typst_library::layout::{Margin, PageElem}; use typst_library::text::{TextElem, TextSize}; @@ -237,7 +237,7 @@ impl TestWorld { Self { print, - main: FileId::detached(), + main: FileId::new(None, VirtualPath::new("main.typ")), library: Prehashed::new(library()), book: Prehashed::new(FontBook::from_fonts(&fonts)), fonts, @@ -555,39 +555,46 @@ fn test_part( // This has one caveat: due to the format of the expected hints, we can not // verify if a hint belongs to a diagnostic or not. That should be irrelevant // however, as the line of the hint is still verified. - let actual_diagnostics: HashSet = diagnostics - .into_iter() - .inspect(|diagnostic| assert!(!diagnostic.span.is_detached())) - .filter(|diagnostic| diagnostic.span.id() == source.id()) - .flat_map(|diagnostic| { - let range = world.range(diagnostic.span); - let message = diagnostic.message.replace('\\', "/"); - let output = match diagnostic.severity { - Severity::Error => UserOutput::Error(range.clone(), message), - Severity::Warning => UserOutput::Warning(range.clone(), message), - }; + let mut actual_diagnostics = HashSet::new(); + for diagnostic in &diagnostics { + // Ignore diagnostics from other files. + if diagnostic.span.id().map_or(false, |id| id != source.id()) { + continue; + } - let hints = diagnostic - .hints - .iter() - .filter(|_| validate_hints) // No unexpected hints should be verified if disabled. - .map(|hint| UserOutput::Hint(range.clone(), hint.to_string())); + let annotation = Annotation { + kind: match diagnostic.severity { + Severity::Error => AnnotationKind::Error, + Severity::Warning => AnnotationKind::Warning, + }, + range: world.range(diagnostic.span), + message: diagnostic.message.replace('\\', "/").into(), + }; - iter::once(output).chain(hints).collect::>() - }) - .collect(); + if validate_hints { + for hint in &diagnostic.hints { + actual_diagnostics.insert(Annotation { + kind: AnnotationKind::Hint, + message: hint.clone(), + range: annotation.range.clone(), + }); + } + } + + actual_diagnostics.insert(annotation); + } // Basically symmetric_difference, but we need to know where an item is coming from. let mut unexpected_outputs = actual_diagnostics - .difference(&metadata.invariants) + .difference(&metadata.annotations) .collect::>(); let mut missing_outputs = metadata - .invariants + .annotations .difference(&actual_diagnostics) .collect::>(); - unexpected_outputs.sort_by_key(|&o| o.start()); - missing_outputs.sort_by_key(|&o| o.start()); + unexpected_outputs.sort_by_key(|&v| v.range.as_ref().map(|r| r.start)); + missing_outputs.sort_by_key(|&v| v.range.as_ref().map(|r| r.start)); // This prints all unexpected emits first, then all missing emits. // Is this reasonable or subject to change? @@ -597,41 +604,34 @@ fn test_part( for unexpected in unexpected_outputs { write!(output, " Not annotated | ").unwrap(); - print_user_output(output, &source, line, unexpected) + print_annotation(output, &source, line, unexpected) } for missing in missing_outputs { write!(output, " Not emitted | ").unwrap(); - print_user_output(output, &source, line, missing) + print_annotation(output, &source, line, missing) } } (ok, compare_ref, frames) } -fn print_user_output( +fn print_annotation( output: &mut String, source: &Source, line: usize, - user_output: &UserOutput, + annotation: &Annotation, ) { - let (range, message) = match &user_output { - UserOutput::Error(r, m) => (r, m), - UserOutput::Warning(r, m) => (r, m), - UserOutput::Hint(r, m) => (r, m), - }; - - let start_line = 1 + line + source.byte_to_line(range.start).unwrap(); - let start_col = 1 + source.byte_to_column(range.start).unwrap(); - let end_line = 1 + line + source.byte_to_line(range.end).unwrap(); - let end_col = 1 + source.byte_to_column(range.end).unwrap(); - let kind = match user_output { - UserOutput::Error(_, _) => "Error", - UserOutput::Warning(_, _) => "Warning", - UserOutput::Hint(_, _) => "Hint", - }; - writeln!(output, "{kind}: {start_line}:{start_col}-{end_line}:{end_col}: {message}") - .unwrap(); + let Annotation { range, message, kind } = annotation; + write!(output, "{kind}: ").unwrap(); + if let Some(range) = range { + let start_line = 1 + line + source.byte_to_line(range.start).unwrap(); + let start_col = 1 + source.byte_to_column(range.start).unwrap(); + let end_line = 1 + line + source.byte_to_line(range.end).unwrap(); + let end_col = 1 + source.byte_to_column(range.end).unwrap(); + write!(output, "{start_line}:{start_col}-{end_line}:{end_col}: ").unwrap(); + } + writeln!(output, "{message}").unwrap(); } struct TestConfiguration { @@ -641,101 +641,93 @@ struct TestConfiguration { struct TestPartMetadata { part_configuration: TestConfiguration, - invariants: HashSet, + annotations: HashSet, } -#[derive(PartialEq, Eq, Debug, Hash)] -enum UserOutput { - Error(Range, String), - Warning(Range, String), - Hint(Range, String), +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +struct Annotation { + range: Option>, + message: EcoString, + kind: AnnotationKind, } -impl UserOutput { - fn start(&self) -> usize { +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] +enum AnnotationKind { + Error, + Warning, + Hint, +} + +impl AnnotationKind { + fn iter() -> impl Iterator { + [AnnotationKind::Error, AnnotationKind::Warning, AnnotationKind::Hint].into_iter() + } + + fn as_str(self) -> &'static str { match self { - UserOutput::Error(r, _) => r.start, - UserOutput::Warning(r, _) => r.start, - UserOutput::Hint(r, _) => r.start, + AnnotationKind::Error => "Error", + AnnotationKind::Warning => "Warning", + AnnotationKind::Hint => "Hint", } } +} - fn error(range: Range, message: String) -> UserOutput { - UserOutput::Error(range, message) - } - - fn warning(range: Range, message: String) -> UserOutput { - UserOutput::Warning(range, message) - } - - fn hint(range: Range, message: String) -> UserOutput { - UserOutput::Hint(range, message) +impl Display for AnnotationKind { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + f.pad(self.as_str()) } } fn parse_part_metadata(source: &Source) -> TestPartMetadata { let mut compare_ref = None; let mut validate_hints = None; - let mut expectations = HashSet::default(); + let mut annotations = HashSet::default(); let lines: Vec<_> = source.text().lines().map(str::trim).collect(); for (i, line) in lines.iter().enumerate() { compare_ref = get_flag_metadata(line, "Ref").or(compare_ref); validate_hints = get_flag_metadata(line, "Hints").or(validate_hints); - fn num(s: &mut Scanner) -> isize { + fn num(s: &mut Scanner) -> Option { let mut first = true; let n = &s.eat_while(|c: char| { let valid = first && c == '-' || c.is_numeric(); first = false; valid }); - n.parse().unwrap_or_else(|e| panic!("{n} is not a number ({e})")) + n.parse().ok() } let comments_until_code = lines[i..].iter().take_while(|line| line.starts_with("//")).count(); - let pos = |s: &mut Scanner| -> usize { - let first = num(s) - 1; + let pos = |s: &mut Scanner| -> Option { + let first = num(s)? - 1; let (delta, column) = - if s.eat_if(':') { (first, num(s) - 1) } else { (0, first) }; - let line = (i + comments_until_code) - .checked_add_signed(delta) - .expect("line number overflowed limits"); - source - .line_column_to_byte( - line, - usize::try_from(column).expect("column number overflowed limits"), - ) - .unwrap() + if s.eat_if(':') { (first, num(s)? - 1) } else { (0, first) }; + let line = (i + comments_until_code).checked_add_signed(delta)?; + source.line_column_to_byte(line, usize::try_from(column).ok()?) }; - let error_factory: fn(Range, String) -> UserOutput = UserOutput::error; - let warning_factory: fn(Range, String) -> UserOutput = UserOutput::warning; - let hint_factory: fn(Range, String) -> UserOutput = UserOutput::hint; + let range = |s: &mut Scanner| -> Option> { + let start = pos(s)?; + let end = if s.eat_if('-') { pos(s)? } else { start }; + Some(start..end) + }; - let error_metadata = get_metadata(line, "Error").map(|s| (s, error_factory)); - let get_warning_metadata = - || get_metadata(line, "Warning").map(|s| (s, warning_factory)); - let get_hint_metadata = || get_metadata(line, "Hint").map(|s| (s, hint_factory)); - - if let Some((expectation, factory)) = error_metadata - .or_else(get_warning_metadata) - .or_else(get_hint_metadata) - { + for kind in AnnotationKind::iter() { + let Some(expectation) = get_metadata(line, kind.as_str()) else { continue }; let mut s = Scanner::new(expectation); - let start = pos(&mut s); - let end = if s.eat_if('-') { pos(&mut s) } else { start }; - let range = start..end; - - expectations.insert(factory(range, s.after().trim().to_string())); - }; + let range = range(&mut s); + let rest = if range.is_some() { s.after() } else { s.string() }; + let message = rest.trim().into(); + annotations.insert(Annotation { kind, range, message }); + } } TestPartMetadata { part_configuration: TestConfiguration { compare_ref, validate_hints }, - invariants: expectations, + annotations, } } diff --git a/tests/typ/meta/state.typ b/tests/typ/meta/state.typ index 1c329a955..86dc70a56 100644 --- a/tests/typ/meta/state.typ +++ b/tests/typ/meta/state.typ @@ -49,10 +49,8 @@ Was: #locate(location => { --- // Make sure that a warning is produced if the layout fails to converge. -// Warning: -3:1-6:1 layout did not converge within 5 attempts -// Hint: -3:1-6:1 check if any states or queries are updating themselves -#let s = state("x", 1) -#locate(loc => { - s.update(s.final(loc) + 1) -}) +// Warning: layout did not converge within 5 attempts +// Hint: check if any states or queries are updating themselves +#let s = state("s", 1) +#locate(loc => s.update(s.final(loc) + 1)) #s.display()