Refactoring: factor out `format_file` and `FormatHandler`

This effectively separates out formatting from other handling.

diff --git a/src/formatting.rs b/src/formatting.rs
index 1c196ca3b751996c61e62f05e550ed9e529bf34d..71ebe9d41026756e7ec9a4eb241b8176582ca4a5 100644 (file)
--- a/src/formatting.rs
+++ b/src/formatting.rs
@@ -20,7 +20,6 @@
 
 // A map of the files of a crate, with their new content
 pub(crate) type FileMap = Vec<FileRecord>;
 
 // A map of the files of a crate, with their new content
 pub(crate) type FileMap = Vec<FileRecord>;

-

 pub(crate) type FileRecord = (FileName, String);
 
 pub(crate) struct FormattingError {
 pub(crate) type FileRecord = (FileName, String);
 
 pub(crate) struct FormattingError {


@@ -297,56 +296,72 @@ enum ParseError<'sess> {
 }
 
 impl<'b, T: Write + 'b> Session<'b, T> {
 }
 
 impl<'b, T: Write + 'b> Session<'b, T> {

-    pub(crate) fn format_input_inner(
-        &mut self,
-        input: Input,
-    ) -> Result<(FileMap, FormatReport), ErrorKind> {
-        syntax_pos::hygiene::set_default_edition(self.config.edition().to_libsyntax_pos_edition());
-
-        if self.config.disable_all_formatting() {
-            // When the input is from stdin, echo back the input.
-            if let Input::Text(ref buf) = input {
-                if let Err(e) = io::stdout().write_all(buf.as_bytes()) {
-                    return Err(From::from(e));
-                }
-            }
-            return Ok((FileMap::new(), FormatReport::new()));
+    pub(crate) fn format_input_inner(&mut self, input: Input) -> Result<FormatReport, ErrorKind> {
+        if !self.config.version_meets_requirement() {
+            return Err(ErrorKind::VersionMismatch);

         }
 
         }
 

-        let mut filemap = FileMap::new();
-        let config = &self.config.clone();
-        let format_result = format_project(input, config, |path, mut result| {
-            if let Some(ref mut out) = self.out {
-                match filemap::write_file(&mut result, &path, out, &self.config) {
-                    Ok(b) if b => self.summary.add_diff(),
-                    Err(e) => {
-                        // Create a new error with path_str to help users see which files failed
-                        let err_msg = format!("{}: {}", path, e);
-                        return Err(io::Error::new(e.kind(), err_msg).into());
+        syntax::with_globals(|| {
+            syntax_pos::hygiene::set_default_edition(
+                self.config.edition().to_libsyntax_pos_edition(),
+            );
+
+            if self.config.disable_all_formatting() {
+                // When the input is from stdin, echo back the input.
+                if let Input::Text(ref buf) = input {
+                    if let Err(e) = io::stdout().write_all(buf.as_bytes()) {
+                        return Err(From::from(e));

                     }
                     }

-                    _ => {}

                 }
                 }

+                return Ok(FormatReport::new());

             }
 
             }
 

-            filemap.push((path, result));
-            Ok(())
-        });
+            let config = &self.config.clone();
+            let format_result = format_project(input, config, self);

 
 

-        format_result.map(|(result, summary)| {
-            self.summary.add(summary);
-            (filemap, result)
+            format_result.map(|(report, summary)| {
+                self.summary.add(summary);
+                report
+            })

         })
     }
 }
 
         })
     }
 }
 

-fn format_project<F>(
+// Handle the results of formatting.
+trait FormatHandler {
+    fn handle_formatted_file(&mut self, path: FileName, result: String) -> Result<(), ErrorKind>;
+}
+
+impl<'b, T: Write + 'b> FormatHandler for Session<'b, T> {
+    // Called for each formatted file.
+    fn handle_formatted_file(
+        &mut self,
+        path: FileName,
+        mut result: String,
+    ) -> Result<(), ErrorKind> {
+        if let Some(ref mut out) = self.out {
+            match filemap::write_file(&mut result, &path, out, &self.config) {
+                Ok(b) if b => self.summary.add_diff(),
+                Err(e) => {
+                    // Create a new error with path_str to help users see which files failed
+                    let err_msg = format!("{}: {}", path, e);
+                    return Err(io::Error::new(e.kind(), err_msg).into());
+                }
+                _ => {}
+            }
+        }
+
+        self.filemap.push((path, result));
+        Ok(())
+    }
+}
+
+// Format an entire crate (or subset of the module tree).
+fn format_project<T: FormatHandler>(

     input: Input,
     config: &Config,
     input: Input,
     config: &Config,

-    mut formatted_file: F,
-) -> Result<(FormatReport, Summary), ErrorKind>
-where
-    F: FnMut(FileName, String) -> Result<(), ErrorKind>,
-{
+    handler: &mut T,
+) -> Result<(FormatReport, Summary), ErrorKind> {

     let mut summary = Summary::default();
     let mut timer = Timer::Initialized(Instant::now());
 
     let mut summary = Summary::default();
     let mut timer = Timer::Initialized(Instant::now());
 


@@ -368,7 +383,7 @@ fn format_project<F>(
                     // Note that if you see this message and want more information,
                     // then go to `parse_input` and run the parse function without
                     // `catch_unwind` so rustfmt panics and you can get a backtrace.
                     // Note that if you see this message and want more information,
                     // then go to `parse_input` and run the parse function without
                     // `catch_unwind` so rustfmt panics and you can get a backtrace.

-                    should_emit_verbose(main_file != FileName::Stdin, config, || {
+                    should_emit_verbose(!input_is_stdin, config, || {

                         println!("The Rust parser panicked")
                     });
                 }
                         println!("The Rust parser panicked")
                     });
                 }


@@ -389,28 +404,86 @@ fn format_project<F>(
     ));
     parse_session.span_diagnostic = Handler::with_emitter(true, false, silent_emitter);
 
     ));
     parse_session.span_diagnostic = Handler::with_emitter(true, false, silent_emitter);
 

-    let report = FormatReport::new();
-
-    let skip_children = config.skip_children();
-    for (path, module) in modules::list_files(&krate, parse_session.codemap())? {
-        if (skip_children && path != main_file) || config.ignore().skip_file(&path) {
+    let mut context = FormatContext::new(
+        &krate,
+        FormatReport::new(),
+        summary,
+        parse_session,
+        config,
+        handler,
+    );
+
+    let files = modules::list_files(&krate, context.parse_session.codemap())?;
+    for (path, module) in files {
+        if (config.skip_children() && path != main_file) || config.ignore().skip_file(&path) {

             continue;
         }
             continue;
         }

-        should_emit_verbose(main_file != FileName::Stdin, config, || {
-            println!("Formatting {}", path)
-        });
-        let filemap = parse_session
+        should_emit_verbose(!input_is_stdin, config, || println!("Formatting {}", path));
+        let is_root = path == main_file;
+        context.format_file(path, module, is_root)?;
+    }
+    timer = timer.done_formatting();
+
+    should_emit_verbose(!input_is_stdin, config, || {
+        println!(
+            "Spent {0:.3} secs in the parsing phase, and {1:.3} secs in the formatting phase",
+            timer.get_parse_time(),
+            timer.get_format_time(),
+        )
+    });
+
+    if context.report.has_warnings() {
+        context.summary.add_formatting_error();
+    }
+    {
+        let report_errs = &context.report.internal.borrow().1;
+        if report_errs.has_check_errors {
+            context.summary.add_check_error();
+        }
+        if report_errs.has_operational_errors {
+            context.summary.add_operational_error();
+        }
+    }
+
+    Ok((context.report, context.summary))
+}
+
+// Used for formatting files.
+#[derive(new)]
+struct FormatContext<'a, T: FormatHandler + 'a> {
+    krate: &'a ast::Crate,
+    report: FormatReport,
+    summary: Summary,
+    parse_session: ParseSess,
+    config: &'a Config,
+    handler: &'a mut T,
+}
+
+impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> {
+    // Formats a single file/module.
+    fn format_file(
+        &mut self,
+        path: FileName,
+        module: &ast::Mod,
+        is_root: bool,
+    ) -> Result<(), ErrorKind> {
+        let filemap = self
+            .parse_session

             .codemap()
             .lookup_char_pos(module.inner.lo())
             .file;
         let big_snippet = filemap.src.as_ref().unwrap();
         let snippet_provider = SnippetProvider::new(filemap.start_pos, big_snippet);
             .codemap()
             .lookup_char_pos(module.inner.lo())
             .file;
         let big_snippet = filemap.src.as_ref().unwrap();
         let snippet_provider = SnippetProvider::new(filemap.start_pos, big_snippet);

-        let mut visitor =
-            FmtVisitor::from_codemap(&parse_session, config, &snippet_provider, report.clone());
+        let mut visitor = FmtVisitor::from_codemap(
+            &self.parse_session,
+            &self.config,
+            &snippet_provider,
+            self.report.clone(),
+        );

         // Format inner attributes if available.
         // Format inner attributes if available.

-        if !krate.attrs.is_empty() && path == main_file {
+        if !self.krate.attrs.is_empty() && is_root {

             visitor.skip_empty_lines(filemap.end_pos);
             visitor.skip_empty_lines(filemap.end_pos);

-            if visitor.visit_attrs(&krate.attrs, ast::AttrStyle::Inner) {
+            if visitor.visit_attrs(&self.krate.attrs, ast::AttrStyle::Inner) {

                 visitor.push_rewrite(module.inner, None);
             } else {
                 visitor.format_separate_mod(module, &*filemap);
                 visitor.push_rewrite(module.inner, None);
             } else {
                 visitor.format_separate_mod(module, &*filemap);


@@ -434,41 +507,17 @@ fn format_project<F>(
             &mut visitor.buffer,
             &path,
             &visitor.skipped_range,
             &mut visitor.buffer,
             &path,
             &visitor.skipped_range,

-            config,
-            &report,
+            &self.config,
+            &self.report,

         );
         );

-        replace_with_system_newlines(&mut visitor.buffer, config);
+        replace_with_system_newlines(&mut visitor.buffer, &self.config);

 
         if visitor.macro_rewrite_failure {
 
         if visitor.macro_rewrite_failure {

-            summary.add_macro_format_failure();
+            self.summary.add_macro_format_failure();

         }
 
         }
 

-        formatted_file(path, visitor.buffer)?;
+        self.handler.handle_formatted_file(path, visitor.buffer)

     }
     }

-    timer = timer.done_formatting();
-
-    should_emit_verbose(input_is_stdin, config, || {
-        println!(
-            "Spent {0:.3} secs in the parsing phase, and {1:.3} secs in the formatting phase",
-            timer.get_parse_time(),
-            timer.get_format_time(),
-        )
-    });
-
-    if report.has_warnings() {
-        summary.add_formatting_error();
-    }
-    {
-        let report_errs = &report.internal.borrow().1;
-        if report_errs.has_check_errors {
-            summary.add_check_error();
-        }
-        if report_errs.has_operational_errors {
-            summary.add_operational_error();
-        }
-    }
-
-    Ok((report, summary))

 }
 
 fn make_parse_sess(codemap: Rc<CodeMap>, config: &Config) -> ParseSess {
 }
 
 fn make_parse_sess(codemap: Rc<CodeMap>, config: &Config) -> ParseSess {

diff --git a/src/lib.rs b/src/lib.rs
index 4d90f5a223eb86dd73e61af9dafb59325bed2586..e21bd6c79be89a144a247fc7d5f158646b7d6052 100644 (file)
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -49,7 +49,7 @@
 
 use comment::LineClasses;
 use failure::Fail;
 
 use comment::LineClasses;
 use failure::Fail;

-use formatting::{FormatErrorMap, FormattingError, ReportedErrors, Summary};
+use formatting::{FileMap, FormatErrorMap, FormattingError, ReportedErrors, Summary};

 use issues::Issue;
 use shape::Indent;
 
 use issues::Issue;
 use shape::Indent;
 


@@ -444,6 +444,7 @@ pub struct Session<'b, T: Write + 'b> {
     pub config: Config,
     pub out: Option<&'b mut T>,
     pub summary: Summary,
     pub config: Config,
     pub out: Option<&'b mut T>,
     pub summary: Summary,

+    filemap: FileMap,

 }
 
 impl<'b, T: Write + 'b> Session<'b, T> {
 }
 
 impl<'b, T: Write + 'b> Session<'b, T> {


@@ -456,17 +457,14 @@ pub fn new(config: Config, out: Option<&'b mut T>) -> Session<'b, T> {
             config,
             out,
             summary: Summary::default(),
             config,
             out,
             summary: Summary::default(),

+            filemap: FileMap::new(),

         }
     }
 
     /// The main entry point for Rustfmt. Formats the given input according to the
     /// given config. `out` is only necessary if required by the configuration.
     pub fn format(&mut self, input: Input) -> Result<FormatReport, ErrorKind> {
         }
     }
 
     /// The main entry point for Rustfmt. Formats the given input according to the
     /// given config. `out` is only necessary if required by the configuration.
     pub fn format(&mut self, input: Input) -> Result<FormatReport, ErrorKind> {

-        if !self.config.version_meets_requirement() {
-            return Err(ErrorKind::VersionMismatch);
-        }
-
-        syntax::with_globals(|| self.format_input_inner(input)).map(|tup| tup.1)
+        self.format_input_inner(input)

     }
 
     pub fn override_config<F, U>(&mut self, mut config: Config, f: F) -> U
     }
 
     pub fn override_config<F, U>(&mut self, mut config: Config, f: F) -> U

diff --git a/src/test/mod.rs b/src/test/mod.rs
index d670bc1d5dbf7f7a8989300464cf3a70ccdc7eda..63b5f244b9067e96f815fd6e2c1ea630088aa8d4 100644 (file)
--- a/src/test/mod.rs
+++ b/src/test/mod.rs
@@ -10,13 +10,12 @@
 
 extern crate assert_cli;
 
 
 extern crate assert_cli;
 

-use syntax;
-

 use std::collections::{HashMap, HashSet};
 use std::env;
 use std::fs;
 use std::io::{self, BufRead, BufReader, Read, Write};
 use std::iter::{Enumerate, Peekable};
 use std::collections::{HashMap, HashSet};
 use std::env;
 use std::fs;
 use std::io::{self, BufRead, BufReader, Read, Write};
 use std::iter::{Enumerate, Peekable};

+use std::mem;

 use std::path::{Path, PathBuf};
 use std::str::Chars;
 
 use std::path::{Path, PathBuf};
 use std::str::Chars;
 


@@ -419,11 +418,11 @@ fn format_file<P: Into<PathBuf>>(filepath: P, config: Config) -> (bool, FileMap,
     let filepath = filepath.into();
     let input = Input::File(filepath);
     let mut session = Session::<io::Stdout>::new(config, None);
     let filepath = filepath.into();
     let input = Input::File(filepath);
     let mut session = Session::<io::Stdout>::new(config, None);

-    syntax::with_globals(|| {
-        let result = session.format_input_inner(input).unwrap();
-        let parsing_errors = session.summary.has_parsing_errors();
-        (parsing_errors, result.0, result.1)
-    })
+    let result = session.format(input).unwrap();
+    let parsing_errors = session.summary.has_parsing_errors();
+    let mut filemap = FileMap::new();
+    mem::swap(&mut session.filemap, &mut filemap);
+    (parsing_errors, filemap, result)

 }
 
 enum IdempotentCheckError {
 }
 
 enum IdempotentCheckError {