Draft: refactor cli with annotate-snippets

[suaviloquence]

Opening this PR not to merge immediately (or even ever) but to show what we could do with annotate-snippets and get feedback on the CLI design (to be more rustc/clippy-like).

Screenshots:

cargo semver-checks --baseline-root test_crates/function_const_removed/old --manifest-path test_crates/function_const_removed/new -Z unstable-options --witness-hints

old

new

when file is not available for span

on manifest_tests/workspace_overrides test crate (warnings + errors)

old

new

On compatibility with clippy/rustc:

This is mostly just a port of our current failure printing to use annotate-snippets (and adding printing the one-line span snippet).

clippy/rustc linters show generally less information per lint, and don't group instances of lints occurring by lint type. We group all lint results and print lots of info for each lint (description, ref link, info link) as well as per-result error info.

sample clippy/rustc lints

I don't know if we actually want parity with rustc in this regard, though.

[obi1kenobi]

I quite like the new format, it looks very promising — especially with the annotated snippets! Are the annotations based on line/column information only, since we don't yet have byte spans in rustdoc JSON?

Some thoughts for further experimentation:

• I'd probably skip the help: ref and help: impl links and save them for the --explain flag and/or a website that describes the lints, when we get one.
• It might be good if we printed error:/warning: only once per error instead of twice (once at the start of the error description and once with the file path).
• Would it make sense to move the annotated snippet up to right under the error: line and move the = help: and = note: lines under the code, instead of having them sort of oddly indented right under the headline?
• For the "code similar to the following", would it make sense to avoid printing that at top level and instead move it to a = note so it's clearly part of the same error report and not a new one?
• The heavily-indented Summary and Warning lines at the very end of the output look a bit out of place now. Would it make sense to change their style to mimic clippy / rustc more closely?

cc @epage, @Muscraft

[suaviloquence]

I'm currently working on the final write-up for GSoC, so I'll respond more thoroughly when I'm done with that, but just a note that this current design is imitating the current c-s-c design where we print info for each lint, then print the different occurrences of this lint. Here's an example where there are more than one instance of the same lint occurring:

old

new

If we wanted more consistency with rustc/clippy, we could not do that first part with the info and move parts of it to each instance of the lint occurring, and some to --explain like you said.

[obi1kenobi]

I think if we're going for clippy / rustc consistency, we should go all the way and tweak our error format to print once-per-instance instead of grouping like we currently do.

The only thing I'd keep is the initial info on which crate is being scanned / how many lints are being run and why etc. For all the actual lint findings, the sweet spots are probably at either end of the spectrum, not in the middle.

[epage]

I think if we're going for clippy / rustc consistency, we should go all the way and tweak our error format to print once-per-instance instead of grouping like we currently do.

If it becomes overwhelming, you can omit some of the details for a lint after the first occurrence.

[suaviloquence]

New screenshots:

warnings + errors

multiple of the same lint

We could also print the lint ID like this:

[obi1kenobi]

This looks very promising, I like it quite a bit. I think the error[lint_name] approach looks better than error: lint_name: text.

Thank you for being open to iterate on this — it's a really, really hard problem to nail down all the edge cases. I would recommend not spending 100% of time on only this: we'll have to iterate a bunch more here, and it's nice to mix in some building & merging every so often just for personal satisfaction and morale :)

A few thoughts for things to look at next:

• re: the consecutive warning lines at the end of the printout, maybe the second one should be a note: instead?
• re: the info line where the templated message goes now, that seems pretty duplicative of the annotated snippet. There are only two pieces of info there that aren't in the snippet, and it might be good to deduplicate + think about how to use that space more effectively:
  • whether the snippet is from "baseline" (previously in lib.rs:1) or "current" (in lib.rs:1)
  • the full importable path of the item that's part of the public API
• re: the snippet's length — what happens if the removed function (or module, etc.) is very long? We don't want to print the whole thing. Printing just one line isn't great either because depending on formatting, we might not get the full function signature.
• re: re-export removals that trigger function_missing, the span will point to the function's definition. But the definition wasn't removed — the re-export removal made some path that was previously public API no longer exist. We don't currently have the span for the re-export that went missing. Showing the function snippet without at least additional context would be quite misleading (and IDK, maybe showing it at all is a bad idea). I'm not sure how to tackle this.

[suaviloquence]

We may need to improve how we handle spans before we can have snippets like this. Currently, we really only have the span_filename and line number of span_begin_line, so we don't know how many lines of a function to show, and we can't annotate things like a reexport, or e.g. a #[must_use] or #[non_exhaustive] annotation.

[obi1kenobi]

100% agreed. Lints could expose span_end_line too, but that's still going to be way too coarse and imprecise in many cases (e.g. pointing to #[non_exhaustive] or to "just the function signature" etc.).

In the long run, we'll eventually probably need to use the coarse spans available in rustdoc JSON as a guide for loading & parsing the affected Rust file, and then extracting more precise spans on-demand as needed.

In the short run, I'm not sure what the best answer is. Maybe we can investigate adding span_end_line and then using some heuristic like "include up to the first 5 lines, then switch to ellipsis" for the snippet?