Merge branch 'master' into issue-4078

Author: EthanTheMaster

CHANGELOG.md

@@ -1063,6 +1063,7 @@ Released 2018-09-13
 [`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug
 [`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion
 [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
+[`manual_mul_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_mul_add
 [`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
 [`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
 [`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
@@ -1191,6 +1192,7 @@ Released 2018-09-13
 [`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting
 [`suspicious_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_map
 [`suspicious_op_assign_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_op_assign_impl
+[`suspicious_unary_op_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_unary_op_formatting
 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments

Cargo.toml

@@ -11,7 +11,7 @@ authors = [
 description = "A bunch of helpful lints to avoid common pitfalls in Rust"
 repository = "https://github.com/rust-lang/rust-clippy"
 readme = "README.md"
-license = "MIT/Apache-2.0"
+license = "MIT OR Apache-2.0"
 keywords = ["clippy", "lint", "plugin"]
 categories = ["development-tools", "development-tools::cargo-plugins"]
 build = "build.rs"

README.md

@@ -2,11 +2,11 @@
 
 [![Build Status](https://travis-ci.com/rust-lang/rust-clippy.svg?branch=master)](https://travis-ci.com/rust-lang/rust-clippy)
 [![Windows Build status](https://ci.appveyor.com/api/projects/status/id677xpw1dguo7iw?svg=true)](https://ci.appveyor.com/project/rust-lang-libs/rust-clippy)
-[![License: MIT/Apache-2.0](https://img.shields.io/crates/l/clippy.svg)](#license)
+[![License: MIT OR Apache-2.0](https://img.shields.io/crates/l/clippy.svg)](#license)
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 320 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 322 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_dummy/Cargo.toml

@@ -9,7 +9,7 @@ build = 'build.rs'
 
 repository = "https://github.com/rust-lang/rust-clippy"
 
-license = "MIT/Apache-2.0"
+license = "MIT OR Apache-2.0"
 keywords = ["clippy", "lint", "plugin"]
 categories = ["development-tools", "development-tools::cargo-plugins"]
 

clippy_lints/Cargo.toml

@@ -12,7 +12,7 @@ authors = [
 description = "A bunch of helpful lints to avoid common pitfalls in Rust"
 repository = "https://github.com/rust-lang/rust-clippy"
 readme = "README.md"
-license = "MPL-2.0"
+license = "MIT OR Apache-2.0"
 keywords = ["clippy", "lint", "plugin"]
 edition = "2018"
 

clippy_lints/src/assertions_on_constants.rs

@@ -1,9 +1,11 @@
-use rustc::hir::{Expr, ExprKind};
+use crate::consts::{constant, Constant};
+use crate::utils::paths;
+use crate::utils::{is_direct_expn_of, is_expn_of, match_def_path, snippet_opt, span_help_and_lint};
+use if_chain::if_chain;
+use rustc::hir::*;
 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::{declare_lint_pass, declare_tool_lint};
-
-use crate::consts::{constant, Constant};
-use crate::utils::{is_direct_expn_of, is_expn_of, span_help_and_lint};
+use syntax::ast::LitKind;
 
 declare_clippy_lint! {
     /// **What it does:** Checks for `assert!(true)` and `assert!(false)` calls.
@@ -31,39 +33,135 @@ declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
-        let lint_assert_cb = |is_debug_assert: bool| {
-            if let ExprKind::Unary(_, ref lit) = e.kind {
-                if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.tables, lit) {
-                    if is_true {
-                        span_help_and_lint(
-                            cx,
-                            ASSERTIONS_ON_CONSTANTS,
-                            e.span,
-                            "`assert!(true)` will be optimized out by the compiler",
-                            "remove it",
-                        );
-                    } else if !is_debug_assert {
-                        span_help_and_lint(
-                            cx,
-                            ASSERTIONS_ON_CONSTANTS,
-                            e.span,
-                            "`assert!(false)` should probably be replaced",
-                            "use `panic!()` or `unreachable!()`",
-                        );
-                    }
-                }
-            }
+        let lint_true = || {
+            span_help_and_lint(
+                cx,
+                ASSERTIONS_ON_CONSTANTS,
+                e.span,
+                "`assert!(true)` will be optimized out by the compiler",
+                "remove it",
+            );
         };
+        let lint_false_without_message = || {
+            span_help_and_lint(
+                cx,
+                ASSERTIONS_ON_CONSTANTS,
+                e.span,
+                "`assert!(false)` should probably be replaced",
+                "use `panic!()` or `unreachable!()`",
+            );
+        };
+        let lint_false_with_message = |panic_message: String| {
+            span_help_and_lint(
+                cx,
+                ASSERTIONS_ON_CONSTANTS,
+                e.span,
+                &format!("`assert!(false, {})` should probably be replaced", panic_message),
+                &format!("use `panic!({})` or `unreachable!({})`", panic_message, panic_message),
+            )
+        };
+
         if let Some(debug_assert_span) = is_expn_of(e.span, "debug_assert") {
             if debug_assert_span.from_expansion() {
                 return;
             }
-            lint_assert_cb(true);
+            if_chain! {
+                if let ExprKind::Unary(_, ref lit) = e.kind;
+                if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.tables, lit);
+                if is_true;
+                then {
+                    lint_true();
+                }
+            };
         } else if let Some(assert_span) = is_direct_expn_of(e.span, "assert") {
             if assert_span.from_expansion() {
                 return;
             }
-            lint_assert_cb(false);
+            if let Some(assert_match) = match_assert_with_message(&cx, e) {
+                match assert_match {
+                    // matched assert but not message
+                    AssertKind::WithoutMessage(false) => lint_false_without_message(),
+                    AssertKind::WithoutMessage(true) | AssertKind::WithMessage(_, true) => lint_true(),
+                    AssertKind::WithMessage(panic_message, false) => lint_false_with_message(panic_message),
+                };
+            }
         }
     }
 }
+
+/// Result of calling `match_assert_with_message`.
+enum AssertKind {
+    WithMessage(String, bool),
+    WithoutMessage(bool),
+}
+
+/// Check if the expression matches
+///
+/// ```rust,ignore
+/// match { let _t = !c; _t } {
+///     true => {
+///         {
+///             ::std::rt::begin_panic(message, _)
+///         }
+///     }
+///     _ => { }
+/// };
+/// ```
+///
+/// where `message` is any expression and `c` is a constant bool.
+fn match_assert_with_message<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option<AssertKind> {
+    if_chain! {
+        if let ExprKind::Match(ref expr, ref arms, _) = expr.kind;
+        // matches { let _t = expr; _t }
+        if let ExprKind::DropTemps(ref expr) = expr.kind;
+        if let ExprKind::Unary(UnOp::UnNot, ref expr) = expr.kind;
+        // bind the first argument of the `assert!` macro
+        if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.tables, expr);
+        // arm 1 pattern
+        if let PatKind::Lit(ref lit_expr) = arms[0].pat.kind;
+        if let ExprKind::Lit(ref lit) = lit_expr.kind;
+        if let LitKind::Bool(true) = lit.node;
+        // arm 1 block
+        if let ExprKind::Block(ref block, _) = arms[0].body.kind;
+        if block.stmts.len() == 0;
+        if let Some(block_expr) = &block.expr;
+        if let ExprKind::Block(ref inner_block, _) = block_expr.kind;
+        if let Some(begin_panic_call) = &inner_block.expr;
+        // function call
+        if let Some(args) = match_function_call(cx, begin_panic_call, &paths::BEGIN_PANIC);
+        if args.len() == 2;
+        // bind the second argument of the `assert!` macro if it exists
+        if let panic_message = snippet_opt(cx, args[0].span);
+        // second argument of begin_panic is irrelevant
+        // as is the second match arm
+        then {
+            // an empty message occurs when it was generated by the macro
+            // (and not passed by the user)
+            return panic_message
+                .filter(|msg| !msg.is_empty())
+                .map(|msg| AssertKind::WithMessage(msg, is_true))
+                .or(Some(AssertKind::WithoutMessage(is_true)));
+        }
+    }
+    None
+}
+
+/// Matches a function call with the given path and returns the arguments.
+///
+/// Usage:
+///
+/// ```rust,ignore
+/// if let Some(args) = match_function_call(cx, begin_panic_call, &paths::BEGIN_PANIC);
+/// ```
+fn match_function_call<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, path: &[&str]) -> Option<&'a [Expr]> {
+    if_chain! {
+        if let ExprKind::Call(ref fun, ref args) = expr.kind;
+        if let ExprKind::Path(ref qpath) = fun.kind;
+        if let Some(fun_def_id) = cx.tables.qpath_res(qpath, fun.hir_id).opt_def_id();
+        if match_def_path(cx, fun_def_id, path);
+        then {
+            return Some(&args)
+        }
+    };
+    None
+}

clippy_lints/src/cargo_common_metadata.rs

@@ -27,7 +27,7 @@ declare_clippy_lint! {
     /// description = "A bunch of helpful lints to avoid common pitfalls in Rust"
     /// repository = "https://github.com/rust-lang/rust-clippy"
     /// readme = "README.md"
-    /// license = "MIT/Apache-2.0"
+    /// license = "MIT OR Apache-2.0"
     /// keywords = ["clippy", "lint", "plugin"]
     /// categories = ["development-tools", "development-tools::cargo-plugins"]
     /// ```

clippy_lints/src/formatting.rs

@@ -1,4 +1,4 @@
-use crate::utils::{differing_macro_contexts, snippet_opt, span_note_and_lint};
+use crate::utils::{differing_macro_contexts, snippet_opt, span_help_and_lint, span_note_and_lint};
 use if_chain::if_chain;
 use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintPass};
 use rustc::{declare_lint_pass, declare_tool_lint};
@@ -22,6 +22,28 @@ declare_clippy_lint! {
     "suspicious formatting of `*=`, `-=` or `!=`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks the formatting of a unary operator on the right hand side
+    /// of a binary operator. It lints if there is no space between the binary and unary operators,
+    /// but there is a space between the unary and its operand.
+    ///
+    /// **Why is this bad?** This is either a typo in the binary operator or confusing.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust,ignore
+    /// if foo <- 30 { // this should be `foo < -30` but looks like a different operator
+    /// }
+    ///
+    /// if foo &&! bar { // this should be `foo && !bar` but looks like a different operator
+    /// }
+    /// ```
+    pub SUSPICIOUS_UNARY_OP_FORMATTING,
+    style,
+    "suspicious formatting of unary `-` or `!` on the RHS of a BinOp"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for formatting of `else`. It lints if the `else`
     /// is followed immediately by a newline or the `else` seems to be missing.
@@ -80,6 +102,7 @@ declare_clippy_lint! {
 
 declare_lint_pass!(Formatting => [
     SUSPICIOUS_ASSIGNMENT_FORMATTING,
+    SUSPICIOUS_UNARY_OP_FORMATTING,
     SUSPICIOUS_ELSE_FORMATTING,
     POSSIBLE_MISSING_COMMA
 ]);
@@ -99,6 +122,7 @@ impl EarlyLintPass for Formatting {
 
     fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
         check_assign(cx, expr);
+        check_unop(cx, expr);
         check_else(cx, expr);
         check_array(cx, expr);
     }
@@ -133,6 +157,45 @@ fn check_assign(cx: &EarlyContext<'_>, expr: &Expr) {
     }
 }
 
+/// Implementation of the `SUSPICIOUS_UNARY_OP_FORMATTING` lint.
+fn check_unop(cx: &EarlyContext<'_>, expr: &Expr) {
+    if_chain! {
+        if let ExprKind::Binary(ref binop, ref lhs, ref rhs) = expr.kind;
+        if !differing_macro_contexts(lhs.span, rhs.span) && !lhs.span.from_expansion();
+        // span between BinOp LHS and RHS
+        let binop_span = lhs.span.between(rhs.span);
+        // if RHS is a UnOp
+        if let ExprKind::Unary(op, ref un_rhs) = rhs.kind;
+        // from UnOp operator to UnOp operand
+        let unop_operand_span = rhs.span.until(un_rhs.span);
+        if let Some(binop_snippet) = snippet_opt(cx, binop_span);
+        if let Some(unop_operand_snippet) = snippet_opt(cx, unop_operand_span);
+        let binop_str = BinOpKind::to_string(&binop.node);
+        // no space after BinOp operator and space after UnOp operator
+        if binop_snippet.ends_with(binop_str) && unop_operand_snippet.ends_with(' ');
+        then {
+            let unop_str = UnOp::to_string(op);
+            let eqop_span = lhs.span.between(un_rhs.span);
+            span_help_and_lint(
+                cx,
+                SUSPICIOUS_UNARY_OP_FORMATTING,
+                eqop_span,
+                &format!(
+                    "by not having a space between `{binop}` and `{unop}` it looks like \
+                     `{binop}{unop}` is a single operator",
+                    binop = binop_str,
+                    unop = unop_str
+                ),
+                &format!(
+                    "put a space between `{binop}` and `{unop}` and remove the space after `{unop}`",
+                    binop = binop_str,
+                    unop = unop_str
+                ),
+            );
+        }
+    }
+}
+
 /// Implementation of the `SUSPICIOUS_ELSE_FORMATTING` lint for weird `else`.
 fn check_else(cx: &EarlyContext<'_>, expr: &Expr) {
     if_chain! {

clippy_lints/src/lib.rs

@@ -224,6 +224,7 @@ pub mod misc_early;
 pub mod missing_const_for_fn;
 pub mod missing_doc;
 pub mod missing_inline;
+pub mod mul_add;
 pub mod multiple_crate_versions;
 pub mod mut_mut;
 pub mod mut_reference;
@@ -604,6 +605,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
     reg.register_late_lint_pass(box inherent_to_string::InherentToString);
     reg.register_late_lint_pass(box trait_bounds::TraitBounds);
     reg.register_late_lint_pass(box comparison_chain::ComparisonChain);
+    reg.register_late_lint_pass(box mul_add::MulAddCheck);
 
     reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -741,6 +743,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         formatting::POSSIBLE_MISSING_COMMA,
         formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
         formatting::SUSPICIOUS_ELSE_FORMATTING,
+        formatting::SUSPICIOUS_UNARY_OP_FORMATTING,
         functions::NOT_UNSAFE_PTR_ARG_DEREF,
         functions::TOO_MANY_ARGUMENTS,
         get_last_with_len::GET_LAST_WITH_LEN,
@@ -837,6 +840,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         misc_early::UNNEEDED_FIELD_PATTERN,
         misc_early::UNNEEDED_WILDCARD_PATTERN,
         misc_early::ZERO_PREFIXED_LITERAL,
+        mul_add::MANUAL_MUL_ADD,
         mut_reference::UNNECESSARY_MUT_PASSED,
         mutex_atomic::MUTEX_ATOMIC,
         needless_bool::BOOL_COMPARISON,
@@ -951,6 +955,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         excessive_precision::EXCESSIVE_PRECISION,
         formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
         formatting::SUSPICIOUS_ELSE_FORMATTING,
+        formatting::SUSPICIOUS_UNARY_OP_FORMATTING,
         infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH,
         inherent_to_string::INHERENT_TO_STRING,
         len_zero::LEN_WITHOUT_IS_EMPTY,
@@ -1175,6 +1180,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         methods::OR_FUN_CALL,
         methods::SINGLE_CHAR_PATTERN,
         misc::CMP_OWNED,
+        mul_add::MANUAL_MUL_ADD,
         mutex_atomic::MUTEX_ATOMIC,
         redundant_clone::REDUNDANT_CLONE,
         slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,