Merge remote-tracking branch 'upstream/master' into rustup

Author: ebroto

.github/driver.sh

@@ -24,16 +24,16 @@ unset CARGO_MANIFEST_DIR
 # FIXME: How to match the clippy invocation in compile-test.rs?
 ./target/debug/clippy-driver -Dwarnings -Aunused -Zui-testing --emit metadata --crate-type bin tests/ui/double_neg.rs 2>double_neg.stderr && exit 1
 sed -e "s,tests/ui,\$DIR," -e "/= help/d" double_neg.stderr >normalized.stderr
-diff normalized.stderr tests/ui/double_neg.stderr
+diff -u normalized.stderr tests/ui/double_neg.stderr
 
 # make sure "clippy-driver --rustc --arg" and "rustc --arg" behave the same
 SYSROOT=$(rustc --print sysroot)
-diff <(LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver --rustc --version --verbose) <(rustc --version --verbose)
+diff -u <(LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver --rustc --version --verbose) <(rustc --version --verbose)
 
 echo "fn main() {}" >target/driver_test.rs
 # we can't run 2 rustcs on the same file at the same time
 CLIPPY=$(LD_LIBRARY_PATH=${SYSROOT}/lib ./target/debug/clippy-driver ./target/driver_test.rs --rustc)
 RUSTC=$(rustc ./target/driver_test.rs)
-diff <($CLIPPY) <($RUSTC)
+diff -u <($CLIPPY) <($RUSTC)
 
 # TODO: CLIPPY_CONF_DIR / CARGO_MANIFEST_DIR

CHANGELOG.md

@@ -1665,6 +1665,7 @@ Released 2018-09-13
 [`cognitive_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#cognitive_complexity
 [`collapsible_if`]: https://rust-lang.github.io/rust-clippy/master/index.html#collapsible_if
 [`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
+[`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
 [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator
 [`create_dir`]: https://rust-lang.github.io/rust-clippy/master/index.html#create_dir
 [`crosspointer_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#crosspointer_transmute
@@ -1802,6 +1803,7 @@ Released 2018-09-13
 [`manual_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_unwrap_or
 [`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
 [`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
+[`map_collect_result_unit`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_collect_result_unit
 [`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
 [`map_err_ignore`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_err_ignore
 [`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten

CONTRIBUTING.md

@@ -63,9 +63,10 @@ To figure out how this syntax structure is encoded in the AST, it is recommended
 Usually the lint will end up to be a nested series of matches and ifs, [like so][deep-nesting].
 But we can make it nest-less by using [if_chain] macro, [like this][nest-less].
 
-[`E-medium`] issues are generally pretty easy too, though it's recommended you work on an E-easy issue first.
-They are mostly classified as [`E-medium`], since they might be somewhat involved code wise,
-but not difficult per-se.
+[`E-medium`] issues are generally pretty easy too, though it's recommended you work on an [`good first issue`]
+first. Sometimes they are only somewhat involved code wise, but not difficult per-se.
+Note that [`E-medium`] issues may require some knowledge of Clippy internals or some 
+debugging to find the actual problem behind the issue. 
 
 [`T-middle`] issues can be more involved and require verifying types. The [`ty`] module contains a
 lot of methods that are useful, though one of the most useful would be `expr_ty` (gives the type of

clippy_dev/src/ra_setup.rs

@@ -11,7 +11,7 @@ use std::path::PathBuf;
 // code. See https://github.com/rust-analyzer/rust-analyzer/issues/3517 and https://github.com/rust-lang/rust-clippy/issues/5514 for details
 
 pub fn run(rustc_path: Option<&str>) {
-    // we can unwrap here because the arg is required here
+    // we can unwrap here because the arg is required by clap
     let rustc_path = PathBuf::from(rustc_path.unwrap());
     assert!(rustc_path.is_dir(), "path is not a directory");
     let rustc_source_basedir = rustc_path.join("compiler");
@@ -49,6 +49,15 @@ fn inject_deps_into_manifest(
     cargo_toml: &str,
     lib_rs: &str,
 ) -> std::io::Result<()> {
+    // do not inject deps if we have aleady done so
+    if cargo_toml.contains("[target.'cfg(NOT_A_PLATFORM)'.dependencies]") {
+        eprintln!(
+            "cargo dev ra-setup: warning: deps already found inside {}, doing nothing.",
+            manifest_path
+        );
+        return Ok(());
+    }
+
     let extern_crates = lib_rs
         .lines()
         // get the deps

clippy_lints/src/arithmetic.rs

@@ -88,9 +88,28 @@ impl<'tcx> LateLintPass<'tcx> for Arithmetic {
 
                 let (l_ty, r_ty) = (cx.typeck_results().expr_ty(l), cx.typeck_results().expr_ty(r));
                 if l_ty.peel_refs().is_integral() && r_ty.peel_refs().is_integral() {
-                    span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
-                    self.expr_span = Some(expr.span);
-                } else if l_ty.peel_refs().is_floating_point() && r_ty.peel_refs().is_floating_point() {
+                    match op.node {
+                        hir::BinOpKind::Div | hir::BinOpKind::Rem => match &r.kind {
+                            hir::ExprKind::Lit(_lit) => (),
+                            hir::ExprKind::Unary(hir::UnOp::UnNeg, expr) => {
+                                if let hir::ExprKind::Lit(lit) = &expr.kind {
+                                    if let rustc_ast::ast::LitKind::Int(1, _) = lit.node {
+                                        span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
+                                        self.expr_span = Some(expr.span);
+                                    }
+                                }
+                            },
+                            _ => {
+                                span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
+                                self.expr_span = Some(expr.span);
+                            },
+                        },
+                        _ => {
+                            span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected");
+                            self.expr_span = Some(expr.span);
+                        },
+                    }
+                } else if r_ty.peel_refs().is_floating_point() && r_ty.peel_refs().is_floating_point() {
                     span_lint(cx, FLOAT_ARITHMETIC, expr.span, "floating-point arithmetic detected");
                     self.expr_span = Some(expr.span);
                 }

clippy_lints/src/future_not_send.rs

@@ -92,13 +92,8 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend {
                         |db| {
                             cx.tcx.infer_ctxt().enter(|infcx| {
                                 for FulfillmentError { obligation, .. } in send_errors {
-                                    infcx.maybe_note_obligation_cause_for_async_await(
-                                        db,
-                                        &obligation,
-                                    );
-                                    if let Trait(trait_pred, _) =
-                                        obligation.predicate.skip_binders()
-                                    {
+                                    infcx.maybe_note_obligation_cause_for_async_await(db, &obligation);
+                                    if let Trait(trait_pred, _) = obligation.predicate.skip_binders() {
                                         db.note(&format!(
                                             "`{}` doesn't implement `{}`",
                                             trait_pred.self_ty(),

clippy_lints/src/items_after_statements.rs

@@ -2,7 +2,8 @@
 
 use crate::utils::span_lint;
 use rustc_ast::ast::{Block, ItemKind, StmtKind};
-use rustc_lint::{EarlyContext, EarlyLintPass};
+use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 
 declare_clippy_lint! {
@@ -53,7 +54,7 @@ declare_lint_pass!(ItemsAfterStatements => [ITEMS_AFTER_STATEMENTS]);
 
 impl EarlyLintPass for ItemsAfterStatements {
     fn check_block(&mut self, cx: &EarlyContext<'_>, item: &Block) {
-        if item.span.from_expansion() {
+        if in_external_macro(cx.sess(), item.span) {
             return;
         }
 
@@ -67,7 +68,7 @@ impl EarlyLintPass for ItemsAfterStatements {
         // lint on all further items
         for stmt in stmts {
             if let StmtKind::Item(ref it) = *stmt {
-                if it.span.from_expansion() {
+                if in_external_macro(cx.sess(), it.span) {
                     return;
                 }
                 if let ItemKind::MacroDef(..) = it.kind {

clippy_lints/src/len_zero.rs

@@ -68,7 +68,44 @@ declare_clippy_lint! {
     "traits or impls with a public `len` method but no corresponding `is_empty` method"
 }
 
-declare_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY]);
+declare_clippy_lint! {
+    /// **What it does:** Checks for comparing to an empty slice such as "" or [],`
+    /// and suggests using `.is_empty()` where applicable.
+    ///
+    /// **Why is this bad?** Some structures can answer `.is_empty()` much faster
+    /// than checking for equality. So it is good to get into the habit of using
+    /// `.is_empty()`, and having it is cheap.
+    /// Besides, it makes the intent clearer than a manual comparison in some contexts.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```ignore
+    /// if s == "" {
+    ///     ..
+    /// }
+    ///
+    /// if arr == [] {
+    ///     ..
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```ignore
+    /// if s.is_empty() {
+    ///     ..
+    /// }
+    ///
+    /// if arr.is_empty() {
+    ///     ..
+    /// }
+    /// ```
+    pub COMPARISON_TO_EMPTY,
+    style,
+    "checking `x == \"\"` or `x == []` (or similar) when `.is_empty()` could be used instead"
+}
+
+declare_lint_pass!(LenZero => [LEN_ZERO, LEN_WITHOUT_IS_EMPTY, COMPARISON_TO_EMPTY]);
 
 impl<'tcx> LateLintPass<'tcx> for LenZero {
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
@@ -221,6 +258,8 @@ fn check_cmp(cx: &LateContext<'_>, span: Span, method: &Expr<'_>, lit: &Expr<'_>
         }
 
         check_len(cx, span, method_path.ident.name, args, &lit.node, op, compare_to)
+    } else {
+        check_empty_expr(cx, span, method, lit, op)
     }
 }
 
@@ -258,6 +297,42 @@ fn check_len(
     }
 }
 
+fn check_empty_expr(cx: &LateContext<'_>, span: Span, lit1: &Expr<'_>, lit2: &Expr<'_>, op: &str) {
+    if (is_empty_array(lit2) || is_empty_string(lit2)) && has_is_empty(cx, lit1) {
+        let mut applicability = Applicability::MachineApplicable;
+        span_lint_and_sugg(
+            cx,
+            COMPARISON_TO_EMPTY,
+            span,
+            "comparison to empty slice",
+            &format!("using `{}is_empty` is clearer and more explicit", op),
+            format!(
+                "{}{}.is_empty()",
+                op,
+                snippet_with_applicability(cx, lit1.span, "_", &mut applicability)
+            ),
+            applicability,
+        );
+    }
+}
+
+fn is_empty_string(expr: &Expr<'_>) -> bool {
+    if let ExprKind::Lit(ref lit) = expr.kind {
+        if let LitKind::Str(lit, _) = lit.node {
+            let lit = lit.as_str();
+            return lit == "";
+        }
+    }
+    false
+}
+
+fn is_empty_array(expr: &Expr<'_>) -> bool {
+    if let ExprKind::Array(ref arr) = expr.kind {
+        return arr.is_empty();
+    }
+    false
+}
+
 /// Checks if this type has an `is_empty` method.
 fn has_is_empty(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
     /// Gets an `AssocItem` and return true if it matches `is_empty(self)`.

clippy_lints/src/lib.rs

@@ -615,6 +615,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &large_const_arrays::LARGE_CONST_ARRAYS,
         &large_enum_variant::LARGE_ENUM_VARIANT,
         &large_stack_arrays::LARGE_STACK_ARRAYS,
+        &len_zero::COMPARISON_TO_EMPTY,
         &len_zero::LEN_WITHOUT_IS_EMPTY,
         &len_zero::LEN_ZERO,
         &let_if_seq::USELESS_LET_IF_SEQ,
@@ -702,6 +703,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::ITER_NTH_ZERO,
         &methods::ITER_SKIP_NEXT,
         &methods::MANUAL_SATURATING_ARITHMETIC,
+        &methods::MAP_COLLECT_RESULT_UNIT,
         &methods::MAP_FLATTEN,
         &methods::MAP_UNWRAP_OR,
         &methods::NEW_RET_NO_SELF,
@@ -1366,6 +1368,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&int_plus_one::INT_PLUS_ONE),
         LintId::of(&large_const_arrays::LARGE_CONST_ARRAYS),
         LintId::of(&large_enum_variant::LARGE_ENUM_VARIANT),
+        LintId::of(&len_zero::COMPARISON_TO_EMPTY),
         LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
         LintId::of(&len_zero::LEN_ZERO),
         LintId::of(&let_underscore::LET_UNDERSCORE_LOCK),
@@ -1427,6 +1430,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::ITER_NTH_ZERO),
         LintId::of(&methods::ITER_SKIP_NEXT),
         LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
+        LintId::of(&methods::MAP_COLLECT_RESULT_UNIT),
         LintId::of(&methods::NEW_RET_NO_SELF),
         LintId::of(&methods::OK_EXPECT),
         LintId::of(&methods::OPTION_AS_REF_DEREF),
@@ -1592,6 +1596,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&functions::RESULT_UNIT_ERR),
         LintId::of(&if_let_some_result::IF_LET_SOME_RESULT),
         LintId::of(&inherent_to_string::INHERENT_TO_STRING),
+        LintId::of(&len_zero::COMPARISON_TO_EMPTY),
         LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
         LintId::of(&len_zero::LEN_ZERO),
         LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING),
@@ -1621,6 +1626,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::ITER_NTH_ZERO),
         LintId::of(&methods::ITER_SKIP_NEXT),
         LintId::of(&methods::MANUAL_SATURATING_ARITHMETIC),
+        LintId::of(&methods::MAP_COLLECT_RESULT_UNIT),
         LintId::of(&methods::NEW_RET_NO_SELF),
         LintId::of(&methods::OK_EXPECT),
         LintId::of(&methods::OPTION_MAP_OR_NONE),