From 3a073f177ca7466cf3e8327a7fc847d495d75fd7 Mon Sep 17 00:00:00 2001 From: Caleb Cartwright Date: Sat, 19 Oct 2019 03:15:13 -0500 Subject: [PATCH] fix: handling of newline_style conflicts (#3850) --- src/emitter/diff.rs | 28 ++++++++++++++++++++++++++++ src/formatting.rs | 10 ++++++++-- src/source_file.rs | 37 ++++++++++++++++++++++++++++--------- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/emitter/diff.rs b/src/emitter/diff.rs index 462a80946ee..9be4fb28f99 100644 --- a/src/emitter/diff.rs +++ b/src/emitter/diff.rs @@ -36,6 +36,13 @@ fn emit_formatted_file( &self.config, ); } + } else if original_text != formatted_text { + // This occurs when the only difference between the original and formatted values + // is the newline style. This happens because The make_diff function compares the + // original and formatted values line by line, independent of line endings. + let file_path = ensure_real_path(filename); + writeln!(output, "Incorrect newline style in {}", file_path.display())?; + return Ok(EmitterResult { has_diff: true }); } return Ok(EmitterResult { has_diff }); @@ -107,4 +114,25 @@ fn prints_file_names_when_config_is_enabled() { format!("{}\n{}\n", bin_file, lib_file), ) } + + #[test] + fn prints_newline_message_with_only_newline_style_diff() { + let mut writer = Vec::new(); + let config = Config::default(); + let mut emitter = DiffEmitter::new(config); + let _ = emitter + .emit_formatted_file( + &mut writer, + FormattedFile { + filename: &FileName::Real(PathBuf::from("src/lib.rs")), + original_text: "fn empty() {}\n", + formatted_text: "fn empty() {}\r\n", + }, + ) + .unwrap(); + assert_eq!( + String::from_utf8(writer).unwrap(), + String::from("Incorrect newline style in src/lib.rs\n") + ); + } } diff --git a/src/formatting.rs b/src/formatting.rs index eec33e6ba79..2822a331260 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -243,8 +243,14 @@ fn handle_formatted_file( report: &mut FormatReport, ) -> Result<(), ErrorKind> { if let Some(ref mut out) = self.out { - match source_file::write_file(Some(source_map), &path, &result, out, &mut *self.emitter) - { + match source_file::write_file( + Some(source_map), + &path, + &result, + out, + &mut *self.emitter, + self.config.newline_style(), + ) { 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 diff --git a/src/source_file.rs b/src/source_file.rs index 28366ca2485..b6764ee7464 100644 --- a/src/source_file.rs +++ b/src/source_file.rs @@ -6,6 +6,7 @@ use crate::config::FileName; use crate::emitter::{self, Emitter}; +use crate::NewlineStyle; #[cfg(test)] use crate::config::Config; @@ -32,7 +33,14 @@ pub(crate) fn write_all_files( emitter.emit_header(out)?; for &(ref filename, ref text) in source_file { - write_file(None, filename, text, out, &mut *emitter)?; + write_file( + None, + filename, + text, + out, + &mut *emitter, + config.newline_style(), + )?; } emitter.emit_footer(out)?; @@ -45,6 +53,7 @@ pub(crate) fn write_file( formatted_text: &str, out: &mut T, emitter: &mut dyn Emitter, + newline_style: NewlineStyle, ) -> Result where T: Write, @@ -65,15 +74,25 @@ fn from(filename: &FileName) -> syntax_pos::FileName { } } - // If parse session is around (cfg(not(test))) then try getting source from - // there instead of hitting the file system. This also supports getting + // SourceFile's in the SourceMap will always have Unix-style line endings + // See: https://github.com/rust-lang/rustfmt/issues/3850 + // So if the user has explicitly overridden the rustfmt `newline_style` + // config and `filename` is FileName::Real, then we must check the file system + // to get the original file value in order to detect newline_style conflicts. + // Otherwise, parse session is around (cfg(not(test))) and newline_style has been + // left as the default value, then try getting source from the parse session + // source map instead of hitting the file system. This also supports getting // original text for `FileName::Stdin`. - let original_text = source_map - .and_then(|x| x.get_source_file(&filename.into())) - .and_then(|x| x.src.as_ref().map(ToString::to_string)); - let original_text = match original_text { - Some(ori) => ori, - None => fs::read_to_string(ensure_real_path(filename))?, + let original_text = if newline_style != NewlineStyle::Auto && *filename != FileName::Stdin { + fs::read_to_string(ensure_real_path(filename))? + } else { + match source_map + .and_then(|x| x.get_source_file(&filename.into())) + .and_then(|x| x.src.as_ref().map(ToString::to_string)) + { + Some(ori) => ori, + None => fs::read_to_string(ensure_real_path(filename))?, + } }; let formatted_file = emitter::FormattedFile { -- 2.44.0