Auto merge of #3686 - flip1995:rollup, r=flip1995

Rollup of 4 pull requests

Successful merges:

 - #3582 (Add assert(true) and assert(false) lints)
 - #3679 (Fix automatic suggestion on `use_self`.)
 - #3684 ("make_return" and "blockify" convenience methods, fixes #3683)
 - #3685 (Rustup)

Failed merges:

r? @ghost

CHANGELOG.md

@@ -616,6 +616,7 @@ All notable changes to this project will be documented in this file.
 [`absurd_extreme_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons
 [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped
 [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
+[`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
 [`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
 [`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops
 [`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask

README.md

@@ -7,7 +7,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 291 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 292 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

clippy_lints/src/arithmetic.rs

@@ -128,7 +128,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Arithmetic {
                 }
                 self.const_span = Some(body_span);
             },
-            hir::BodyOwnerKind::Fn => (),
+            hir::BodyOwnerKind::Fn | hir::BodyOwnerKind::Closure => (),
         }
     }
 

clippy_lints/src/assertions_on_constants.rs

@@ -0,0 +1,87 @@
+// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use crate::consts::{constant, Constant};
+use crate::rustc::hir::{Expr, ExprKind};
+use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use crate::rustc::{declare_tool_lint, lint_array};
+use crate::syntax::ast::LitKind;
+use crate::utils::{is_direct_expn_of, span_help_and_lint};
+use if_chain::if_chain;
+
+/// **What it does:** Check to call assert!(true/false)
+///
+/// **Why is this bad?** Will be optimized out by the compiler or should probably be replaced by a
+/// panic!() or unreachable!()
+///
+/// **Known problems:** None
+///
+/// **Example:**
+/// ```rust
+/// assert!(false)
+/// // or
+/// assert!(true)
+/// // or
+/// const B: bool = false;
+/// assert!(B)
+/// ```
+declare_clippy_lint! {
+    pub ASSERTIONS_ON_CONSTANTS,
+    style,
+    "assert!(true/false) will be optimized out by the compiler/should probably be replaced by a panic!() or unreachable!()"
+}
+
+pub struct AssertionsOnConstants;
+
+impl LintPass for AssertionsOnConstants {
+    fn get_lints(&self) -> LintArray {
+        lint_array![ASSERTIONS_ON_CONSTANTS]
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
+    fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
+        if_chain! {
+            if is_direct_expn_of(e.span, "assert").is_some();
+            if let ExprKind::Unary(_, ref lit) = e.node;
+            then {
+                if let ExprKind::Lit(ref inner) = lit.node {
+                    match inner.node {
+                        LitKind::Bool(true) => {
+                            span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span,
+                                "assert!(true) will be optimized out by the compiler",
+                                "remove it");
+                        },
+                        LitKind::Bool(false) => {
+                            span_help_and_lint(
+                                cx, ASSERTIONS_ON_CONSTANTS, e.span,
+                                "assert!(false) should probably be replaced",
+                                "use panic!() or unreachable!()");
+                        },
+                        _ => (),
+                    }
+                } else if let Some(bool_const) = constant(cx, cx.tables, lit) {
+                    match bool_const.0 {
+                        Constant::Bool(true) => {
+                            span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span,
+                                "assert!(const: true) will be optimized out by the compiler",
+                                "remove it");
+                        },
+                        Constant::Bool(false) => {
+                            span_help_and_lint(cx, ASSERTIONS_ON_CONSTANTS, e.span,
+                                "assert!(const: false) should probably be replaced",
+                                "use panic!() or unreachable!()");
+                        },
+                        _ => (),
+                    }
+                }
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -78,6 +78,7 @@ mod utils;
 // begin lints modules, do not remove this comment, it’s used in `update_lints`
 pub mod approx_const;
 pub mod arithmetic;
+pub mod assertions_on_constants;
 pub mod assign_ops;
 pub mod attrs;
 pub mod bit_mask;
@@ -477,6 +478,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box redundant_clone::RedundantClone);
     reg.register_late_lint_pass(box slow_vector_initialization::Pass);
     reg.register_late_lint_pass(box types::RefToMut);
+    reg.register_late_lint_pass(box assertions_on_constants::AssertionsOnConstants);
 
     reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -554,6 +556,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
 
     reg.register_lint_group("clippy::all", Some("clippy"), vec![
         approx_const::APPROX_CONSTANT,
+        assertions_on_constants::ASSERTIONS_ON_CONSTANTS,
         assign_ops::ASSIGN_OP_PATTERN,
         assign_ops::MISREFACTORED_ASSIGN_OP,
         attrs::DEPRECATED_CFG_ATTR,
@@ -776,6 +779,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     ]);
 
     reg.register_lint_group("clippy::style", Some("clippy_style"), vec![
+        assertions_on_constants::ASSERTIONS_ON_CONSTANTS,
         assign_ops::ASSIGN_OP_PATTERN,
         attrs::UNKNOWN_CLIPPY_LINTS,
         bit_mask::VERBOSE_BIT_MASK,

clippy_lints/src/needless_bool.rs

@@ -70,16 +70,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool {
             let reduce = |ret, not| {
                 let mut applicability = Applicability::MachineApplicable;
                 let snip = Sugg::hir_with_applicability(cx, pred, "<predicate>", &mut applicability);
-                let snip = if not { !snip } else { snip };
+                let mut snip = if not { !snip } else { snip };
 
-                let mut hint = if ret {
-                    format!("return {}", snip)
-                } else {
-                    snip.to_string()
-                };
+                if ret {
+                    snip = snip.make_return();
+                }
 
                 if parent_node_is_if_expr(&e, &cx) {
-                    hint = format!("{{ {} }}", hint);
+                    snip = snip.blockify()
                 }
 
                 span_lint_and_sugg(
@@ -88,7 +86,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBool {
                     e.span,
                     "this if-then-else expression returns a bool literal",
                     "you can reduce it to",
-                    hint,
+                    snip.to_string(),
                     applicability,
                 );
             };

clippy_lints/src/use_self.rs

@@ -56,10 +56,15 @@ impl LintPass for UseSelf {
 const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
 
 fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path) {
+    // path segments only include actual path, no methods or fields
+    let last_path_span = path.segments.last().expect(SEGMENTS_MSG).ident.span;
+    // only take path up to the end of last_path_span
+    let span = path.span.with_hi(last_path_span.hi());
+
     span_lint_and_sugg(
         cx,
         USE_SELF,
-        path.span,
+        span,
         "unnecessary structure name repetition",
         "use the applicable keyword",
         "Self".to_owned(),

clippy_lints/src/utils/mod.rs

@@ -64,9 +64,25 @@ pub fn differing_macro_contexts(lhs: Span, rhs: Span) -> bool {
 /// ```
 pub fn in_constant(cx: &LateContext<'_, '_>, id: NodeId) -> bool {
     let parent_id = cx.tcx.hir().get_parent(id);
-    match cx.tcx.hir().body_owner_kind(parent_id) {
-        hir::BodyOwnerKind::Fn => false,
-        hir::BodyOwnerKind::Const | hir::BodyOwnerKind::Static(..) => true,
+    match cx.tcx.hir().get(parent_id) {
+        Node::Item(&Item {
+            node: ItemKind::Const(..),
+            ..
+        })
+        | Node::TraitItem(&TraitItem {
+            node: TraitItemKind::Const(..),
+            ..
+        })
+        | Node::ImplItem(&ImplItem {
+            node: ImplItemKind::Const(..),
+            ..
+        })
+        | Node::AnonConst(_)
+        | Node::Item(&Item {
+            node: ItemKind::Static(..),
+            ..
+        }) => true,
+        _ => false,
     }
 }
 

clippy_lints/src/utils/sugg.rs

@@ -206,6 +206,17 @@ impl<'a> Sugg<'a> {
         make_unop("&mut *", self)
     }
 
+    /// Convenience method to transform suggestion into a return call
+    pub fn make_return(self) -> Sugg<'static> {
+        Sugg::NonParen(Cow::Owned(format!("return {}", self)))
+    }
+
+    /// Convenience method to transform suggestion into a block
+    /// where the suggestion is a trailing expression
+    pub fn blockify(self) -> Sugg<'static> {
+        Sugg::NonParen(Cow::Owned(format!("{{ {} }}", self)))
+    }
+
     /// Convenience method to create the `<lhs>..<rhs>` or `<lhs>...<rhs>`
     /// suggestion.
     #[allow(dead_code)]
@@ -578,3 +589,21 @@ impl<'a, 'b, 'c, T: LintContext<'c>> DiagnosticBuilderExt<'c, T> for rustc_error
         self.span_suggestion_with_applicability(remove_span, msg, String::new(), applicability);
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::Sugg;
+    use std::borrow::Cow;
+
+    const SUGGESTION: Sugg<'static> = Sugg::NonParen(Cow::Borrowed("function_call()"));
+
+    #[test]
+    fn make_return_transform_sugg_into_a_return_call() {
+        assert_eq!("return function_call()", SUGGESTION.make_return().to_string());
+    }
+
+    #[test]
+    fn blockify_transforms_sugg_into_a_block() {
+        assert_eq!("{ function_call() }", SUGGESTION.blockify().to_string());
+    }
+}

tests/ui/assertions_on_constants.rs

@@ -0,0 +1,21 @@
+// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn main() {
+    assert!(true);
+    assert!(false);
+    assert!(true, "true message");
+    assert!(false, "false message");
+
+    const B: bool = true;
+    assert!(B);
+
+    const C: bool = false;
+    assert!(C);
+}

tests/ui/assertions_on_constants.stderr

@@ -0,0 +1,51 @@
+error: assert!(true) will be optimized out by the compiler
+  --> $DIR/assertions_on_constants.rs:11:5
+   |
+LL |     assert!(true);
+   |     ^^^^^^^^^^^^^^
+   |
+   = note: `-D clippy::assertions-on-constants` implied by `-D warnings`
+   = help: remove it
+
+error: assert!(false) should probably be replaced
+  --> $DIR/assertions_on_constants.rs:12:5
+   |
+LL |     assert!(false);
+   |     ^^^^^^^^^^^^^^^
+   |
+   = help: use panic!() or unreachable!()
+
+error: assert!(true) will be optimized out by the compiler
+  --> $DIR/assertions_on_constants.rs:13:5
+   |
+LL |     assert!(true, "true message");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: remove it
+
+error: assert!(false) should probably be replaced
+  --> $DIR/assertions_on_constants.rs:14:5
+   |
+LL |     assert!(false, "false message");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: use panic!() or unreachable!()
+
+error: assert!(const: true) will be optimized out by the compiler
+  --> $DIR/assertions_on_constants.rs:17:5
+   |
+LL |     assert!(B);
+   |     ^^^^^^^^^^^
+   |
+   = help: remove it
+
+error: assert!(const: false) should probably be replaced
+  --> $DIR/assertions_on_constants.rs:20:5
+   |
+LL |     assert!(C);
+   |     ^^^^^^^^^^^
+   |
+   = help: use panic!() or unreachable!()
+
+error: aborting due to 6 previous errors
+

tests/ui/attrs.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::inline_always, clippy::deprecated_semver)]
-
+#![allow(clippy::assertions_on_constants::assertions_on_constants)]
 #[inline(always)]
 fn test_attr_lint() {
     assert!(true)

tests/ui/empty_line_after_outer_attribute.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::empty_line_after_outer_attr)]
-
+#![allow(clippy::assertions_on_constants::assertions_on_constants)]
 // This should produce a warning
 #[crate_type = "lib"]
 

tests/ui/panic_unimplemented.rs

@@ -1,5 +1,5 @@
 #![warn(clippy::panic_params, clippy::unimplemented)]
-
+#![allow(clippy::assertions_on_constants::assertions_on_constants)]
 fn missing() {
     if true {
         panic!("{}");