Skip to content

Read the original file for comparing/detecting newline #4236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/config/file_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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 {
Expand Down
40 changes: 3 additions & 37 deletions src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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(),
};

Expand Down
34 changes: 22 additions & 12 deletions src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -136,7 +135,7 @@ fn format_project(
&module,
&format_report,
original_snippet.clone(),
);
)?;
}
timer = timer.done_formatting();

Expand All @@ -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());
Expand Down Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to pass the original file content to apply_newline_style.


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)]
Expand Down
12 changes: 5 additions & 7 deletions src/formatting/report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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>) {
Expand Down
7 changes: 0 additions & 7 deletions src/formatting/syntux/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is no longer used, and thus removed.

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.
Expand Down
3 changes: 0 additions & 3 deletions src/formatting/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ impl SnippetProvider {
}
}

pub(crate) fn entire_snippet(&self) -> &str {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is no longer used, and thus removed.

self.big_snippet.as_str()
}
pub(crate) fn start_pos(&self) -> BytePos {
BytePos::from_usize(self.start_pos)
}
Expand Down
3 changes: 3 additions & 0 deletions src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down