Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add config flag for reborrows in explicit_iter_loop #11418

Merged
merged 2 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5570,4 +5570,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 -->
24 changes: 24 additions & 0 deletions book/src/lint_configuration.md
Expand Up @@ -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)


3 changes: 2 additions & 1 deletion clippy_lints/src/lib.rs
Expand Up @@ -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));
Expand Down
17 changes: 13 additions & 4 deletions clippy_lints/src/loops/explicit_iter_loop.rs
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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])
Expand Down
10 changes: 7 additions & 3 deletions clippy_lints/src/loops/mod.rs
Expand Up @@ -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 => [
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 20 additions & 0 deletions clippy_lints/src/utils/conf.rs
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions tests/ui/explicit_iter_loop.fixed
Expand Up @@ -4,6 +4,7 @@
clippy::similar_names,
clippy::needless_borrow,
clippy::deref_addrof,
clippy::unnecessary_mut_passed,
dead_code
)]

Expand All @@ -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] {}
Expand Down
1 change: 1 addition & 0 deletions tests/ui/explicit_iter_loop.rs
Expand Up @@ -4,6 +4,7 @@
clippy::similar_names,
clippy::needless_borrow,
clippy::deref_addrof,
clippy::unnecessary_mut_passed,
dead_code
)]

Expand Down
64 changes: 19 additions & 45 deletions 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`
Expand All @@ -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