From c58a5c6272f1846b0c5fd7726970183dcccdfe59 Mon Sep 17 00:00:00 2001 From: Johannes Schilling Date: Tue, 19 Nov 2019 20:03:41 +0100 Subject: [PATCH 1/4] Add suggested good cases in docs for lifetimes lint --- clippy_lints/src/lifetimes.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index fad54c3de659..fe8938c581fa 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -24,9 +24,15 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad: unnecessary lifetime annotations /// fn in_and_out<'a>(x: &'a u8, y: u8) -> &'a u8 { /// x /// } + /// + /// // Good + /// fn elided(x: &u8, y: u8) -> &u8 { + /// x + /// } /// ``` pub NEEDLESS_LIFETIMES, complexity, @@ -46,9 +52,15 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad: unnecessary lifetimes /// fn unused_lifetime<'a>(x: u8) { /// // .. /// } + /// + /// // Good + /// fn no_lifetime(x: u8) { + /// // ... + /// } /// ``` pub EXTRA_UNUSED_LIFETIMES, complexity, From c6e6b292bd194660bbc42fa60447e0d35c46004b Mon Sep 17 00:00:00 2001 From: Johannes Schilling Date: Tue, 19 Nov 2019 20:14:09 +0100 Subject: [PATCH 2/4] add a good example for the approx_const lint --- clippy_lints/src/approx_const.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clippy_lints/src/approx_const.rs b/clippy_lints/src/approx_const.rs index fac75cffeba6..9f665a3361da 100644 --- a/clippy_lints/src/approx_const.rs +++ b/clippy_lints/src/approx_const.rs @@ -23,7 +23,13 @@ declare_clippy_lint! { /// /// **Example:** /// ```rust + /// // Bad /// let x = 3.14; + /// let y = 1_f64 / x; + /// + /// // Good + /// let x = std::f32::consts::PI; + /// let y = std::f64::consts::FRAC_1_PI; /// ``` pub APPROX_CONSTANT, correctness, From fff9a8ea9c56d5075c23d9d5b2d8b0838eee0da3 Mon Sep 17 00:00:00 2001 From: Tim Bodeit Date: Sat, 23 Nov 2019 22:35:21 +0100 Subject: [PATCH 3/4] [comparison_chain] #4827 Check `core::cmp::Ord` is implemented Only emit lint, if `cmp` is actually available on the type being compared. Don't emit lint in cases where only `PartialOrd` is implemented. --- clippy_lints/src/comparison_chain.rs | 12 +++++- tests/ui/comparison_chain.rs | 61 ++++++++++++++++++++++++++++ tests/ui/comparison_chain.stderr | 42 ++++++++++++++++++- 3 files changed, 113 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/comparison_chain.rs b/clippy_lints/src/comparison_chain.rs index 6968d8f65599..087bceaffd98 100644 --- a/clippy_lints/src/comparison_chain.rs +++ b/clippy_lints/src/comparison_chain.rs @@ -1,4 +1,6 @@ -use crate::utils::{if_sequence, parent_node_is_if_expr, span_help_and_lint, SpanlessEq}; +use crate::utils::{ + get_trait_def_id, if_sequence, implements_trait, parent_node_is_if_expr, paths, span_help_and_lint, SpanlessEq, +}; use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::{declare_lint_pass, declare_tool_lint}; @@ -84,6 +86,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ComparisonChain { { return; } + + // Check that the type being compared implements `core::cmp::Ord` + let ty = cx.tables.expr_ty(lhs1); + let is_ord = get_trait_def_id(cx, &paths::ORD).map_or(false, |id| implements_trait(cx, ty, id, &[])); + + if !is_ord { + return; + } } else { // We only care about comparison chains return; diff --git a/tests/ui/comparison_chain.rs b/tests/ui/comparison_chain.rs index b697413b6e03..9c2128469de9 100644 --- a/tests/ui/comparison_chain.rs +++ b/tests/ui/comparison_chain.rs @@ -76,4 +76,65 @@ fn f(x: u8, y: u8, z: u8) { } } +#[allow(clippy::float_cmp)] +fn g(x: f64, y: f64, z: f64) { + // Ignored: f64 doesn't implement Ord + if x > y { + a() + } else if x < y { + b() + } + + // Ignored: f64 doesn't implement Ord + if x > y { + a() + } else if x < y { + b() + } else { + c() + } + + // Ignored: f64 doesn't implement Ord + if x > y { + a() + } else if y > x { + b() + } else { + c() + } + + // Ignored: f64 doesn't implement Ord + if x > 1.0 { + a() + } else if x < 1.0 { + b() + } else if x == 1.0 { + c() + } +} + +fn h(x: T, y: T, z: T) { + if x > y { + a() + } else if x < y { + b() + } + + if x > y { + a() + } else if x < y { + b() + } else { + c() + } + + if x > y { + a() + } else if y > x { + b() + } else { + c() + } +} + fn main() {} diff --git a/tests/ui/comparison_chain.stderr b/tests/ui/comparison_chain.stderr index 575181dd7194..69db88b03b5b 100644 --- a/tests/ui/comparison_chain.stderr +++ b/tests/ui/comparison_chain.stderr @@ -53,5 +53,45 @@ LL | | } | = help: Consider rewriting the `if` chain to use `cmp` and `match`. -error: aborting due to 4 previous errors +error: `if` chain can be rewritten with `match` + --> $DIR/comparison_chain.rs:117:5 + | +LL | / if x > y { +LL | | a() +LL | | } else if x < y { +LL | | b() +LL | | } + | |_____^ + | + = help: Consider rewriting the `if` chain to use `cmp` and `match`. + +error: `if` chain can be rewritten with `match` + --> $DIR/comparison_chain.rs:123:5 + | +LL | / if x > y { +LL | | a() +LL | | } else if x < y { +LL | | b() +LL | | } else { +LL | | c() +LL | | } + | |_____^ + | + = help: Consider rewriting the `if` chain to use `cmp` and `match`. + +error: `if` chain can be rewritten with `match` + --> $DIR/comparison_chain.rs:131:5 + | +LL | / if x > y { +LL | | a() +LL | | } else if y > x { +LL | | b() +LL | | } else { +LL | | c() +LL | | } + | |_____^ + | + = help: Consider rewriting the `if` chain to use `cmp` and `match`. + +error: aborting due to 7 previous errors From 47ef5394dea149f0a76d97a4b11f8f1a12673f9c Mon Sep 17 00:00:00 2001 From: Andre Bogus Date: Mon, 25 Nov 2019 14:06:34 +0100 Subject: [PATCH 4/4] fixing a typo --- clippy_lints/src/methods/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 78188527cbac..2cf5835e697f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1075,7 +1075,7 @@ declare_clippy_lint! { /// /// **Example:** /// ```ignore - /// unsafe { (&() as *const ()).offest(1) }; + /// unsafe { (&() as *const ()).offset(1) }; /// ``` pub ZST_OFFSET, correctness,