From 069c4fc50871acfcf41f8726107daa26a29b488c Mon Sep 17 00:00:00 2001 From: Nick Cameron Date: Tue, 24 Jul 2018 08:42:33 +1200 Subject: [PATCH] Refactoring: summary Move the timer from Summary to Session. Move Summary from config to formatting. --- src/config/mod.rs | 1 - src/config/summary.rs | 160 ------------------------------------------ src/formatting.rs | 145 +++++++++++++++++++++++++++++++++++--- src/lib.rs | 7 +- src/test/mod.rs | 3 +- 5 files changed, 143 insertions(+), 173 deletions(-) delete mode 100644 src/config/summary.rs diff --git a/src/config/mod.rs b/src/config/mod.rs index ba6596c3da7..42686483419 100644 --- a/src/config/mod.rs +++ b/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: diff --git a/src/config/summary.rs b/src/config/summary.rs deleted file mode 100644 index 1ef6d18a540..00000000000 --- a/src/config/summary.rs +++ /dev/null @@ -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 or the MIT license -// , 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 { - 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 { - 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"), - } - } -} diff --git a/src/formatting.rs b/src/formatting.rs index d0184d6ea0e..43651fb4559 100644 --- a/src/formatting.rs +++ b/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; @@ -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( &mut self, @@ -389,7 +387,7 @@ fn format_ast( 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( 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 { + 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 { + 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, } + +#[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; + } +} diff --git a/src/lib.rs b/src/lib.rs index c35fb25f777..2cbe5c46e30 100644 --- a/src/lib.rs +++ b/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()), } } diff --git a/src/test/mod.rs b/src/test/mod.rs index f02cfbc858c..d670bc1d5db 100644 --- a/src/test/mod.rs +++ b/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}; -- 2.44.0