From 0437bf7a7d968af885dc05931e4e7d1eaddaadbf Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 1 Mar 2019 17:59:38 +0100 Subject: [PATCH 1/5] Allow for stdin input in EmitMode::ModifiedLines --- src/formatting.rs | 16 ++++++++---- src/lib.rs | 2 ++ src/source_file.rs | 63 ++++++++++++++++++++++++++++++---------------- src/test/mod.rs | 26 ++++++++++++++++++- 4 files changed, 80 insertions(+), 27 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index 38440802ef2..f6daab0f487 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -181,8 +181,12 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { self.report .add_non_formatted_ranges(visitor.skipped_range.clone()); - self.handler - .handle_formatted_file(path, visitor.buffer.to_owned(), &mut self.report) + self.handler.handle_formatted_file( + self.parse_session.source_map(), + path, + visitor.buffer.to_owned(), + &mut self.report, + ) } } @@ -190,6 +194,7 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> { trait FormatHandler { fn handle_formatted_file( &mut self, + source_map: &SourceMap, path: FileName, result: String, report: &mut FormatReport, @@ -200,13 +205,14 @@ impl<'b, T: Write + 'b> FormatHandler for Session<'b, T> { // Called for each formatted file. fn handle_formatted_file( &mut self, + source_map: &SourceMap, path: FileName, result: String, report: &mut FormatReport, ) -> Result<(), ErrorKind> { if let Some(ref mut out) = self.out { - match source_file::write_file(&result, &path, out, &self.config) { - Ok(b) if b => report.add_diff(), + match source_file::write_file(Some(source_map), &path, &result, out, &self.config) { + Ok(has_diff) if 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); @@ -299,7 +305,7 @@ impl FormattingError { pub(crate) type FormatErrorMap = HashMap>; -#[derive(Default, Debug)] +#[derive(Default, Debug, PartialEq)] pub(crate) struct ReportedErrors { // Encountered e.g., an IO error. pub(crate) has_operational_errors: bool, diff --git a/src/lib.rs b/src/lib.rs index 520a6e66fcd..2a64d58ba52 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,6 +33,8 @@ pub use crate::config::{ Range, Verbosity, }; +pub use crate::rustfmt_diff::make_diff; + #[macro_use] mod utils; diff --git a/src/source_file.rs b/src/source_file.rs index 07c1e37fbed..5cbb01dd123 100644 --- a/src/source_file.rs +++ b/src/source_file.rs @@ -1,5 +1,8 @@ use std::fs; use std::io::{self, Write}; +use std::path::Path; + +use syntax::source_map::SourceMap; use crate::checkstyle::output_checkstyle_file; use crate::config::{Config, EmitMode, FileName, Verbosity}; @@ -26,7 +29,7 @@ where write!(out, "{}", crate::checkstyle::header())?; } for &(ref filename, ref text) in source_file { - write_file(text, filename, out, config)?; + write_file(None, filename, text, out, config)?; } if config.emit_mode() == EmitMode::Checkstyle { write!(out, "{}", crate::checkstyle::footer())?; @@ -36,24 +39,46 @@ where } pub fn write_file( - formatted_text: &str, + source_map: Option<&SourceMap>, filename: &FileName, + formatted_text: &str, out: &mut T, config: &Config, ) -> Result where T: Write, { - let filename_to_path = || match *filename { - FileName::Real(ref path) => path, - _ => panic!("cannot format `{}` and emit to files", filename), + fn ensure_real_path(filename: &FileName) -> &Path { + match *filename { + FileName::Real(ref path) => path, + _ => panic!("cannot format `{}` and emit to files", filename), + } + } + + impl From<&FileName> for syntax_pos::FileName { + fn from(filename: &FileName) -> syntax_pos::FileName { + match filename { + FileName::Real(path) => syntax_pos::FileName::Real(path.to_owned()), + FileName::Stdin => syntax_pos::FileName::Custom("stdin".to_owned()), + } + } + } + + // If parse session is around (cfg(not(test))) then try getting source from + // there 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(|x| x.to_string())); + let original_text = match original_text { + Some(ori) => ori, + None => fs::read_to_string(ensure_real_path(filename))?, }; match config.emit_mode() { EmitMode::Files if config.make_backup() => { - let filename = filename_to_path(); - let ori = fs::read_to_string(filename)?; - if ori != formatted_text { + 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. @@ -67,9 +92,9 @@ where } EmitMode::Files => { // Write text directly over original file if there is a diff. - let filename = filename_to_path(); - let ori = fs::read_to_string(filename)?; - if ori != formatted_text { + let filename = ensure_real_path(filename); + + if original_text != formatted_text { fs::write(filename, formatted_text)?; } } @@ -80,27 +105,23 @@ where write!(out, "{}", formatted_text)?; } EmitMode::ModifiedLines => { - let filename = filename_to_path(); - let ori = fs::read_to_string(filename)?; - let mismatch = make_diff(&ori, formatted_text, 0); + let mismatch = make_diff(&original_text, formatted_text, 0); let has_diff = !mismatch.is_empty(); output_modified(out, mismatch); return Ok(has_diff); } EmitMode::Checkstyle => { - let filename = filename_to_path(); - let ori = fs::read_to_string(filename)?; - let diff = make_diff(&ori, formatted_text, 3); + let filename = ensure_real_path(filename); + + let diff = make_diff(&original_text, formatted_text, 3); output_checkstyle_file(out, filename, diff)?; } EmitMode::Diff => { - let filename = filename_to_path(); - let ori = fs::read_to_string(filename)?; - let mismatch = make_diff(&ori, formatted_text, 3); + 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.display(), line_num), + |line_num| format!("Diff in {} at line {}:", filename, line_num), config, ); return Ok(has_diff); diff --git a/src/test/mod.rs b/src/test/mod.rs index 2d1a62ceda5..5d9c103ed03 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -9,7 +9,7 @@ use std::process::{Command, Stdio}; use std::str::Chars; use crate::config::{Color, Config, EmitMode, FileName, ReportTactic}; -use crate::formatting::{ModifiedChunk, SourceFile}; +use crate::formatting::{ModifiedChunk, ReportedErrors, SourceFile}; use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, OutputWriter}; use crate::source_file; use crate::{FormatReport, Input, Session}; @@ -290,6 +290,30 @@ fn stdin_parser_panic_caught() { } } +/// Ensures that `EmitMode::ModifiedLines` works with input from `stdin`. Useful +/// when embedding Rustfmt (e.g. inside RLS). +#[test] +fn stdin_works_with_modified_lines() { + let input = "\nfn\n some( )\n{\n}\nfn main () {}\n"; + let output = "1 6 2\nfn some() {}\nfn main() {}\n"; + + let input = Input::Text(input.to_owned()); + let mut config = Config::default(); + config.set().emit_mode(EmitMode::ModifiedLines); + let mut buf: Vec = vec![]; + { + let mut session = Session::new(config, Some(&mut buf)); + session.format(input).unwrap(); + let errors = ReportedErrors { + has_diff: true, + ..Default::default() + }; + assert_eq!(session.errors, errors); + } + let newline = if cfg!(windows) { "\r\n" } else { "\n" }; + assert_eq!(buf, output.replace('\n', newline).as_bytes()); +} + #[test] fn stdin_disable_all_formatting_test() { match option_env!("CFG_RELEASE_CHANNEL") { From b3c28dba830fb7c88133a0a402f02a5dff9d0738 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Fri, 1 Mar 2019 19:20:57 +0100 Subject: [PATCH 2/5] Expose ModifiedLines and implement parsing data from the string output This moves `Modified{Chunks,Lines}` from `src/formatting.rs` to `src/rustfmt_diff.rs` and reexports it in `src/lib.rs`. With this, a conversion from `Vec` to `ModifiedLines` was implemented and now this implements complementary `Display` and `FromStr`, which simplified the previously used `output_modified` function and which allows to parse the raw data emitted with `EmitMode::ModifiedLines`. --- src/formatting.rs | 19 ----- src/lib.rs | 2 +- src/rustfmt_diff.rs | 184 ++++++++++++++++++++++++++++++++++---------- src/source_file.rs | 4 +- src/test/mod.rs | 4 +- 5 files changed, 150 insertions(+), 63 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index f6daab0f487..9ec887ebbdf 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -338,25 +338,6 @@ impl ReportedErrors { } } -/// A single span of changed lines, with 0 or more removed lines -/// and a vector of 0 or more inserted lines. -#[derive(Debug, PartialEq, Eq)] -pub(crate) struct ModifiedChunk { - /// The first to be removed from the original text - pub line_number_orig: u32, - /// The number of lines which have been replaced - pub lines_removed: u32, - /// The new lines - pub lines: Vec, -} - -/// Set of changed sections of a file. -#[derive(Debug, PartialEq, Eq)] -pub(crate) struct ModifiedLines { - /// The set of changed chunks. - pub chunks: Vec, -} - #[derive(Clone, Copy, Debug)] enum Timer { Disabled, diff --git a/src/lib.rs b/src/lib.rs index 2a64d58ba52..53e74210ede 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,7 +33,7 @@ pub use crate::config::{ Range, Verbosity, }; -pub use crate::rustfmt_diff::make_diff; +pub use crate::rustfmt_diff::{ModifiedChunk, ModifiedLines}; #[macro_use] mod utils; diff --git a/src/rustfmt_diff.rs b/src/rustfmt_diff.rs index c6262dcab15..c796db09c87 100644 --- a/src/rustfmt_diff.rs +++ b/src/rustfmt_diff.rs @@ -1,4 +1,5 @@ use std::collections::VecDeque; +use std::fmt; use std::io; use std::io::Write; @@ -33,6 +34,118 @@ impl Mismatch { } } +/// A single span of changed lines, with 0 or more removed lines +/// and a vector of 0 or more inserted lines. +#[derive(Debug, PartialEq, Eq)] +pub struct ModifiedChunk { + /// The first to be removed from the original text + pub line_number_orig: u32, + /// The number of lines which have been replaced + pub lines_removed: u32, + /// The new lines + pub lines: Vec, +} + +/// Set of changed sections of a file. +#[derive(Debug, PartialEq, Eq)] +pub struct ModifiedLines { + /// The set of changed chunks. + pub chunks: Vec, +} + +impl From> for ModifiedLines { + fn from(mismatches: Vec) -> ModifiedLines { + let chunks = mismatches.into_iter().map(|mismatch| { + let lines = || mismatch.lines.iter(); + let num_removed = lines() + .filter(|line| match line { + DiffLine::Resulting(_) => true, + _ => false, + }) + .count(); + + let new_lines = mismatch.lines.into_iter().filter_map(|line| match line { + DiffLine::Context(_) | DiffLine::Resulting(_) => None, + DiffLine::Expected(str) => Some(str), + }); + + ModifiedChunk { + line_number_orig: mismatch.line_number_orig, + lines_removed: num_removed as u32, + lines: new_lines.collect(), + } + }); + + ModifiedLines { + chunks: chunks.collect(), + } + } +} + +// Converts a `Mismatch` into a serialized form, which just includes +// enough information to modify the original file. +// Each section starts with a line with three integers, space separated: +// lineno num_removed num_added +// followed by (`num_added`) lines of added text. The line numbers are +// relative to the original file. +impl fmt::Display for ModifiedLines { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + for chunk in &self.chunks { + writeln!( + f, + "{} {} {}", + chunk.line_number_orig, + chunk.lines_removed, + chunk.lines.iter().count() + )?; + + for line in &chunk.lines { + writeln!(f, "{}", line)?; + } + } + + Ok(()) + } +} + +// Allows to convert `Display`ed `ModifiedLines` back to the structural data. +impl std::str::FromStr for ModifiedLines { + type Err = (); + + fn from_str(s: &str) -> Result { + let mut chunks = vec![]; + + let mut lines = s.lines(); + while let Some(header) = lines.next() { + let mut header = header.split_whitespace(); + let (orig, rem, new_lines) = match (header.next(), header.next(), header.next()) { + (Some(orig), Some(removed), Some(added)) => (orig, removed, added), + _ => return Err(()), + }; + eprintln!("{} {} {}", orig, rem, new_lines); + let (orig, rem, new_lines): (u32, u32, usize) = + match (orig.parse(), rem.parse(), new_lines.parse()) { + (Ok(a), Ok(b), Ok(c)) => (a, b, c), + _ => return Err(()), + }; + eprintln!("{} {} {}", orig, rem, new_lines); + let lines = lines.by_ref().take(new_lines); + let lines: Vec<_> = lines.map(ToOwned::to_owned).collect(); + if lines.len() != new_lines { + return Err(()); + } + + chunks.push(ModifiedChunk { + line_number_orig: orig, + lines_removed: rem, + lines, + }); + } + + Ok(ModifiedLines { chunks }) + } +} + // This struct handles writing output to stdout and abstracts away the logic // of printing in color, if it's possible in the executing environment. pub struct OutputWriter { @@ -174,49 +287,11 @@ where } } -/// Converts a `Mismatch` into a serialized form, which just includes -/// enough information to modify the original file. -/// Each section starts with a line with three integers, space separated: -/// lineno num_removed num_added -/// followed by (`num_added`) lines of added text. The line numbers are -/// relative to the original file. -pub fn output_modified(mut out: W, diff: Vec) -where - W: Write, -{ - for mismatch in diff { - let (num_removed, num_added) = - mismatch - .lines - .iter() - .fold((0, 0), |(rem, add), line| match *line { - DiffLine::Context(_) => panic!("No Context expected"), - DiffLine::Expected(_) => (rem, add + 1), - DiffLine::Resulting(_) => (rem + 1, add), - }); - // Write a header with enough information to separate the modified lines. - writeln!( - out, - "{} {} {}", - mismatch.line_number_orig, num_removed, num_added - ) - .unwrap(); - - for line in mismatch.lines { - match line { - DiffLine::Context(_) | DiffLine::Resulting(_) => (), - DiffLine::Expected(ref str) => { - writeln!(out, "{}", str).unwrap(); - } - } - } - } -} - #[cfg(test)] mod test { use super::DiffLine::*; use super::{make_diff, Mismatch}; + use super::{ModifiedChunk, ModifiedLines}; #[test] fn diff_simple() { @@ -298,4 +373,35 @@ mod test { }] ); } + + #[test] + fn modified_lines_from_str() { + use std::str::FromStr; + + let src = "1 6 2\nfn some() {}\nfn main() {}\n25 3 1\n struct Test {}"; + let lines = ModifiedLines::from_str(src).unwrap(); + assert_eq!( + lines, + ModifiedLines { + chunks: vec![ + ModifiedChunk { + line_number_orig: 1, + lines_removed: 6, + lines: vec!["fn some() {}".to_owned(), "fn main() {}".to_owned(),] + }, + ModifiedChunk { + line_number_orig: 25, + lines_removed: 3, + lines: vec![" struct Test {}".to_owned()] + } + ] + } + ); + + let src = "1 5 3"; + assert_eq!(ModifiedLines::from_str(src), Err(())); + + let src = "1 5 3\na\nb"; + assert_eq!(ModifiedLines::from_str(src), Err(())); + } } diff --git a/src/source_file.rs b/src/source_file.rs index 5cbb01dd123..a779c4dfbe2 100644 --- a/src/source_file.rs +++ b/src/source_file.rs @@ -6,7 +6,7 @@ use syntax::source_map::SourceMap; use crate::checkstyle::output_checkstyle_file; use crate::config::{Config, EmitMode, FileName, Verbosity}; -use crate::rustfmt_diff::{make_diff, output_modified, print_diff}; +use crate::rustfmt_diff::{make_diff, print_diff, ModifiedLines}; #[cfg(test)] use crate::formatting::FileRecord; @@ -107,7 +107,7 @@ where EmitMode::ModifiedLines => { let mismatch = make_diff(&original_text, formatted_text, 0); let has_diff = !mismatch.is_empty(); - output_modified(out, mismatch); + write!(out, "{}", ModifiedLines::from(mismatch))?; return Ok(has_diff); } EmitMode::Checkstyle => { diff --git a/src/test/mod.rs b/src/test/mod.rs index 5d9c103ed03..24772d70c16 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -9,8 +9,8 @@ use std::process::{Command, Stdio}; use std::str::Chars; use crate::config::{Color, Config, EmitMode, FileName, ReportTactic}; -use crate::formatting::{ModifiedChunk, ReportedErrors, SourceFile}; -use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, OutputWriter}; +use crate::formatting::{ReportedErrors, SourceFile}; +use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, ModifiedChunk, OutputWriter}; use crate::source_file; use crate::{FormatReport, Input, Session}; From 8aa0f44d15fe133f10830e4b58cd20f207bbf769 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Sat, 2 Mar 2019 23:21:47 +0100 Subject: [PATCH 3/5] Remove eprintln --- src/rustfmt_diff.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/rustfmt_diff.rs b/src/rustfmt_diff.rs index c796db09c87..8081221af49 100644 --- a/src/rustfmt_diff.rs +++ b/src/rustfmt_diff.rs @@ -122,13 +122,11 @@ impl std::str::FromStr for ModifiedLines { (Some(orig), Some(removed), Some(added)) => (orig, removed, added), _ => return Err(()), }; - eprintln!("{} {} {}", orig, rem, new_lines); let (orig, rem, new_lines): (u32, u32, usize) = match (orig.parse(), rem.parse(), new_lines.parse()) { (Ok(a), Ok(b), Ok(c)) => (a, b, c), _ => return Err(()), }; - eprintln!("{} {} {}", orig, rem, new_lines); let lines = lines.by_ref().take(new_lines); let lines: Vec<_> = lines.map(ToOwned::to_owned).collect(); if lines.len() != new_lines { From 6b993c8b8b72faa3b768ffef52a42c19ac134227 Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Mon, 4 Mar 2019 18:28:25 +0100 Subject: [PATCH 4/5] Fix stdin_works... test on Windows --- src/test/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/mod.rs b/src/test/mod.rs index 24772d70c16..1992190bb62 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -8,7 +8,7 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::str::Chars; -use crate::config::{Color, Config, EmitMode, FileName, ReportTactic}; +use crate::config::{Color, Config, EmitMode, FileName, NewlineStyle, ReportTactic}; use crate::formatting::{ReportedErrors, SourceFile}; use crate::rustfmt_diff::{make_diff, print_diff, DiffLine, Mismatch, ModifiedChunk, OutputWriter}; use crate::source_file; @@ -299,6 +299,7 @@ fn stdin_works_with_modified_lines() { let input = Input::Text(input.to_owned()); let mut config = Config::default(); + config.set().newline_style(NewlineStyle::Unix); config.set().emit_mode(EmitMode::ModifiedLines); let mut buf: Vec = vec![]; { @@ -310,8 +311,7 @@ fn stdin_works_with_modified_lines() { }; assert_eq!(session.errors, errors); } - let newline = if cfg!(windows) { "\r\n" } else { "\n" }; - assert_eq!(buf, output.replace('\n', newline).as_bytes()); + assert_eq!(buf, output.as_bytes()); } #[test] From fbfda614be1f666a57e44ae2f981b0ff0ef64c5b Mon Sep 17 00:00:00 2001 From: Igor Matuszewski Date: Mon, 4 Mar 2019 18:32:58 +0100 Subject: [PATCH 5/5] Remove redundant closure --- src/rustfmt_diff.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rustfmt_diff.rs b/src/rustfmt_diff.rs index 8081221af49..c83eb1080a6 100644 --- a/src/rustfmt_diff.rs +++ b/src/rustfmt_diff.rs @@ -56,8 +56,8 @@ pub struct ModifiedLines { impl From> for ModifiedLines { fn from(mismatches: Vec) -> ModifiedLines { let chunks = mismatches.into_iter().map(|mismatch| { - let lines = || mismatch.lines.iter(); - let num_removed = lines() + let lines = mismatch.lines.iter(); + let num_removed = lines .filter(|line| match line { DiffLine::Resulting(_) => true, _ => false,