Use trait to abstract emit modes (#3616)

src/checkstyle.rs

@@ -1,102 +0,0 @@
-use std::fmt::{self, Display};
-use std::io::{self, Write};
-use std::path::Path;
-
-use crate::rustfmt_diff::{DiffLine, Mismatch};
-
-/// The checkstyle header - should be emitted before the output of Rustfmt.
-///
-/// Note that emitting checkstyle output is not stable and may removed in a
-/// future version of Rustfmt.
-pub(crate) fn header() -> String {
-    let mut xml_heading = String::new();
-    xml_heading.push_str(r#"<?xml version="1.0" encoding="utf-8"?>"#);
-    xml_heading.push_str("\n");
-    xml_heading.push_str(r#"<checkstyle version="4.3">"#);
-    xml_heading
-}
-
-/// The checkstyle footer - should be emitted after the output of Rustfmt.
-///
-/// Note that emitting checkstyle output is not stable and may removed in a
-/// future version of Rustfmt.
-pub(crate) fn footer() -> String {
-    "</checkstyle>\n".to_owned()
-}
-
-pub(crate) fn output_checkstyle_file<T>(
-    mut writer: T,
-    filename: &Path,
-    diff: Vec<Mismatch>,
-) -> Result<(), io::Error>
-where
-    T: Write,
-{
-    write!(writer, r#"<file name="{}">"#, filename.display())?;
-    for mismatch in diff {
-        for line in mismatch.lines {
-            // Do nothing with `DiffLine::Context` and `DiffLine::Resulting`.
-            if let DiffLine::Expected(message) = line {
-                write!(
-                    writer,
-                    r#"<error line="{}" severity="warning" message="Should be `{}`" />"#,
-                    mismatch.line_number,
-                    XmlEscaped(&message)
-                )?;
-            }
-        }
-    }
-    write!(writer, "</file>")?;
-    Ok(())
-}
-
-/// Convert special characters into XML entities.
-/// This is needed for checkstyle output.
-struct XmlEscaped<'a>(&'a str);
-
-impl<'a> Display for XmlEscaped<'a> {
-    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
-        for char in self.0.chars() {
-            match char {
-                '<' => write!(formatter, "&lt;"),
-                '>' => write!(formatter, "&gt;"),
-                '"' => write!(formatter, "&quot;"),
-                '\'' => write!(formatter, "&apos;"),
-                '&' => write!(formatter, "&amp;"),
-                _ => write!(formatter, "{}", char),
-            }?;
-        }
-
-        Ok(())
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn special_characters_are_escaped() {
-        assert_eq!(
-            "&lt;&gt;&quot;&apos;&amp;",
-            format!("{}", XmlEscaped(r#"<>"'&"#)),
-        );
-    }
-
-    #[test]
-    fn special_characters_are_escaped_in_string_with_other_characters() {
-        assert_eq!(
-            "The quick brown &quot;🦊&quot; jumps &lt;over&gt; the lazy 🐶",
-            format!(
-                "{}",
-                XmlEscaped(r#"The quick brown "🦊" jumps <over> the lazy 🐶"#)
-            ),
-        );
-    }
-
-    #[test]
-    fn other_characters_are_not_escaped() {
-        let string = "The quick brown 🦊 jumps over the lazy 🐶";
-        assert_eq!(string, format!("{}", XmlEscaped(string)));
-    }
-}

src/emitter.rs

@@ -0,0 +1,50 @@
+pub(crate) use self::checkstyle::*;
+pub(crate) use self::diff::*;
+pub(crate) use self::files::*;
+pub(crate) use self::files_with_backup::*;
+pub(crate) use self::modified_lines::*;
+pub(crate) use self::stdout::*;
+use crate::FileName;
+use std::io::{self, Write};
+use std::path::Path;
+
+mod checkstyle;
+mod diff;
+mod files;
+mod files_with_backup;
+mod modified_lines;
+mod stdout;
+
+pub(crate) struct FormattedFile<'a> {
+    pub(crate) filename: &'a FileName,
+    pub(crate) original_text: &'a str,
+    pub(crate) formatted_text: &'a str,
+}
+
+#[derive(Debug, Default, Clone)]
+pub(crate) struct EmitterResult {
+    pub(crate) has_diff: bool,
+}
+
+pub(crate) trait Emitter {
+    fn emit_formatted_file(
+        &self,
+        output: &mut dyn Write,
+        formatted_file: FormattedFile<'_>,
+    ) -> Result<EmitterResult, io::Error>;
+
+    fn emit_header(&self, _output: &mut dyn Write) -> Result<(), io::Error> {
+        Ok(())
+    }
+
+    fn emit_footer(&self, _output: &mut dyn Write) -> Result<(), io::Error> {
+        Ok(())
+    }
+}
+
+fn ensure_real_path(filename: &FileName) -> &Path {
+    match *filename {
+        FileName::Real(ref path) => path,
+        _ => panic!("cannot format `{}` and emit to files", filename),
+    }
+}

src/emitter/checkstyle.rs

@@ -0,0 +1,64 @@
+use self::xml::XmlEscaped;
+use super::*;
+use crate::rustfmt_diff::{make_diff, DiffLine, Mismatch};
+use std::io::{self, Write};
+use std::path::Path;
+
+mod xml;
+
+#[derive(Debug, Default)]
+pub(crate) struct CheckstyleEmitter;
+
+impl Emitter for CheckstyleEmitter {
+    fn emit_header(&self, output: &mut dyn Write) -> Result<(), io::Error> {
+        writeln!(output, r#"<?xml version="1.0" encoding="utf-8"?>"#)?;
+        write!(output, r#"<checkstyle version="4.3">"#)?;
+        Ok(())
+    }
+
+    fn emit_footer(&self, output: &mut dyn Write) -> Result<(), io::Error> {
+        writeln!(output, "</checkstyle>")
+    }
+
+    fn emit_formatted_file(
+        &self,
+        output: &mut dyn Write,
+        FormattedFile {
+            filename,
+            original_text,
+            formatted_text,
+        }: FormattedFile<'_>,
+    ) -> Result<EmitterResult, io::Error> {
+        const CONTEXT_SIZE: usize = 3;
+        let filename = ensure_real_path(filename);
+        let diff = make_diff(original_text, formatted_text, CONTEXT_SIZE);
+        output_checkstyle_file(output, filename, diff)?;
+        Ok(EmitterResult::default())
+    }
+}
+
+pub(crate) fn output_checkstyle_file<T>(
+    mut writer: T,
+    filename: &Path,
+    diff: Vec<Mismatch>,
+) -> Result<(), io::Error>
+where
+    T: Write,
+{
+    write!(writer, r#"<file name="{}">"#, filename.display())?;
+    for mismatch in diff {
+        for line in mismatch.lines {
+            // Do nothing with `DiffLine::Context` and `DiffLine::Resulting`.
+            if let DiffLine::Expected(message) = line {
+                write!(
+                    writer,
+                    r#"<error line="{}" severity="warning" message="Should be `{}`" />"#,
+                    mismatch.line_number,
+                    XmlEscaped(&message)
+                )?;
+            }
+        }
+    }
+    write!(writer, "</file>")?;
+    Ok(())
+}

src/emitter/checkstyle/xml.rs

@@ -0,0 +1,52 @@
+use std::fmt::{self, Display};
+
+/// Convert special characters into XML entities.
+/// This is needed for checkstyle output.
+pub(super) struct XmlEscaped<'a>(pub(super) &'a str);
+
+impl<'a> Display for XmlEscaped<'a> {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for char in self.0.chars() {
+            match char {
+                '<' => write!(formatter, "&lt;"),
+                '>' => write!(formatter, "&gt;"),
+                '"' => write!(formatter, "&quot;"),
+                '\'' => write!(formatter, "&apos;"),
+                '&' => write!(formatter, "&amp;"),
+                _ => write!(formatter, "{}", char),
+            }?;
+        }
+
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn special_characters_are_escaped() {
+        assert_eq!(
+            "&lt;&gt;&quot;&apos;&amp;",
+            format!("{}", XmlEscaped(r#"<>"'&"#)),
+        );
+    }
+
+    #[test]
+    fn special_characters_are_escaped_in_string_with_other_characters() {
+        assert_eq!(
+            "The quick brown &quot;🦊&quot; jumps &lt;over&gt; the lazy 🐶",
+            format!(
+                "{}",
+                XmlEscaped(r#"The quick brown "🦊" jumps <over> the lazy 🐶"#)
+            ),
+        );
+    }
+
+    #[test]
+    fn other_characters_are_not_escaped() {
+        let string = "The quick brown 🦊 jumps over the lazy 🐶";
+        assert_eq!(string, format!("{}", XmlEscaped(string)));
+    }
+}

src/emitter/diff.rs

@@ -0,0 +1,35 @@
+use super::*;
+use crate::config::Config;
+use crate::rustfmt_diff::{make_diff, print_diff};
+
+pub(crate) struct DiffEmitter {
+    config: Config,
+}
+
+impl DiffEmitter {
+    pub(crate) fn new(config: Config) -> Self {
+        Self { config }
+    }
+}
+
+impl Emitter for DiffEmitter {
+    fn emit_formatted_file(
+        &self,
+        _output: &mut dyn Write,
+        FormattedFile {
+            filename,
+            original_text,
+            formatted_text,
+        }: FormattedFile<'_>,
+    ) -> Result<EmitterResult, io::Error> {
+        const CONTEXT_SIZE: usize = 3;
+        let mismatch = make_diff(&original_text, formatted_text, CONTEXT_SIZE);
+        let has_diff = !mismatch.is_empty();
+        print_diff(
+            mismatch,
+            |line_num| format!("Diff in {} at line {}:", filename, line_num),
+            &self.config,
+        );
+        return Ok(EmitterResult { has_diff });
+    }
+}

src/emitter/files.rs

@@ -0,0 +1,24 @@
+use super::*;
+use std::fs;
+
+#[derive(Debug, Default)]
+pub(crate) struct FilesEmitter;
+
+impl Emitter for FilesEmitter {
+    fn emit_formatted_file(
+        &self,
+        _output: &mut dyn Write,
+        FormattedFile {
+            filename,
+            original_text,
+            formatted_text,
+        }: FormattedFile<'_>,
+    ) -> Result<EmitterResult, io::Error> {
+        // Write text directly over original file if there is a diff.
+        let filename = ensure_real_path(filename);
+        if original_text != formatted_text {
+            fs::write(filename, formatted_text)?;
+        }
+        Ok(EmitterResult::default())
+    }
+}

src/emitter/files_with_backup.rs

@@ -0,0 +1,31 @@
+use super::*;
+use std::fs;
+
+#[derive(Debug, Default)]
+pub(crate) struct FilesWithBackupEmitter;
+
+impl Emitter for FilesWithBackupEmitter {
+    fn emit_formatted_file(
+        &self,
+        _output: &mut dyn Write,
+        FormattedFile {
+            filename,
+            original_text,
+            formatted_text,
+        }: FormattedFile<'_>,
+    ) -> Result<EmitterResult, io::Error> {
+        let filename = ensure_real_path(filename);
+        if original_text != formatted_text {
+            // Do a little dance to make writing safer - write to a temp file
+            // rename the original to a .bk, then rename the temp file to the
+            // original.
+            let tmp_name = filename.with_extension("tmp");
+            let bk_name = filename.with_extension("bk");
+
+            fs::write(&tmp_name, formatted_text)?;
+            fs::rename(filename, bk_name)?;
+            fs::rename(tmp_name, filename)?;
+        }
+        Ok(EmitterResult::default())
+    }
+}

src/emitter/modified_lines.rs

@@ -0,0 +1,24 @@
+use super::*;
+use crate::rustfmt_diff::{make_diff, ModifiedLines};
+use std::io::Write;
+
+#[derive(Debug, Default)]
+pub(crate) struct ModifiedLinesEmitter;
+
+impl Emitter for ModifiedLinesEmitter {
+    fn emit_formatted_file(
+        &self,
+        output: &mut dyn Write,
+        FormattedFile {
+            original_text,
+            formatted_text,
+            ..
+        }: FormattedFile<'_>,
+    ) -> Result<EmitterResult, io::Error> {
+        const CONTEXT_SIZE: usize = 0;
+        let mismatch = make_diff(original_text, formatted_text, CONTEXT_SIZE);
+        let has_diff = !mismatch.is_empty();
+        write!(output, "{}", ModifiedLines::from(mismatch))?;
+        Ok(EmitterResult { has_diff })
+    }
+}

src/emitter/stdout.rs

@@ -0,0 +1,32 @@
+use super::*;
+use crate::config::Verbosity;
+use std::io::Write;
+
+#[derive(Debug)]
+pub(crate) struct StdoutEmitter {
+    verbosity: Verbosity,
+}
+
+impl StdoutEmitter {
+    pub(crate) fn new(verbosity: Verbosity) -> Self {
+        Self { verbosity }
+    }
+}
+
+impl Emitter for StdoutEmitter {
+    fn emit_formatted_file(
+        &self,
+        output: &mut dyn Write,
+        FormattedFile {
+            filename,
+            formatted_text,
+            ..
+        }: FormattedFile<'_>,
+    ) -> Result<EmitterResult, io::Error> {
+        if self.verbosity != Verbosity::Quiet {
+            writeln!(output, "{}:\n", filename)?;
+        }
+        write!(output, "{}", formatted_text)?;
+        Ok(EmitterResult::default())
+    }
+}

src/formatting.rs

@@ -233,8 +233,8 @@ impl<'b, T: Write + 'b> FormatHandler for Session<'b, T> {
         report: &mut FormatReport,
     ) -> Result<(), ErrorKind> {
         if let Some(ref mut out) = self.out {
-            match source_file::write_file(Some(source_map), &path, &result, out, &self.config) {
-                Ok(has_diff) if has_diff => report.add_diff(),
+            match source_file::write_file(Some(source_map), &path, &result, out, &*self.emitter) {
+                Ok(ref result) if result.has_diff => report.add_diff(),
                 Err(e) => {
                     // Create a new error with path_str to help users see which files failed
                     let err_msg = format!("{}: {}", path, e);

src/lib.rs

@@ -23,6 +23,7 @@ use ignore;
 use syntax::{ast, parse::DirectoryOwnership};
 
 use crate::comment::LineClasses;
+use crate::emitter::Emitter;
 use crate::formatting::{FormatErrorMap, FormattingError, ReportedErrors, SourceFile};
 use crate::issues::Issue;
 use crate::shape::Indent;
@@ -45,10 +46,10 @@ mod release_channel;
 
 mod attr;
 mod chains;
-pub(crate) mod checkstyle;
 mod closures;
 mod comment;
 pub(crate) mod config;
+mod emitter;
 mod expr;
 mod format_report_formatter;
 pub(crate) mod formatting;
@@ -403,17 +404,21 @@ pub struct Session<'b, T: Write> {
     pub out: Option<&'b mut T>,
     pub(crate) errors: ReportedErrors,
     source_file: SourceFile,
+    emitter: Box<dyn Emitter + 'b>,
 }
 
 impl<'b, T: Write + 'b> Session<'b, T> {
-    pub fn new(config: Config, out: Option<&'b mut T>) -> Session<'b, T> {
-        if config.emit_mode() == EmitMode::Checkstyle {
-            println!("{}", checkstyle::header());
+    pub fn new(config: Config, mut out: Option<&'b mut T>) -> Session<'b, T> {
+        let emitter = create_emitter(&config);
+
+        if let Some(ref mut out) = out {
+            let _ = emitter.emit_header(out);
         }
 
         Session {
             config,
             out,
+            emitter,
             errors: ReportedErrors::default(),
             source_file: SourceFile::new(),
         }
@@ -469,10 +474,25 @@ impl<'b, T: Write + 'b> Session<'b, T> {
     }
 }
 
+pub(crate) fn create_emitter<'a>(config: &Config) -> Box<dyn Emitter + 'a> {
+    match config.emit_mode() {
+        EmitMode::Files if config.make_backup() => {
+            Box::new(emitter::FilesWithBackupEmitter::default())
+        }
+        EmitMode::Files => Box::new(emitter::FilesEmitter::default()),
+        EmitMode::Stdout | EmitMode::Coverage => {
+            Box::new(emitter::StdoutEmitter::new(config.verbose()))
+        }
+        EmitMode::ModifiedLines => Box::new(emitter::ModifiedLinesEmitter::default()),
+        EmitMode::Checkstyle => Box::new(emitter::CheckstyleEmitter::default()),
+        EmitMode::Diff => Box::new(emitter::DiffEmitter::new(config.clone())),
+    }
+}
+
 impl<'b, T: Write + 'b> Drop for Session<'b, T> {
     fn drop(&mut self) {
-        if self.config.emit_mode() == EmitMode::Checkstyle {
-            println!("{}", checkstyle::footer());
+        if let Some(ref mut out) = self.out {
+            let _ = self.emitter.emit_footer(out);
         }
     }
 }

src/source_file.rs

@@ -4,10 +4,13 @@ use std::path::Path;
 
 use syntax::source_map::SourceMap;
 
-use crate::checkstyle::output_checkstyle_file;
-use crate::config::{Config, EmitMode, FileName, Verbosity};
-use crate::rustfmt_diff::{make_diff, print_diff, ModifiedLines};
+use crate::config::FileName;
+use crate::emitter::{self, Emitter};
 
+#[cfg(test)]
+use crate::config::Config;
+#[cfg(test)]
+use crate::create_emitter;
 #[cfg(test)]
 use crate::formatting::FileRecord;
 
@@ -25,15 +28,13 @@ pub(crate) fn write_all_files<T>(
 where
     T: Write,
 {
-    if config.emit_mode() == EmitMode::Checkstyle {
-        write!(out, "{}", crate::checkstyle::header())?;
-    }
+    let emitter = create_emitter(config);
+
+    emitter.emit_header(out)?;
     for &(ref filename, ref text) in source_file {
-        write_file(None, filename, text, out, config)?;
-    }
-    if config.emit_mode() == EmitMode::Checkstyle {
-        write!(out, "{}", crate::checkstyle::footer())?;
+        write_file(None, filename, text, out, &*emitter)?;
     }
+    emitter.emit_footer(out)?;
 
     Ok(())
 }
@@ -43,8 +44,8 @@ pub(crate) fn write_file<T>(
     filename: &FileName,
     formatted_text: &str,
     out: &mut T,
-    config: &Config,
-) -> Result<bool, io::Error>
+    emitter: &dyn Emitter,
+) -> Result<emitter::EmitterResult, io::Error>
 where
     T: Write,
 {
@@ -75,59 +76,11 @@ where
         None => fs::read_to_string(ensure_real_path(filename))?,
     };
 
-    match config.emit_mode() {
-        EmitMode::Files if config.make_backup() => {
-            let filename = ensure_real_path(filename);
-            if original_text != formatted_text {
-                // Do a little dance to make writing safer - write to a temp file
-                // rename the original to a .bk, then rename the temp file to the
-                // original.
-                let tmp_name = filename.with_extension("tmp");
-                let bk_name = filename.with_extension("bk");
+    let formatted_file = emitter::FormattedFile {
+        filename,
+        original_text: &original_text,
+        formatted_text,
+    };
 
-                fs::write(&tmp_name, formatted_text)?;
-                fs::rename(filename, bk_name)?;
-                fs::rename(tmp_name, filename)?;
-            }
-        }
-        EmitMode::Files => {
-            // Write text directly over original file if there is a diff.
-            let filename = ensure_real_path(filename);
-
-            if original_text != formatted_text {
-                fs::write(filename, formatted_text)?;
-            }
-        }
-        EmitMode::Stdout | EmitMode::Coverage => {
-            if config.verbose() != Verbosity::Quiet {
-                println!("{}:\n", filename);
-            }
-            write!(out, "{}", formatted_text)?;
-        }
-        EmitMode::ModifiedLines => {
-            let mismatch = make_diff(&original_text, formatted_text, 0);
-            let has_diff = !mismatch.is_empty();
-            write!(out, "{}", ModifiedLines::from(mismatch))?;
-            return Ok(has_diff);
-        }
-        EmitMode::Checkstyle => {
-            let filename = ensure_real_path(filename);
-
-            let diff = make_diff(&original_text, formatted_text, 3);
-            output_checkstyle_file(out, filename, diff)?;
-        }
-        EmitMode::Diff => {
-            let mismatch = make_diff(&original_text, formatted_text, 3);
-            let has_diff = !mismatch.is_empty();
-            print_diff(
-                mismatch,
-                |line_num| format!("Diff in {} at line {}:", filename, line_num),
-                config,
-            );
-            return Ok(has_diff);
-        }
-    }
-
-    // when we are not in diff mode, don't indicate differing files
-    Ok(false)
+    emitter.emit_formatted_file(out, formatted_file)
 }

src/test/mod.rs

@@ -9,7 +9,7 @@ use std::process::{Command, Stdio};
 use std::str::Chars;
 use std::thread;
 
-use crate::config::{Color, Config, EmitMode, FileName, NewlineStyle, ReportTactic};
+use crate::config::{Color, Config, EmitMode, FileName, NewlineStyle, ReportTactic, Verbosity};
 use crate::formatting::{ReportedErrors, SourceFile};
 use crate::is_nightly_channel;
 use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, ModifiedChunk, OutputWriter};
@@ -344,9 +344,9 @@ fn stdin_formatting_smoke_test() {
     }
 
     #[cfg(not(windows))]
-    assert_eq!(buf, "fn main() {}\n".as_bytes());
+    assert_eq!(buf, "stdin:\n\nfn main() {}\n".as_bytes());
     #[cfg(windows)]
-    assert_eq!(buf, "fn main() {}\r\n".as_bytes());
+    assert_eq!(buf, "stdin:\n\nfn main() {}\r\n".as_bytes());
 }
 
 #[test]
@@ -838,6 +838,7 @@ impl ConfigCodeBlock {
 
     fn get_block_config(&self) -> Config {
         let mut config = Config::default();
+        config.set().verbose(Verbosity::Quiet);
         if self.config_name.is_some() && self.config_value.is_some() {
             config.override_value(
                 self.config_name.as_ref().unwrap(),