src/syntax.rs

@@ -152,12 +152,12 @@ impl<'a> From<&'a ParserError> for Diagnostic<usize> {
                 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<usize> {
                 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(),
                     )

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<usize> {
         Label::primary(self.file_idx, self.offset..self.offset)
     }
 
+    /// 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<usize> {
         Label::secondary(self.file_idx, self.offset..self.offset)
     }
 
-    pub fn range_label(&self, end: &Location) -> Vec<Label<usize>> {
-        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<Label<usize>> {
+        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<usize> {
         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<usize> {
         Diagnostic::error().with_labels(vec![Label::primary(
             self.file_idx,