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

Stabilise --emit=checkstyle #4614

Open
adamncasey opened this issue Dec 29, 2020 · 4 comments
Open

Stabilise --emit=checkstyle #4614

adamncasey opened this issue Dec 29, 2020 · 4 comments
Labels
stabilisation-request A request to stabilise an option or argument

Comments

@adamncasey
Copy link

--emit=checkstyle has existed in rustfmt for a number of years. Although there have been some bugs fixed in the past, nothing has been mentioned in the last year or so when it was suggested checkstyle/json were considered stable: #3947 (comment) I believe using these features still requires a nightly build of rustfmt: https://github.com/rust-lang/rustfmt/blob/rustfmt-1.4.31/src/bin/main.rs#L496

This feature is useful for integrating rustfmt into existing code lint pipelines which support checkstyle from other languages. It would be helpful to me to have this support in a stable rust release to avoid managing a nightly build for this feature.

I can't see any documentation or issues which describe why some emit options are behind a nightly release, so opening this to make progress or at least document the current state.

Two questions to start with:

  1. Is there a path forward for these features to become available in stable builds of rustfmt & therefore stable builds of rust/cargo/etc?
  2. Would a change to stablise this on a pre-2.0 feature branch be accepted? An untested/draft patch

PS Thank you for the effort put into this project. I can see there's a near constant stream of issues/bug reports here. I'm hoping this doesn't just add to the noise.

@adamncasey adamncasey changed the title Stablise --emit=checkstyle Stabilise --emit=checkstyle Dec 29, 2020
@calebcartwright
Copy link
Member

Thank you for the question @adamncasey.

When it comes to stabilizing things wsithin rustfmt, the criteria enumerated specifically for config options here actually tend to be generally applicable everywhere, including for emit modes.

Additionally, for emit modes we need to have a high degree of confidence that the schema/format is future proof due given how seriously Rust treats stability. Unfortunately some times that means stabilization of rustfmt features moves at a somewhat glacial pace, but that's because once we've crossed that stabilization line we can't go back.

It still seems like we're not quite there yet for the stabilization of the checkstyle emit mode. With both the json and checkstyle emit modes, there's still some gaps and challenges related to non-misformatted errors. For example, any rustc parser or rustfmt errors are sent to stderr and are not included in the corresponding json/xml (#4369) and in certain scenarios this also causes issues with the actual output (#1635, I'm actually not sure why this was closed as I believe it still exists).

In order to be able to stabilize the checkstyle mode, at a minimum we'd need to confirm that #1635 no longer occurs in any case, as well as having an answer on if/how we could incorporate non-formatting errors (parser errors, operational errors, etc.) into the output without requiring breaking changes to the schema/consumers.

@calebcartwright calebcartwright added stabilisation-request A request to stabilise an option or argument and removed feature-request labels Dec 30, 2020
@davidBar-On
Copy link
Contributor

.... we'd need to confirm that #1635 no longer occurs ...

I don't have the file that was used for #1635, but with a simple test I run setting --emit=checkstyle and using a file with a line too long, the "line exceeded maximum length" error messages appeared *before the JSON output, so it seems that the issue was resolved.

@calebcartwright
Copy link
Member

*before the JSON output

The checkstyle output should be xml, so just want to confirm you meant to say xml instead of json? Or were you running with the json emitter instead?

@davidBar-On
Copy link
Contributor

.. just want to confirm you meant to say xml instead of json?

Yes, I meant XML

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stabilisation-request A request to stabilise an option or argument
Projects
None yet
Development

No branches or pull requests

3 participants