Merge branch 'better-error-handling'

bin/build-data-crate.rs

@@ -1,24 +1,32 @@
 use std::env::args;
+use std::error::Error as ErrorTrait;
 use std::io::prelude::*;
 use std::io::BufReader;
-use std::io::Result as IoResult;
+use std::io::Result as IOResult;
 use std::fs::{File, OpenOptions, create_dir, metadata};
 use std::path::{Path, PathBuf};
 use std::process::exit;
 
 extern crate datetime;
-use datetime::LocalDateTime;
+use datetime::{LocalDateTime, ISO};
 
 extern crate zoneinfo_parse;
-use zoneinfo_parse::{Line, TableBuilder, Table, Structure, Child};
+use zoneinfo_parse::line::{Line};
+use zoneinfo_parse::table::{Table, TableBuilder};
+use zoneinfo_parse::structure::{Structure, Child};
+use zoneinfo_parse::transitions::{TableTransitions};
 
 
 fn main() {
     let args: Vec<_> = args().skip(1).collect();
 
     let data_crate = match DataCrate::new(&args[0], &args[1..]) {
         Ok(dc) => dc,
-        Err(_) => {
+        Err(errs) => {
+            for err in errs {
+                println!("{}:{}: {}", err.filename, err.line, err.error);
+            }
+
             println!("Errors occurred - not going any further.");
             exit(1);
         },
@@ -40,17 +48,17 @@ struct DataCrate {
 
 impl DataCrate {
 
-    fn new<P>(base_path: P, input_file_paths: &[String]) -> Result<DataCrate, u32>
+    fn new<P>(base_path: P, input_file_paths: &[String]) -> Result<DataCrate, Vec<Error>>
     where P: Into<PathBuf> {
 
         let mut builder = TableBuilder::new();
-        let mut errors = 0;
+        let mut errors = Vec::new();
 
         for arg in input_file_paths {
             let f = File::open(arg).unwrap();
             let reader = BufReader::new(f);
 
-            for line in reader.lines() {
+            for (line_number, line) in reader.lines().enumerate() {
                 let line = line.unwrap();
 
                 // Strip out the comment portion from the line, if any.
@@ -62,9 +70,14 @@ impl DataCrate {
                 let result = match Line::from_str(line_portion) {
 
                     // If there’s an error, then display which line failed to parse.
-                    Err(_) => {
-                        println!("Failed to parse line: {:?}", line_portion);
-                        errors += 1;
+                    Err(e) => {
+                        let error = Error {
+                            filename: arg.clone(),
+                            line: line_number + 1,
+                            error: e.description().to_owned(),
+                        };
+
+                        errors.push(error);
                         continue;
                     },
 
@@ -78,13 +91,18 @@ impl DataCrate {
                 };
 
                 if let Err(e) = result {
-                    errors += 1;
-                    println!("Error: {:?}", e);
+                    let error = Error {
+                        filename: arg.clone(),
+                        line: line_number + 1,
+                        error: e.description().to_owned(),
+                    };
+
+                    errors.push(error);
                 }
             }
         }
 
-        if errors == 0 {
+        if errors.is_empty() {
             Ok(DataCrate {
                 base_path: base_path.into(),
                 table: builder.build()
@@ -95,13 +113,13 @@ impl DataCrate {
         }
     }
 
-    fn run(&self) -> IoResult<()> {
+    fn run(&self) -> IOResult<()> {
         try!(self.create_structure_directories());
         try!(self.write_zonesets());
         Ok(())
     }
 
-    fn create_structure_directories(&self) -> IoResult<()> {
+    fn create_structure_directories(&self) -> IOResult<()> {
         let base_mod_path = self.base_path.join("mod.rs");
         let mut base_w = try!(OpenOptions::new().write(true).create(true).truncate(true).open(base_mod_path));
         try!(writeln!(base_w, "{}", WARNING_HEADER));
@@ -159,7 +177,7 @@ impl DataCrate {
         Ok(())
     }
 
-    fn write_zonesets(&self) -> IoResult<()> {
+    fn write_zonesets(&self) -> IOResult<()> {
         for name in self.table.zonesets.keys().chain(self.table.links.keys()) {
             let components: PathBuf = name.split('/').map(sanitise_name).collect();
             let zoneset_path = self.base_path.join(components).with_extension("rs");
@@ -182,7 +200,7 @@ impl DataCrate {
             try!(writeln!(w, "        rest: &["));
 
             for t in &set.rest {
-                try!(writeln!(w, "        ({:?}, FixedTimespan {{  // {:?} UTC", t.0, LocalDateTime::at(t.0)));
+                try!(writeln!(w, "        ({:?}, FixedTimespan {{  // {} UTC", t.0, LocalDateTime::at(t.0).iso()));
 
                 // Write the total offset (the only value that gets used)
                 // and both the offsets that get added together, as a
@@ -211,6 +229,12 @@ fn sanitise_name(name: &str) -> String {
     name.replace("-", "_")
 }
 
+struct Error {
+    filename: String,
+    line: usize,
+    error: String,
+}
+
 const WARNING_HEADER: &'static str = r##"
 // ------
 // This file is autogenerated!

src/lib.rs

@@ -1,5 +1,9 @@
 //! Parsing Olson DB formats.
 
+#![crate_name = "zoneinfo_parse"]
+#![crate_type = "rlib"]
+#![crate_type = "dylib"]
+
 #![warn(missing_copy_implementations)]
 #![warn(missing_debug_implementations)]
 #![warn(missing_docs)]
@@ -12,11 +16,7 @@ extern crate datetime;
 extern crate regex;
 #[macro_use] extern crate lazy_static;
 
-mod line;
-pub use line::Line;
-
-mod table;
-pub use table::{Table, TableBuilder, Saving, FixedTimespanSet, FixedTimespan};
-
-mod structure;
-pub use structure::*;
+pub mod line;
+pub mod table;
+pub mod transitions;
+pub mod structure;

src/line.rs

@@ -9,6 +9,8 @@
 //! `table` module.
 
 use std::ascii::AsciiExt;
+use std::error::Error as ErrorTrait;
+use std::fmt;
 use std::str::FromStr;
 
 use datetime::{LocalDate, LocalTime, LocalDateTime, Month, Weekday};
@@ -521,8 +523,7 @@ impl DaySpec {
     /// Converts this day specification to a concrete date, given the year and
     /// month it should occur in.
     pub fn to_concrete_date(&self, year: i64, month: Month) -> LocalDate {
-        use datetime::{LocalDate, DatePiece};
-        use datetime::iter::Year;
+        use datetime::{LocalDate, DatePiece, Year};
 
         match *self {
             DaySpec::Ordinal(day)           => LocalDate::ymd(year, month, day).unwrap(),
@@ -612,7 +613,6 @@ pub struct TimeSpecAndType(pub TimeSpec, pub TimeType);
 impl TimeSpec {
 
     /// Shorthand `TimeSpecAndType` constructor for testing.
-    #[cfg(test)]
     pub fn with_type(self, time_type: TimeType) -> TimeSpecAndType {
         TimeSpecAndType(self, time_type)
     }
@@ -693,6 +693,21 @@ pub enum Error {
     Fail
 }
 
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.description())
+    }
+}
+
+impl ErrorTrait for Error {
+    fn description(&self) -> &str {
+        "parse error"
+    }
+
+    fn cause(&self) -> Option<&ErrorTrait> {
+        None
+    }
+}
 
 /// A type of valid line that has been parsed.
 #[derive(PartialEq, Debug, Copy, Clone)]

src/structure.rs

@@ -38,7 +38,7 @@ use table::Table;
 /// Trait to put the `structure` method on Tables.
 pub trait Structure {
 
-    /// The **Structure** of the zonesets.
+    /// Returns an iterator over the structure of this table.
     fn structure(&self) -> TableStructure;
 }