From 5e47dddd4cdfa07e0fb1321c6d72521b191d52a6 Mon Sep 17 00:00:00 2001
From: Joscha <joscha@plugh.de>
Date: Wed, 5 Jan 2022 02:59:03 +0100
Subject: [PATCH] Make error formatting more generic

Now, some errors are generic over the FileId type, meaning I can now
pretty-print command line argument errors as well as normal errors with
the same machinery. This is accomplished via the Eprint trait, whose
generics are a tad complex but probably still fine.
---
 src/cli.rs         | 22 +++++++++----------
 src/error.rs       | 28 ++++++++++++++++++++++++
 src/eval/error.rs  | 38 +++++++++++++++------------------
 src/files.rs       | 53 ++++++++++++++++++++++++++++++++++------------
 src/files/error.rs | 28 ++++++++++++------------
 src/main.rs        |  1 +
 6 files changed, 111 insertions(+), 59 deletions(-)
 create mode 100644 src/error.rs

diff --git a/src/cli.rs b/src/cli.rs
index 2decacf..10b4e36 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -3,9 +3,11 @@ use std::str::FromStr;
 use std::{process, result};
 
 use chrono::{NaiveDate, NaiveDateTime};
+use codespan_reporting::files::SimpleFile;
 use directories::ProjectDirs;
 use structopt::StructOpt;
 
+use crate::error;
 // use crate::eval::{DateRange, Entry, EntryMode, SourceInfo};
 use crate::files::arguments::Range;
 use crate::files::{self, Files};
@@ -72,8 +74,6 @@ fn load_files(opt: &Opt, files: &mut Files) -> result::Result<(), files::Error>
     files.load(&file)
 }
 
-/*
-
 fn find_now(opt: &Opt, files: &Files) -> NaiveDateTime {
     let now = files.now().naive_local();
     if let Some(date) = opt.date {
@@ -83,6 +83,8 @@ fn find_now(opt: &Opt, files: &Files) -> NaiveDateTime {
     }
 }
 
+/*
+
 fn find_entries(files: &Files, range: DateRange) -> Result<Vec<Entry>> {
     Ok(files.eval(EntryMode::Relevant, range)?)
 }
@@ -136,24 +138,20 @@ pub fn run() {
 
     let mut files = Files::new();
     if let Err(e) = load_files(&opt, &mut files) {
-        e.print(&files);
+        error::eprint_error(&files, &e);
         process::exit(1);
     }
 
-    /*
-
     let now = find_now(&opt, &files);
 
     // Kinda ugly, but it can stay for now (until it grows at least).
     let range = match Range::from_str(&opt.range) {
-        Ok(range) => match range.eval(0, now.date()) {
+        Ok(range) => match range.eval((), now.date()) {
             Ok(range) => range,
             Err(e) => {
                 eprintln!("Failed to evaluate --range:");
-                e.print(&[SourceInfo {
-                    name: Some("--range".to_string()),
-                    content: &opt.range,
-                }]);
+                let file = SimpleFile::new("--range", &opt.range);
+                error::eprint_error(&file, &e);
                 process::exit(1)
             }
         },
@@ -163,6 +161,8 @@ pub fn run() {
         }
     };
 
+    /*
+
     if let Err(e) = run_command(&opt, &mut files, range, now) {
         e.print(&files.sources());
         process::exit(1);
@@ -171,7 +171,7 @@ pub fn run() {
     */
 
     if let Err(e) = files.save() {
-        e.print(&files);
+        error::eprint_error(&files, &e);
         process::exit(1);
     }
 }
diff --git a/src/error.rs b/src/error.rs
new file mode 100644
index 0000000..89e85e7
--- /dev/null
+++ b/src/error.rs
@@ -0,0 +1,28 @@
+use codespan_reporting::diagnostic::Diagnostic;
+use codespan_reporting::files::Files;
+use codespan_reporting::term::{self, Config};
+use termcolor::StandardStream;
+
+pub trait Eprint<'a, F: Files<'a>> {
+    fn eprint_diagnostic<'f: 'a>(
+        files: &'f F,
+        config: &Config,
+        diagnostic: &Diagnostic<F::FileId>,
+    ) {
+        let mut out = StandardStream::stderr(termcolor::ColorChoice::Auto);
+        if let Err(e) = term::emit(&mut out, config, files, diagnostic) {
+            panic!("Error while reporting error: {}", e);
+        }
+    }
+
+    fn eprint<'f: 'a>(&self, files: &'f F, config: &Config);
+}
+
+pub fn eprint_error<'a, 'f: 'a, F, E>(files: &'f F, e: &E)
+where
+    F: Files<'a>,
+    E: Eprint<'a, F>,
+{
+    let config = Config::default();
+    e.eprint(files, &config);
+}
diff --git a/src/eval/error.rs b/src/eval/error.rs
index 16062b9..4e5bd45 100644
--- a/src/eval/error.rs
+++ b/src/eval/error.rs
@@ -1,8 +1,10 @@
 use chrono::NaiveDate;
 use codespan_reporting::diagnostic::{Diagnostic, Label};
+use codespan_reporting::files::Files;
+use codespan_reporting::term::Config;
 
+use crate::error::Eprint;
 use crate::files::primitives::{Span, Time};
-use crate::files::{FileSource, Files};
 
 #[derive(Debug, thiserror::Error)]
 pub enum Error<S> {
@@ -76,15 +78,17 @@ pub enum Error<S> {
     },
 }
 
-impl Error<FileSource> {
+impl<S> Error<S> {
     fn fmt_date_time(date: NaiveDate, time: Option<Time>) -> String {
         match time {
             None => format!("{}", date),
             Some(time) => format!("{} {}", date, time),
         }
     }
+}
 
-    pub fn print(&self, files: &Files) {
+impl<'a, F: Files<'a>> Eprint<'a, F> for Error<F::FileId> {
+    fn eprint<'f: 'a>(&self, files: &'f F, config: &Config) {
         let diagnostic = match self {
             Error::DeltaInvalidStep {
                 index,
@@ -99,7 +103,7 @@ impl Error<FileSource> {
                 Diagnostic::error()
                     .with_message("Delta step resulted in invalid date")
                     .with_labels(vec![
-                        Label::primary(files.cs_id(*index), span).with_message("At this step")
+                        Label::primary(*index, span).with_message("At this step")
                     ])
                     .with_notes(vec![
                         format!("Date before applying delta: {}", start_str),
@@ -114,7 +118,7 @@ impl Error<FileSource> {
             } => Diagnostic::error()
                 .with_message("Time-based delta step applied to date without time")
                 .with_labels(vec![
-                    Label::primary(files.cs_id(*index), span).with_message("At this step")
+                    Label::primary(*index, span).with_message("At this step")
                 ])
                 .with_notes(vec![
                     format!("Date before applying delta: {}", start),
@@ -127,9 +131,7 @@ impl Error<FileSource> {
                 to,
             } => Diagnostic::error()
                 .with_message("Repeat delta did not move forwards")
-                .with_labels(vec![
-                    Label::primary(files.cs_id(*index), span).with_message("This delta")
-                ])
+                .with_labels(vec![Label::primary(*index, span).with_message("This delta")])
                 .with_notes(vec![format!("Moved from {} to {}", from, to)]),
             Error::RemindDidNotMoveBackwards {
                 index,
@@ -138,30 +140,24 @@ impl Error<FileSource> {
                 to,
             } => Diagnostic::error()
                 .with_message("Remind delta did not move backwards")
-                .with_labels(vec![
-                    Label::primary(files.cs_id(*index), span).with_message("This delta")
-                ])
+                .with_labels(vec![Label::primary(*index, span).with_message("This delta")])
                 .with_notes(vec![format!("Moved from {} to {}", from, to)]),
             Error::MoveWithoutSource { index, span } => Diagnostic::error()
                 .with_message("Tried to move nonexistent entry")
-                .with_labels(vec![
-                    Label::primary(files.cs_id(*index), span).with_message("Here")
-                ]),
+                .with_labels(vec![Label::primary(*index, span).with_message("Here")]),
             Error::TimedMoveWithoutTime { index, span } => Diagnostic::error()
                 .with_message("Tried to move un-timed entry to new time")
-                .with_labels(vec![
-                    Label::primary(files.cs_id(*index), span).with_message("Here")
-                ]),
+                .with_labels(vec![Label::primary(*index, span).with_message("Here")]),
             Error::DivByZero { index, span, date } => Diagnostic::error()
                 .with_message("Tried to divide by zero")
                 .with_labels(vec![
-                    Label::primary(files.cs_id(*index), span).with_message("This expression")
+                    Label::primary(*index, span).with_message("This expression")
                 ])
                 .with_notes(vec![format!("At date: {}", date)]),
             Error::ModByZero { index, span, date } => Diagnostic::error()
                 .with_message("Tried to modulo by zero")
                 .with_labels(vec![
-                    Label::primary(files.cs_id(*index), span).with_message("This expression")
+                    Label::primary(*index, span).with_message("This expression")
                 ])
                 .with_notes(vec![format!("At date: {}", date)]),
             Error::Easter {
@@ -172,13 +168,13 @@ impl Error<FileSource> {
             } => Diagnostic::error()
                 .with_message("Failed to calculate easter")
                 .with_labels(vec![
-                    Label::primary(files.cs_id(*index), span).with_message("This expression")
+                    Label::primary(*index, span).with_message("This expression")
                 ])
                 .with_notes(vec![
                     format!("At date: {}", date),
                     format!("Reason: {}", msg),
                 ]),
         };
-        files.eprint_diagnostic(&diagnostic);
+        Self::eprint_diagnostic(files, config, &diagnostic);
     }
 }
diff --git a/src/files.rs b/src/files.rs
index 1ac07a6..3ba0573 100644
--- a/src/files.rs
+++ b/src/files.rs
@@ -1,13 +1,10 @@
 use std::collections::hash_map::Entry;
 use std::collections::{HashMap, HashSet};
-use std::fs;
 use std::path::{Path, PathBuf};
+use std::{fs, result};
 
 use chrono::{DateTime, NaiveDate, Utc};
-use codespan_reporting::diagnostic::Diagnostic;
 use codespan_reporting::files::SimpleFiles;
-use codespan_reporting::term::{self, Config};
-use termcolor::StandardStream;
 use tzfile::Tz;
 
 use self::commands::{Command, File};
@@ -54,7 +51,7 @@ pub struct Source {
 }
 
 // TODO Rename to `SourceFile`?
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct FileSource(usize);
 
 impl Source {
@@ -82,6 +79,42 @@ pub struct Files {
     logs: HashMap<NaiveDate, Source>,
 }
 
+impl<'a> codespan_reporting::files::Files<'a> for Files {
+    type FileId = FileSource;
+    type Name = String;
+    type Source = &'a str;
+
+    fn name(
+        &'a self,
+        id: Self::FileId,
+    ) -> result::Result<Self::Name, codespan_reporting::files::Error> {
+        self.cs_files.name(self.cs_id(id))
+    }
+
+    fn source(
+        &'a self,
+        id: Self::FileId,
+    ) -> result::Result<Self::Source, codespan_reporting::files::Error> {
+        self.cs_files.source(self.cs_id(id))
+    }
+
+    fn line_index(
+        &'a self,
+        id: Self::FileId,
+        byte_index: usize,
+    ) -> result::Result<usize, codespan_reporting::files::Error> {
+        self.cs_files.line_index(self.cs_id(id), byte_index)
+    }
+
+    fn line_range(
+        &'a self,
+        id: Self::FileId,
+        line_index: usize,
+    ) -> result::Result<std::ops::Range<usize>, codespan_reporting::files::Error> {
+        self.cs_files.line_range(self.cs_id(id), line_index)
+    }
+}
+
 impl Files {
     /* Loading */
 
@@ -323,15 +356,7 @@ impl Files {
 
     /* Errors */
 
-    pub fn cs_id(&self, file: FileSource) -> usize {
+    fn cs_id(&self, file: FileSource) -> usize {
         self.files[file.0].cs_id
     }
-
-    pub fn eprint_diagnostic(&self, diagnostic: &Diagnostic<usize>) {
-        let mut out = StandardStream::stderr(termcolor::ColorChoice::Auto);
-        let config = Config::default();
-        if let Err(e) = term::emit(&mut out, &config, &self.cs_files, diagnostic) {
-            panic!("Error while reporting error: {}", e);
-        }
-    }
 }
diff --git a/src/files/error.rs b/src/files/error.rs
index 3b8dc34..14f2a8c 100644
--- a/src/files/error.rs
+++ b/src/files/error.rs
@@ -3,6 +3,9 @@ use std::{io, result};
 
 use chrono::NaiveDate;
 use codespan_reporting::diagnostic::{Diagnostic, Label};
+use codespan_reporting::term::Config;
+
+use crate::error::Eprint;
 
 use super::primitives::Span;
 use super::{parse, FileSource, Files};
@@ -45,8 +48,8 @@ pub enum Error {
     },
 }
 
-impl Error {
-    pub fn print(&self, files: &Files) {
+impl<'a> Eprint<'a, Files> for Error {
+    fn eprint<'f: 'a>(&self, files: &'f Files, config: &Config) {
         match self {
             Error::ResolvePath { path, error } => {
                 eprintln!("Could not resolve path {:?}:", path);
@@ -68,10 +71,11 @@ impl Error {
             } => {
                 let diagnostic = Diagnostic::error()
                     .with_message(format!("Could not resolve time zone {}", tz))
-                    .with_labels(vec![Label::primary(files.cs_id(*file), span)
-                        .with_message("Time zone defined here")])
+                    .with_labels(vec![
+                        Label::primary(*file, span).with_message("Time zone defined here")
+                    ])
                     .with_notes(vec![format!("{}", error)]);
-                files.eprint_diagnostic(&diagnostic);
+                Self::eprint_diagnostic(files, config, &diagnostic);
             }
             Error::LocalTz { error } => {
                 eprintln!("Could not determine local timezone:");
@@ -90,15 +94,13 @@ impl Error {
                 let diagnostic = Diagnostic::error()
                     .with_message(format!("Time zone conflict between {} and {}", tz1, tz2))
                     .with_labels(vec![
-                        Label::primary(files.cs_id(*file1), span1)
-                            .with_message("Time zone defined here"),
-                        Label::primary(files.cs_id(*file2), span2)
-                            .with_message("Time zone defined here"),
+                        Label::primary(*file1, span1).with_message("Time zone defined here"),
+                        Label::primary(*file2, span2).with_message("Time zone defined here"),
                     ])
                     .with_notes(vec![
                         "All TIMEZONE commands must set the same time zone.".to_string()
                     ]);
-                files.eprint_diagnostic(&diagnostic);
+                Self::eprint_diagnostic(files, config, &diagnostic);
             }
             Error::LogConflict {
                 file1,
@@ -110,11 +112,11 @@ impl Error {
                 let diagnostic = Diagnostic::error()
                     .with_message(format!("Duplicate log entries for {}", date))
                     .with_labels(vec![
-                        Label::primary(files.cs_id(*file1), span1).with_message("Log defined here"),
-                        Label::primary(files.cs_id(*file2), span2).with_message("Log defined here"),
+                        Label::primary(*file1, span1).with_message("Log defined here"),
+                        Label::primary(*file2, span2).with_message("Log defined here"),
                     ])
                     .with_notes(vec!["A day can have at most one LOG entry.".to_string()]);
-                files.eprint_diagnostic(&diagnostic);
+                Self::eprint_diagnostic(files, config, &diagnostic);
             }
         }
     }
diff --git a/src/main.rs b/src/main.rs
index 9053d12..5d5922f 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -4,6 +4,7 @@
 #![warn(clippy::use_self)]
 
 mod cli;
+mod error;
 mod eval;
 mod files;