-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Adapting the lint list to Clippy's new metadata format #7279
Adapting the lint list to Clippy's new metadata format #7279
Conversation
974580e
to
c0f8291
Compare
New to-dos:
|
☔ The latest upstream changes (presumably #7352) made this pull request unmergeable. Please resolve the merge conflicts. |
I got another idea for the website (definitely not for this PR though): Add a link to playground for |
Hey, I think that should be possible to implement this in some form, rustdoc already has similar functionality. I believe the biggest hurdle would be to adapt all lint documentation to have valid code. Some examples are currently using just a code snipped that would be invalid on its own. (Examples: (1) (2) (3)). The code would also need to include a lint level for the lint like What is the main objective of this feature? @flip1995 I would only use it to view the output of such a lint. However, I think that that use case might be better covered by an implementation that includes the lint output in the documentation itself. Rustc does this by running the example code and replacing the string // Bad
let _ = "Hello".bytes().nth(3);
// Good
let _ = "Hello".as_bytes().get(3); Lint output
(Sorry if the last part totally misses the point of your question. This was a thing that I thought about while working on a lint in rustc and my mind made the connection to your question) |
All code in the documentation tagged with rust-clippy/clippy_lints/src/swap.rs Lines 50 to 55 in 6ae0835
Yeah my intention is pretty much to run Clippy on the code and see the output. I don't think this feature will be used very often, so it shouldn't be a priority. This is more something that would be nice to have, if you or someone else has an idea on how to add this non-intrusively. If it is much work to add it, I really don't think it is worth adding at all. |
Okay, I'll add that as an idea to the tracking issue 🙃 |
4026896
to
e5f8927
Compare
// This removes the leading space that the macro translation introduces | ||
if let Some(stripped_doc) = line.strip_prefix(' ') { | ||
docs.push_str(stripped_doc); | ||
} else if !line.is_empty() { | ||
docs.push_str(&line); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rebase might make it difficult to see where and how the conflicts were resolved. I added this block to strip the leading space on every filled line and added a doc comment to the method itself, the rest stayed the same. I also removed the JS implementation of the language extraction in a new commit.
e5f8927
to
3958620
Compare
@bors treeclosed=1000 |
I don't understand CSS or JS, so I can't really review those changes, but a quick skim over the changes and looking at the resulting website LGTM. Approving to do a rollup. @bors r+ rollup |
📌 Commit 08a9cb9 has been approved by |
🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened. |
…ormat, r=flip1995 Adapting the lint list to Clippy's new metadata format This is close to the end of a long living project to rewrite the lint metadata collection for Clippy. Progress on this has been tracked in rust-lang#7172. This PR adds one of the last missing puzzle pieces, the actual display of all the changes that have been done in the background. A preview can be seen here: [Clippy's lint list](https://xfrednet.github.io/rust-clippy/master/index.html) The styling has been discussed on [zulip](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Styling.20feedback.20for.20Clippy's.20lint.20list/near/239601067) but is still open to suggestion. Side note: It has been fun working on the website where we don't have unit tests and everything is just tested by playing around. However, it's good that this chaos is contained into this one part of Clippy. 🐛 --- Closes: rust-lang#1303 Closes: rust-lang#4310 This actually closes fewer things than I thought it would... changelog: Reworked Clippy's [website](https://rust-lang.github.io/rust-clippy/master/index.html): changelog: * Added applicability information about lints changelog: * Added a link to jump to the specific lint implementation changelog: * Adapted some styling and improved loading time r? `@flip1995`
…ednet,flip1995 Switch CI to new metadata collection r? `@xFrednet` Things we have to keep in mind: - This removes the template files and the scripts used for deployment from the checkout. This was added in rust-lang#5517. I don't think we ever needed those there. Not sure though. - ~~As a result, we can't remove the python scripts yet. We have to wait until this hits a stable Clippy release.~~ I'll just break the next stable deploy and do it by hand once. - This should be merged together with rust-lang#7279. Me and `@xFrednet` will coordinate the switch - ...? I still have to try out some things: - [x] Is it worth caching? Yes - [x] ~~Is it worth to do a release build?~~ Nope - [x] Does it actually work? With a few changes, yes - [ ] ...? changelog: Clippy now uses a lint to generate its documentation 🎉
08a9cb9
to
dbddce1
Compare
Changes included: - Minimum adaption to the new `lints.json` format - Fixing filtering for the new `lints.json` format; hardcoding the lint groups in the index - Recreating the original doc styling for the new format - Fixed sytax highlighting for rust,ignore code blocks - Fixed markdown table extraction in the metadata collector and fixed lint level output - Adding the additional information row for lints - Changed the website title to Clippy's lint list - Flexing the website for mobile users - Added (?) references for lint levels and groups - Making deprecated lints look dead - Removed JS code block language extraction in favor of a rust implementation `rust-clippy#7352` - Added the suspicious lint group to the lint list - Remove trailing whitespaces from index.html - Fix code highlighting - Use default value if the docVersion is empty Co-authored-by: Philipp Krones <[email protected]>
dbddce1
to
322a768
Compare
@bors r+ rollup |
📌 Commit 322a768 has been approved by |
🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened. |
…ednet,flip1995 Switch CI to new metadata collection r? `@xFrednet` Things we have to keep in mind: - This removes the template files and the scripts used for deployment from the checkout. This was added in rust-lang#5517. I don't think we ever needed those there. Not sure though. - ~~As a result, we can't remove the python scripts yet. We have to wait until this hits a stable Clippy release.~~ I'll just break the next stable deploy and do it by hand once. - This should be merged together with rust-lang#7279. Me and `@xFrednet` will coordinate the switch - ...? I still have to try out some things: - [x] Is it worth caching? Yes - [x] ~~Is it worth to do a release build?~~ Nope - [x] Does it actually work? With a few changes, yes - [ ] ...? changelog: Clippy now uses a lint to generate its documentation 🎉
Updated changelog for 1.55 This has again been a bit of work, but I'm happy to notice that my English is still improving, and I'm getting faster at these things. That's a very nice side effect of contributing and getting feedback on reviews 😊 Moving on, there were a few things that I was unsure about: * The PR rust-lang/rust#86717 changes an old entry in the change log, is this worth mentioning? I've left it out for now. * The stabilization of `cargo clippy --fix` is quite awesome and important IMO. It sadly gets a bit lost in the *Other* entry, as it's the last one. Do we maybe want to move it somewhere else or change the headline order for this release? * I've listed the introduction of the new `suspicious` group under the *Moves and Deprecations* section. Is this alright, or should it be moved to the *Other* section as well? * Last but definitely not least, some fun! I've used the 🎉 emoji in the `cargo clippy --fix` entry, is this okay? Sorry for the bombardment of questions xD --- The PR already includes the entries for the new metadata collection and website updates. These are not merged yet, but should probably be to make this correct. This might also require the commit hashes to be updated (Not sure on this, though). It would actually be super fitting to get this into this release as we also stabilize `--fix`. TODOs: * [x] Merge metadata collection PRs: 1. #7279 2. #7298 3. #7420 (Hope to not get any merge conflicts) --- [Rendered 📰](https://github.com/xFrednet/rust-clippy/blob/changelog-1-55/CHANGELOG.md) r? `@flip1995` changelog: none
This is close to the end of a long living project to rewrite the lint metadata collection for Clippy. Progress on this has been tracked in #7172. This PR adds one of the last missing puzzle pieces, the actual display of all the changes that have been done in the background. A preview can be seen here: Clippy's lint list
The styling has been discussed on zulip but is still open to suggestion.
Side note: It has been fun working on the website where we don't have unit tests and everything is just tested by playing around. However, it's good that this chaos is contained into this one part of Clippy. 🐛
Closes: #1303
Closes: #4310
This actually closes fewer things than I thought it would...
changelog: Reworked Clippy's website:
changelog: * Added applicability information about lints
changelog: * Added a link to jump to the specific lint implementation
changelog: * Adapted some styling and improved loading time
r? @flip1995