Refactoring: summary

Move the timer from Summary to Session.

Move Summary from config to formatting.

src/config/mod.rs

@@ -29,7 +29,6 @@
 pub mod file_lines;
 pub mod license;
 pub mod lists;
-pub mod summary;
 
 /// This macro defines configuration options used in rustfmt. Each option
 /// is defined as follows:

src/config/summary.rs

@@ -1,160 +0,0 @@
-// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
-// file at the top-level directory of this distribution and at
-// http://rust-lang.org/COPYRIGHT.
-//
-// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
-// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
-// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
-// option. This file may not be copied, modified, or distributed
-// except according to those terms.
-
-use std::default::Default;
-use std::time::{Duration, Instant};
-
-/// A summary of a Rustfmt run.
-#[derive(Debug, Default, Clone, Copy)]
-pub struct Summary {
-    // Encountered e.g. an IO error.
-    has_operational_errors: bool,
-
-    // Failed to reformat code because of parsing errors.
-    has_parsing_errors: bool,
-
-    // Code is valid, but it is impossible to format it properly.
-    has_formatting_errors: bool,
-
-    // Code contains macro call that was unable to format.
-    pub(crate) has_macro_format_failure: bool,
-
-    // Failed a check, such as the license check or other opt-in checking.
-    has_check_errors: bool,
-
-    /// Formatted code differs from existing code (--check only).
-    pub has_diff: bool,
-
-    // Keeps track of time spent in parsing and formatting steps.
-    timer: Timer,
-}
-
-impl Summary {
-    pub(crate) fn mark_parse_time(&mut self) {
-        self.timer = self.timer.done_parsing();
-    }
-
-    pub(crate) fn mark_format_time(&mut self) {
-        self.timer = self.timer.done_formatting();
-    }
-
-    /// Returns the time it took to parse the source files in nanoseconds.
-    pub(crate) fn get_parse_time(&self) -> Option<Duration> {
-        match self.timer {
-            Timer::DoneParsing(init, parse_time) | Timer::DoneFormatting(init, parse_time, _) => {
-                // This should never underflow since `Instant::now()` guarantees monotonicity.
-                Some(parse_time.duration_since(init))
-            }
-            Timer::Initialized(..) => None,
-        }
-    }
-
-    /// Returns the time it took to go from the parsed AST to the formatted output. Parsing time is
-    /// not included.
-    pub(crate) fn get_format_time(&self) -> Option<Duration> {
-        match self.timer {
-            Timer::DoneFormatting(_init, parse_time, format_time) => {
-                Some(format_time.duration_since(parse_time))
-            }
-            Timer::DoneParsing(..) | Timer::Initialized(..) => None,
-        }
-    }
-
-    pub fn has_operational_errors(&self) -> bool {
-        self.has_operational_errors
-    }
-
-    pub fn has_parsing_errors(&self) -> bool {
-        self.has_parsing_errors
-    }
-
-    pub fn has_formatting_errors(&self) -> bool {
-        self.has_formatting_errors
-    }
-
-    pub fn has_check_errors(&self) -> bool {
-        self.has_check_errors
-    }
-
-    pub(crate) fn has_macro_formatting_failure(&self) -> bool {
-        self.has_macro_format_failure
-    }
-
-    pub fn add_operational_error(&mut self) {
-        self.has_operational_errors = true;
-    }
-
-    pub(crate) fn add_parsing_error(&mut self) {
-        self.has_parsing_errors = true;
-    }
-
-    pub(crate) fn add_formatting_error(&mut self) {
-        self.has_formatting_errors = true;
-    }
-
-    pub(crate) fn add_check_error(&mut self) {
-        self.has_check_errors = true;
-    }
-
-    pub(crate) fn add_diff(&mut self) {
-        self.has_diff = true;
-    }
-
-    pub(crate) fn add_macro_foramt_failure(&mut self) {
-        self.has_macro_format_failure = true;
-    }
-
-    pub fn has_no_errors(&self) -> bool {
-        !(self.has_operational_errors
-            || self.has_parsing_errors
-            || self.has_formatting_errors
-            || self.has_diff)
-    }
-
-    /// Combine two summaries together.
-    pub fn add(&mut self, other: Summary) {
-        self.has_operational_errors |= other.has_operational_errors;
-        self.has_formatting_errors |= other.has_formatting_errors;
-        self.has_parsing_errors |= other.has_parsing_errors;
-        self.has_check_errors |= other.has_check_errors;
-        self.has_diff |= other.has_diff;
-    }
-}
-
-#[derive(Clone, Copy, Debug)]
-enum Timer {
-    Initialized(Instant),
-    DoneParsing(Instant, Instant),
-    DoneFormatting(Instant, Instant, Instant),
-}
-
-impl Default for Timer {
-    fn default() -> Self {
-        Timer::Initialized(Instant::now())
-    }
-}
-
-impl Timer {
-    fn done_parsing(self) -> Self {
-        match self {
-            Timer::Initialized(init_time) => Timer::DoneParsing(init_time, Instant::now()),
-            _ => panic!("Timer can only transition to DoneParsing from Initialized state"),
-        }
-    }
-
-    fn done_formatting(self) -> Self {
-        match self {
-            Timer::DoneParsing(init_time, parse_time) => {
-                Timer::DoneFormatting(init_time, parse_time, Instant::now())
-            }
-            _ => panic!("Timer can only transition to DoneFormatting from DoneParsing state"),
-        }
-    }
-}

src/formatting.rs

@@ -4,7 +4,7 @@
 use std::io::{self, Write};
 use std::panic::{catch_unwind, AssertUnwindSafe};
 use std::rc::Rc;
-use std::time::Duration;
+use std::time::{Duration, Instant};
 
 use syntax::ast;
 use syntax::codemap::{CodeMap, FilePathMapping, Span};
@@ -13,12 +13,11 @@
 use syntax::parse::{self, ParseSess};
 
 use comment::{CharClasses, FullCodeCharKind};
+use config::{Config, FileName, NewlineStyle, Verbosity};
 use issues::BadIssueSeeker;
 use visitor::{FmtVisitor, SnippetProvider};
 use {filemap, modules, ErrorKind, FormatReport, Input, Session};
 
-use config::{Config, FileName, NewlineStyle, Verbosity};
-
 // A map of the files of a crate, with their new content
 pub(crate) type FileMap = Vec<FileRecord>;
 
@@ -343,8 +342,8 @@ fn duration_to_f32(d: Duration) -> f32 {
 
             println!(
                 "Spent {0:.3} secs in the parsing phase, and {1:.3} secs in the formatting phase",
-                duration_to_f32(self.summary.get_parse_time().unwrap()),
-                duration_to_f32(self.summary.get_format_time().unwrap()),
+                duration_to_f32(self.get_parse_time().unwrap()),
+                duration_to_f32(self.get_format_time().unwrap()),
             )
         });
 
@@ -352,7 +351,6 @@ fn duration_to_f32(d: Duration) -> f32 {
     }
 
     // TODO name, only uses config and summary
-    // TODO move timing from summary to Session
     // Formatting which depends on the AST.
     fn format_ast<F>(
         &mut self,
@@ -389,7 +387,7 @@ fn format_ast<F>(
                 return Err(ErrorKind::ParseError);
             }
         };
-        self.summary.mark_parse_time();
+        self.timer = self.timer.done_parsing();
 
         // Suppress error output if we have to do any further parsing.
         let silent_emitter = Box::new(EmitterWriter::new(
@@ -460,7 +458,7 @@ fn format_ast<F>(
 
             formatted_file(self, path, visitor.buffer)?;
         }
-        self.summary.mark_format_time();
+        self.timer = self.timer.done_formatting();
 
         if report.has_warnings() {
             self.summary.add_formatting_error();
@@ -527,6 +525,28 @@ fn replace_with_system_newlines(&self, text: &mut String) -> () {
             NewlineStyle::Native => unreachable!(),
         }
     }
+
+    /// Returns the time it took to parse the source files in nanoseconds.
+    fn get_parse_time(&self) -> Option<Duration> {
+        match self.timer {
+            Timer::DoneParsing(init, parse_time) | Timer::DoneFormatting(init, parse_time, _) => {
+                // This should never underflow since `Instant::now()` guarantees monotonicity.
+                Some(parse_time.duration_since(init))
+            }
+            Timer::Initialized(..) => None,
+        }
+    }
+
+    /// Returns the time it took to go from the parsed AST to the formatted output. Parsing time is
+    /// not included.
+    fn get_format_time(&self) -> Option<Duration> {
+        match self.timer {
+            Timer::DoneFormatting(_init, parse_time, format_time) => {
+                Some(format_time.duration_since(parse_time))
+            }
+            Timer::DoneParsing(..) | Timer::Initialized(..) => None,
+        }
+    }
 }
 
 /// A single span of changed lines, with 0 or more removed lines
@@ -547,3 +567,112 @@ pub(crate) struct ModifiedLines {
     /// The set of changed chunks.
     pub chunks: Vec<ModifiedChunk>,
 }
+
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum Timer {
+    Initialized(Instant),
+    DoneParsing(Instant, Instant),
+    DoneFormatting(Instant, Instant, Instant),
+}
+
+impl Timer {
+    fn done_parsing(self) -> Self {
+        match self {
+            Timer::Initialized(init_time) => Timer::DoneParsing(init_time, Instant::now()),
+            _ => panic!("Timer can only transition to DoneParsing from Initialized state"),
+        }
+    }
+
+    fn done_formatting(self) -> Self {
+        match self {
+            Timer::DoneParsing(init_time, parse_time) => {
+                Timer::DoneFormatting(init_time, parse_time, Instant::now())
+            }
+            _ => panic!("Timer can only transition to DoneFormatting from DoneParsing state"),
+        }
+    }
+}
+
+/// A summary of a Rustfmt run.
+#[derive(Debug, Default, Clone, Copy)]
+pub struct Summary {
+    // Encountered e.g. an IO error.
+    has_operational_errors: bool,
+
+    // Failed to reformat code because of parsing errors.
+    has_parsing_errors: bool,
+
+    // Code is valid, but it is impossible to format it properly.
+    has_formatting_errors: bool,
+
+    // Code contains macro call that was unable to format.
+    pub(crate) has_macro_format_failure: bool,
+
+    // Failed a check, such as the license check or other opt-in checking.
+    has_check_errors: bool,
+
+    /// Formatted code differs from existing code (--check only).
+    pub has_diff: bool,
+}
+
+impl Summary {
+    pub fn has_operational_errors(&self) -> bool {
+        self.has_operational_errors
+    }
+
+    pub fn has_parsing_errors(&self) -> bool {
+        self.has_parsing_errors
+    }
+
+    pub fn has_formatting_errors(&self) -> bool {
+        self.has_formatting_errors
+    }
+
+    pub fn has_check_errors(&self) -> bool {
+        self.has_check_errors
+    }
+
+    pub(crate) fn has_macro_formatting_failure(&self) -> bool {
+        self.has_macro_format_failure
+    }
+
+    pub fn add_operational_error(&mut self) {
+        self.has_operational_errors = true;
+    }
+
+    pub(crate) fn add_parsing_error(&mut self) {
+        self.has_parsing_errors = true;
+    }
+
+    pub(crate) fn add_formatting_error(&mut self) {
+        self.has_formatting_errors = true;
+    }
+
+    pub(crate) fn add_check_error(&mut self) {
+        self.has_check_errors = true;
+    }
+
+    pub(crate) fn add_diff(&mut self) {
+        self.has_diff = true;
+    }
+
+    pub(crate) fn add_macro_foramt_failure(&mut self) {
+        self.has_macro_format_failure = true;
+    }
+
+    pub fn has_no_errors(&self) -> bool {
+        !(self.has_operational_errors
+            || self.has_parsing_errors
+            || self.has_formatting_errors
+            || self.has_diff)
+    }
+
+    /// Combine two summaries together.
+    pub fn add(&mut self, other: Summary) {
+        self.has_operational_errors |= other.has_operational_errors;
+        self.has_formatting_errors |= other.has_formatting_errors;
+        self.has_parsing_errors |= other.has_parsing_errors;
+        self.has_check_errors |= other.has_check_errors;
+        self.has_diff |= other.has_diff;
+    }
+}

src/lib.rs

@@ -45,15 +45,15 @@
 use std::mem;
 use std::path::PathBuf;
 use std::rc::Rc;
+use std::time::Instant;
 use syntax::ast;
 
 use comment::LineClasses;
 use failure::Fail;
-use formatting::{FormatErrorMap, FormattingError, ReportedErrors};
+use formatting::{FormatErrorMap, FormattingError, ReportedErrors, Summary, Timer};
 use issues::Issue;
 use shape::Indent;
 
-pub use config::summary::Summary;
 pub use config::{
     load_config, CliOptions, Color, Config, EmitMode, FileLines, FileName, NewlineStyle, Range,
     Verbosity,
@@ -445,6 +445,8 @@ pub struct Session<'b, T: Write + 'b> {
     pub config: Config,
     pub out: Option<&'b mut T>,
     pub summary: Summary,
+    // Keeps track of time spent in parsing and formatting steps.
+    timer: Timer,
 }
 
 impl<'b, T: Write + 'b> Session<'b, T> {
@@ -457,6 +459,7 @@ pub fn new(config: Config, out: Option<&'b mut T>) -> Session<'b, T> {
             config,
             out,
             summary: Summary::default(),
+            timer: Timer::Initialized(Instant::now()),
         }
     }
 

src/test/mod.rs

@@ -20,10 +20,9 @@
 use std::path::{Path, PathBuf};
 use std::str::Chars;
 
-use config::summary::Summary;
 use config::{Color, Config, EmitMode, FileName, ReportTactic};
 use filemap;
-use formatting::{FileMap, ModifiedChunk};
+use formatting::{FileMap, ModifiedChunk, Summary};
 use rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, OutputWriter};
 use {FormatReport, Input, Session};