reversed_empty_ranges: don't lint N..N except in for loop arg

1 files changed, 29 insertions, 33 deletions

diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs
index 1eb26d97ed4..45de4d29375 100644
--- a/clippy_lints/src/ranges.rs
+++ b/clippy_lints/src/ranges.rs
@@ -241,14 +241,26 @@ fn check_inclusive_range_minus_one(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
 }
 
 fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
-    fn inside_indexing_expr<'a>(cx: &'a LateContext<'_, '_>, expr: &Expr<'_>) -> Option<&'a Expr<'a>> {
-        match get_parent_expr(cx, expr) {
-            parent_expr @ Some(Expr {
+    fn inside_indexing_expr<'a>(cx: &'a LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
+        matches!(
+            get_parent_expr(cx, expr),
+            Some(Expr {
                 kind: ExprKind::Index(..),
                 ..
-            }) => parent_expr,
-            _ => None,
+            })
+        )
+    }
+
+    fn is_for_loop_arg(cx: &LateContext<'_, '_>, expr: &Expr<'_>) -> bool {
+        let mut cur_expr = expr;
+        while let Some(parent_expr) = get_parent_expr(cx, cur_expr) {
+            match higher::for_loop(parent_expr) {
+                Some((_, args, _)) if args.hir_id == expr.hir_id => return true,
+                _ => cur_expr = parent_expr,
+            }
         }
+
+        false
     }
 
     fn is_empty_range(limits: RangeLimits, ordering: Ordering) -> bool {
@@ -267,34 +279,18 @@ fn check_reversed_empty_range(cx: &LateContext<'_, '_>, expr: &Expr<'_>) {
         if let Some(ordering) = Constant::partial_cmp(cx.tcx, ty, &start_idx, &end_idx);
         if is_empty_range(limits, ordering);
         then {
-            if let Some(parent_expr) = inside_indexing_expr(cx, expr) {
-                let (reason, outcome) = if ordering == Ordering::Equal {
-                    ("empty", "always yield an empty slice")
-                } else {
-                    ("reversed", "panic at run-time")
-                };
-
-                span_lint_and_then(
-                    cx,
-                    REVERSED_EMPTY_RANGES,
-                    expr.span,
-                    &format!("this range is {} and using it to index a slice will {}", reason, outcome),
-                    |diag| {
-                        if_chain! {
-                            if ordering == Ordering::Equal;
-                            if let ty::Slice(slice_ty) = cx.tables.expr_ty(parent_expr).kind;
-                            then {
-                                diag.span_suggestion(
-                                    parent_expr.span,
-                                    "if you want an empty slice, use",
-                                    format!("[] as &[{}]", slice_ty),
-                                    Applicability::MaybeIncorrect
-                                );
-                            }
-                        }
-                    }
-                );
-            } else {
+            if inside_indexing_expr(cx, expr) {
+                // Avoid linting `N..N` as it has proven to be useful, see #5689 and #5628 ...
+                if ordering != Ordering::Equal {
+                    span_lint(
+                        cx,
+                        REVERSED_EMPTY_RANGES,
+                        expr.span,
+                        "this range is reversed and using it to index a slice will panic at run-time",
+                    );
+                }
+            // ... except in for loop arguments for backwards compatibility with `reverse_range_loop`
+            } else if ordering != Ordering::Equal || is_for_loop_arg(cx, expr) {
                 span_lint_and_then(
                     cx,
                     REVERSED_EMPTY_RANGES,