Auto merge of #6109 - patrickelectric:single_element_for_check, r=flip1995

Add linter for a single element for loop

changelog: Fixes #1540, check for vectors that contain a single element in a for loop

CHANGELOG.md

@@ -1938,6 +1938,7 @@ Released 2018-09-13
 [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
 [`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str
 [`single_component_path_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports
+[`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop
 [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
 [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
 [`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next

clippy_lints/src/lib.rs

@@ -634,6 +634,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &loops::NEEDLESS_RANGE_LOOP,
         &loops::NEVER_LOOP,
         &loops::SAME_ITEM_PUSH,
+        &loops::SINGLE_ELEMENT_LOOP,
         &loops::WHILE_IMMUTABLE_CONDITION,
         &loops::WHILE_LET_LOOP,
         &loops::WHILE_LET_ON_ITERATOR,
@@ -1368,6 +1369,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&loops::NEEDLESS_RANGE_LOOP),
         LintId::of(&loops::NEVER_LOOP),
         LintId::of(&loops::SAME_ITEM_PUSH),
+        LintId::of(&loops::SINGLE_ELEMENT_LOOP),
         LintId::of(&loops::WHILE_IMMUTABLE_CONDITION),
         LintId::of(&loops::WHILE_LET_LOOP),
         LintId::of(&loops::WHILE_LET_ON_ITERATOR),
@@ -1669,6 +1671,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
         LintId::of(&loops::EXPLICIT_COUNTER_LOOP),
         LintId::of(&loops::MUT_RANGE_BOUND),
+        LintId::of(&loops::SINGLE_ELEMENT_LOOP),
         LintId::of(&loops::WHILE_LET_LOOP),
         LintId::of(&manual_strip::MANUAL_STRIP),
         LintId::of(&manual_unwrap_or::MANUAL_UNWRAP_OR),

clippy_lints/src/loops.rs

@@ -4,9 +4,10 @@ use crate::utils::sugg::Sugg;
 use crate::utils::usage::{is_unused, mutated_variables};
 use crate::utils::{
     contains_name, get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
-    is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment, match_trait_method,
-    match_type, match_var, multispan_sugg, qpath_res, snippet, snippet_with_applicability, snippet_with_macro_callsite,
-    span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
+    indent_of, is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment,
+    match_trait_method, match_type, match_var, multispan_sugg, qpath_res, single_segment_path, snippet,
+    snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
+    span_lint_and_then, sugg, SpanlessEq,
 };
 use if_chain::if_chain;
 use rustc_ast::ast;
@@ -452,6 +453,31 @@ declare_clippy_lint! {
     "the same item is pushed inside of a for loop"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks whether a for loop has a single element.
+    ///
+    /// **Why is this bad?** There is no reason to have a loop of a
+    /// single element.
+    /// **Known problems:** None
+    ///
+    /// **Example:**
+    /// ```rust
+    /// let item1 = 2;
+    /// for item in &[item1] {
+    ///     println!("{}", item);
+    /// }
+    /// ```
+    /// could be written as
+    /// ```rust
+    /// let item1 = 2;
+    /// let item = &item1;
+    /// println!("{}", item);
+    /// ```
+    pub SINGLE_ELEMENT_LOOP,
+    complexity,
+    "there is no reason to have a single element loop"
+}
+
 declare_lint_pass!(Loops => [
     MANUAL_MEMCPY,
     NEEDLESS_RANGE_LOOP,
@@ -469,6 +495,7 @@ declare_lint_pass!(Loops => [
     MUT_RANGE_BOUND,
     WHILE_IMMUTABLE_CONDITION,
     SAME_ITEM_PUSH,
+    SINGLE_ELEMENT_LOOP,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -777,6 +804,7 @@ fn check_for_loop<'tcx>(
     check_for_loop_arg(cx, pat, arg, expr);
     check_for_loop_over_map_kv(cx, pat, arg, body, expr);
     check_for_mut_range_bound(cx, arg, body);
+    check_for_single_element_loop(cx, pat, arg, body, expr);
     detect_same_item_push(cx, pat, arg, body, expr);
 }
 
@@ -1866,6 +1894,43 @@ fn check_for_loop_over_map_kv<'tcx>(
     }
 }
 
+fn check_for_single_element_loop<'tcx>(
+    cx: &LateContext<'tcx>,
+    pat: &'tcx Pat<'_>,
+    arg: &'tcx Expr<'_>,
+    body: &'tcx Expr<'_>,
+    expr: &'tcx Expr<'_>,
+) {
+    if_chain! {
+        if let ExprKind::AddrOf(BorrowKind::Ref, _, ref arg_expr) = arg.kind;
+        if let PatKind::Binding(.., target, _) = pat.kind;
+        if let ExprKind::Array(ref arg_expr_list) = arg_expr.kind;
+        if let [arg_expression] = arg_expr_list;
+        if let ExprKind::Path(ref list_item) = arg_expression.kind;
+        if let Some(list_item_name) = single_segment_path(list_item).map(|ps| ps.ident.name);
+        if let ExprKind::Block(ref block, _) = body.kind;
+        if !block.stmts.is_empty();
+
+        then {
+            let for_span = get_span_of_entire_for_loop(expr);
+            let mut block_str = snippet(cx, block.span, "..").into_owned();
+            block_str.remove(0);
+            block_str.pop();
+
+
+            span_lint_and_sugg(
+                cx,
+                SINGLE_ELEMENT_LOOP,
+                for_span,
+                "for loop over a single element",
+                "try",
+                format!("{{\n{}let {} = &{};{}}}", " ".repeat(indent_of(cx, block.stmts[0].span).unwrap_or(0)), target.name, list_item_name, block_str),
+                Applicability::MachineApplicable
+            )
+        }
+    }
+}
+
 struct MutatePairDelegate<'a, 'tcx> {
     cx: &'a LateContext<'tcx>,
     hir_id_low: Option<HirId>,

src/lintlist/mod.rs

@@ -2139,6 +2139,13 @@ vec![
         deprecation: None,
         module: "single_component_path_imports",
     },
+    Lint {
+        name: "single_element_loop",
+        group: "complexity",
+        desc: "there is no reason to have a single element loop",
+        deprecation: None,
+        module: "loops",
+    },
     Lint {
         name: "single_match",
         group: "style",

tests/ui/if_same_then_else2.rs

@@ -3,7 +3,8 @@
     clippy::blacklisted_name,
     clippy::collapsible_if,
     clippy::ifs_same_cond,
-    clippy::needless_return
+    clippy::needless_return,
+    clippy::single_element_loop
 )]
 
 fn if_same_then_else2() -> Result<&'static str, ()> {

tests/ui/if_same_then_else2.stderr

@@ -1,5 +1,5 @@
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else2.rs:19:12
+  --> $DIR/if_same_then_else2.rs:20:12
    |
 LL |       } else {
    |  ____________^
@@ -13,7 +13,7 @@ LL | |     }
    |
    = note: `-D clippy::if-same-then-else` implied by `-D warnings`
 note: same as this
-  --> $DIR/if_same_then_else2.rs:10:13
+  --> $DIR/if_same_then_else2.rs:11:13
    |
 LL |       if true {
    |  _____________^
@@ -26,7 +26,7 @@ LL | |     } else {
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else2.rs:33:12
+  --> $DIR/if_same_then_else2.rs:34:12
    |
 LL |       } else {
    |  ____________^
@@ -36,7 +36,7 @@ LL | |     }
    | |_____^
    |
 note: same as this
-  --> $DIR/if_same_then_else2.rs:31:13
+  --> $DIR/if_same_then_else2.rs:32:13
    |
 LL |       if true {
    |  _____________^
@@ -45,7 +45,7 @@ LL | |     } else {
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else2.rs:40:12
+  --> $DIR/if_same_then_else2.rs:41:12
    |
 LL |       } else {
    |  ____________^
@@ -55,7 +55,7 @@ LL | |     }
    | |_____^
    |
 note: same as this
-  --> $DIR/if_same_then_else2.rs:38:13
+  --> $DIR/if_same_then_else2.rs:39:13
    |
 LL |       if true {
    |  _____________^
@@ -64,7 +64,7 @@ LL | |     } else {
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else2.rs:90:12
+  --> $DIR/if_same_then_else2.rs:91:12
    |
 LL |       } else {
    |  ____________^
@@ -74,7 +74,7 @@ LL | |     };
    | |_____^
    |
 note: same as this
-  --> $DIR/if_same_then_else2.rs:88:21
+  --> $DIR/if_same_then_else2.rs:89:21
    |
 LL |       let _ = if true {
    |  _____________________^
@@ -83,7 +83,7 @@ LL | |     } else {
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else2.rs:97:12
+  --> $DIR/if_same_then_else2.rs:98:12
    |
 LL |       } else {
    |  ____________^
@@ -93,7 +93,7 @@ LL | |     }
    | |_____^
    |
 note: same as this
-  --> $DIR/if_same_then_else2.rs:95:13
+  --> $DIR/if_same_then_else2.rs:96:13
    |
 LL |       if true {
    |  _____________^
@@ -102,7 +102,7 @@ LL | |     } else {
    | |_____^
 
 error: this `if` has identical blocks
-  --> $DIR/if_same_then_else2.rs:122:12
+  --> $DIR/if_same_then_else2.rs:123:12
    |
 LL |       } else {
    |  ____________^
@@ -112,7 +112,7 @@ LL | |     }
    | |_____^
    |
 note: same as this
-  --> $DIR/if_same_then_else2.rs:119:20
+  --> $DIR/if_same_then_else2.rs:120:20
    |
 LL |       } else if true {
    |  ____________________^

tests/ui/single_element_loop.fixed

@@ -0,0 +1,11 @@
+// run-rustfix
+// Tests from for_loop.rs that don't have suggestions
+
+#[warn(clippy::single_element_loop)]
+fn main() {
+    let item1 = 2;
+    {
+        let item = &item1;
+        println!("{}", item);
+    }
+}

tests/ui/single_element_loop.rs

@@ -0,0 +1,10 @@
+// run-rustfix
+// Tests from for_loop.rs that don't have suggestions
+
+#[warn(clippy::single_element_loop)]
+fn main() {
+    let item1 = 2;
+    for item in &[item1] {
+        println!("{}", item);
+    }
+}

tests/ui/single_element_loop.stderr

@@ -0,0 +1,19 @@
+error: for loop over a single element
+  --> $DIR/single_element_loop.rs:7:5
+   |
+LL | /     for item in &[item1] {
+LL | |         println!("{}", item);
+LL | |     }
+   | |_____^
+   |
+   = note: `-D clippy::single-element-loop` implied by `-D warnings`
+help: try
+   |
+LL |     {
+LL |         let item = &item1;
+LL |         println!("{}", item);
+LL |     }
+   |
+
+error: aborting due to previous error
+