Skip to content
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

Make --check work when running from stdin. #3896

Merged
merged 16 commits into from
Nov 6, 2019
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
31 changes: 16 additions & 15 deletions src/bin/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,10 @@ pub enum OperationError {
/// An io error during reading or writing.
#[fail(display = "io error: {}", _0)]
IoError(IoError),
/// Attempt to use --check with stdin, which isn't currently
/// supported.
#[fail(display = "The `--check` option is not supported with standard input.")]
CheckWithStdin,
/// Attempt to use --emit=json with stdin, which isn't currently
/// supported.
#[fail(display = "Using `--emit` other than stdout is not supported with standard input.")]
EmitWithStdin,
/// Attempt to use --emit with a mode which is not currently
/// supported with stdandard input.
#[fail(display = "Emit mode {} not supported with standard output.", _0)]
StdinBadEmit(EmitMode),
}

impl From<IoError> for OperationError {
Expand Down Expand Up @@ -251,15 +247,20 @@ fn format_string(input: String, options: GetOptsOptions) -> Result<i32, FailureE
let (mut config, _) = load_config(Some(Path::new(".")), Some(options.clone()))?;

if options.check {
return Err(OperationError::CheckWithStdin.into());
}
if let Some(emit_mode) = options.emit_mode {
if emit_mode != EmitMode::Stdout {
return Err(OperationError::EmitWithStdin.into());
config.set().emit_mode(EmitMode::Diff);
} else {
match options.emit_mode {
// Emit modes which work with standard input
// None means default, which is Stdout.
None | Some(EmitMode::Stdout) | Some(EmitMode::Checkstyle) | Some(EmitMode::Json) => {}
Some(emit_mode) => {
return Err(OperationError::StdinBadEmit(emit_mode).into());
}
}
config
.set()
.emit_mode(options.emit_mode.unwrap_or(EmitMode::Stdout));
}
// emit mode is always Stdout for Stdin.
config.set().emit_mode(EmitMode::Stdout);
config.set().verbose(Verbosity::Quiet);

// parse file_lines
Expand Down
12 changes: 7 additions & 5 deletions src/emitter/checkstyle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use self::xml::XmlEscaped;
use super::*;
use crate::rustfmt_diff::{make_diff, DiffLine, Mismatch};
use std::io::{self, Write};
use std::path::Path;

mod xml;

Expand Down Expand Up @@ -30,7 +29,6 @@ impl Emitter for CheckstyleEmitter {
}: FormattedFile<'_>,
) -> Result<EmitterResult, io::Error> {
const CONTEXT_SIZE: usize = 0;
let filename = ensure_real_path(filename);
let diff = make_diff(original_text, formatted_text, CONTEXT_SIZE);
output_checkstyle_file(output, filename, diff)?;
Ok(EmitterResult::default())
Expand All @@ -39,13 +37,13 @@ impl Emitter for CheckstyleEmitter {

pub(crate) fn output_checkstyle_file<T>(
mut writer: T,
filename: &Path,
filename: &FileName,
diff: Vec<Mismatch>,
) -> Result<(), io::Error>
where
T: Write,
{
write!(writer, r#"<file name="{}">"#, filename.display())?;
write!(writer, r#"<file name="{}">"#, filename)?;
for mismatch in diff {
let begin_line = mismatch.line_number;
let mut current_line;
Expand Down Expand Up @@ -77,7 +75,11 @@ mod tests {
fn emits_empty_record_on_file_with_no_mismatches() {
let file_name = "src/well_formatted.rs";
let mut writer = Vec::new();
let _ = output_checkstyle_file(&mut writer, &PathBuf::from(file_name), vec![]);
let _ = output_checkstyle_file(
&mut writer,
&FileName::Real(PathBuf::from(file_name)),
vec![],
);
assert_eq!(
&writer[..],
format!(r#"<file name="{}"></file>"#, file_name).as_bytes()
Expand Down
20 changes: 14 additions & 6 deletions src/emitter/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::rustfmt_diff::{make_diff, DiffLine, Mismatch};
use serde::Serialize;
use serde_json::to_string as to_json_string;
use std::io::{self, Write};
use std::path::Path;

#[derive(Debug, Default)]
pub(crate) struct JsonEmitter {
Expand Down Expand Up @@ -47,7 +46,6 @@ impl Emitter for JsonEmitter {
}: FormattedFile<'_>,
) -> Result<EmitterResult, io::Error> {
const CONTEXT_SIZE: usize = 0;
let filename = ensure_real_path(filename);
let diff = make_diff(original_text, formatted_text, CONTEXT_SIZE);
let has_diff = !diff.is_empty();

Expand All @@ -62,7 +60,7 @@ impl Emitter for JsonEmitter {

fn output_json_file<T>(
mut writer: T,
filename: &Path,
filename: &FileName,
diff: Vec<Mismatch>,
num_emitted_files: u32,
) -> Result<(), io::Error>
Expand Down Expand Up @@ -106,7 +104,7 @@ where
});
}
let json = to_json_string(&MismatchedFile {
name: String::from(filename.to_str().unwrap()),
name: format!("{}", filename),
mismatches,
})?;
let prefix = if num_emitted_files > 0 { "," } else { "" };
Expand Down Expand Up @@ -148,7 +146,12 @@ mod tests {

let mut writer = Vec::new();
let exp_json = to_json_string(&mismatched_file).unwrap();
let _ = output_json_file(&mut writer, &PathBuf::from(file), vec![mismatch], 0);
let _ = output_json_file(
&mut writer,
&FileName::Real(PathBuf::from(file)),
vec![mismatch],
0,
);
assert_eq!(&writer[..], format!("{}", exp_json).as_bytes());
}

Expand Down Expand Up @@ -188,7 +191,12 @@ mod tests {

let mut writer = Vec::new();
let exp_json = to_json_string(&mismatched_file).unwrap();
let _ = output_json_file(&mut writer, &PathBuf::from(file), vec![mismatch], 0);
let _ = output_json_file(
&mut writer,
&FileName::Real(PathBuf::from(file)),
vec![mismatch],
0,
);
assert_eq!(&writer[..], format!("{}", exp_json).as_bytes());
}

Expand Down
93 changes: 93 additions & 0 deletions src/test/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,52 @@ fn assert_output(source: &Path, expected_filename: &Path) {
}
}

// Helper function for comparing the results of rustfmt
// to a known output generated by one of the write modes.
fn assert_stdin_output(
source: &Path,
expected_filename: &Path,
emit_mode: EmitMode,
has_diff: bool,
) {
let mut config = Config::default();
config.set().newline_style(NewlineStyle::Unix);
config.set().emit_mode(emit_mode);

let mut source_file = fs::File::open(&source).expect("couldn't open source");
let mut source_text = String::new();
source_file
.read_to_string(&mut source_text)
.expect("Failed reading target");
let input = Input::Text(source_text);

// Populate output by writing to a vec.
let mut buf: Vec<u8> = vec![];
{
let mut session = Session::new(config, Some(&mut buf));
session.format(input).unwrap();
let errors = ReportedErrors {
has_diff: has_diff,
..Default::default()
};
assert_eq!(session.errors, errors);
}

let mut expected_file = fs::File::open(&expected_filename).expect("couldn't open target");
let mut expected_text = String::new();
expected_file
.read_to_string(&mut expected_text)
.expect("Failed reading target");

let output = String::from_utf8(buf).unwrap();
let compare = make_diff(&expected_text, &output, DIFF_CONTEXT_SIZE);
if !compare.is_empty() {
let mut failures = HashMap::new();
failures.insert(source.to_owned(), compare);
print_mismatches_default_message(failures);
panic!("Text does not match expected output");
}
}
// Idempotence tests. Files in tests/target are checked to be unaltered by
// rustfmt.
#[test]
Expand Down Expand Up @@ -428,6 +474,30 @@ fn stdin_works_with_modified_lines() {
assert_eq!(buf, output.as_bytes());
}

/// Ensures that `EmitMode::Json` works with input from `stdin`.
#[test]
fn stdin_works_with_json() {
init_log();
assert_stdin_output(
Path::new("tests/writemode/source/stdin.rs"),
Path::new("tests/writemode/target/stdin.json"),
EmitMode::Json,
true,
);
}

/// Ensures that `EmitMode::Checkstyle` works with input from `stdin`.
#[test]
fn stdin_works_with_checkstyle() {
init_log();
assert_stdin_output(
Path::new("tests/writemode/source/stdin.rs"),
Path::new("tests/writemode/target/stdin.xml"),
EmitMode::Checkstyle,
false,
);
}

#[test]
fn stdin_disable_all_formatting_test() {
init_log();
Expand Down Expand Up @@ -865,3 +935,26 @@ fn verify_check_works() {
.status()
.expect("run with check option failed");
}

#[test]
fn verify_check_works_with_stdin() {
init_log();

let mut child = Command::new(rustfmt().to_str().unwrap())
.arg("--check")
.stdin(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.expect("run with check option failed");

{
let stdin = child.stdin.as_mut().expect("Failed to open stdin");
stdin
.write_all("fn main() {}\n".as_bytes())
.expect("Failed to write to rustfmt --check");
}
let output = child
.wait_with_output()
.expect("Failed to wait on rustfmt child");
assert!(output.status.success());
}
6 changes: 6 additions & 0 deletions tests/writemode/source/stdin.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

fn
some( )
{
}
fn main () {}
1 change: 1 addition & 0 deletions tests/writemode/target/stdin.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"name":"stdin","mismatches":[{"original_begin_line":1,"original_end_line":6,"expected_begin_line":1,"expected_end_line":2,"original":"\nfn\n some( )\n{\n}\nfn main () {}","expected":"fn some() {}\nfn main() {}"}]}]
2 changes: 2 additions & 0 deletions tests/writemode/target/stdin.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<?xml version="1.0" encoding="utf-8"?>
<checkstyle version="4.3"><file name="stdin"><error line="1" severity="warning" message="Should be `fn some() {}`" /><error line="2" severity="warning" message="Should be `fn main() {}`" /></file></checkstyle>