Skip to content

Commit 93b7de5

Browse files
jugglerchriscalebcartwright
authored andcommitted
Make --check work when running from stdin. (#3896)
# Conflicts: # src/bin/main.rs
1 parent 737e6f7 commit 93b7de5

File tree

7 files changed

+139
-26
lines changed

7 files changed

+139
-26
lines changed

Diff for: src/bin/main.rs

+16-15
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,10 @@ pub enum OperationError {
7474
/// An io error during reading or writing.
7575
#[error("{0}")]
7676
IoError(IoError),
77-
/// Attempt to use --check with stdin, which isn't currently
78-
/// supported.
79-
#[error("The `--check` option is not supported with standard input.")]
80-
CheckWithStdin,
81-
/// Attempt to use --emit=json with stdin, which isn't currently
82-
/// supported.
83-
#[error("Using `--emit` other than stdout is not supported with standard input.")]
84-
EmitWithStdin,
77+
/// Attempt to use --emit with a mode which is not currently
78+
/// supported with stdandard input.
79+
#[error("Emit mode {0} not supported with standard output.")]
80+
StdinBadEmit(EmitMode),
8581
}
8682

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

257253
if options.check {
258-
return Err(OperationError::CheckWithStdin.into());
259-
}
260-
if let Some(emit_mode) = options.emit_mode {
261-
if emit_mode != EmitMode::Stdout {
262-
return Err(OperationError::EmitWithStdin.into());
254+
config.set().emit_mode(EmitMode::Diff);
255+
} else {
256+
match options.emit_mode {
257+
// Emit modes which work with standard input
258+
// None means default, which is Stdout.
259+
None | Some(EmitMode::Stdout) | Some(EmitMode::Checkstyle) | Some(EmitMode::Json) => {}
260+
Some(emit_mode) => {
261+
return Err(OperationError::StdinBadEmit(emit_mode).into());
262+
}
263263
}
264+
config
265+
.set()
266+
.emit_mode(options.emit_mode.unwrap_or(EmitMode::Stdout));
264267
}
265-
// emit mode is always Stdout for Stdin.
266-
config.set().emit_mode(EmitMode::Stdout);
267268
config.set().verbose(Verbosity::Quiet);
268269

269270
// parse file_lines

Diff for: src/emitter/checkstyle.rs

+7-5
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use self::xml::XmlEscaped;
22
use super::*;
33
use crate::rustfmt_diff::{make_diff, DiffLine, Mismatch};
44
use std::io::{self, Write};
5-
use std::path::Path;
65

76
mod xml;
87

@@ -30,7 +29,6 @@ impl Emitter for CheckstyleEmitter {
3029
}: FormattedFile<'_>,
3130
) -> Result<EmitterResult, io::Error> {
3231
const CONTEXT_SIZE: usize = 0;
33-
let filename = ensure_real_path(filename);
3432
let diff = make_diff(original_text, formatted_text, CONTEXT_SIZE);
3533
output_checkstyle_file(output, filename, diff)?;
3634
Ok(EmitterResult::default())
@@ -39,13 +37,13 @@ impl Emitter for CheckstyleEmitter {
3937

4038
pub(crate) fn output_checkstyle_file<T>(
4139
mut writer: T,
42-
filename: &Path,
40+
filename: &FileName,
4341
diff: Vec<Mismatch>,
4442
) -> Result<(), io::Error>
4543
where
4644
T: Write,
4745
{
48-
write!(writer, r#"<file name="{}">"#, filename.display())?;
46+
write!(writer, r#"<file name="{}">"#, filename)?;
4947
for mismatch in diff {
5048
let begin_line = mismatch.line_number;
5149
let mut current_line;
@@ -77,7 +75,11 @@ mod tests {
7775
fn emits_empty_record_on_file_with_no_mismatches() {
7876
let file_name = "src/well_formatted.rs";
7977
let mut writer = Vec::new();
80-
let _ = output_checkstyle_file(&mut writer, &PathBuf::from(file_name), vec![]);
78+
let _ = output_checkstyle_file(
79+
&mut writer,
80+
&FileName::Real(PathBuf::from(file_name)),
81+
vec![],
82+
);
8183
assert_eq!(
8284
&writer[..],
8385
format!(r#"<file name="{}"></file>"#, file_name).as_bytes()

Diff for: src/emitter/json.rs

+14-6
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use crate::rustfmt_diff::{make_diff, DiffLine, Mismatch};
33
use serde::Serialize;
44
use serde_json::to_string as to_json_string;
55
use std::io::{self, Write};
6-
use std::path::Path;
76

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

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

6361
fn output_json_file<T>(
6462
mut writer: T,
65-
filename: &Path,
63+
filename: &FileName,
6664
diff: Vec<Mismatch>,
6765
num_emitted_files: u32,
6866
) -> Result<(), io::Error>
@@ -106,7 +104,7 @@ where
106104
});
107105
}
108106
let json = to_json_string(&MismatchedFile {
109-
name: String::from(filename.to_str().unwrap()),
107+
name: format!("{}", filename),
110108
mismatches,
111109
})?;
112110
let prefix = if num_emitted_files > 0 { "," } else { "" };
@@ -148,7 +146,12 @@ mod tests {
148146

149147
let mut writer = Vec::new();
150148
let exp_json = to_json_string(&mismatched_file).unwrap();
151-
let _ = output_json_file(&mut writer, &PathBuf::from(file), vec![mismatch], 0);
149+
let _ = output_json_file(
150+
&mut writer,
151+
&FileName::Real(PathBuf::from(file)),
152+
vec![mismatch],
153+
0,
154+
);
152155
assert_eq!(&writer[..], format!("{}", exp_json).as_bytes());
153156
}
154157

@@ -188,7 +191,12 @@ mod tests {
188191

189192
let mut writer = Vec::new();
190193
let exp_json = to_json_string(&mismatched_file).unwrap();
191-
let _ = output_json_file(&mut writer, &PathBuf::from(file), vec![mismatch], 0);
194+
let _ = output_json_file(
195+
&mut writer,
196+
&FileName::Real(PathBuf::from(file)),
197+
vec![mismatch],
198+
0,
199+
);
192200
assert_eq!(&writer[..], format!("{}", exp_json).as_bytes());
193201
}
194202

Diff for: src/test/mod.rs

+93
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,52 @@ fn assert_output(source: &Path, expected_filename: &Path) {
307307
}
308308
}
309309

310+
// Helper function for comparing the results of rustfmt
311+
// to a known output generated by one of the write modes.
312+
fn assert_stdin_output(
313+
source: &Path,
314+
expected_filename: &Path,
315+
emit_mode: EmitMode,
316+
has_diff: bool,
317+
) {
318+
let mut config = Config::default();
319+
config.set().newline_style(NewlineStyle::Unix);
320+
config.set().emit_mode(emit_mode);
321+
322+
let mut source_file = fs::File::open(&source).expect("couldn't open source");
323+
let mut source_text = String::new();
324+
source_file
325+
.read_to_string(&mut source_text)
326+
.expect("Failed reading target");
327+
let input = Input::Text(source_text);
328+
329+
// Populate output by writing to a vec.
330+
let mut buf: Vec<u8> = vec![];
331+
{
332+
let mut session = Session::new(config, Some(&mut buf));
333+
session.format(input).unwrap();
334+
let errors = ReportedErrors {
335+
has_diff: has_diff,
336+
..Default::default()
337+
};
338+
assert_eq!(session.errors, errors);
339+
}
340+
341+
let mut expected_file = fs::File::open(&expected_filename).expect("couldn't open target");
342+
let mut expected_text = String::new();
343+
expected_file
344+
.read_to_string(&mut expected_text)
345+
.expect("Failed reading target");
346+
347+
let output = String::from_utf8(buf).unwrap();
348+
let compare = make_diff(&expected_text, &output, DIFF_CONTEXT_SIZE);
349+
if !compare.is_empty() {
350+
let mut failures = HashMap::new();
351+
failures.insert(source.to_owned(), compare);
352+
print_mismatches_default_message(failures);
353+
panic!("Text does not match expected output");
354+
}
355+
}
310356
// Idempotence tests. Files in tests/target are checked to be unaltered by
311357
// rustfmt.
312358
#[nightly_only_test]
@@ -463,6 +509,30 @@ fn stdin_works_with_modified_lines() {
463509
assert_eq!(buf, output.as_bytes());
464510
}
465511

512+
/// Ensures that `EmitMode::Json` works with input from `stdin`.
513+
#[test]
514+
fn stdin_works_with_json() {
515+
init_log();
516+
assert_stdin_output(
517+
Path::new("tests/writemode/source/stdin.rs"),
518+
Path::new("tests/writemode/target/stdin.json"),
519+
EmitMode::Json,
520+
true,
521+
);
522+
}
523+
524+
/// Ensures that `EmitMode::Checkstyle` works with input from `stdin`.
525+
#[test]
526+
fn stdin_works_with_checkstyle() {
527+
init_log();
528+
assert_stdin_output(
529+
Path::new("tests/writemode/source/stdin.rs"),
530+
Path::new("tests/writemode/target/stdin.xml"),
531+
EmitMode::Checkstyle,
532+
false,
533+
);
534+
}
535+
466536
#[test]
467537
fn stdin_disable_all_formatting_test() {
468538
init_log();
@@ -896,3 +966,26 @@ fn verify_check_works() {
896966
.status()
897967
.expect("run with check option failed");
898968
}
969+
970+
#[test]
971+
fn verify_check_works_with_stdin() {
972+
init_log();
973+
974+
let mut child = Command::new(rustfmt().to_str().unwrap())
975+
.arg("--check")
976+
.stdin(Stdio::piped())
977+
.stderr(Stdio::piped())
978+
.spawn()
979+
.expect("run with check option failed");
980+
981+
{
982+
let stdin = child.stdin.as_mut().expect("Failed to open stdin");
983+
stdin
984+
.write_all("fn main() {}\n".as_bytes())
985+
.expect("Failed to write to rustfmt --check");
986+
}
987+
let output = child
988+
.wait_with_output()
989+
.expect("Failed to wait on rustfmt child");
990+
assert!(output.status.success());
991+
}

Diff for: tests/writemode/source/stdin.rs

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
fn
3+
some( )
4+
{
5+
}
6+
fn main () {}

Diff for: tests/writemode/target/stdin.json

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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() {}"}]}]

Diff for: tests/writemode/target/stdin.xml

+2
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<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>

0 commit comments

Comments
 (0)