]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/needless_pass_by_value.rs
Auto merge of #3680 - g-bartoszek:needless-bool-else-if-brackets, r=oli-obk
[rust.git] / clippy_lints / src / needless_pass_by_value.rs
index 82e85f3453a1526d5c1ebaaf74c37a2d3cb4feca..cb1fe475a1eeeb1363bea725bbbbdf01823aca96 100644 (file)
@@ -1,23 +1,25 @@
+use crate::utils::ptr::get_spans;
+use crate::utils::{
+    get_trait_def_id, implements_trait, in_macro, is_copy, is_self, match_type, multispan_sugg, paths, snippet,
+    snippet_opt, span_lint_and_then,
+};
+use if_chain::if_chain;
 use matches::matches;
-use rustc::hir::*;
-use rustc::hir::map::*;
 use rustc::hir::intravisit::FnKind;
-use rustc::lint::*;
-use rustc::{declare_lint, lint_array};
-use if_chain::if_chain;
-use rustc::ty::{self, RegionKind, TypeFoldable};
-use rustc::traits;
+use rustc::hir::*;
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
 use rustc::middle::expr_use_visitor as euv;
 use rustc::middle::mem_categorization as mc;
+use rustc::traits;
+use rustc::ty::{self, RegionKind, TypeFoldable};
+use rustc::{declare_tool_lint, lint_array};
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_errors::Applicability;
 use rustc_target::spec::abi::Abi;
+use std::borrow::Cow;
 use syntax::ast::NodeId;
-use syntax_pos::Span;
 use syntax::errors::DiagnosticBuilder;
-use crate::utils::{get_trait_def_id, implements_trait, in_macro, is_copy, is_self, match_type, multispan_sugg, paths,
-            snippet, snippet_opt, span_lint_and_then};
-use crate::utils::ptr::get_spans;
-use std::collections::{HashMap, HashSet};
-use std::borrow::Cow;
+use syntax_pos::Span;
 
 /// **What it does:** Checks for functions taking arguments by value, but not
 /// consuming them in its
@@ -44,7 +46,7 @@
 /// ```
 declare_clippy_lint! {
     pub NEEDLESS_PASS_BY_VALUE,
-    style,
+    pedantic,
     "functions taking arguments by value, but not consuming them in its body"
 }
 
@@ -57,7 +59,13 @@ fn get_lints(&self) -> LintArray {
 }
 
 macro_rules! need {
-    ($e: expr) => { if let Some(x) = $e { x } else { return; } };
+    ($e: expr) => {
+        if let Some(x) = $e {
+            x
+        } else {
+            return;
+        }
+    };
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
@@ -90,7 +98,7 @@ fn check_fn(
         }
 
         // Exclude non-inherent impls
-        if let Some(NodeItem(item)) = cx.tcx.hir.find(cx.tcx.hir.get_parent_node(node_id)) {
+        if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(node_id)) {
             if matches!(item.node, ItemKind::Impl(_, _, _, _, Some(_), _, _) |
                 ItemKind::Trait(..))
             {
@@ -104,18 +112,19 @@ fn check_fn(
             need!(cx.tcx.lang_items().fn_trait()),
             need!(cx.tcx.lang_items().fn_once_trait()),
             need!(cx.tcx.lang_items().fn_mut_trait()),
-            need!(get_trait_def_id(cx, &paths::RANGE_ARGUMENT_TRAIT))
+            need!(get_trait_def_id(cx, &paths::RANGE_ARGUMENT_TRAIT)),
         ];
 
         let sized_trait = need!(cx.tcx.lang_items().sized_trait());
 
-        let fn_def_id = cx.tcx.hir.local_def_id(node_id);
+        let fn_def_id = cx.tcx.hir().local_def_id(node_id);
 
         let preds = traits::elaborate_predicates(cx.tcx, cx.param_env.caller_bounds.to_vec())
             .filter(|p| !p.is_global())
             .filter_map(|pred| {
                 if let ty::Predicate::Trait(poly_trait_ref) = pred {
-                    if poly_trait_ref.def_id() == sized_trait || poly_trait_ref.skip_binder().has_escaping_regions() {
+                    if poly_trait_ref.def_id() == sized_trait || poly_trait_ref.skip_binder().has_escaping_bound_vars()
+                    {
                         return None;
                     }
                     Some(poly_trait_ref)
@@ -142,12 +151,7 @@ fn check_fn(
         let fn_sig = cx.tcx.fn_sig(fn_def_id);
         let fn_sig = cx.tcx.erase_late_bound_regions(&fn_sig);
 
-        for (idx, ((input, &ty), arg)) in decl.inputs
-            .iter()
-            .zip(fn_sig.inputs())
-            .zip(&body.arguments)
-            .enumerate()
-        {
+        for (idx, ((input, &ty), arg)) in decl.inputs.iter().zip(fn_sig.inputs()).zip(&body.arguments).enumerate() {
             // All spans generated from a proc-macro invocation are the same...
             if span == input.span {
                 return;
@@ -162,9 +166,10 @@ fn check_fn(
                 }
             }
 
+            //
             // * Exclude a type that is specifically bounded by `Borrow`.
-            // * Exclude a type whose reference also fulfills its bound.
-            //   (e.g. `std::convert::AsRef`, `serde::Serialize`)
+            // * Exclude a type whose reference also fulfills its bound. (e.g. `std::convert::AsRef`,
+            //   `serde::Serialize`)
             let (implements_borrow_trait, all_borrowable_trait) = {
                 let preds = preds
                     .iter()
@@ -173,18 +178,18 @@ fn check_fn(
 
                 (
                     preds.iter().any(|t| t.def_id() == borrow_trait),
-                    !preds.is_empty() && preds.iter().all(|t| {
-                        implements_trait(
-                            cx,
-                            cx.tcx.mk_imm_ref(&RegionKind::ReErased, ty),
-                            t.def_id(),
-                            &t.skip_binder()
-                                .input_types()
+                    !preds.is_empty()
+                        && preds.iter().all(|t| {
+                            let ty_params = &t
+                                .skip_binder()
+                                .trait_ref
+                                .substs
+                                .iter()
                                 .skip(1)
-                                .map(|ty| ty.into())
-                                .collect::<Vec<_>>(),
-                        )
-                    }),
+                                .cloned()
+                                .collect::<Vec<_>>();
+                            implements_trait(cx, cx.tcx.mk_imm_ref(&RegionKind::ReErased, ty), t.def_id(), ty_params)
+                        }),
                 )
             };
 
@@ -205,8 +210,8 @@ fn check_fn(
 
                     // Dereference suggestion
                     let sugg = |db: &mut DiagnosticBuilder<'_>| {
-                        if let ty::TypeVariants::TyAdt(def, ..) = ty.sty {
-                            if let Some(span) = cx.tcx.hir.span_if_local(def.did) {
+                        if let ty::Adt(def, ..) = ty.sty {
+                            if let Some(span) = cx.tcx.hir().span_if_local(def.did) {
                                 if cx.param_env.can_type_implement_copy(cx.tcx, ty).is_ok() {
                                     db.span_help(span, "consider marking this type as Copy");
                                 }
@@ -228,19 +233,23 @@ fn check_fn(
                                 }).unwrap());
                             then {
                                 let slice_ty = format!("&[{}]", snippet(cx, elem_ty.span, "_"));
-                                db.span_suggestion(input.span,
-                                                "consider changing the type to",
-                                                slice_ty);
+                                db.span_suggestion_with_applicability(
+                                    input.span,
+                                    "consider changing the type to",
+                                    slice_ty,
+                                    Applicability::Unspecified,
+                                );
 
                                 for (span, suggestion) in clone_spans {
-                                    db.span_suggestion(
+                                    db.span_suggestion_with_applicability(
                                         span,
                                         &snippet_opt(cx, span)
                                             .map_or(
                                                 "change the call to".into(),
                                                 |x| Cow::from(format!("change `{}` to", x)),
                                             ),
-                                        suggestion.into()
+                                        suggestion.into(),
+                                        Applicability::Unspecified,
                                     );
                                 }
 
@@ -253,10 +262,15 @@ fn check_fn(
                         if match_type(cx, ty, &paths::STRING) {
                             if let Some(clone_spans) =
                                 get_spans(cx, Some(body.id()), idx, &[("clone", ".to_string()"), ("as_str", "")]) {
-                                db.span_suggestion(input.span, "consider changing the type to", "&str".to_string());
+                                db.span_suggestion_with_applicability(
+                                    input.span,
+                                    "consider changing the type to",
+                                    "&str".to_string(),
+                                    Applicability::Unspecified,
+                                );
 
                                 for (span, suggestion) in clone_spans {
-                                    db.span_suggestion(
+                                    db.span_suggestion_with_applicability(
                                         span,
                                         &snippet_opt(cx, span)
                                             .map_or(
@@ -264,6 +278,7 @@ fn check_fn(
                                                 |x| Cow::from(format!("change `{}` to", x))
                                             ),
                                         suggestion.into(),
+                                        Applicability::Unspecified,
                                     );
                                 }
 
@@ -302,18 +317,18 @@ fn check_fn(
 
 struct MovedVariablesCtxt<'a, 'tcx: 'a> {
     cx: &'a LateContext<'a, 'tcx>,
-    moved_vars: HashSet<NodeId>,
+    moved_vars: FxHashSet<NodeId>,
     /// Spans which need to be prefixed with `*` for dereferencing the
     /// suggested additional reference.
-    spans_need_deref: HashMap<NodeId, HashSet<Span>>,
+    spans_need_deref: FxHashMap<NodeId, FxHashSet<Span>>,
 }
 
 impl<'a, 'tcx> MovedVariablesCtxt<'a, 'tcx> {
     fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
         Self {
             cx,
-            moved_vars: HashSet::new(),
-            spans_need_deref: HashMap::new(),
+            moved_vars: FxHashSet::default(),
+            spans_need_deref: FxHashMap::default(),
         }
     }
 
@@ -331,34 +346,33 @@ fn non_moving_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>) {
         if let mc::Categorization::Local(vid) = cmt.cat {
             let mut id = matched_pat.id;
             loop {
-                let parent = self.cx.tcx.hir.get_parent_node(id);
+                let parent = self.cx.tcx.hir().get_parent_node(id);
                 if id == parent {
                     // no parent
                     return;
                 }
                 id = parent;
 
-                if let Some(node) = self.cx.tcx.hir.find(id) {
+                if let Some(node) = self.cx.tcx.hir().find(id) {
                     match node {
-                        map::Node::NodeExpr(e) => {
+                        Node::Expr(e) => {
                             // `match` and `if let`
                             if let ExprKind::Match(ref c, ..) = e.node {
                                 self.spans_need_deref
                                     .entry(vid)
-                                    .or_insert_with(HashSet::new)
+                                    .or_insert_with(FxHashSet::default)
                                     .insert(c.span);
                             }
                         },
 
-                        map::Node::NodeStmt(s) => {
+                        Node::Stmt(s) => {
                             // `let <pat> = x;`
                             if_chain! {
-                                if let StmtKind::Decl(ref decl, _) = s.node;
-                                if let DeclKind::Local(ref local) = decl.node;
+                                if let StmtKind::Local(ref local) = s.node;
                                 then {
                                     self.spans_need_deref
                                         .entry(vid)
-                                        .or_insert_with(HashSet::new)
+                                        .or_insert_with(FxHashSet::default)
                                         .insert(local.init
                                             .as_ref()
                                             .map(|e| e.span)
@@ -396,14 +410,22 @@ fn consume_pat(&mut self, consume_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::Co
         }
     }
 
-    fn borrow(&mut self, _: NodeId, _: Span, _: &mc::cmt_<'tcx>, _: ty::Region<'_>, _: ty::BorrowKind, _: euv::LoanCause) {}
+    fn borrow(
+        &mut self,
+        _: NodeId,
+        _: Span,
+        _: &mc::cmt_<'tcx>,
+        _: ty::Region<'_>,
+        _: ty::BorrowKind,
+        _: euv::LoanCause,
+    ) {
+    }
 
     fn mutate(&mut self, _: NodeId, _: Span, _: &mc::cmt_<'tcx>, _: euv::MutateMode) {}
 
     fn decl_without_init(&mut self, _: NodeId, _: Span) {}
 }
 
-
 fn unwrap_downcast_or_interior<'a, 'tcx>(mut cmt: &'a mc::cmt_<'tcx>) -> mc::cmt_<'tcx> {
     loop {
         match cmt.cat {
@@ -412,5 +434,5 @@ fn unwrap_downcast_or_interior<'a, 'tcx>(mut cmt: &'a mc::cmt_<'tcx>) -> mc::cmt
             },
             _ => return (*cmt).clone(),
         }
-    };
+    }
 }