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.

Sign up for GitHub

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

Fixed naive doc formatting for #[must_use] lints #7827

Merged
merged 1 commit into from
Oct 16, 2021
Merged

Fixed naive doc formatting for #[must_use] lints #7827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions clippy_lints/src/functions/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,9 @@ declare_clippy_lint! {

declare_clippy_lint! {
/// ### What it does
/// Checks for a [`#[must_use]`] attribute on
/// Checks for a `#[must_use]` attribute on
/// unit-returning functions and methods.
///
/// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
///
/// ### Why is this bad?
/// Unit values are useless. The attribute is likely
/// a remnant of a refactoring that removed the return type.
Expand All @@ -112,12 +110,10 @@ declare_clippy_lint! {

declare_clippy_lint! {
/// ### What it does
/// Checks for a [`#[must_use]`] attribute without
/// Checks for a `#[must_use]` attribute without
/// further information on functions and methods that return a type already
/// marked as `#[must_use]`.
///
/// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
///
/// ### Why is this bad?
/// The attribute isn't needed. Not using the result
/// will already be reported. Alternatively, one can add some text to the
Expand All @@ -138,11 +134,9 @@ declare_clippy_lint! {
declare_clippy_lint! {
/// ### What it does
/// Checks for public functions that have no
/// [`#[must_use]`] attribute, but return something not already marked
/// `#[must_use]` attribute, but return something not already marked
/// must-use, have no mutable arg and mutate no statics.
///
/// [`#[must_use]`]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute
///
/// ### Why is this bad?
/// Not bad at all, this lint just shows places where
/// you could add the attribute.
Expand Down
7 changes: 3 additions & 4 deletions clippy_lints/src/let_underscore.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@ use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// ### What it does
/// Checks for `let _ = <expr>`
/// where expr is #[must_use]
/// Checks for `let _ = <expr>` where expr is `#[must_use]`
///
/// ### Why is this bad?
/// It's better to explicitly
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, most of these doc comments intentionally use an extra link for must_use. These comments are exported and rendered on Clippy's lint list which doesn't automatically lint to Rust's attributes and symbols. Therefore, it wouldn't be good to change it in the lint documentation.

This adjustment is correct, though. Could you revert, the other changes and add a link for the attributes in this lint documentation? 🙃

Copy link
Contributor Author

@zvavybir zvavybir Oct 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but I don't quite understand you. For me this website looks so (screenshot from the must_use_candidate lint):
Screenshot_20211016_175219
Is this here shown behaviour really the desired one? I started this PR to get the outer [] away and when I looked more closely I saw that this "link" doesn't actual work, so I removed it too. Was this wrong?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I believe that GH actually accepts code like that. Let's see [#[must_use]]

[#[must_use]]: https://doc.rust-lang.org/reference/attributes/diagnostics.html#the-must_use-attribute

Apparently not, TIL. I thought that markdown would accept that, but apparently it doesn't. This makes a lot more sense now. Thank you for checking 🙃

/// handle the value of a #[must_use] expr
/// It's better to explicitly handle the value of a `#[must_use]`
/// expr
///
/// ### Example
/// ```rust
Expand Down