Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion clippy_lints/src/approx_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ declare_clippy_lint! {
/// let x = 3.14;
/// let y = 1_f64 / x;
/// ```
/// Use predefined constants instead:
/// Use instead:
/// ```rust
/// let x = std::f32::consts::PI;
/// let y = std::f64::consts::FRAC_1_PI;
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/as_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ declare_clippy_lint! {
/// f(a as u16);
/// ```
///
/// Usually better represents the semantics you expect:
/// Use instead:
/// ```rust,ignore
/// f(a.try_into()?);
/// ```
Expand Down
10 changes: 8 additions & 2 deletions clippy_lints/src/assign_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,16 @@ declare_clippy_lint! {
/// let mut a = 5;
/// let b = 0;
/// // ...
/// // Bad
///
/// a = a + b;
/// ```
///
/// Use instead:
/// ```rust
/// let mut a = 5;
/// let b = 0;
/// // ...
///
/// // Good
/// a += b;
/// ```
#[clippy::version = "pre 1.29.0"]
Expand Down
32 changes: 17 additions & 15 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,14 @@ declare_clippy_lint! {
///
/// ### Example
/// ```ignore
/// // Bad
/// #[deny(dead_code)]
/// extern crate foo;
/// #[forbid(dead_code)]
/// use foo::bar;
/// ```
///
/// // Ok
/// Use instead:
/// ```rust,ignore
/// #[allow(unused_imports)]
/// use foo::baz;
/// #[allow(unused_imports)]
Expand Down Expand Up @@ -146,15 +147,19 @@ declare_clippy_lint! {
///
/// ### Example
/// ```rust
/// #[allow(dead_code)]
///
/// fn not_quite_good_code() { }
/// ```
///
/// Use instead:
/// ```rust
/// // Good (as inner attribute)
/// #![allow(dead_code)]
///
/// fn this_is_fine() { }
///
/// // Bad
/// #[allow(dead_code)]
///
/// fn not_quite_good_code() { }
/// // or
///
/// // Good (as outer attribute)
/// #[allow(dead_code)]
Expand All @@ -175,12 +180,11 @@ declare_clippy_lint! {
/// These lints should only be enabled on a lint-by-lint basis and with careful consideration.
///
/// ### Example
/// Bad:
/// ```rust
/// #![deny(clippy::restriction)]
/// ```
///
/// Good:
/// Use instead:
/// ```rust
/// #![deny(clippy::as_conversions)]
/// ```
Expand All @@ -205,13 +209,12 @@ declare_clippy_lint! {
/// [#3123](https://github.com/rust-lang/rust-clippy/pull/3123#issuecomment-422321765)
///
/// ### Example
/// Bad:
/// ```rust
/// #[cfg_attr(rustfmt, rustfmt_skip)]
/// fn main() { }
/// ```
///
/// Good:
/// Use instead:
/// ```rust
/// #[rustfmt::skip]
/// fn main() { }
Expand All @@ -231,19 +234,19 @@ declare_clippy_lint! {
/// by the conditional compilation engine.
///
/// ### Example
/// Bad:
/// ```rust
/// #[cfg(linux)]
/// fn conditional() { }
/// ```
///
/// Good:
/// Use instead:
/// ```rust
/// #[cfg(target_os = "linux")]
/// fn conditional() { }
/// ```
///
/// Or:
/// or
///
/// ```rust
/// #[cfg(unix)]
/// fn conditional() { }
Expand All @@ -266,14 +269,13 @@ declare_clippy_lint! {
/// ensure that others understand the reasoning
///
/// ### Example
/// Bad:
/// ```rust
/// #![feature(lint_reasons)]
///
/// #![allow(clippy::some_lint)]
/// ```
///
/// Good:
/// Use instead:
/// ```rust
/// #![feature(lint_reasons)]
///
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/blocks_in_if_conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ declare_clippy_lint! {
/// if true { /* ... */ }
/// ```
///
/// // or
/// or
Copy link
Contributor

@xFrednet xFrednet May 31, 2022

Choose a reason for hiding this comment

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

In cases with or we might want to consider removing the // bad comment and replacing the // good with // use instead instead. What do you think? And would you mind updating that in another commit?

Thinking about this a bit more, I think it's good to remove the "// bad" comments, as it can be seen as degrading the actual code. But I might also be over thinking this a bit too much 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, we could also use multiple ### Example headlines, but I believe that that would look weird 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to fix this one, I just noticed the misplaced comment 😄. I think "or" should be removed entirely and all "bad" snippets should go in the same example, and all of the "good" ones in the Use instead section:

Examples

if { true } { /* ... */ }

if { let x = somefunc(); x } { /* ... */ }

Use instead:

if true { /* ... */ }

let res = { let x = somefunc(); x };
if res { /* ... */ }

I think that's a lot cleaner, what do you think?

Copy link
Contributor

@xFrednet xFrednet Jun 1, 2022

Choose a reason for hiding this comment

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

In this example, I agree 👍 . There might be some cases where this could be overwhelming, though. Could you review the other "// or" cases as well? If it's the same story, then we can just remove the "// bad" comments and separate the examples into the two blocks. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the other occurrences look alright if they were to be combined. It doesn't seem to be that common of a pattern, and when it's used the examples are pretty short.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perfect. Can you combine them in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Can I replace the // Good/// Bad comments in another PR? There are too many to do at once 😄.

///
/// ```rust
/// # fn somefunc() -> bool { true };
Expand Down
19 changes: 15 additions & 4 deletions clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,14 @@ declare_clippy_lint! {
///
/// ### Example
/// ```ignore
/// if a && true // should be: if a
/// if !(a == b) // should be: if a != b
/// if a && true {}
/// if !(a == b) {}
/// ```
///
/// Use instead:
/// ```rust,ignore
/// if a {}
/// if a != b {}
/// ```
#[clippy::version = "pre 1.29.0"]
pub NONMINIMAL_BOOL,
Expand All @@ -48,10 +54,15 @@ declare_clippy_lint! {
/// Ignores short circuiting behavior.
///
/// ### Example
/// ```ignore
/// ```rust,ignore
/// // The `b` is unnecessary, the expression is equivalent to `if a`.
/// if a && b || a { ... }
/// ```
/// The `b` is unnecessary, the expression is equivalent to `if a`.
///
/// Use instead:
/// ```rust,ignore
/// if a {}
/// ```
#[clippy::version = "pre 1.29.0"]
pub LOGIC_BUG,
correctness,
Expand Down
8 changes: 7 additions & 1 deletion clippy_lints/src/bytecount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ declare_clippy_lint! {
/// ### Example
/// ```rust
/// # let vec = vec![1_u8];
/// &vec.iter().filter(|x| **x == 0u8).count(); // use bytecount::count instead
/// let count = vec.iter().filter(|x| **x == 0u8).count();
/// ```
///
/// Use instead:
/// ```rust,ignore
/// # let vec = vec![1_u8];
/// let count = bytecount::count(&vec, 0u8);
/// ```
#[clippy::version = "pre 1.29.0"]
pub NAIVE_BYTECOUNT,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/cognitive_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ declare_clippy_lint! {
/// complexity.
///
/// ### Example
/// No. You'll see it when you get the warning.
/// You'll see it when you get the warning.
#[clippy::version = "1.35.0"]
pub COGNITIVE_COMPLEXITY,
nursery,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ declare_clippy_lint! {
///
/// ```
///
/// Should be written:
/// Use instead:
///
/// ```rust,ignore
/// if x && y {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/comparison_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ declare_clippy_lint! {
/// }
/// ```
///
/// Could be written:
/// Use instead:
///
/// ```rust,ignore
/// use std::cmp::Ordering;
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ declare_clippy_lint! {
/// };
/// ```
///
/// Could be written as:
/// Use instead:
/// ```ignore
/// println!("Hello World");
/// let foo = if … {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/derivable_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ declare_clippy_lint! {
/// bar: bool
/// }
///
/// impl std::default::Default for Foo {
/// impl Default for Foo {
/// fn default() -> Self {
/// Self {
/// bar: false
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/double_comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ declare_clippy_lint! {
/// if x == y || x < y {}
/// ```
///
/// Could be written as:
/// Use instead:
///
/// ```rust
/// # let x = 1;
Expand Down
18 changes: 10 additions & 8 deletions clippy_lints/src/duration_subsec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@ declare_clippy_lint! {
/// ### Example
/// ```rust
/// # use std::time::Duration;
/// let dur = Duration::new(5, 0);
///
/// // Bad
/// let _micros = dur.subsec_nanos() / 1_000;
/// let _millis = dur.subsec_nanos() / 1_000_000;
/// # let duration = Duration::new(5, 0);
/// let micros = duration.subsec_nanos() / 1_000;
/// let millis = duration.subsec_nanos() / 1_000_000;
/// ```
///
/// // Good
/// let _micros = dur.subsec_micros();
/// let _millis = dur.subsec_millis();
/// Use instead:
/// ```rust
/// # use std::time::Duration;
/// # let duration = Duration::new(5, 0);
/// let micros = duration.subsec_micros();
/// let millis = duration.subsec_millis();
/// ```
#[clippy::version = "pre 1.29.0"]
pub DURATION_SUBSEC,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/else_if_without_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ declare_clippy_lint! {
/// }
/// ```
///
/// Could be written:
/// Use instead:
///
/// ```rust
/// # fn a() {}
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/empty_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,11 @@ declare_clippy_lint! {
///
///
/// ### Example
/// Bad:
/// ```rust
/// enum Test {}
/// ```
///
/// Good:
/// Use instead:
/// ```rust
/// #![feature(never_type)]
///
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ declare_clippy_lint! {
/// map.insert(k, v);
/// }
/// ```
/// can both be rewritten as:
/// Use instead:
/// ```rust
/// # use std::collections::HashMap;
/// # let mut map = HashMap::new();
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/enum_variants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ declare_clippy_lint! {
/// BattenbergCake,
/// }
/// ```
/// Could be written as:
/// Use instead:
/// ```rust
/// enum Cake {
/// BlackForest,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/equatable_if_let.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ declare_clippy_lint! {
/// do_thing();
/// }
/// ```
/// Should be written
/// Use instead:
/// ```rust,ignore
/// if x == Some(2) {
/// do_thing();
Expand Down
6 changes: 4 additions & 2 deletions clippy_lints/src/escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,14 @@ declare_clippy_lint! {
/// ### Example
/// ```rust
/// # fn foo(bar: usize) {}
/// // Bad
/// let x = Box::new(1);
/// foo(*x);
/// println!("{}", *x);
/// ```
///
/// // Good
/// Use instead:
/// ```rust
/// # fn foo(bar: usize) {}
/// let x = 1;
/// foo(x);
/// println!("{}", x);
Expand Down
3 changes: 1 addition & 2 deletions clippy_lints/src/excessive_bools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ declare_clippy_lint! {
/// readability and API.
///
/// ### Example
/// Bad:
/// ```rust
/// struct S {
/// is_pending: bool,
Expand All @@ -27,7 +26,7 @@ declare_clippy_lint! {
/// }
/// ```
///
/// Good:
/// Use instead:
/// ```rust
/// enum S {
/// Pending,
Expand Down
10 changes: 9 additions & 1 deletion clippy_lints/src/explicit_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,16 @@ declare_clippy_lint! {
/// ```rust
/// # use std::io::Write;
/// # let bar = "furchtbar";
/// // this would be clearer as `eprintln!("foo: {:?}", bar);`
/// writeln!(&mut std::io::stderr(), "foo: {:?}", bar).unwrap();
/// writeln!(&mut std::io::stdout(), "foo: {:?}", bar).unwrap();
/// ```
///
/// Use instead:
/// ```rust
/// # use std::io::Write;
/// # let bar = "furchtbar";
/// eprintln!("foo: {:?}", bar);
/// println!("foo: {:?}", bar);
/// ```
#[clippy::version = "pre 1.29.0"]
pub EXPLICIT_WRITE,
Expand Down
Loading