Refactor

Author: topecongiro

Configurations.md

@@ -311,14 +311,6 @@ By default this option is set as a percentage of [`max_width`](#max_width) provi
 
 See also [`max_width`](#max_width) and [`width_heuristics`](#width_heuristics)
 
-## `color`
-
-Whether to use colored output or not.
-
-- **Default value**: `"Auto"`
-- **Possible values**: "Auto", "Always", "Never"
-- **Stable**: No (tracking issue: [#3385](https://github.com/rust-lang/rustfmt/issues/3385))
-
 ## `combine_control_expr`
 
 Combine control expressions with function calls.
@@ -2629,17 +2621,4 @@ Break comments to fit on the line
 // magna aliqua. Ut enim ad minim veniam, quis nostrud
 // exercitation ullamco laboris nisi ut aliquip ex ea
 // commodo consequat.
-```
-
-# Internal Options
-
-## `emit_mode`
-
-Internal option
-
-## `print_misformatted_file_names`
-
-Internal option, use `-l` or `--files-with-diff`
-
-## `recursive`
-Internal option, use `-r` or `--recursive`
+```

rustfmt-core/rustfmt-bin/src/bin/main.rs

@@ -14,12 +14,14 @@ use structopt::StructOpt;
 use thiserror::Error;
 
 use rustfmt_lib::{
-    load_config, CliOptions, Config, Edition, EmitMode, FileLines, FileName,
-    FormatReportFormatterBuilder, Input, Session, Verbosity,
+    load_config, write_all_files, CliOptions, Config, Edition, EmitMode, EmitterConfig, FileLines,
+    FileName, FormatReport, FormatReportFormatterBuilder, Input, RustFormatterBuilder, Session,
+    Verbosity,
 };
 
 fn main() {
     env_logger::init();
+
     let opt: Opt = Opt::from_args();
 
     let exit_code = match execute(opt) {
@@ -127,6 +129,32 @@ struct Opt {
     files: Vec<PathBuf>,
 }
 
+impl Opt {
+    fn verbosity(&self) -> Verbosity {
+        if self.verbose {
+            Verbosity::Verbose
+        } else if self.quiet {
+            Verbosity::Quiet
+        } else {
+            Verbosity::Normal
+        }
+    }
+
+    fn emitter_config(&self, default_emit_mode: EmitMode) -> EmitterConfig {
+        let emit_mode = if self.check {
+            EmitMode::Diff
+        } else {
+            self.emit.map_or(default_emit_mode, Emit::to_emit_mode)
+        };
+        EmitterConfig {
+            emit_mode,
+            verbosity: self.verbosity(),
+            print_filename: self.files_with_diff,
+            ..EmitterConfig::default()
+        }
+    }
+}
+
 #[derive(Debug, Clone)]
 struct InlineConfig(HashMap<String, String>, bool /* is help */);
 
@@ -307,29 +335,13 @@ impl From<IoError> for OperationError {
 
 impl CliOptions for Opt {
     fn apply_to(&self, config: &mut Config) {
-        if self.verbose {
-            config.set().verbose(Verbosity::Verbose);
-        } else if self.quiet {
-            config.set().verbose(Verbosity::Quiet);
-        }
         config.set().file_lines(self.file_lines.clone());
-        if self.recursive {
-            config.set().recursive(true);
-        }
         if self.error_on_unformatted {
             config.set().error_on_unformatted(true);
         }
         if let Some(ref edition) = self.edition {
             config.set().edition((*edition).clone());
         }
-        if self.check {
-            config.set().emit_mode(EmitMode::Diff);
-        } else if let Some(emit) = self.emit {
-            config.set().emit_mode(emit.to_emit_mode());
-        }
-        if self.files_with_diff {
-            config.set().print_misformatted_file_names(true);
-        }
         if let Some(ref inline_configs) = self.inline_config {
             for inline_config in inline_configs {
                 for (k, v) in &inline_config.0 {
@@ -392,17 +404,8 @@ fn format_string(input: String, opt: Opt) -> Result<i32> {
     // try to read config from local directory
     let (mut config, _) = load_config(Some(Path::new(".")), Some(&opt))?;
 
-    if opt.check {
-        config.set().emit_mode(EmitMode::Diff);
-    } else {
-        config
-            .set()
-            .emit_mode(opt.emit.map_or(EmitMode::Stdout, Emit::to_emit_mode));
-    }
-    config.set().verbose(Verbosity::Quiet);
-
     // parse file_lines
-    config.set().file_lines(opt.file_lines);
+    config.set().file_lines(opt.file_lines.clone());
     for f in config.file_lines().files() {
         match *f {
             FileName::Stdin => {}
@@ -411,15 +414,20 @@ fn format_string(input: String, opt: Opt) -> Result<i32> {
     }
 
     let out = &mut stdout();
-    let mut session = Session::new(config, Some(out));
-    format_and_emit_report(&mut session, Input::Text(input));
-
-    let exit_code = if session.has_operational_errors() || session.has_parsing_errors() {
-        1
-    } else {
-        0
-    };
-    Ok(exit_code)
+    let mut session = RustFormatterBuilder::default()
+        .recursive(opt.recursive)
+        .verbosity(Verbosity::Quiet)
+        .build();
+    let mut format_report = FormatReport::new();
+    // FIXME: Add error handling.
+    format_and_emit_report(
+        &mut session,
+        Input::Text(input),
+        &config,
+        &mut format_report,
+    );
+    let has_diff = write_all_files(format_report, out, opt.emitter_config(EmitMode::Stdout))?;
+    Ok(if opt.check && has_diff { 1 } else { 0 })
 }
 
 enum FileConfig {
@@ -484,21 +492,24 @@ fn format(opt: Opt) -> Result<i32> {
 
     let (config, config_path) = load_config(None, Some(&opt))?;
 
-    if config.verbose() == Verbosity::Verbose {
+    if opt.verbose {
         if let Some(path) = config_path.as_ref() {
             println!("Using rustfmt config file {}", path.display());
         }
     }
 
-    let out = &mut stdout();
-    let mut session = Session::new(config, Some(out));
+    let mut session = RustFormatterBuilder::default()
+        .recursive(opt.recursive)
+        .verbosity(opt.verbosity())
+        .build();
+    let mut format_report = FormatReport::new();
 
     for pair in FileConfigPairIter::new(&opt, config_path.is_some()) {
         let file = pair.file;
 
         if let FileConfig::Local(local_config, config_path) = pair.config {
             if let Some(path) = config_path {
-                if local_config.verbose() == Verbosity::Verbose {
+                if opt.verbose {
                     println!(
                         "Using rustfmt config file {} for {}",
                         path.display(),
@@ -507,54 +518,50 @@ fn format(opt: Opt) -> Result<i32> {
                 }
             }
 
-            session.override_config(local_config, |sess| {
-                format_and_emit_report(sess, Input::File(file.to_path_buf()))
-            });
+            format_and_emit_report(
+                &mut session,
+                Input::File(file.to_path_buf()),
+                &local_config,
+                &mut format_report,
+            );
         } else {
-            format_and_emit_report(&mut session, Input::File(file.to_path_buf()));
+            format_and_emit_report(
+                &mut session,
+                Input::File(file.to_path_buf()),
+                &config,
+                &mut format_report,
+            );
         }
     }
 
-    let exit_code = if session.has_operational_errors()
-        || session.has_parsing_errors()
-        || ((session.has_diff() || session.has_check_errors()) && opt.check)
-    {
-        1
-    } else {
-        0
-    };
-    Ok(exit_code)
+    if format_report.has_errors() {
+        eprintln!(
+            "{}",
+            FormatReportFormatterBuilder::new(&format_report)
+                .enable_colors(true)
+                .build()
+        );
+    }
+
+    let out = &mut stdout();
+    let has_diff = write_all_files(format_report, out, opt.emitter_config(EmitMode::Files))?;
+
+    Ok(if opt.check && has_diff { 1 } else { 0 })
 }
 
-fn format_and_emit_report<T: Write>(session: &mut Session<'_, T>, input: Input) {
-    match session.format(input) {
+fn format_and_emit_report(
+    session: &mut Session,
+    input: Input,
+    config: &Config,
+    format_report: &mut FormatReport,
+) {
+    match session.format(input, config) {
         Ok(report) => {
-            if report.has_warnings() {
-                eprintln!(
-                    "{}",
-                    FormatReportFormatterBuilder::new(&report)
-                        .enable_colors(should_print_with_colors(session))
-                        .build()
-                );
-            }
-        }
-        Err(msg) => {
-            eprintln!("Error writing files: {}", msg);
-            session.add_operational_error();
+            format_report.merge(report);
         }
-    }
-}
-
-fn should_print_with_colors<T: Write>(session: &mut Session<'_, T>) -> bool {
-    match term::stderr() {
-        Some(ref t)
-            if session.config.color().use_colored_tty()
-                && t.supports_color()
-                && t.supports_attr(term::Attr::Bold) =>
-        {
-            true
+        Err(err) => {
+            eprintln!("{}", err);
         }
-        _ => false,
     }
 }
 
@@ -567,31 +574,6 @@ mod test {
         let _ = env_logger::builder().is_test(true).try_init();
     }
 
-    #[test]
-    fn format_lines_errors_are_reported() {
-        init_log();
-        let long_identifier = String::from_utf8(vec![b'a'; 239]).unwrap();
-        let input = Input::Text(format!("fn {}() {{}}", long_identifier));
-        let mut config = Config::default();
-        config.set().error_on_line_overflow(true);
-        let mut session = Session::<io::Stdout>::new(config, None);
-        session.format(input).unwrap();
-        assert!(session.has_formatting_errors());
-    }
-
-    #[test]
-    fn format_lines_errors_are_reported_with_tabs() {
-        init_log();
-        let long_identifier = String::from_utf8(vec![b'a'; 97]).unwrap();
-        let input = Input::Text(format!("fn a() {{\n\t{}\n}}", long_identifier));
-        let mut config = Config::default();
-        config.set().error_on_line_overflow(true);
-        config.set().hard_tabs(true);
-        let mut session = Session::<io::Stdout>::new(config, None);
-        session.format(input).unwrap();
-        assert!(session.has_formatting_errors());
-    }
-
     struct TempFile {
         path: PathBuf,
     }
@@ -704,7 +686,7 @@ mod test {
         let output = child
             .wait_with_output()
             .expect("Failed to wait on rustfmt child");
-        assert!(output.status.success());
+        assert!(!output.status.success());
         assert_eq!(std::str::from_utf8(&output.stdout).unwrap(), "stdin\n");
     }
 

rustfmt-core/rustfmt-bin/src/git-rustfmt/main.rs

@@ -7,7 +7,10 @@ use std::process::Command;
 
 use structopt::StructOpt;
 
-use rustfmt_lib::{load_config, CliOptions, FormatReportFormatterBuilder, Input, Session};
+use rustfmt_lib::{
+    load_config, write_all_files, CliOptions, EmitterConfig, FormatReportFormatterBuilder, Input,
+    Session,
+};
 
 fn prune_files(files: Vec<&str>) -> Vec<&str> {
     let prefixes: Vec<_> = files
@@ -57,19 +60,19 @@ fn fmt_files(files: &[&str]) -> i32 {
     let (config, _) =
         load_config::<NullOptions>(Some(Path::new(".")), None).expect("couldn't load config");
 
-    let mut exit_code = 0;
     let mut out = stdout();
-    let mut session = Session::new(config, Some(&mut out));
+    let mut session = Session::default();
     for file in files {
-        let report = session.format(Input::File(PathBuf::from(file))).unwrap();
+        let report = session
+            .format(Input::File(PathBuf::from(file)), &config)
+            .unwrap();
         if report.has_warnings() {
             eprintln!("{}", FormatReportFormatterBuilder::new(&report).build());
         }
-        if !session.has_no_errors() {
-            exit_code = 1;
-        }
+        write_all_files(report, &mut out, EmitterConfig::default()).unwrap();
     }
-    exit_code
+
+    todo!("Fix error handling")
 }
 
 struct NullOptions;

rustfmt-core/rustfmt-bin/tests/rustfmt/main.rs

@@ -77,9 +77,9 @@ fn inline_config() {
             "--print-config",
             "current",
             ".",
-            "--config=color=Never,edition=2018"
+            "--config=max_width=50,edition=2018"
         ],
-        contains("color = \"Never\"") && contains("edition = \"2018\"")
+        contains("max_width = 50") && contains("edition = \"2018\"")
     );
 
     // multiple overriding invocations
@@ -89,11 +89,11 @@ fn inline_config() {
             "current",
             ".",
             "--config",
-            "color=never,edition=2018",
+            "max_width=80,edition=2018",
             "--config",
             "color=always,format_strings=true"
         ],
-        contains("color = \"Always\"")
+        contains("max_width = 80")
             && contains("edition = \"2018\"")
             && contains("format_strings = true")
     );