]> git.lizzy.rs Git - rust.git/blobdiff - clippy_lints/src/needless_pass_by_value.rs
Auto merge of #4551 - mikerite:fix-ice-reporting, r=llogiq
[rust.git] / clippy_lints / src / needless_pass_by_value.rs
index a3748d87a83c96bbf8ca58b8faf142283545fcf4..16693d8209bc4ae80be5c811e7a8b3c18c440a1f 100644 (file)
@@ -1,7 +1,7 @@
 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,
+    get_trait_def_id, implements_trait, is_copy, is_self, is_type_diagnostic_item, match_type, multispan_sugg, paths,
+    snippet, snippet_opt, span_lint_and_then,
 };
 use if_chain::if_chain;
 use matches::matches;
 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::{declare_lint_pass, declare_tool_lint};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_errors::Applicability;
 use rustc_target::spec::abi::Abi;
 use std::borrow::Cow;
+use syntax::ast::Attribute;
 use syntax::errors::DiagnosticBuilder;
-use syntax_pos::Span;
+use syntax_pos::{Span, Symbol};
 
 declare_clippy_lint! {
     /// **What it does:** Checks for functions taking arguments by value, but not
@@ -39,6 +40,9 @@
     /// fn foo(v: Vec<i32>) {
     ///     assert_eq!(v.len(), 42);
     /// }
+    /// ```
+    ///
+    /// ```rust
     /// // should be
     /// fn foo(v: &[i32]) {
     ///     assert_eq!(v.len(), 42);
     "functions taking arguments by value, but not consuming them in its body"
 }
 
-pub struct NeedlessPassByValue;
-
-impl LintPass for NeedlessPassByValue {
-    fn get_lints(&self) -> LintArray {
-        lint_array![NEEDLESS_PASS_BY_VALUE]
-    }
-
-    fn name(&self) -> &'static str {
-        "NeedlessPassByValue"
-    }
-}
+declare_lint_pass!(NeedlessPassByValue => [NEEDLESS_PASS_BY_VALUE]);
 
 macro_rules! need {
     ($e: expr) => {
@@ -82,31 +76,22 @@ fn check_fn(
         span: Span,
         hir_id: HirId,
     ) {
-        if in_macro(span) {
+        if span.from_expansion() {
             return;
         }
 
         match kind {
             FnKind::ItemFn(.., header, _, attrs) => {
-                if header.abi != Abi::Rust {
+                if header.abi != Abi::Rust || requires_exact_signature(attrs) {
                     return;
                 }
-                for a in attrs {
-                    if a.meta_item_list().is_some() && a.name() == "proc_macro_derive" {
-                        return;
-                    }
-                }
             },
             FnKind::Method(..) => (),
             _ => return,
         }
 
         // Exclude non-inherent impls
-        if let Some(Node::Item(item)) = cx
-            .tcx
-            .hir()
-            .find_by_hir_id(cx.tcx.hir().get_parent_node_by_hir_id(hir_id))
-        {
+        if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) {
             if matches!(item.node, ItemKind::Impl(_, _, _, _, Some(_), _, _) |
                 ItemKind::Trait(..))
             {
@@ -125,7 +110,7 @@ fn check_fn(
 
         let sized_trait = need!(cx.tcx.lang_items().sized_trait());
 
-        let fn_def_id = cx.tcx.hir().local_def_id_from_hir_id(hir_id);
+        let fn_def_id = cx.tcx.hir().local_def_id(hir_id);
 
         let preds = traits::elaborate_predicates(cx.tcx, cx.param_env.caller_bounds.to_vec())
             .filter(|p| !p.is_global())
@@ -151,15 +136,23 @@ fn check_fn(
         } = {
             let mut ctx = MovedVariablesCtxt::new(cx);
             let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
-            euv::ExprUseVisitor::new(&mut ctx, cx.tcx, cx.param_env, region_scope_tree, cx.tables, None)
-                .consume_body(body);
+            euv::ExprUseVisitor::new(
+                &mut ctx,
+                cx.tcx,
+                fn_def_id,
+                cx.param_env,
+                region_scope_tree,
+                cx.tables,
+                None,
+            )
+            .consume_body(body);
             ctx
         };
 
         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.params).enumerate() {
             // All spans generated from a proc-macro invocation are the same...
             if span == input.span {
                 return;
@@ -176,7 +169,7 @@ 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`,
+            // * 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
@@ -203,7 +196,7 @@ fn check_fn(
 
             if_chain! {
                 if !is_self(arg);
-                if !ty.is_mutable_pointer();
+                if !ty.is_mutable_ptr();
                 if !is_copy(cx, ty);
                 if !whitelisted_traits.iter().any(|&t| implements_trait(cx, ty, t, &[]));
                 if !implements_borrow_trait;
@@ -228,12 +221,12 @@ fn check_fn(
 
                         let deref_span = spans_need_deref.get(&canonical_id);
                         if_chain! {
-                            if match_type(cx, ty, &paths::VEC);
+                            if is_type_diagnostic_item(cx, ty, Symbol::intern("vec_type"));
                             if let Some(clone_spans) =
                                 get_spans(cx, Some(body.id()), idx, &[("clone", ".to_owned()")]);
                             if let TyKind::Path(QPath::Resolved(_, ref path)) = input.node;
                             if let Some(elem_ty) = path.segments.iter()
-                                .find(|seg| seg.ident.name == "Vec")
+                                .find(|seg| seg.ident.name == sym!(Vec))
                                 .and_then(|ps| ps.args.as_ref())
                                 .map(|params| params.args.iter().find_map(|arg| match arg {
                                     GenericArg::Type(ty) => Some(ty),
@@ -323,7 +316,16 @@ fn check_fn(
     }
 }
 
-struct MovedVariablesCtxt<'a, 'tcx: 'a> {
+/// Functions marked with these attributes must have the exact signature.
+fn requires_exact_signature(attrs: &[Attribute]) -> bool {
+    attrs.iter().any(|attr| {
+        [sym!(proc_macro), sym!(proc_macro_attribute), sym!(proc_macro_derive)]
+            .iter()
+            .any(|&allow| attr.check_name(allow))
+    })
+}
+
+struct MovedVariablesCtxt<'a, 'tcx> {
     cx: &'a LateContext<'a, 'tcx>,
     moved_vars: FxHashSet<HirId>,
     /// Spans which need to be prefixed with `*` for dereferencing the
@@ -354,14 +356,14 @@ 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.hir_id;
             loop {
-                let parent = self.cx.tcx.hir().get_parent_node_by_hir_id(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_by_hir_id(id) {
+                if let Some(node) = self.cx.tcx.hir().find(id) {
                     match node {
                         Node::Expr(e) => {
                             // `match` and `if let`