Read the original file for comparing/detecting newline (#4236)

Author: topecongiro

Diff for: src/config/file_lines.rs

@@ -2,7 +2,7 @@
 
 use itertools::Itertools;
 use std::collections::HashMap;
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::rc::Rc;
 use std::{cmp, fmt, iter, str};
 
@@ -26,6 +26,15 @@ pub enum FileName {
     Stdin,
 }
 
+impl FileName {
+    pub(crate) fn as_path(&self) -> Option<&Path> {
+        match self {
+            FileName::Real(ref path) => Some(path),
+            _ => None,
+        }
+    }
+}
+
 impl From<rustc_span::FileName> for FileName {
     fn from(name: rustc_span::FileName) -> FileName {
         match name {

Diff for: src/emitter.rs

@@ -5,15 +5,11 @@ pub use self::json::*;
 pub use self::modified_lines::*;
 pub use self::stdout::*;
 
-use std::fs;
 use std::io::{self, Write};
-use std::path::Path;
-use std::rc::Rc;
 
 use thiserror::Error;
 
-use crate::formatting::ParseSess;
-use crate::{config::FileName, FormatReport, FormatResult, NewlineStyle};
+use crate::{config::FileName, FormatReport, FormatResult};
 
 pub mod checkstyle;
 pub mod diff;
@@ -166,15 +162,14 @@ where
 
     emitter.emit_header(out)?;
     for (filename, format_result) in format_report.format_result_as_rc().borrow().iter() {
-        has_diff |= write_file(None, filename, &format_result, out, &mut *emitter)?.has_diff;
+        has_diff |= write_file(filename, &format_result, out, &mut *emitter)?.has_diff;
     }
     emitter.emit_footer(out)?;
 
     Ok(has_diff)
 }
 
 pub(crate) fn write_file<T>(
-    parse_sess: Option<&ParseSess>,
     filename: &FileName,
     formatted_result: &FormatResult,
     out: &mut T,
@@ -183,38 +178,9 @@ pub(crate) fn write_file<T>(
 where
     T: Write,
 {
-    fn ensure_real_path(filename: &FileName) -> &Path {
-        match *filename {
-            FileName::Real(ref path) => path,
-            _ => panic!("cannot format `{}` and emit to files", filename),
-        }
-    }
-
-    // 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 =
-        if formatted_result.newline_style() != NewlineStyle::Auto && *filename != FileName::Stdin {
-            Rc::new(fs::read_to_string(ensure_real_path(filename))?)
-        } else {
-            match formatted_result.original_text() {
-                Some(original_snippet) => Rc::new(original_snippet.to_owned()),
-                None => match parse_sess.and_then(|sess| sess.get_original_snippet(filename)) {
-                    Some(ori) => ori,
-                    None => Rc::new(fs::read_to_string(ensure_real_path(filename))?),
-                },
-            }
-        };
-
     let formatted_file = FormattedFile {
         filename,
-        original_text: original_text.as_str(),
+        original_text: formatted_result.original_text(),
         formatted_text: formatted_result.formatted_text(),
     };
 

Diff for: src/formatting.rs

@@ -7,19 +7,18 @@ use rustc_span::symbol;
 
 pub(crate) use syntux::session::ParseSess;
 
-use self::newline_style::apply_newline_style;
 use crate::config::{Config, FileName};
-use crate::formatting::modules::Module;
 use crate::formatting::{
     comment::{CharClasses, FullCodeCharKind},
+    modules::Module,
+    newline_style::apply_newline_style,
     report::NonFormattedRange,
     syntux::parser::{DirectoryOwnership, Parser, ParserError},
     utils::count_newlines,
     visitor::FmtVisitor,
 };
-use crate::result::OperationError;
 use crate::{
-    result::{ErrorKind, FormatError},
+    result::{ErrorKind, FormatError, OperationError},
     FormatReport, FormatResult, Input, OperationSetting, Verbosity,
 };
 
@@ -136,7 +135,7 @@ fn format_project(
             &module,
             &format_report,
             original_snippet.clone(),
-        );
+        )?;
     }
     timer = timer.done_formatting();
 
@@ -159,7 +158,7 @@ fn format_file(
     module: &Module<'_>,
     report: &FormatReport,
     original_snippet: Option<String>,
-) {
+) -> Result<(), OperationError> {
     let snippet_provider = parse_session.snippet_provider(module.as_ref().inner);
     let mut visitor =
         FmtVisitor::from_parse_sess(&parse_session, config, &snippet_provider, report.clone());
@@ -187,22 +186,33 @@ fn format_file(
         report.clone(),
     );
 
-    apply_newline_style(
-        config.newline_style(),
-        &mut visitor.buffer,
-        snippet_provider.entire_snippet(),
-    );
+    // SourceFile's in the SourceMap will always have Unix-style line endings
+    // See: https://github.com/rust-lang/rustfmt/issues/3850
+    // So 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.
+    let original_text = match original_snippet {
+        Some(snippet) => snippet,
+        None => std::fs::read_to_string(path.as_path().ok_or(OperationError::IoError(
+            std::io::Error::from(std::io::ErrorKind::InvalidInput),
+        ))?)?,
+    };
+    apply_newline_style(config.newline_style(), &mut visitor.buffer, &original_text);
 
     if visitor.macro_rewrite_failure {
         report.add_macro_format_failure(path.clone());
     }
     let format_result = FormatResult::success(
         visitor.buffer.to_owned(),
         visitor.skipped_range.borrow().clone(),
-        original_snippet,
+        original_text,
         config.newline_style(),
     );
     report.add_format_result(path, format_result);
+
+    Ok(())
 }
 
 #[derive(Clone, Copy, Debug)]

Diff for: src/formatting/report.rs

@@ -20,7 +20,7 @@ pub struct FormatReport {
 /// errors and warning arose while formatting.
 #[derive(Debug, Clone, Default)]
 pub struct FormatResult {
-    original_snippet: Option<String>,
+    original_snippet: String,
     formatted_snippet: FormattedSnippet,
     format_errors: HashSet<FormatError>,
     newline_style: NewlineStyle,
@@ -37,7 +37,7 @@ impl FormatResult {
     pub(crate) fn success(
         snippet: String,
         non_formatted_ranges: Vec<NonFormattedRange>,
-        original_snippet: Option<String>,
+        original_snippet: String,
         newline_style: NewlineStyle,
     ) -> Self {
         let formatted_snippet = FormattedSnippet {
@@ -67,8 +67,8 @@ impl FormatResult {
         self.newline_style.clone()
     }
 
-    pub fn original_text(&self) -> Option<&str> {
-        self.original_snippet.as_ref().map(|s| s.as_str())
+    pub fn original_text(&self) -> &str {
+        &self.original_snippet
     }
 
     pub fn formatted_text(&self) -> &str {
@@ -120,9 +120,7 @@ impl FormatReport {
         original_format_result
             .format_errors
             .extend(format_result.format_errors);
-        if original_format_result.original_snippet.is_none() {
-            original_format_result.original_snippet = format_result.original_snippet;
-        }
+        original_format_result.original_snippet = format_result.original_snippet;
     }
 
     pub(crate) fn append_errors(&self, f: FileName, errors: impl Iterator<Item = FormatError>) {

Diff for: src/formatting/syntux/session.rs

@@ -206,13 +206,6 @@ impl ParseSess {
             Rc::clone(source_file.src.as_ref().unwrap()),
         )
     }
-
-    pub(crate) fn get_original_snippet(&self, file_name: &FileName) -> Option<Rc<String>> {
-        self.parse_sess
-            .source_map()
-            .get_source_file(&file_name.into())
-            .and_then(|source_file| source_file.src.clone())
-    }
 }
 
 // Methods that should be restricted within the syntux module.

Diff for: src/formatting/visitor.rs

@@ -62,9 +62,6 @@ impl SnippetProvider {
         }
     }
 
-    pub(crate) fn entire_snippet(&self) -> &str {
-        self.big_snippet.as_str()
-    }
     pub(crate) fn start_pos(&self) -> BytePos {
         BytePos::from_usize(self.start_pos)
     }

Diff for: src/result.rs

@@ -120,6 +120,9 @@ pub enum OperationError {
     /// Parse error occurred while parsing the input.
     #[error("failed to parse {input:?}")]
     ParseError { input: FileName, is_panic: bool },
+    /// Io error.
+    #[error("{0}")]
+    IoError(#[from] std::io::Error),
 }
 
 impl OperationError {