Remove reverse_range_loop lint

Author: ebroto

Diff for: CHANGELOG.md

@@ -1545,7 +1545,6 @@ Released 2018-09-13
 [`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
 [`result_map_unwrap_or_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unwrap_or_else
 [`result_unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_unwrap_used
-[`reverse_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#reverse_range_loop
 [`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
 [`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition
 [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some

Diff for: clippy_lints/src/lib.rs

@@ -624,7 +624,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &loops::NEEDLESS_COLLECT,
         &loops::NEEDLESS_RANGE_LOOP,
         &loops::NEVER_LOOP,
-        &loops::REVERSE_RANGE_LOOP,
         &loops::WHILE_IMMUTABLE_CONDITION,
         &loops::WHILE_LET_LOOP,
         &loops::WHILE_LET_ON_ITERATOR,
@@ -1284,7 +1283,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&loops::NEEDLESS_COLLECT),
         LintId::of(&loops::NEEDLESS_RANGE_LOOP),
         LintId::of(&loops::NEVER_LOOP),
-        LintId::of(&loops::REVERSE_RANGE_LOOP),
         LintId::of(&loops::WHILE_IMMUTABLE_CONDITION),
         LintId::of(&loops::WHILE_LET_LOOP),
         LintId::of(&loops::WHILE_LET_ON_ITERATOR),
@@ -1658,7 +1656,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&loops::FOR_LOOP_OVER_RESULT),
         LintId::of(&loops::ITER_NEXT_LOOP),
         LintId::of(&loops::NEVER_LOOP),
-        LintId::of(&loops::REVERSE_RANGE_LOOP),
         LintId::of(&loops::WHILE_IMMUTABLE_CONDITION),
         LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
         LintId::of(&mem_replace::MEM_REPLACE_WITH_UNINIT),
@@ -1788,6 +1785,10 @@ fn register_removed_non_tool_lints(store: &mut rustc_lint::LintStore) {
         "unsafe_vector_initialization",
         "the replacement suggested by this lint had substantially different behavior",
     );
+    store.register_removed(
+        "reverse_range_loop",
+        "this lint is now included in reversed_empty_ranges",
+    );
 }
 
 /// Register renamed lints.

Diff for: clippy_lints/src/loops.rs

@@ -1,4 +1,4 @@
-use crate::consts::{constant, Constant};
+use crate::consts::constant;
 use crate::reexport::Name;
 use crate::utils::paths;
 use crate::utils::usage::{is_unused, mutated_variables};
@@ -8,7 +8,7 @@ use crate::utils::{
     multispan_sugg, snippet, snippet_opt, snippet_with_applicability, span_lint, span_lint_and_help,
     span_lint_and_sugg, span_lint_and_then, SpanlessEq,
 };
-use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sext, sugg};
+use crate::utils::{is_type_diagnostic_item, qpath_res, same_tys, sugg};
 use if_chain::if_chain;
 use rustc_ast::ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
@@ -270,30 +270,6 @@ declare_clippy_lint! {
     "collecting an iterator when collect is not needed"
 }
 
-declare_clippy_lint! {
-    /// **What it does:** Checks for loops over ranges `x..y` where both `x` and `y`
-    /// are constant and `x` is greater or equal to `y`, unless the range is
-    /// reversed or has a negative `.step_by(_)`.
-    ///
-    /// **Why is it bad?** Such loops will either be skipped or loop until
-    /// wrap-around (in debug code, this may `panic!()`). Both options are probably
-    /// not intended.
-    ///
-    /// **Known problems:** The lint cannot catch loops over dynamically defined
-    /// ranges. Doing this would require simulating all possible inputs and code
-    /// paths through the program, which would be complex and error-prone.
-    ///
-    /// **Example:**
-    /// ```ignore
-    /// for x in 5..10 - 5 {
-    ///     ..
-    /// } // oops, stray `-`
-    /// ```
-    pub REVERSE_RANGE_LOOP,
-    correctness,
-    "iteration over an empty range, such as `10..0` or `5..5`"
-}
-
 declare_clippy_lint! {
     /// **What it does:** Checks `for` loops over slices with an explicit counter
     /// and suggests the use of `.enumerate()`.
@@ -463,7 +439,6 @@ declare_lint_pass!(Loops => [
     FOR_LOOP_OVER_OPTION,
     WHILE_LET_LOOP,
     NEEDLESS_COLLECT,
-    REVERSE_RANGE_LOOP,
     EXPLICIT_COUNTER_LOOP,
     EMPTY_LOOP,
     WHILE_LET_ON_ITERATOR,
@@ -761,7 +736,6 @@ fn check_for_loop<'a, 'tcx>(
     expr: &'tcx Expr<'_>,
 ) {
     check_for_loop_range(cx, pat, arg, body, expr);
-    check_for_loop_reverse_range(cx, arg, expr);
     check_for_loop_arg(cx, pat, arg, expr);
     check_for_loop_explicit_counter(cx, pat, arg, body, expr);
     check_for_loop_over_map_kv(cx, pat, arg, body, expr);
@@ -1248,78 +1222,6 @@ fn is_end_eq_array_len<'tcx>(
     false
 }
 
-fn check_for_loop_reverse_range<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arg: &'tcx Expr<'_>, expr: &'tcx Expr<'_>) {
-    // if this for loop is iterating over a two-sided range...
-    if let Some(higher::Range {
-        start: Some(start),
-        end: Some(end),
-        limits,
-    }) = higher::range(cx, arg)
-    {
-        // ...and both sides are compile-time constant integers...
-        if let Some((start_idx, _)) = constant(cx, cx.tables, start) {
-            if let Some((end_idx, _)) = constant(cx, cx.tables, end) {
-                // ...and the start index is greater than the end index,
-                // this loop will never run. This is often confusing for developers
-                // who think that this will iterate from the larger value to the
-                // smaller value.
-                let ty = cx.tables.expr_ty(start);
-                let (sup, eq) = match (start_idx, end_idx) {
-                    (Constant::Int(start_idx), Constant::Int(end_idx)) => (
-                        match ty.kind {
-                            ty::Int(ity) => sext(cx.tcx, start_idx, ity) > sext(cx.tcx, end_idx, ity),
-                            ty::Uint(_) => start_idx > end_idx,
-                            _ => false,
-                        },
-                        start_idx == end_idx,
-                    ),
-                    _ => (false, false),
-                };
-
-                if sup {
-                    let start_snippet = snippet(cx, start.span, "_");
-                    let end_snippet = snippet(cx, end.span, "_");
-                    let dots = if limits == ast::RangeLimits::Closed {
-                        "..="
-                    } else {
-                        ".."
-                    };
-
-                    span_lint_and_then(
-                        cx,
-                        REVERSE_RANGE_LOOP,
-                        expr.span,
-                        "this range is empty so this for loop will never run",
-                        |diag| {
-                            diag.span_suggestion(
-                                arg.span,
-                                "consider using the following if you are attempting to iterate over this \
-                                 range in reverse",
-                                format!(
-                                    "({end}{dots}{start}).rev()",
-                                    end = end_snippet,
-                                    dots = dots,
-                                    start = start_snippet
-                                ),
-                                Applicability::MaybeIncorrect,
-                            );
-                        },
-                    );
-                } else if eq && limits != ast::RangeLimits::Closed {
-                    // if they are equal, it's also problematic - this loop
-                    // will never run.
-                    span_lint(
-                        cx,
-                        REVERSE_RANGE_LOOP,
-                        expr.span,
-                        "this range is empty so this for loop will never run",
-                    );
-                }
-            }
-        }
-    }
-}
-
 fn lint_iter_method(cx: &LateContext<'_, '_>, args: &[Expr<'_>], arg: &Expr<'_>, method_name: &str) {
     let mut applicability = Applicability::MachineApplicable;
     let object = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);

Diff for: src/lintlist/mod.rs

@@ -1922,11 +1922,11 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         module: "methods",
     },
     Lint {
-        name: "reverse_range_loop",
+        name: "reversed_empty_ranges",
         group: "correctness",
-        desc: "iteration over an empty range, such as `10..0` or `5..5`",
+        desc: "reversing the limits of range expressions, resulting in empty ranges",
         deprecation: None,
-        module: "loops",
+        module: "ranges",
     },
     Lint {
         name: "same_functions_in_if_condition",

Diff for: tests/ui/for_loop_fixable.fixed

@@ -21,7 +21,6 @@ impl Unrelated {
     clippy::explicit_iter_loop,
     clippy::explicit_into_iter_loop,
     clippy::iter_next_loop,
-    clippy::reverse_range_loop,
     clippy::for_kv_map
 )]
 #[allow(
@@ -32,61 +31,8 @@ impl Unrelated {
 )]
 #[allow(clippy::many_single_char_names, unused_variables)]
 fn main() {
-    const MAX_LEN: usize = 42;
     let mut vec = vec![1, 2, 3, 4];
 
-    for i in (0..10).rev() {
-        println!("{}", i);
-    }
-
-    for i in (0..=10).rev() {
-        println!("{}", i);
-    }
-
-    for i in (0..MAX_LEN).rev() {
-        println!("{}", i);
-    }
-
-    for i in 5..=5 {
-        // not an error, this is the range with only one element “5”
-        println!("{}", i);
-    }
-
-    for i in 0..10 {
-        // not an error, the start index is less than the end index
-        println!("{}", i);
-    }
-
-    for i in -10..0 {
-        // not an error
-        println!("{}", i);
-    }
-
-    for i in (10..0).map(|x| x * 2) {
-        // not an error, it can't be known what arbitrary methods do to a range
-        println!("{}", i);
-    }
-
-    // testing that the empty range lint folds constants
-    for i in (5 + 4..10).rev() {
-        println!("{}", i);
-    }
-
-    for i in ((3 - 1)..(5 + 2)).rev() {
-        println!("{}", i);
-    }
-
-    for i in (2 * 2)..(2 * 3) {
-        // no error, 4..6 is fine
-        println!("{}", i);
-    }
-
-    let x = 42;
-    for i in x..10 {
-        // no error, not constant-foldable
-        println!("{}", i);
-    }
-
     // See #601
     for i in 0..10 {
         // no error, id_col does not exist outside the loop

Diff for: tests/ui/for_loop_fixable.rs

@@ -21,7 +21,6 @@ impl Unrelated {
     clippy::explicit_iter_loop,
     clippy::explicit_into_iter_loop,
     clippy::iter_next_loop,
-    clippy::reverse_range_loop,
     clippy::for_kv_map
 )]
 #[allow(
@@ -32,61 +31,8 @@ impl Unrelated {
 )]
 #[allow(clippy::many_single_char_names, unused_variables)]
 fn main() {
-    const MAX_LEN: usize = 42;
     let mut vec = vec![1, 2, 3, 4];
 
-    for i in 10..0 {
-        println!("{}", i);
-    }
-
-    for i in 10..=0 {
-        println!("{}", i);
-    }
-
-    for i in MAX_LEN..0 {
-        println!("{}", i);
-    }
-
-    for i in 5..=5 {
-        // not an error, this is the range with only one element “5”
-        println!("{}", i);
-    }
-
-    for i in 0..10 {
-        // not an error, the start index is less than the end index
-        println!("{}", i);
-    }
-
-    for i in -10..0 {
-        // not an error
-        println!("{}", i);
-    }
-
-    for i in (10..0).map(|x| x * 2) {
-        // not an error, it can't be known what arbitrary methods do to a range
-        println!("{}", i);
-    }
-
-    // testing that the empty range lint folds constants
-    for i in 10..5 + 4 {
-        println!("{}", i);
-    }
-
-    for i in (5 + 2)..(3 - 1) {
-        println!("{}", i);
-    }
-
-    for i in (2 * 2)..(2 * 3) {
-        // no error, 4..6 is fine
-        println!("{}", i);
-    }
-
-    let x = 42;
-    for i in x..10 {
-        // no error, not constant-foldable
-        println!("{}", i);
-    }
-
     // See #601
     for i in 0..10 {
         // no error, id_col does not exist outside the loop