Skip to content

Commit

Permalink
Add config flag for reborrows in explicit_iter_loop
Browse files Browse the repository at this point in the history
This commit 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 lint is not enabled.
  • Loading branch information
Benjscho committed Aug 28, 2023
1 parent 8c20739 commit be55a96
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 56 deletions.
3 changes: 2 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit be55a96

Please sign in to comment.