From 6cd2b2148d266ee40f7e398969c126e3638f4326 Mon Sep 17 00:00:00 2001 From: Adam Wick Date: Sun, 7 May 2023 18:09:10 -0700 Subject: [PATCH] Add documentation around Locations. --- src/syntax.rs | 8 ++-- src/syntax/location.rs | 88 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 82 insertions(+), 14 deletions(-) diff --git a/src/syntax.rs b/src/syntax.rs index 8356999..e6f0fbc 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -152,12 +152,12 @@ impl<'a> From<&'a ParserError> for Diagnostic { let expected_str = format!("unexpected token {}{}", token, display_expected(expected)); let unexpected_str = format!("unexpected token {}", token); - let mut labels = start.range_label(end); + let labels = start.range_label(end); Diagnostic::error() .with_labels( labels - .drain(..) + .into_iter() .map(|l| l.with_message(unexpected_str.clone())) .collect(), ) @@ -169,12 +169,12 @@ impl<'a> From<&'a ParserError> for Diagnostic { let expected_str = format!("unexpected token {} after the expected end of file", token); let unexpected_str = format!("unexpected token {}", token); - let mut labels = start.range_label(end); + let labels = start.range_label(end); Diagnostic::error() .with_labels( labels - .drain(..) + .into_iter() .map(|l| l.with_message(unexpected_str.clone())) .collect(), ) diff --git a/src/syntax/location.rs b/src/syntax/location.rs index 65e1402..420de82 100644 --- a/src/syntax/location.rs +++ b/src/syntax/location.rs @@ -1,5 +1,9 @@ use codespan_reporting::diagnostic::{Diagnostic, Label}; +/// A source location, for use in pointing users towards warnings and errors. +/// +/// Internally, locations are very tied to the `codespan_reporting` library, +/// and the primary use of them is to serve as anchors within that library. #[derive(Clone, Debug, Eq, PartialEq)] pub struct Location { file_idx: usize, @@ -7,10 +11,22 @@ pub struct Location { } impl Location { + /// Generate a new `Location` from a file index and an offset from the + /// start of the file. + /// + /// The file index is based on the file database being used. See the + /// `codespan_reporting::files::SimpleFiles::add` function, which is + /// normally where we get this index. pub fn new(file_idx: usize, offset: usize) -> Self { Location { file_idx, offset } } + /// Generate a `Location` for a completely manufactured bit of code. + /// + /// Ideally, this is used only in testing, as any code we generate as + /// part of the compiler should, theoretically, be tied to some actual + /// location in the source code. That being said, this can be used in + /// a pinch ... just maybe try to avoid it if you can. pub fn manufactured() -> Self { Location { file_idx: 0, @@ -18,27 +34,73 @@ impl Location { } } + /// Generate a primary label for a [`Diagnostic`], based on this source + /// location. + /// + /// Note, this is just the [`Label`], you'll want to fill in the [`Diagnostic`] + /// with a lot more information. + /// + /// Primary labels are the things that are they key cause of the message. + /// If, for example, it was an error to bind a variable named "x", and + /// then have another binding of a variable named "x", the second one + /// would likely be the primary label (because that's where the error + /// actually happened), but you'd probably want to make the first location + /// the secondary label to help users find it. pub fn primary_label(&self) -> Label { Label::primary(self.file_idx, self.offset..self.offset) } - pub fn secondary_label(&self) -> Label { + /// Generate a secondary label for a [`Diagnostic`], based on this source + /// location. + /// + /// Note, this is just the [`Label`], you'll want to fill in the [`Diagnostic`] + /// with a lot more information. + /// + /// Secondary labels are the things that are involved in the message, but + /// aren't necessarily a problem in and of themselves. If, for example, it + /// was an error to bind a variable named "x", and then have another binding + /// of a variable named "x", the second one would likely be the primary + /// label (because that's where the error actually happened), but you'd + /// probably want to make the first location the secondary label to help + /// users find it. + pub fn secondary_label(&self) -> Label { Label::secondary(self.file_idx, self.offset..self.offset) } - pub fn range_label(&self, end: &Location) -> Vec> { - if self.file_idx == end.file_idx { - vec![Label::primary(self.file_idx, self.offset..end.offset)] - } else if self.file_idx == 0 { - // if this is a manufactured item, then ... just try the other one - vec![Label::primary(end.file_idx, end.offset..end.offset)] + /// Given this location and another, generate a primary label that + /// specifies the area between those two locations. + /// + /// See [`Self::primary_label`] for some discussion of primary versus + /// secondary labels. If the two locations are the same, this method does + /// the exact same thing as [`Self::primary_label`]. If this item was + /// generated by [`Self::manufactured`], it will act as if you'd called + /// `primary_label` on the argument. Otherwise, it will generate the obvious + /// span. + /// + /// This function will return `None` only in the case that you provide + /// labels from two different files, which it cannot sensibly handle. + pub fn range_label(&self, end: &Location) -> Option> { + if self.file_idx == 0 { + return Some(end.primary_label()); + } + + if self.file_idx != end.file_idx { + return None; + } + + if self.offset > end.offset { + Some(Label::primary(self.file_idx, end.offset..self.offset)) } else { - // we'll just pick the first location if this is in two different - // files - vec![Label::primary(self.file_idx, self.offset..self.offset)] + Some(Label::primary(self.file_idx, self.offset..end.offset)) } } + /// Return an error diagnostic centered at this location. + /// + /// Note that this [`Diagnostic`] will have no information associated with + /// it other than that (a) there is an error, and (b) that the error is at + /// this particular location. You'll need to extend it with actually useful + /// information, like what kind of error it is. pub fn error(&self) -> Diagnostic { Diagnostic::error().with_labels(vec![Label::primary( self.file_idx, @@ -46,6 +108,12 @@ impl Location { )]) } + /// Return an error diagnostic centered at this location, with the given message. + /// + /// This is much more useful than [`Self::error`], because it actually provides + /// the user with some guidance. That being said, you still might want to add + /// even more information to ut, using [`Diagnostic::with_labels`], + /// [`Diagnostic::with_notes`], or [`Diagnostic::with_code`]. pub fn labelled_error(&self, msg: &str) -> Diagnostic { Diagnostic::error().with_labels(vec![Label::primary( self.file_idx,