Print parse errors with codespan-reporting

src/cli.rs

@@ -7,9 +7,9 @@ use codespan_reporting::files::SimpleFile;
 use directories::ProjectDirs;
 use structopt::StructOpt;
 
-use crate::eval::{DateRange, Entry, EntryMode};
+use crate::eval::{self, DateRange, Entry, EntryMode};
 use crate::files::arguments::CliRange;
-use crate::files::{self, FileSource, Files};
+use crate::files::{self, FileSource, Files, ParseError};
 
 use self::error::Error;
 use self::layout::line::LineLayout;
@@ -95,6 +95,24 @@ fn find_layout(
     layout::layout(files, entries, range, now)
 }
 
+fn parse_eval_arg<T, R>(
+    name: &str,
+    text: &str,
+    eval: impl FnOnce(T) -> Result<R, eval::Error<()>>,
+) -> Option<R>
+where
+    T: FromStr<Err = ParseError<()>>,
+{
+    match T::from_str(text) {
+        Ok(value) => match eval(value) {
+            Ok(result) => return Some(result),
+            Err(e) => crate::error::eprint_error(&SimpleFile::new(name, text), &e),
+        },
+        Err(e) => crate::error::eprint_error(&SimpleFile::new(name, text), &e),
+    }
+    None
+}
+
 fn run_command(
     opt: &Opt,
     files: &mut Files,
@@ -144,21 +162,11 @@ pub fn run() {
 
     let now = find_now(&opt, &files);
 
-    // Kinda ugly, but it can stay for now (until it grows at least).
-    let range = match CliRange::from_str(&opt.range) {
-        Ok(range) => match range.eval((), now.date()) {
-            Ok(range) => range,
-            Err(e) => {
-                eprintln!("Failed to evaluate --range:");
-                let file = SimpleFile::new("--range", &opt.range);
-                crate::error::eprint_error(&file, &e);
-                process::exit(1)
-            }
-        },
-        Err(e) => {
-            eprintln!("Failed to parse --range:\n{}", e.with_path("--range"));
-            process::exit(1)
-        }
+    let range = match parse_eval_arg("--range", &opt.range, |range: CliRange| {
+        range.eval((), now.date())
+    }) {
+        Some(range) => range,
+        None => process::exit(1),
     };
 
     if let Err(e) = run_command(&opt, &mut files, range, now) {

src/files.rs

@@ -8,7 +8,7 @@ use codespan_reporting::files::SimpleFiles;
 use tzfile::Tz;
 
 use self::commands::{Command, Done, File, Log};
-pub use self::error::{Error, Result};
+pub use self::error::{Error, ParseError, Result};
 use self::primitives::Spanned;
 
 pub mod arguments;
@@ -162,9 +162,26 @@ impl Files {
             file: path.clone(),
             error: e,
         })?;
+        let cs_id = self
+            .cs_files
+            .add(name.to_string_lossy().to_string(), content.clone());
 
         // Using `name` instead of `path` for the unwrap below.
-        let file = parse::parse(name, &content)?;
+        let file = match parse::parse(name, &content) {
+            Ok(file) => file,
+            Err(error) => {
+                // Using a dummy file. This should be fine since we return an
+                // error immediately after and the user must never call `load`
+                // twice. Otherwise, we run the danger of overwriting a file
+                // with empty content.
+                self.files
+                    .push(LoadedFile::new(name.to_owned(), cs_id, File::dummy()));
+                return Err(Error::Parse {
+                    file: FileSource(self.files.len() - 1),
+                    error,
+                });
+            }
+        };
 
         let includes = file
             .commands
@@ -175,10 +192,7 @@ impl Files {
             })
             .collect::<Vec<_>>();
 
-        loaded.insert(path.clone());
-        let cs_id = self
-            .cs_files
-            .add(path.to_string_lossy().to_string(), content);
+        loaded.insert(path);
         self.files
             .push(LoadedFile::new(name.to_owned(), cs_id, file));
 

src/files/arguments.rs

@@ -1,3 +1,4 @@
+use std::result;
 use std::str::FromStr;
 
 use chrono::NaiveDate;
@@ -5,7 +6,8 @@ use pest::iterators::Pair;
 use pest::Parser;
 
 use super::commands::Delta;
-use super::parse::{self, Error, Result, Rule, TodayfileParser};
+use super::parse::{self, Result, Rule, TodayfileParser};
+use super::ParseError;
 
 #[derive(Debug)]
 pub enum CliDatum {
@@ -61,14 +63,15 @@ fn parse_cli_ident(p: Pair<'_, Rule>) -> Result<CliIdent> {
 }
 
 impl FromStr for CliIdent {
-    type Err = Error;
+    type Err = ParseError<()>;
 
-    fn from_str(s: &str) -> Result<Self> {
-        let mut pairs = TodayfileParser::parse(Rule::cli_ident, s)?;
+    fn from_str(s: &str) -> result::Result<Self, ParseError<()>> {
+        let mut pairs =
+            TodayfileParser::parse(Rule::cli_ident, s).map_err(|e| ParseError::new((), e))?;
         let p = pairs.next().unwrap();
         assert_eq!(pairs.next(), None);
 
-        parse_cli_ident(p)
+        parse_cli_ident(p).map_err(|e| ParseError::new((), e))
     }
 }
 
@@ -132,13 +135,14 @@ fn parse_cli_range(p: Pair<'_, Rule>) -> Result<CliRange> {
 }
 
 impl FromStr for CliRange {
-    type Err = Error;
+    type Err = ParseError<()>;
 
-    fn from_str(s: &str) -> Result<Self> {
-        let mut pairs = TodayfileParser::parse(Rule::cli_range, s)?;
+    fn from_str(s: &str) -> result::Result<Self, ParseError<()>> {
+        let mut pairs =
+            TodayfileParser::parse(Rule::cli_range, s).map_err(|e| ParseError::new((), e))?;
         let p = pairs.next().unwrap();
         assert_eq!(pairs.next(), None);
 
-        parse_cli_range(p)
+        parse_cli_range(p).map_err(|e| ParseError::new((), e))
     }
 }

src/files/commands.rs

@@ -357,3 +357,14 @@ pub struct File {
     pub contents: String,
     pub commands: Vec<Command>,
 }
+
+impl File {
+    /// Create an empty dummy file. This file should only be used as a
+    /// placeholder value.
+    pub fn dummy() -> Self {
+        Self {
+            contents: String::new(),
+            commands: vec![],
+        }
+    }
+}

src/files/error.rs

@@ -4,12 +4,85 @@ use std::{io, result};
 use chrono::NaiveDate;
 use codespan_reporting::diagnostic::{Diagnostic, Label};
 use codespan_reporting::term::Config;
+use pest::error::{ErrorVariant, InputLocation};
 
 use crate::error::Eprint;
 
 use super::primitives::Span;
 use super::{parse, FileSource, Files};
 
+#[derive(Debug, thiserror::Error)]
+#[error("{error}")]
+pub struct ParseError<S> {
+    file: S,
+    error: parse::Error,
+}
+
+impl<S> ParseError<S> {
+    pub fn new(file: S, error: parse::Error) -> Self {
+        Self { file, error }
+    }
+
+    fn rule_name(rule: parse::Rule) -> String {
+        // TODO Rename rules to be more readable?
+        format!("{:?}", rule)
+    }
+
+    fn enumerate(rules: &[parse::Rule]) -> String {
+        match rules.len() {
+            0 => "something".to_string(),
+            1 => Self::rule_name(rules[0]),
+            n => {
+                let except_last = rules
+                    .iter()
+                    .take(n - 1)
+                    .map(|rule| Self::rule_name(*rule))
+                    .collect::<Vec<_>>()
+                    .join(", ");
+                let last = Self::rule_name(rules[n - 1]);
+                format!("{} or {}", except_last, last)
+            }
+        }
+    }
+
+    fn notes(&self) -> Vec<String> {
+        match &self.error.variant {
+            ErrorVariant::ParsingError {
+                positives,
+                negatives,
+            } => {
+                let mut notes = vec![];
+                if !positives.is_empty() {
+                    notes.push(format!("expected {}", Self::enumerate(positives)))
+                }
+                if !negatives.is_empty() {
+                    notes.push(format!("unexpected {}", Self::enumerate(negatives)))
+                }
+                notes
+            }
+            ErrorVariant::CustomError { message } => vec![message.clone()],
+        }
+    }
+}
+
+impl<'a, F> Eprint<'a, F> for ParseError<F::FileId>
+where
+    F: codespan_reporting::files::Files<'a>,
+{
+    fn eprint<'f: 'a>(&self, files: &'f F, config: &Config) {
+        let range = match self.error.location {
+            InputLocation::Pos(at) => at..at,
+            InputLocation::Span((from, to)) => from..to,
+        };
+        let name = files.name(self.file).expect("file exists");
+        let diagnostic = Diagnostic::error()
+            .with_message(format!("Could not parse {}", name))
+            .with_labels(vec![Label::primary(self.file, range)])
+            .with_notes(self.notes());
+        Self::eprint_diagnostic(files, config, &diagnostic);
+    }
+}
+
 #[derive(Debug, thiserror::Error)]
 pub enum Error {
     #[error("Could not resolve {path}: {error}")]
@@ -27,8 +100,11 @@ pub enum Error {
     },
     #[error("Could not determine local timezone: {error}")]
     LocalTz { error: io::Error },
-    #[error("{0}")]
-    Parse(#[from] parse::Error),
+    #[error("{error}")]
+    Parse {
+        file: FileSource,
+        error: parse::Error,
+    },
     #[error("Conflicting time zones {tz1} and {tz2}")]
     TzConflict {
         file1: FileSource,
@@ -81,8 +157,9 @@ impl<'a> Eprint<'a, Files> for Error {
                 eprintln!("Could not determine local timezone:");
                 eprintln!("  {}", error);
             }
-            // TODO Format using codespan-reporting as well
-            Error::Parse(error) => eprintln!("{}", error),
+            Error::Parse { file, error } => {
+                ParseError::new(*file, error.clone()).eprint(files, config)
+            }
             Error::TzConflict {
                 file1,
                 span1,