Fix needless_pass_by_value

This also accidentally improved the spans of the suggestions

3 files changed, 15 insertions, 109 deletions

diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs
index 932660da19c..acad6459cf2 100644
--- a/clippy_lints/src/needless_pass_by_value.rs
+++ b/clippy_lints/src/needless_pass_by_value.rs
@@ -134,7 +134,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
             spans_need_deref,
             ..
         } = {
-            let mut ctx = MovedVariablesCtxt::new(cx);
+            let mut ctx = MovedVariablesCtxt::default();
             let region_scope_tree = &cx.tcx.region_scope_tree(fn_def_id);
             euv::ExprUseVisitor::new(
                 &mut ctx,
@@ -324,98 +324,28 @@ fn requires_exact_signature(attrs: &[Attribute]) -> bool {
     })
 }
 
-struct MovedVariablesCtxt<'a, 'tcx> {
-    cx: &'a LateContext<'a, 'tcx>,
+#[derive(Default)]
+struct MovedVariablesCtxt {
     moved_vars: FxHashSet<HirId>,
     /// Spans which need to be prefixed with `*` for dereferencing the
     /// suggested additional reference.
     spans_need_deref: FxHashMap<HirId, FxHashSet<Span>>,
 }
 
-impl<'a, 'tcx> MovedVariablesCtxt<'a, 'tcx> {
-    fn new(cx: &'a LateContext<'a, 'tcx>) -> Self {
-        Self {
-            cx,
-            moved_vars: FxHashSet::default(),
-            spans_need_deref: FxHashMap::default(),
-        }
-    }
-
-    fn move_common(&mut self, _consume_id: HirId, _span: Span, cmt: &mc::cmt_<'tcx>) {
+impl MovedVariablesCtxt {
+    fn move_common(&mut self, cmt: &mc::cmt_<'_>) {
         let cmt = unwrap_downcast_or_interior(cmt);
 
         if let mc::Categorization::Local(vid) = cmt.cat {
             self.moved_vars.insert(vid);
         }
     }
-
-    fn non_moving_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>) {
-        let cmt = unwrap_downcast_or_interior(cmt);
-
-        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(id);
-                if id == parent {
-                    // no parent
-                    return;
-                }
-                id = parent;
-
-                if let Some(node) = self.cx.tcx.hir().find(id) {
-                    match node {
-                        Node::Expr(e) => {
-                            // `match` and `if let`
-                            if let ExprKind::Match(ref c, ..) = e.kind {
-                                self.spans_need_deref
-                                    .entry(vid)
-                                    .or_insert_with(FxHashSet::default)
-                                    .insert(c.span);
-                            }
-                        },
-
-                        Node::Stmt(s) => {
-                            // `let <pat> = x;`
-                            if_chain! {
-                                if let StmtKind::Local(ref local) = s.kind;
-                                then {
-                                    self.spans_need_deref
-                                        .entry(vid)
-                                        .or_insert_with(FxHashSet::default)
-                                        .insert(local.init
-                                            .as_ref()
-                                            .map(|e| e.span)
-                                            .expect("`let` stmt without init aren't caught by match_pat"));
-                                }
-                            }
-                        },
-
-                        _ => {},
-                    }
-                }
-            }
-        }
-    }
 }
 
-impl<'a, 'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt<'a, 'tcx> {
+impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt {
     fn consume(&mut self, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
         if let euv::ConsumeMode::Move = mode {
-            self.move_common(cmt.hir_id, cmt.span, cmt);
-        }
-    }
-
-    fn matched_pat(&mut self, matched_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::MatchMode) {
-        if let euv::MatchMode::MovingMatch = mode {
-            self.move_common(matched_pat.hir_id, matched_pat.span, cmt);
-        } else {
-            self.non_moving_pat(matched_pat, cmt);
-        }
-    }
-
-    fn consume_pat(&mut self, consume_pat: &Pat, cmt: &mc::cmt_<'tcx>, mode: euv::ConsumeMode) {
-        if let euv::ConsumeMode::Move(_) = mode {
-            self.move_common(consume_pat.hir_id, consume_pat.span, cmt);
+            self.move_common(cmt);
         }
     }
 
diff --git a/tests/ui/mut_range_bound.stderr b/tests/ui/mut_range_bound.stderr
index 50e94efde53..0eeb76e0ec5 100644
--- a/tests/ui/mut_range_bound.stderr
+++ b/tests/ui/mut_range_bound.stderr
@@ -2,7 +2,7 @@ error: attempt to mutate range bound within loop; note that the range of the loo
   --> $DIR/mut_range_bound.rs:16:9
    |
 LL |         m = 5;
-   |         ^^^^^
+   |         ^
    |
    = note: `-D clippy::mut-range-bound` implied by `-D warnings`
 
@@ -10,19 +10,19 @@ error: attempt to mutate range bound within loop; note that the range of the loo
   --> $DIR/mut_range_bound.rs:23:9
    |
 LL |         m *= 2;
-   |         ^^^^^^
+   |         ^
 
 error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
   --> $DIR/mut_range_bound.rs:31:9
    |
 LL |         m = 5;
-   |         ^^^^^
+   |         ^
 
 error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
   --> $DIR/mut_range_bound.rs:32:9
    |
 LL |         n = 7;
-   |         ^^^^^
+   |         ^
 
 error: attempt to mutate range bound within loop; note that the range of the loop is unchanged
   --> $DIR/mut_range_bound.rs:46:22
diff --git a/tests/ui/needless_pass_by_value.stderr b/tests/ui/needless_pass_by_value.stderr
index 5efeea0685c..3cb64a227f1 100644
--- a/tests/ui/needless_pass_by_value.stderr
+++ b/tests/ui/needless_pass_by_value.stderr
@@ -28,12 +28,7 @@ error: this argument is passed by value, but not consumed in the function body
   --> $DIR/needless_pass_by_value.rs:49:18
    |
 LL | fn test_match(x: Option<Option<String>>, y: Option<Option<String>>) {
-   |                  ^^^^^^^^^^^^^^^^^^^^^^
-help: consider taking a reference instead
-   |
-LL | fn test_match(x: &Option<Option<String>>, y: Option<Option<String>>) {
-LL |     match *x {
-   |
+   |                  ^^^^^^^^^^^^^^^^^^^^^^ help: consider taking a reference instead: `&Option<Option<String>>`
 
 error: this argument is passed by value, but not consumed in the function body
   --> $DIR/needless_pass_by_value.rs:62:24
@@ -45,14 +40,7 @@ error: this argument is passed by value, but not consumed in the function body
   --> $DIR/needless_pass_by_value.rs:62:36
    |
 LL | fn test_destructure(x: Wrapper, y: Wrapper, z: Wrapper) {
-   |                                    ^^^^^^^
-help: consider taking a reference instead
-   |
-LL | fn test_destructure(x: Wrapper, y: &Wrapper, z: Wrapper) {
-LL |     let Wrapper(s) = z; // moved
-LL |     let Wrapper(ref t) = *y; // not moved
-LL |     let Wrapper(_) = *y; // still not moved
-   |
+   |                                    ^^^^^^^ help: consider taking a reference instead: `&Wrapper`
 
 error: this argument is passed by value, but not consumed in the function body
   --> $DIR/needless_pass_by_value.rs:78:49
@@ -152,37 +140,25 @@ error: this argument is passed by value, but not consumed in the function body
   --> $DIR/needless_pass_by_value.rs:131:45
    |
 LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
-   |                                             ^^^^^^^^^^^
+   |                                             ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
    |
 help: consider marking this type as Copy
   --> $DIR/needless_pass_by_value.rs:123:1
    |
 LL | struct CopyWrapper(u32);
    | ^^^^^^^^^^^^^^^^^^^^^^^^
-help: consider taking a reference instead
-   |
-LL | fn test_destructure_copy(x: CopyWrapper, y: &CopyWrapper, z: CopyWrapper) {
-LL |     let CopyWrapper(s) = z; // moved
-LL |     let CopyWrapper(ref t) = *y; // not moved
-LL |     let CopyWrapper(_) = *y; // still not moved
-   |
 
 error: this argument is passed by value, but not consumed in the function body
   --> $DIR/needless_pass_by_value.rs:131:61
    |
 LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: CopyWrapper) {
-   |                                                             ^^^^^^^^^^^
+   |                                                             ^^^^^^^^^^^ help: consider taking a reference instead: `&CopyWrapper`
    |
 help: consider marking this type as Copy
   --> $DIR/needless_pass_by_value.rs:123:1
    |
 LL | struct CopyWrapper(u32);
    | ^^^^^^^^^^^^^^^^^^^^^^^^
-help: consider taking a reference instead
-   |
-LL | fn test_destructure_copy(x: CopyWrapper, y: CopyWrapper, z: &CopyWrapper) {
-LL |     let CopyWrapper(s) = *z; // moved
-   |
 
 error: this argument is passed by value, but not consumed in the function body
   --> $DIR/needless_pass_by_value.rs:143:40