Auto merge of #4855 - phansch:rollup-x7yail7, r=phansch

Rollup of 3 pull requests Successful merges: - #4832 (Add some positive examples to lint docs) - #4842 ([comparison_chain] #4827 Check `core::cmp::Ord` is implemented) - #4847 (fixing a typo) Failed merges: changelog: none r? @ghost
rust-lang · Nov 28, 2019 · dbdd75a · dbdd75a
2 parents f3288eb + a05f3cb
commit dbdd75a
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 3 deletions.
diff --git 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,

diff --git 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/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,

diff --git 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,

diff --git 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<T: Ord>(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
@@ -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