Skip to content

Commit

Permalink
fix: handling of newline_style conflicts (#3850)
Browse files Browse the repository at this point in the history
  • Loading branch information
calebcartwright authored and topecongiro committed Oct 19, 2019
1 parent 5327c36 commit 3a073f1
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 11 deletions.
28 changes: 28 additions & 0 deletions src/emitter/diff.rs
Expand Up @@ -36,6 +36,13 @@ impl Emitter for DiffEmitter {
&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 });
Expand Down Expand Up @@ -107,4 +114,25 @@ mod tests {
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")
);
}
}
10 changes: 8 additions & 2 deletions src/formatting.rs
Expand Up @@ -243,8 +243,14 @@ 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, &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
Expand Down
37 changes: 28 additions & 9 deletions src/source_file.rs
Expand Up @@ -6,6 +6,7 @@ use syntax::source_map::SourceMap;

use crate::config::FileName;
use crate::emitter::{self, Emitter};
use crate::NewlineStyle;

#[cfg(test)]
use crate::config::Config;
Expand All @@ -32,7 +33,14 @@ where

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)?;

Expand All @@ -45,6 +53,7 @@ pub(crate) fn write_file<T>(
formatted_text: &str,
out: &mut T,
emitter: &mut dyn Emitter,
newline_style: NewlineStyle,
) -> Result<emitter::EmitterResult, io::Error>
where
T: Write,
Expand All @@ -65,15 +74,25 @@ where
}
}

// 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 {
Expand Down

0 comments on commit 3a073f1

Please sign in to comment.