Update Clippy

CHANGELOG.md

@@ -1093,6 +1093,7 @@ Released 2018-09-13
 [`extend_from_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#extend_from_slice
 [`extra_unused_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes
 [`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from
+[`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file
 [`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map
 [`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next
 [`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next

COPYRIGHT

@@ -1,4 +1,4 @@
-Copyright 2014-2019 The Rust Project Developers
+Copyright 2014-2020 The Rust Project Developers
 
 Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
 http://www.apache.org/licenses/LICENSE-2.0> or the MIT license

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 346 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 347 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/doc.rs

@@ -1,4 +1,4 @@
-use crate::utils::{match_type, paths, return_ty, span_lint};
+use crate::utils::{is_entrypoint_fn, match_type, paths, return_ty, span_lint};
 use itertools::Itertools;
 use rustc::lint::in_external_macro;
 use rustc_data_structures::fx::FxHashSet;
@@ -153,7 +153,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
         let headers = check_attrs(cx, &self.valid_idents, &item.attrs);
         match item.kind {
             hir::ItemKind::Fn(ref sig, ..) => {
-                if !in_external_macro(cx.tcx.sess, item.span) {
+                if !(is_entrypoint_fn(cx, cx.tcx.hir().local_def_id(item.hir_id))
+                    || in_external_macro(cx.tcx.sess, item.span))
+                {
                     lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
                 }
             },

clippy_lints/src/lib.rs

@@ -614,6 +614,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::CLONE_ON_COPY,
         &methods::CLONE_ON_REF_PTR,
         &methods::EXPECT_FUN_CALL,
+        &methods::FILETYPE_IS_FILE,
         &methods::FILTER_MAP,
         &methods::FILTER_MAP_NEXT,
         &methods::FILTER_NEXT,
@@ -1007,6 +1008,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&matches::WILDCARD_ENUM_MATCH_ARM),
         LintId::of(&mem_forget::MEM_FORGET),
         LintId::of(&methods::CLONE_ON_REF_PTR),
+        LintId::of(&methods::FILETYPE_IS_FILE),
         LintId::of(&methods::GET_UNWRAP),
         LintId::of(&methods::OPTION_EXPECT_USED),
         LintId::of(&methods::OPTION_UNWRAP_USED),

clippy_lints/src/loops.rs

@@ -1,37 +1,35 @@
+use crate::consts::{constant, Constant};
 use crate::reexport::*;
+use crate::utils::paths;
+use crate::utils::usage::{is_unused, mutated_variables};
+use crate::utils::{
+    get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
+    is_integer_const, is_refutable, last_path_segment, match_trait_method, match_type, match_var, multispan_sugg,
+    snippet, snippet_opt, snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg,
+    span_lint_and_then, SpanlessEq,
+};
+use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg};
 use if_chain::if_chain;
 use itertools::Itertools;
+use rustc::hir::map::Map;
 use rustc::lint::in_external_macro;
 use rustc::middle::region;
+use rustc::ty::{self, Ty};
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id;
 use rustc_hir::intravisit::{walk_block, walk_expr, walk_pat, walk_stmt, NestedVisitorMap, Visitor};
 use rustc_hir::*;
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-// use rustc::middle::region::CodeExtent;
-use crate::consts::{constant, Constant};
-use crate::utils::usage::mutated_variables;
-use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg};
-use rustc::hir::map::Map;
-use rustc::ty::{self, Ty};
-use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_errors::Applicability;
 use rustc_span::source_map::Span;
 use rustc_span::{BytePos, Symbol};
 use rustc_typeck::expr_use_visitor::*;
 use std::iter::{once, Iterator};
 use std::mem;
 use syntax::ast;
 
-use crate::utils::paths;
-use crate::utils::{
-    get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
-    is_integer_const, is_refutable, last_path_segment, match_trait_method, match_type, match_var, multispan_sugg,
-    snippet, snippet_opt, snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg,
-    span_lint_and_then, SpanlessEq,
-};
-
 declare_clippy_lint! {
     /// **What it does:** Checks for for-loops that manually copy items between
     /// slices that could be optimized by having a memcpy.
@@ -1689,39 +1687,11 @@ fn check_for_mutation(
 fn pat_is_wild<'tcx>(pat: &'tcx PatKind<'_>, body: &'tcx Expr<'_>) -> bool {
     match *pat {
         PatKind::Wild => true,
-        PatKind::Binding(.., ident, None) if ident.as_str().starts_with('_') => {
-            let mut visitor = UsedVisitor {
-                var: ident.name,
-                used: false,
-            };
-            walk_expr(&mut visitor, body);
-            !visitor.used
-        },
+        PatKind::Binding(.., ident, None) if ident.as_str().starts_with('_') => is_unused(&ident, body),
         _ => false,
     }
 }
 
-struct UsedVisitor {
-    var: ast::Name, // var to look for
-    used: bool,     // has the var been used otherwise?
-}
-
-impl<'tcx> Visitor<'tcx> for UsedVisitor {
-    type Map = Map<'tcx>;
-
-    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
-        if match_var(expr, self.var) {
-            self.used = true;
-        } else {
-            walk_expr(self, expr);
-        }
-    }
-
-    fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> {
-        NestedVisitorMap::None
-    }
-}
-
 struct LocalUsedVisitor<'a, 'tcx> {
     cx: &'a LateContext<'a, 'tcx>,
     local: HirId,

clippy_lints/src/matches.rs

@@ -1,8 +1,9 @@
 use crate::consts::{constant, miri_to_const, Constant};
 use crate::utils::paths;
 use crate::utils::sugg::Sugg;
+use crate::utils::usage::is_unused;
 use crate::utils::{
-    expr_block, is_allowed, is_expn_of, match_qpath, match_type, multispan_sugg, remove_blocks, snippet,
+    expr_block, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks, snippet,
     snippet_with_applicability, span_help_and_lint, span_lint_and_sugg, span_lint_and_then, span_note_and_lint,
     walk_ptrs_ty,
 };
@@ -461,33 +462,40 @@ fn check_overlapping_arms<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ex: &'tcx Expr<'
     }
 }
 
-fn is_wild<'tcx>(pat: &impl std::ops::Deref<Target = Pat<'tcx>>) -> bool {
-    match pat.kind {
-        PatKind::Wild => true,
-        _ => false,
-    }
-}
-
 fn check_wild_err_arm(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>]) {
     let ex_ty = walk_ptrs_ty(cx.tables.expr_ty(ex));
     if match_type(cx, ex_ty, &paths::RESULT) {
         for arm in arms {
             if let PatKind::TupleStruct(ref path, ref inner, _) = arm.pat.kind {
                 let path_str = print::to_string(print::NO_ANN, |s| s.print_qpath(path, false));
-                if_chain! {
-                    if path_str == "Err";
-                    if inner.iter().any(is_wild);
-                    if let ExprKind::Block(ref block, _) = arm.body.kind;
-                    if is_panic_block(block);
-                    then {
-                        // `Err(_)` arm with `panic!` found
-                        span_note_and_lint(cx,
-                                           MATCH_WILD_ERR_ARM,
-                                           arm.pat.span,
-                                           "`Err(_)` will match all errors, maybe not a good idea",
-                                           arm.pat.span,
-                                           "to remove this warning, match each error separately \
-                                            or use `unreachable!` macro");
+                if path_str == "Err" {
+                    let mut matching_wild = inner.iter().any(is_wild);
+                    let mut ident_bind_name = String::from("_");
+                    if !matching_wild {
+                        // Looking for unused bindings (i.e.: `_e`)
+                        inner.iter().for_each(|pat| {
+                            if let PatKind::Binding(.., ident, None) = &pat.kind {
+                                if ident.as_str().starts_with('_') && is_unused(ident, arm.body) {
+                                    ident_bind_name = (&ident.name.as_str()).to_string();
+                                    matching_wild = true;
+                                }
+                            }
+                        });
+                    }
+                    if_chain! {
+                        if matching_wild;
+                        if let ExprKind::Block(ref block, _) = arm.body.kind;
+                        if is_panic_block(block);
+                        then {
+                            // `Err(_)` or `Err(_e)` arm with `panic!` found
+                            span_note_and_lint(cx,
+                                MATCH_WILD_ERR_ARM,
+                                arm.pat.span,
+                                &format!("`Err({})` matches all errors", &ident_bind_name),
+                                arm.pat.span,
+                                "match each error separately or use the error output",
+                            );
+                        }
                     }
                 }
             }

clippy_lints/src/methods/mod.rs

@@ -1131,6 +1131,40 @@ declare_clippy_lint! {
     "Check for offset calculations on raw pointers to zero-sized types"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for `FileType::is_file()`.
+    ///
+    /// **Why is this bad?** When people testing a file type with `FileType::is_file`
+    /// they are testing whether a path is something they can get bytes from. But
+    /// `is_file` doesn't cover special file types in unix-like systems, and doesn't cover
+    /// symlink in windows. Using `!FileType::is_dir()` is a better way to that intention.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust,ignore
+    /// let metadata = std::fs::metadata("foo.txt")?;
+    /// let filetype = metadata.file_type();
+    ///
+    /// if filetype.is_file() {
+    ///     // read file
+    /// }
+    /// ```
+    ///
+    /// should be written as:
+    ///
+    /// ```rust,ignore
+    /// let metadata = std::fs::metadata("foo.txt")?;
+    /// let filetype = metadata.file_type();
+    ///
+    /// if !filetype.is_dir() {
+    ///     // read file
+    /// }
+    /// ```
+    pub FILETYPE_IS_FILE,
+    restriction,
+    "`FileType::is_file` is not recommended to test for readable file type"
+}
+
 declare_lint_pass!(Methods => [
     OPTION_UNWRAP_USED,
     RESULT_UNWRAP_USED,
@@ -1178,6 +1212,7 @@ declare_lint_pass!(Methods => [
     UNINIT_ASSUMED_INIT,
     MANUAL_SATURATING_ARITHMETIC,
     ZST_OFFSET,
+    FILETYPE_IS_FILE,
 ]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
@@ -1241,6 +1276,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
             ["add"] | ["offset"] | ["sub"] | ["wrapping_offset"] | ["wrapping_add"] | ["wrapping_sub"] => {
                 check_pointer_offset(cx, expr, arg_lists[0])
             },
+            ["is_file", ..] => lint_filetype_is_file(cx, expr, arg_lists[0]),
             _ => {},
         }
 
@@ -3225,3 +3261,35 @@ fn check_pointer_offset(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, args: &[
         }
     }
 }
+
+fn lint_filetype_is_file(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
+    let ty = cx.tables.expr_ty(&args[0]);
+
+    if !match_type(cx, ty, &paths::FILE_TYPE) {
+        return;
+    }
+
+    let span: Span;
+    let verb: &str;
+    let lint_unary: &str;
+    let help_unary: &str;
+    if_chain! {
+        if let Some(parent) = get_parent_expr(cx, expr);
+        if let hir::ExprKind::Unary(op, _) = parent.kind;
+        if op == hir::UnOp::UnNot;
+        then {
+            lint_unary = "!";
+            verb = "denies";
+            help_unary = "";
+            span = parent.span;
+        } else {
+            lint_unary = "";
+            verb = "covers";
+            help_unary = "!";
+            span = expr.span;
+        }
+    }
+    let lint_msg = format!("`{}FileType::is_file()` only {} regular files", lint_unary, verb);
+    let help_msg = format!("use `{}FileType::is_dir()` instead", help_unary);
+    span_help_and_lint(cx, FILETYPE_IS_FILE, span, &lint_msg, &help_msg);
+}

clippy_lints/src/utils/mod.rs

@@ -127,6 +127,14 @@ pub fn is_present_in_source<T: LintContext>(cx: &T, span: Span) -> bool {
     true
 }
 
+/// Checks if given pattern is a wildcard (`_`)
+pub fn is_wild<'tcx>(pat: &impl std::ops::Deref<Target = Pat<'tcx>>) -> bool {
+    match pat.kind {
+        PatKind::Wild => true,
+        _ => false,
+    }
+}
+
 /// Checks if type is struct, enum or union type with the given def path.
 pub fn match_type(cx: &LateContext<'_, '_>, ty: Ty<'_>, path: &[&str]) -> bool {
     match ty.kind {

clippy_lints/src/utils/paths.rs

@@ -28,6 +28,7 @@ pub const DROP_TRAIT: [&str; 4] = ["core", "ops", "drop", "Drop"];
 pub const DURATION: [&str; 3] = ["core", "time", "Duration"];
 pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"];
 pub const EXIT: [&str; 3] = ["std", "process", "exit"];
+pub const FILE_TYPE: [&str; 3] = ["std", "fs", "FileType"];
 pub const FMT_ARGUMENTS_NEW_V1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"];
 pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"];
 pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"];

clippy_lints/src/utils/usage.rs

@@ -1,9 +1,14 @@
+use crate::utils::match_var;
+use rustc::hir::map::Map;
 use rustc::ty;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::def::Res;
+use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
 use rustc_hir::*;
 use rustc_lint::LateContext;
+use rustc_span::symbol::Ident;
 use rustc_typeck::expr_use_visitor::*;
+use syntax::ast;
 
 /// Returns a set of mutated local variable IDs, or `None` if mutations could not be determined.
 pub fn mutated_variables<'a, 'tcx>(expr: &'tcx Expr<'_>, cx: &'a LateContext<'a, 'tcx>) -> Option<FxHashSet<HirId>> {
@@ -70,3 +75,33 @@ impl<'tcx> Delegate<'tcx> for MutVarsDelegate {
         self.update(&cmt)
     }
 }
+
+pub struct UsedVisitor {
+    pub var: ast::Name, // var to look for
+    pub used: bool,     // has the var been used otherwise?
+}
+
+impl<'tcx> Visitor<'tcx> for UsedVisitor {
+    type Map = Map<'tcx>;
+
+    fn visit_expr(&mut self, expr: &'tcx Expr<'_>) {
+        if match_var(expr, self.var) {
+            self.used = true;
+        } else {
+            walk_expr(self, expr);
+        }
+    }
+
+    fn nested_visit_map(&mut self) -> NestedVisitorMap<'_, Self::Map> {
+        NestedVisitorMap::None
+    }
+}
+
+pub fn is_unused<'tcx>(ident: &'tcx Ident, body: &'tcx Expr<'_>) -> bool {
+    let mut visitor = UsedVisitor {
+        var: ident.name,
+        used: false,
+    };
+    walk_expr(&mut visitor, body);
+    !visitor.used
+}