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

update json emitter to better handle errors #3953

Merged
merged 1 commit into from
Dec 9, 2019

Conversation

calebcartwright
Copy link
Member

Resolves #3952 and relevant for #3947

This changes the json emitter to emit the full json output for the formatting results as part of emit_footer whereas the current approach incrementally emitted incomplete json snippets as each file was processed and used emit_header and emit_footer to try to sandwich it all together into valid json (which was not always viable as described in #3952).

Copy link
Contributor

@topecongiro topecongiro left a comment

Choose a reason for hiding this comment

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

LGTM, thank you! Also, I like that we now have a newline at the end of the output.

@@ -120,6 +109,9 @@ mod tests {

#[test]
fn expected_line_range_correct_when_single_line_split() {
let mut emitter = JsonEmitter {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: we can use JsonEmitter::default().

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. I need to make a similar update to the checkstyle xml emitter at some point (which suffers from the same type of invalid output in this scenario) and will tweak this accordingly when I do

@topecongiro topecongiro merged commit 04a37b5 into rust-lang:master Dec 9, 2019
@calebcartwright calebcartwright deleted the json-emitter-fix branch December 11, 2019 01:37
@calebcartwright
Copy link
Member Author

Also, I like that we now have a newline at the end of the output.

Me too. I feel like one day it would be good to see if we can also have the errors emitted as part of the json/xml output instead of stderr when the user specifies the json or checkstyle emit mode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

json emit mode produces invalid json in terminal if rustfmt errs
3 participants