Auto merge of #11418 - Benjscho:explicit_iter_loop_config, r=llogiq

Add config flag for reborrows in explicit_iter_loop This PR adds a config flag for enforcing explicit into iter lint for reborrowed values. The config flag, `enforce_iter_loop_reborrow`, can be added to clippy.toml files to enable the linting behaviour. By default the reborrow lint is disabled. fixes: #11074 changelog: [`explicit_iter_loop`]: add config flag `enforce_iter_loop_reborrow` to disable reborrow linting by default
rust-lang · Aug 31, 2023 · c50d86f · c50d86f
2 parents 3da21b0 + 55bd0fe
commit c50d86f
Show file tree

Hide file tree

Showing 10 changed files with 93 additions and 56 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5571,4 +5571,5 @@ Released 2018-09-13
 [`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
 [`absolute-paths-max-segments`]: https://doc.rust-lang.org/clippy/lint_configuration.html#absolute-paths-max-segments
 [`absolute-paths-allowed-crates`]: https://doc.rust-lang.org/clippy/lint_configuration.html#absolute-paths-allowed-crates
+[`enforce-iter-loop-reborrow`]: https://doc.rust-lang.org/clippy/lint_configuration.html#enforce-iter-loop-reborrow
 <!-- end autogenerated links to configuration documentation -->
diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md
@@ -751,3 +751,27 @@ Which crates to allow absolute paths from
 * [`absolute_paths`](https://rust-lang.github.io/rust-clippy/master/index.html#absolute_paths)
 
 
+## `enforce-iter-loop-reborrow`
+#### Example
+```
+let mut vec = vec![1, 2, 3];
+let rmvec = &mut vec;
+for _ in rmvec.iter() {}
+for _ in rmvec.iter_mut() {}
+```
+
+Use instead:
+```
+let mut vec = vec![1, 2, 3];
+let rmvec = &mut vec;
+for _ in &*rmvec {}
+for _ in &mut *rmvec {}
+```
+
+**Default Value:** `false` (`bool`)
+
+---
+**Affected lints:**
+* [`explicit_iter_loop`](https://rust-lang.github.io/rust-clippy/master/index.html#explicit_iter_loop)
+
+
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -695,7 +695,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     });
     store.register_late_pass(|_| Box::<shadow::Shadow>::default());
     store.register_late_pass(|_| Box::new(unit_types::UnitTypes));
-    store.register_late_pass(move |_| Box::new(loops::Loops::new(msrv())));
+    let enforce_iter_loop_reborrow = conf.enforce_iter_loop_reborrow;
+    store.register_late_pass(move |_| Box::new(loops::Loops::new(msrv(), enforce_iter_loop_reborrow)));
     store.register_late_pass(|_| Box::<main_recursion::MainRecursion>::default());
     store.register_late_pass(|_| Box::new(lifetimes::Lifetimes));
     store.register_late_pass(|_| Box::new(entry::HashMapPass));

diff --git a/clippy_lints/src/loops/explicit_iter_loop.rs b/clippy_lints/src/loops/explicit_iter_loop.rs
@@ -13,8 +13,14 @@ use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMut
 use rustc_middle::ty::{self, EarlyBinder, Ty, TypeAndMut};
 use rustc_span::sym;
 
-pub(super) fn check(cx: &LateContext<'_>, self_arg: &Expr<'_>, call_expr: &Expr<'_>, msrv: &Msrv) {
-    let Some((adjust, ty)) = is_ref_iterable(cx, self_arg, call_expr) else {
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    self_arg: &Expr<'_>,
+    call_expr: &Expr<'_>,
+    msrv: &Msrv,
+    enforce_iter_loop_reborrow: bool,
+) {
+    let Some((adjust, ty)) = is_ref_iterable(cx, self_arg, call_expr, enforce_iter_loop_reborrow) else {
         return;
     };
     if let ty::Array(_, count) = *ty.peel_refs().kind() {
@@ -102,6 +108,7 @@ fn is_ref_iterable<'tcx>(
     cx: &LateContext<'tcx>,
     self_arg: &Expr<'_>,
     call_expr: &Expr<'_>,
+    enforce_iter_loop_reborrow: bool,
 ) -> Option<(AdjustKind, Ty<'tcx>)> {
     let typeck = cx.typeck_results();
     if let Some(trait_id) = cx.tcx.get_diagnostic_item(sym::IntoIterator)
@@ -142,7 +149,8 @@ fn is_ref_iterable<'tcx>(
                 {
                     return Some((AdjustKind::None, self_ty));
                 }
-            } else if let ty::Ref(region, ty, Mutability::Mut) = *self_ty.kind()
+            } else if enforce_iter_loop_reborrow
+                && let ty::Ref(region, ty, Mutability::Mut) = *self_ty.kind()
                 && let Some(mutbl) = mutbl
             {
                 // Attempt to reborrow the mutable reference
@@ -186,7 +194,8 @@ fn is_ref_iterable<'tcx>(
                 },
                 ..
             ] => {
-                if target != self_ty
+                if enforce_iter_loop_reborrow
+                    && target != self_ty
                     && implements_trait(cx, target, trait_id, &[])
                     && let Some(ty) =
                         make_normalized_projection(cx.tcx, cx.param_env, trait_id, sym!(IntoIter), [target])

diff --git a/clippy_lints/src/loops/mod.rs b/clippy_lints/src/loops/mod.rs
@@ -609,10 +609,14 @@ declare_clippy_lint! {
 
 pub struct Loops {
     msrv: Msrv,
+    enforce_iter_loop_reborrow: bool,
 }
 impl Loops {
-    pub fn new(msrv: Msrv) -> Self {
-        Self { msrv }
+    pub fn new(msrv: Msrv, enforce_iter_loop_reborrow: bool) -> Self {
+        Self {
+            msrv,
+            enforce_iter_loop_reborrow,
+        }
     }
 }
 impl_lint_pass!(Loops => [
@@ -719,7 +723,7 @@ impl Loops {
         if let ExprKind::MethodCall(method, self_arg, [], _) = arg.kind {
             match method.ident.as_str() {
                 "iter" | "iter_mut" => {
-                    explicit_iter_loop::check(cx, self_arg, arg, &self.msrv);
+                    explicit_iter_loop::check(cx, self_arg, arg, &self.msrv, self.enforce_iter_loop_reborrow);
                 },
                 "into_iter" => {
                     explicit_into_iter_loop::check(cx, self_arg, arg);

diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
@@ -561,6 +561,26 @@ define_Conf! {
     /// Which crates to allow absolute paths from
     (absolute_paths_allowed_crates: rustc_data_structures::fx::FxHashSet<String> =
         rustc_data_structures::fx::FxHashSet::default()),
+    /// Lint: EXPLICIT_ITER_LOOP
+    ///
+    /// Whether to recommend using implicit into iter for reborrowed values.
+    ///
+    /// #### Example
+    /// ```
+    /// let mut vec = vec![1, 2, 3];
+    /// let rmvec = &mut vec;
+    /// for _ in rmvec.iter() {}
+    /// for _ in rmvec.iter_mut() {}
+    /// ```
+    ///
+    /// Use instead:
+    /// ```
+    /// let mut vec = vec![1, 2, 3];
+    /// let rmvec = &mut vec;
+    /// for _ in &*rmvec {}
+    /// for _ in &mut *rmvec {}
+    /// ```
+    (enforce_iter_loop_reborrow: bool = false),
 }
 
 /// Search for the configuration file.

diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
@@ -28,6 +28,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            disallowed-types
            doc-valid-idents
            enable-raw-pointer-heuristic-for-send
+           enforce-iter-loop-reborrow
            enforced-import-renames
            enum-variant-name-threshold
            enum-variant-size-threshold
@@ -99,6 +100,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            disallowed-types
            doc-valid-idents
            enable-raw-pointer-heuristic-for-send
+           enforce-iter-loop-reborrow
            enforced-import-renames
            enum-variant-name-threshold
            enum-variant-size-threshold

diff --git a/tests/ui/explicit_iter_loop.fixed b/tests/ui/explicit_iter_loop.fixed
@@ -4,6 +4,7 @@
     clippy::similar_names,
     clippy::needless_borrow,
     clippy::deref_addrof,
+    clippy::unnecessary_mut_passed,
     dead_code
 )]
 
@@ -20,15 +21,15 @@ fn main() {
     for _ in rvec {}
 
     let rmvec = &mut vec;
-    for _ in &*rmvec {}
-    for _ in &mut *rmvec {}
+    for _ in rmvec.iter() {}
+    for _ in rmvec.iter_mut() {}
 
     for _ in &vec {} // these are fine
     for _ in &mut vec {} // these are fine
 
     for _ in &[1, 2, 3] {}
 
-    for _ in &*(&mut [1, 2, 3]) {}
+    for _ in (&mut [1, 2, 3]).iter() {}
 
     for _ in &[0; 32] {}
     for _ in &[0; 33] {}

diff --git a/tests/ui/explicit_iter_loop.rs b/tests/ui/explicit_iter_loop.rs
@@ -4,6 +4,7 @@
     clippy::similar_names,
     clippy::needless_borrow,
     clippy::deref_addrof,
+    clippy::unnecessary_mut_passed,
     dead_code
 )]
 

diff --git a/tests/ui/explicit_iter_loop.stderr b/tests/ui/explicit_iter_loop.stderr
@@ -1,5 +1,5 @@
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:16:14
+  --> $DIR/explicit_iter_loop.rs:17:14
    |
 LL |     for _ in vec.iter() {}
    |              ^^^^^^^^^^ help: to write this more concisely, try: `&vec`
@@ -11,132 +11,106 @@ LL | #![deny(clippy::explicit_iter_loop)]
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:17:14
+  --> $DIR/explicit_iter_loop.rs:18:14
    |
 LL |     for _ in vec.iter_mut() {}
    |              ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut vec`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:20:14
+  --> $DIR/explicit_iter_loop.rs:21:14
    |
 LL |     for _ in rvec.iter() {}
    |              ^^^^^^^^^^^ help: to write this more concisely, try: `rvec`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:23:14
-   |
-LL |     for _ in rmvec.iter() {}
-   |              ^^^^^^^^^^^^ help: to write this more concisely, try: `&*rmvec`
-
-error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:24:14
-   |
-LL |     for _ in rmvec.iter_mut() {}
-   |              ^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut *rmvec`
-
-error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:29:14
+  --> $DIR/explicit_iter_loop.rs:30:14
    |
 LL |     for _ in [1, 2, 3].iter() {}
    |              ^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[1, 2, 3]`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:31:14
-   |
-LL |     for _ in (&mut [1, 2, 3]).iter() {}
-   |              ^^^^^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&*(&mut [1, 2, 3])`
-
-error: the method `iter` doesn't need a mutable reference
-  --> $DIR/explicit_iter_loop.rs:31:14
-   |
-LL |     for _ in (&mut [1, 2, 3]).iter() {}
-   |              ^^^^^^^^^^^^^^^^
-   |
-   = note: `-D clippy::unnecessary-mut-passed` implied by `-D warnings`
-
-error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:33:14
+  --> $DIR/explicit_iter_loop.rs:34:14
    |
 LL |     for _ in [0; 32].iter() {}
    |              ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 32]`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:34:14
+  --> $DIR/explicit_iter_loop.rs:35:14
    |
 LL |     for _ in [0; 33].iter() {}
    |              ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 33]`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:37:14
+  --> $DIR/explicit_iter_loop.rs:38:14
    |
 LL |     for _ in ll.iter() {}
    |              ^^^^^^^^^ help: to write this more concisely, try: `&ll`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:39:14
+  --> $DIR/explicit_iter_loop.rs:40:14
    |
 LL |     for _ in rll.iter() {}
    |              ^^^^^^^^^^ help: to write this more concisely, try: `rll`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:42:14
+  --> $DIR/explicit_iter_loop.rs:43:14
    |
 LL |     for _ in vd.iter() {}
    |              ^^^^^^^^^ help: to write this more concisely, try: `&vd`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:44:14
+  --> $DIR/explicit_iter_loop.rs:45:14
    |
 LL |     for _ in rvd.iter() {}
    |              ^^^^^^^^^^ help: to write this more concisely, try: `rvd`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:47:14
+  --> $DIR/explicit_iter_loop.rs:48:14
    |
 LL |     for _ in bh.iter() {}
    |              ^^^^^^^^^ help: to write this more concisely, try: `&bh`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:50:14
+  --> $DIR/explicit_iter_loop.rs:51:14
    |
 LL |     for _ in hm.iter() {}
    |              ^^^^^^^^^ help: to write this more concisely, try: `&hm`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:53:14
+  --> $DIR/explicit_iter_loop.rs:54:14
    |
 LL |     for _ in bt.iter() {}
    |              ^^^^^^^^^ help: to write this more concisely, try: `&bt`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:56:14
+  --> $DIR/explicit_iter_loop.rs:57:14
    |
 LL |     for _ in hs.iter() {}
    |              ^^^^^^^^^ help: to write this more concisely, try: `&hs`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:59:14
+  --> $DIR/explicit_iter_loop.rs:60:14
    |
 LL |     for _ in bs.iter() {}
    |              ^^^^^^^^^ help: to write this more concisely, try: `&bs`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:148:14
+  --> $DIR/explicit_iter_loop.rs:149:14
    |
 LL |     for _ in x.iter() {}
    |              ^^^^^^^^ help: to write this more concisely, try: `&x`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:149:14
+  --> $DIR/explicit_iter_loop.rs:150:14
    |
 LL |     for _ in x.iter_mut() {}
    |              ^^^^^^^^^^^^ help: to write this more concisely, try: `&mut x`
 
 error: it is more concise to loop over references to containers instead of using explicit iteration methods
-  --> $DIR/explicit_iter_loop.rs:152:14
+  --> $DIR/explicit_iter_loop.rs:153:14
    |
 LL |     for _ in r.iter() {}
    |              ^^^^^^^^ help: to write this more concisely, try: `r`
 
-error: aborting due to 22 previous errors
+error: aborting due to 18 previous errors