Return an error if --check or --emit json are used with stdin.
#3875
Conversation
src/bin/main.rs
Outdated
| @@ -464,6 +472,14 @@ fn determine_operation(matches: &Matches) -> Result<Operation, OperationError> { | |||
| if minimal_config_path.is_some() { | |||
| return Err(OperationError::MinimalPathWithStdin); | |||
| } | |||
| if matches.opt_present("check") { | |||
There was a problem hiding this comment.
Would it be better to move these checks into format_string (https://github.com/rust-lang/rustfmt/blob/master/src/bin/main.rs#L241) so that we can use the already parsed values? (options.check and emit_mode)
There was a problem hiding this comment.
That sounds reasonable, yes - it didn't feel quite right comparing with a literal there! I'll take a look there, thanks.
There was a problem hiding this comment.
I've moved the check into format_string. I'm still returning Err(OperationError) but with added .into() to convert to FailureError - maybe ? would be better?
|
The CI failures seem to be unrelated to the changes here - I think it's the same as rust-lang/rust#65424 |
src/bin/main.rs
Outdated
| return Err(OperationError::CheckWithStdin); | ||
| } | ||
| if let Some(mode) = matches.opt_str("emit") { | ||
| if mode == "json" { |
There was a problem hiding this comment.
All of the emit modes other than stdout will currently fail with stdin (not just json), so I think this should check for any emit mode other than stdout and fail accordingly
There was a problem hiding this comment.
Thanks, I'll tweak it.
There was a problem hiding this comment.
This is a bit broken, since the default is --emit=files. format_string just overrides whatever it was with stdout, but I think it needs to distinguish between an explicit --emit=files (which would be an error) and not being specified (in which case default to stdout), which suggests that options.emit_mode should be an Option.
Correct |
…or other --emit values.
be defaulted depending on the mode.
The intention is to implement these cases, but return an error is better than silently ignoring them in the meantime.
See #3871 .