Extend the diagnostics plugin with uniqueness tests and JSON output. #24884
Conversation
|
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
|
cc @brson |
|
This is my first time using |
|
@michaelsproul you don't need to do anything - some people leave comments, but it is not necessary |
| pub use self::Uniqueness::*; | ||
|
|
||
| // Directory to use for extended error JSON. | ||
| const ERROR_METADATA_DIR: &'static str = "tmp/extended-errors"; |
There was a problem hiding this comment.
It would be nice if the user could customise this path, perhaps through an environment variable.
|
Looks good, and I'm excited to have proper checking of error diagnostics across crates. One problem I see here is that we might compare metadata from an old build of a crate with a new build of the current crate. I think to avoid some false positives, that when a crate is built, we should delete its file and the files of any dependencies. Unfortunately, I don't know how to do that because rustc doesn't have access to the deps. I guess this could be built in to our build system, but that sounds kinda tough. Anyway, if you've got any ideas for how to do this, it would be great, otherwise, I think this can land as is (well, with the other comments addressed). Also, I think, if possible, it would be good to pull out all the diagnostics stuff into its own crate, rather than have it in libsyntax (obvs, doesn't need to be done to land this, but if you'd like to do it in the future it would be appreciated). |
|
@nrc: I've addressed your comments as best I could. In light of lints possibly getting error codes (#24900) I completed removed all mentions of crates from the metadata module. I also made the metadata dir adjustable via the As far as stale errors go, I think we're unlikely to run into issues unless a crate is renamed or an error is moved from one crate to another. In either case the error message about where the duplicate was found should allow the problem to be resolved quickly. The current code deals with old metadata for the same crate by ignoring any metadata with a matching name. I'm also happy to pull the diagnostics stuff into its own crate at some point 😄 |
|
Oh, I also marked some more stuff public, because I realised I'll need it for the HTML page generator. |
|
@bors r+ |
|
📌 Commit 096d72a has been approved by |
|
⌛ Testing commit 096d72a with merge 79e8da7... |
|
💔 Test failed - auto-mac-64-opt |
Diagnostic errors are now checked for uniqueness across the compiler and error metadata is written to JSON files.
|
Whoops! I've fixed the test case and pushed a new commit. |
I've been working on improving the diagnostic registration system so that it can:
* Check uniqueness of error codes *across the whole compiler*. The current method using `errorck.py` is prone to failure as it relies on simple text search - I found that it breaks when referencing an error's ident within a string (e.g. `"See also E0303"`).
* Provide JSON output of error metadata, to eventually facilitate HTML output, as well as tracking of which errors need descriptions. The current schema is:
```
<error code>: {
"description": <long description>,
"use_site": {
"filename": <filename where error is used>,
"line": <line in file where error is used>
}
}
```
[Here's][metadata-dump] a pretty-printed sample dump for `librustc`.
One thing to note is that I had to move the diagnostics arrays out of the diagnostics modules. I really wanted to be able to capture error usage information, which only becomes available as a crate is compiled. Hence all invocations of `__build_diagnostics_array!` have been moved to the ends of their respective `lib.rs` files. I tried to avoid moving the array by making a plugin that expands to nothing but couldn't invoke it in item position and gave up on hackily generating a fake item. I also briefly considered using a lint, but it seemed like it would impossible to get access to the data stored in the thread-local storage.
The next step will be to generate a web page that lists each error with its rendered description and use site. Simple mapping and filtering of the metadata files also allows us to work out which error numbers are absent, which errors are unused and which need descriptions.
[metadata-dump]: https://gist.github.com/michaelsproul/3246846ff1bea71bd049
This PR adds a program which uses the JSON output from #24884 to generate a webpage with descriptions of each diagnostic error. The page is constructed by hand, with calls to `rustdoc`'s markdown renderers where needed. I opted to generate HTML directly as I think it's more flexible than generating a markdown file and feeding it into the `rustdoc` executable. I envision adding the ability to filter errors by their properties (description, no description, used, unused), which is infeasible using the whole-file markdown approach due to the need to wrap each error in a `<div>` (markdown inside tags isn't rendered). Architecturally, I wasn't sure how to add this generator to the distribution. For the moment I've settled on a separate Rust program in `src/etc/` that gets compiled and run by a custom makefile rule. This approach doesn't seem too hackish, but I'm unsure if my usage of makefile variables is correct, particularly the call to `rustc` (and the `LD_LIBRARY_PATH` weirdness). Other options I considered were: * Integrate the error-index generator into `rustdoc` so that it gets invoked via a flag and can be built as part of `rustdoc`. * Add the error-index-generator as a "tool" to the `TOOLS` array, and make use of the facilities for building tools. The main reason I didn't do this was because it seemed like I'd need to add lots of stuff. I'm happy to investigate this further if it's the preferred method.
I've been working on improving the diagnostic registration system so that it can:
errorck.pyis prone to failure as it relies on simple text search - I found that it breaks when referencing an error's ident within a string (e.g."See also E0303").Here's a pretty-printed sample dump for
librustc.One thing to note is that I had to move the diagnostics arrays out of the diagnostics modules. I really wanted to be able to capture error usage information, which only becomes available as a crate is compiled. Hence all invocations of
__build_diagnostics_array!have been moved to the ends of their respectivelib.rsfiles. I tried to avoid moving the array by making a plugin that expands to nothing but couldn't invoke it in item position and gave up on hackily generating a fake item. I also briefly considered using a lint, but it seemed like it would impossible to get access to the data stored in the thread-local storage.The next step will be to generate a web page that lists each error with its rendered description and use site. Simple mapping and filtering of the metadata files also allows us to work out which error numbers are absent, which errors are unused and which need descriptions.