Add lint ref_mut_iter_method_chain

Author: Jarcho

CHANGELOG.md

@@ -2895,6 +2895,7 @@ Released 2018-09-13
 [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
 [`ref_binding_to_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_binding_to_reference
 [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
+[`ref_mut_iter_method_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_mut_iter_method_chain
 [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref
 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
 [`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once

clippy_lints/src/lib.rs

@@ -792,6 +792,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         methods::OPTION_FILTER_MAP,
         methods::OPTION_MAP_OR_NONE,
         methods::OR_FUN_CALL,
+        methods::REF_MUT_ITER_METHOD_CHAIN,
         methods::RESULT_MAP_OR_INTO_OPTION,
         methods::SEARCH_IS_SOME,
         methods::SHOULD_IMPLEMENT_TRAIT,
@@ -1345,6 +1346,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(methods::OPTION_FILTER_MAP),
         LintId::of(methods::OPTION_MAP_OR_NONE),
         LintId::of(methods::OR_FUN_CALL),
+        LintId::of(methods::REF_MUT_ITER_METHOD_CHAIN),
         LintId::of(methods::RESULT_MAP_OR_INTO_OPTION),
         LintId::of(methods::SEARCH_IS_SOME),
         LintId::of(methods::SHOULD_IMPLEMENT_TRAIT),
@@ -1547,6 +1549,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(methods::NEW_RET_NO_SELF),
         LintId::of(methods::OK_EXPECT),
         LintId::of(methods::OPTION_MAP_OR_NONE),
+        LintId::of(methods::REF_MUT_ITER_METHOD_CHAIN),
         LintId::of(methods::RESULT_MAP_OR_INTO_OPTION),
         LintId::of(methods::SHOULD_IMPLEMENT_TRAIT),
         LintId::of(methods::SINGLE_CHAR_ADD_STR),

clippy_lints/src/methods/mod.rs

@@ -44,6 +44,7 @@ mod option_as_ref_deref;
 mod option_map_or_none;
 mod option_map_unwrap_or;
 mod or_fun_call;
+mod ref_mut_iter_method_chain;
 mod search_is_some;
 mod single_char_add_str;
 mod single_char_insert_string;
@@ -1796,6 +1797,30 @@ declare_clippy_lint! {
     "replace `.splitn(2, pat)` with `.split_once(pat)`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Check for `&mut iter` followed by a method call.
+    ///
+    /// ### Why is this bad?
+    /// This requires using parenthesis to signify precedence.
+    ///
+    /// ### Example
+    /// ```rust
+    /// let mut iter = ['a', 'b', '.', 'd'].iter();
+    /// let before_dot = (&mut iter).take_while(|&&c| c != '.').collect::<Vec<_>>();
+    /// let after_dot = iter.collect::<Vec<_>>();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// let mut iter = ['a', 'b', '.', 'd'].iter();
+    /// let before_dot = iter.by_ref().take_while(|&&c| c != '.').collect::<Vec<_>>();
+    /// let after_dot = iter.collect::<Vec<_>>();
+    /// ```
+    pub REF_MUT_ITER_METHOD_CHAIN,
+    style,
+    "`&mut iter` used in a method chain"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Option<RustcVersion>,
@@ -1874,7 +1899,8 @@ impl_lint_pass!(Methods => [
     SUSPICIOUS_SPLITN,
     MANUAL_STR_REPEAT,
     EXTEND_WITH_DRAIN,
-    MANUAL_SPLIT_ONCE
+    MANUAL_SPLIT_ONCE,
+    REF_MUT_ITER_METHOD_CHAIN
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -1908,7 +1934,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
             hir::ExprKind::Call(func, args) => {
                 from_iter_instead_of_collect::check(cx, expr, args, func);
             },
-            hir::ExprKind::MethodCall(method_call, ref method_span, args, _) => {
+            hir::ExprKind::MethodCall(method_call, ref method_span, args @ [self_arg, ..], _) => {
                 or_fun_call::check(cx, expr, *method_span, &method_call.ident.as_str(), args);
                 expect_fun_call::check(cx, expr, *method_span, &method_call.ident.as_str(), args);
                 clone_on_copy::check(cx, expr, method_call.ident.name, args);
@@ -1917,6 +1943,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
                 single_char_add_str::check(cx, expr, args);
                 into_iter_on_ref::check(cx, expr, *method_span, method_call.ident.name, args);
                 single_char_pattern::check(cx, expr, method_call.ident.name, args);
+                ref_mut_iter_method_chain::check(cx, self_arg);
             },
             hir::ExprKind::Binary(op, lhs, rhs) if op.node == hir::BinOpKind::Eq || op.node == hir::BinOpKind::Ne => {
                 let mut info = BinaryExprInfo {

clippy_lints/src/methods/ref_mut_iter_method_chain.rs

@@ -0,0 +1,40 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::in_macro;
+use clippy_utils::source::snippet_with_context;
+use clippy_utils::ty::implements_trait;
+use if_chain::if_chain;
+use rustc_errors::Applicability;
+use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, UnOp};
+use rustc_lint::LateContext;
+use rustc_span::sym;
+
+use super::REF_MUT_ITER_METHOD_CHAIN;
+
+pub(crate) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>) {
+    if_chain! {
+        if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, base_expr) = self_arg.kind;
+        if !in_macro(self_arg.span);
+        if let Some(&iter_trait) = cx.tcx.all_diagnostic_items(()).get(&sym::Iterator);
+        if implements_trait(cx, cx.typeck_results().expr_ty(base_expr).peel_refs(), iter_trait, &[]);
+        then {
+            let snip_span = match base_expr.kind {
+                ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty(e).is_ref() && !in_macro(base_expr.span)
+                    => e.span,
+                _ => base_expr.span,
+            };
+            let mut app = Applicability::MachineApplicable;
+            span_lint_and_sugg(
+                cx,
+                REF_MUT_ITER_METHOD_CHAIN,
+                self_arg.span,
+                "use of `&mut` on an iterator in a method chain",
+                "try",
+                format!(
+                    "{}.by_ref()",
+                    snippet_with_context(cx, snip_span, self_arg.span.ctxt(), "..", &mut app).0,
+                ),
+                app,
+            );
+        }
+    }
+}

tests/ui/ref_mut_iter_method_chain.fixed

@@ -0,0 +1,34 @@
+// run-rustfix
+#![warn(clippy::ref_mut_iter_method_chain)]
+
+macro_rules! m {
+    ($i:ident) => {
+        $i
+    };
+    (&mut $i:ident) => {
+        &mut $i
+    };
+    (($i:expr).$m:ident($arg:expr)) => {
+        ($i).$m($arg)
+    };
+}
+
+fn main() {
+    let mut iter = [0, 1, 2].iter();
+    let _ = iter.by_ref().find(|&&x| x == 1);
+    let _ = m!(iter).by_ref().find(|&&x| x == 1);
+
+    // Don't lint. `&mut` comes from macro expansion.
+    let _ = m!(&mut iter).find(|&&x| x == 1);
+
+    // Don't lint. Method call from expansion
+    let _ = m!((&mut iter).find(|&&x| x == 1));
+
+    // Don't lint. No method chain.
+    for &x in &mut iter {
+        print!("{}", x)
+    }
+
+    let iter = &mut iter;
+    iter.by_ref().find(|&&x| x == 1);
+}

tests/ui/ref_mut_iter_method_chain.rs

@@ -0,0 +1,34 @@
+// run-rustfix
+#![warn(clippy::ref_mut_iter_method_chain)]
+
+macro_rules! m {
+    ($i:ident) => {
+        $i
+    };
+    (&mut $i:ident) => {
+        &mut $i
+    };
+    (($i:expr).$m:ident($arg:expr)) => {
+        ($i).$m($arg)
+    };
+}
+
+fn main() {
+    let mut iter = [0, 1, 2].iter();
+    let _ = (&mut iter).find(|&&x| x == 1);
+    let _ = (&mut m!(iter)).find(|&&x| x == 1);
+
+    // Don't lint. `&mut` comes from macro expansion.
+    let _ = m!(&mut iter).find(|&&x| x == 1);
+
+    // Don't lint. Method call from expansion
+    let _ = m!((&mut iter).find(|&&x| x == 1));
+
+    // Don't lint. No method chain.
+    for &x in &mut iter {
+        print!("{}", x)
+    }
+
+    let iter = &mut iter;
+    (&mut *iter).find(|&&x| x == 1);
+}

tests/ui/ref_mut_iter_method_chain.stderr

@@ -0,0 +1,22 @@
+error: use of `&mut` on an iterator in a method chain
+  --> $DIR/ref_mut_iter_method_chain.rs:18:13
+   |
+LL |     let _ = (&mut iter).find(|&&x| x == 1);
+   |             ^^^^^^^^^^^ help: try: `iter.by_ref()`
+   |
+   = note: `-D clippy::ref-mut-iter-method-chain` implied by `-D warnings`
+
+error: use of `&mut` on an iterator in a method chain
+  --> $DIR/ref_mut_iter_method_chain.rs:19:13
+   |
+LL |     let _ = (&mut m!(iter)).find(|&&x| x == 1);
+   |             ^^^^^^^^^^^^^^^ help: try: `m!(iter).by_ref()`
+
+error: use of `&mut` on an iterator in a method chain
+  --> $DIR/ref_mut_iter_method_chain.rs:33:5
+   |
+LL |     (&mut *iter).find(|&&x| x == 1);
+   |     ^^^^^^^^^^^^ help: try: `iter.by_ref()`
+
+error: aborting due to 3 previous errors
+