add --explain subcommand

[llogiq]

This closes #8291.

---

changelog: add cargo clippy -- --explain <lintname> subcommand

[rust-highfive]

r? @dswij

(rust-highfive has picked a reviewer for you, use r? to override)

[llogiq]

I started out trying to get a proc macro to work, but then I found Alexey Kladow's blog post about self-modifying code and I thought this might be just the right application for the technique.

[xFrednet]

This one looks interesting, and my reviewing queue is currently relatively small. I would take over the review. (If you want to keep it @dswij, feel free to say so)

r? @xFrednet

@llogiq could you maybe link the blog post? Your comment reminded me of a post, I've read before. Also, as a side node, I sometimes find it confusing, when to use the double dashes after cargo clippy and when not. Therefore, I think it would be cool, if the --explain comment would also be accepted by cargo-clippy and then passed to clippy-driver. But that can also be done in a followup PR 🙃

[llogiq]

Updated my previus comment with the link.

[dswij]

Sure, go ahead!

[llogiq]

@xFrednet I'm fine with merging this & doing a follow-up for cargo-clippy later.

[llogiq]

I noted that -E is already used for "error".

[xFrednet]

The implementation itself looks good to be. I haven't done a full test, but the initial tries seem good.

While I like the concept of using a test, it feels a bit off in Clippy's codebase since we already do this kind of generation in clippy_dev. This introduces another location with little benefit in comparison to adding it to clippy_dev. At least that's my impression. clippy_dev doesn't have the regex crate, but that should be simple to avoid/rewrite.

cc: @Serial-ATA This might be interesting, since you're working on the metadata collection/lint list. Feel free to ignore this as well 🙃

[Serial-ATA]

Couldn't docs.rs be generated during the metadata collection lint alongside lints.json? Just so you don't have to reinvent the wheel to get the docs.

[flip1995]

Couldn't docs.rs be generated during the metadata collection lint alongside lints.json?

collect-metadata is only run during deployment and not even in bors CI. However, collect-metadata could use this file to generate the documentation instead of doing it itself.

[SeniorMars]

This would be a really cool thing to have -- especially if we can somehow add this feature to rust-analyzer. Thank you for your work!

[llogiq]

So, I see two ways to improve on this:

• I'll move the docs creation to cargo-dev so it's included with update_lints and re-use our current infrastructure.
• I'll change the generation to emit a file per lint and a single file to include_str! all of those doc files. This way we won't get too many merge conflicts from docs.

[llogiq]

This version should deal with the merge conflicts arising from doc changes.

I didn't get to changing the code to use cargo dev update_lints yet. Using the metadata collector would not be optimal, because we don't run it in CI. On the other hand, extending update_lints to also parse the doc comments would duplicate some machinery. Oh well.

[llogiq]

Hmmm...I should not name the files ".md" when they don't contain markdown...

[llogiq]

With the following force-push, I removed the final newline from the md files as well as # -ignored code lines.

@dswij r?

[xFrednet]

Sorry, for the late replay, I moved last week and life is currently still a bit chaotic. Reviewing this is on my todo list for this weekend. If @dswij want's to take over that would also be fine for me 🙃

[llogiq]

@xFrednet @dswij ping?

[xFrednet]

Overall, looks good to me. I also like the location in cargo-clippy. I've also played around with it and to works well :). My GitHub is actually struggling a bit with this many changes ^^.

Thank you for your patience, the last weeks have been very full for me 😅

[xFrednet]

Hey @rust-lang/clippy, this PR adds the --explain comment to cargo-clippy. For this, it stores the lint description in a text file inside src/docs/<lint_name>.txt. The documentation files are automatically checked and updated by cargo dev update_lints. This seems to work quite well and looks good to me. Does anyone have any concerns regarding this? 🙃

[llogiq]

I need to rebase to make the tests work.

[llogiq]

We discussed wanting this three meetings ago, and I agreed to implement it. I don't think there were any objections during that meeting.

Re the implementation: Splitting the files and using include_str! to pull them together was done to reduce merge conflicts. Merging this will fail unless it's rebased on a sufficiently late version (i.e. no docs changes in between), so please ping me before r+ing it.

[xFrednet]

We discussed wanting this three meetings ago, and I agreed to implement it. I don't think there were any objections during that meeting.

I just wanted to make sure now that we had an actual implementation to play around with. It looks like everyone is cool with this :)

---

This looks good to me, you can r=me after a rebase and lint update 🙃 Thank you for the implementation, happy to finally have this in Clippy 🥳 🎉

[llogiq]

@bors r=xFredNet

[bors]

📌 Commit ad72aee has been approved by xFredNet

It is now in the queue for this repository.

[bors]

⌛ Testing commit ad72aee with merge 30e4532...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFredNet
Pushing 30e4532 to master...

[llogiq]

Thank you for the review, @xFrednet.