Skip to content

Commit

Permalink
Add linter for a single element for loop
Browse files Browse the repository at this point in the history
Signed-off-by: Patrick José Pereira <patrickelectric@gmail.com>
  • Loading branch information
patrickelectric committed Oct 19, 2020
1 parent 3d35072 commit ec23db9
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -1936,6 +1936,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
Expand Down
3 changes: 3 additions & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -633,6 +633,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,
Expand Down Expand Up @@ -1363,6 +1364,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),
Expand Down Expand Up @@ -1664,6 +1666,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),
Expand Down
71 changes: 68 additions & 3 deletions clippy_lints/src/loops.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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>,
Expand Down
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
Expand Up @@ -2125,6 +2125,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",
Expand Down
11 changes: 11 additions & 0 deletions 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);
}
}
10 changes: 10 additions & 0 deletions 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);
}
}
19 changes: 19 additions & 0 deletions 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

0 comments on commit ec23db9

Please sign in to comment.