From 0813a12fefd14d6739818753f024348dd361d41d Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Sat, 18 Sep 2021 16:49:25 -0400 Subject: [PATCH] Add lint `ref_mut_iter_method_chain` --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 3 ++ clippy_lints/src/methods/mod.rs | 31 +++++++++++++- .../src/methods/ref_mut_iter_method_chain.rs | 40 +++++++++++++++++++ tests/ui/ref_mut_iter_method_chain.fixed | 34 ++++++++++++++++ tests/ui/ref_mut_iter_method_chain.rs | 34 ++++++++++++++++ tests/ui/ref_mut_iter_method_chain.stderr | 22 ++++++++++ 7 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 clippy_lints/src/methods/ref_mut_iter_method_chain.rs create mode 100644 tests/ui/ref_mut_iter_method_chain.fixed create mode 100644 tests/ui/ref_mut_iter_method_chain.rs create mode 100644 tests/ui/ref_mut_iter_method_chain.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 72fe6ae31735..4363c564d692 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 59d87aa96dcf..939bd561efcb 100644 --- a/clippy_lints/src/lib.rs +++ b/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), diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index e89b2d295b92..28bdd76906ab 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/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::>(); + /// let after_dot = iter.collect::>(); + /// ``` + /// Use instead: + /// ```rust + /// let mut iter = ['a', 'b', '.', 'd'].iter(); + /// let before_dot = iter.by_ref().take_while(|&&c| c != '.').collect::>(); + /// let after_dot = iter.collect::>(); + /// ``` + pub REF_MUT_ITER_METHOD_CHAIN, + style, + "`&mut iter` used in a method chain" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Option, @@ -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 { diff --git a/clippy_lints/src/methods/ref_mut_iter_method_chain.rs b/clippy_lints/src/methods/ref_mut_iter_method_chain.rs new file mode 100644 index 000000000000..a68129317827 --- /dev/null +++ b/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, + ); + } + } +} diff --git a/tests/ui/ref_mut_iter_method_chain.fixed b/tests/ui/ref_mut_iter_method_chain.fixed new file mode 100644 index 000000000000..dc30a16c0e5a --- /dev/null +++ b/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); +} diff --git a/tests/ui/ref_mut_iter_method_chain.rs b/tests/ui/ref_mut_iter_method_chain.rs new file mode 100644 index 000000000000..8414cf8c6039 --- /dev/null +++ b/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); +} diff --git a/tests/ui/ref_mut_iter_method_chain.stderr b/tests/ui/ref_mut_iter_method_chain.stderr new file mode 100644 index 000000000000..8a10888fe669 --- /dev/null +++ b/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 +