Skip to content

Commit 1bc1552

Browse files
Add linter for a single element for loop
Signed-off-by: Patrick José Pereira <[email protected]>
1 parent 7118d37 commit 1bc1552

File tree

6 files changed

+83
-2
lines changed

6 files changed

+83
-2
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1934,6 +1934,7 @@ Released 2018-09-13
19341934
[`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
19351935
[`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str
19361936
[`single_component_path_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports
1937+
[`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop
19371938
[`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
19381939
[`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
19391940
[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next

clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
631631
&loops::NEEDLESS_RANGE_LOOP,
632632
&loops::NEVER_LOOP,
633633
&loops::SAME_ITEM_PUSH,
634+
&loops::SINGLE_ELEMENT_LOOP,
634635
&loops::WHILE_IMMUTABLE_CONDITION,
635636
&loops::WHILE_LET_LOOP,
636637
&loops::WHILE_LET_ON_ITERATOR,
@@ -1358,6 +1359,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13581359
LintId::of(&loops::NEEDLESS_RANGE_LOOP),
13591360
LintId::of(&loops::NEVER_LOOP),
13601361
LintId::of(&loops::SAME_ITEM_PUSH),
1362+
LintId::of(&loops::SINGLE_ELEMENT_LOOP),
13611363
LintId::of(&loops::WHILE_IMMUTABLE_CONDITION),
13621364
LintId::of(&loops::WHILE_LET_LOOP),
13631365
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
@@ -1657,6 +1659,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16571659
LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
16581660
LintId::of(&loops::EXPLICIT_COUNTER_LOOP),
16591661
LintId::of(&loops::MUT_RANGE_BOUND),
1662+
LintId::of(&loops::SINGLE_ELEMENT_LOOP),
16601663
LintId::of(&loops::WHILE_LET_LOOP),
16611664
LintId::of(&manual_strip::MANUAL_STRIP),
16621665
LintId::of(&map_identity::MAP_IDENTITY),

clippy_lints/src/loops.rs

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@ use crate::utils::usage::{is_unused, mutated_variables};
55
use crate::utils::{
66
contains_name, get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
77
is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment, match_trait_method,
8-
match_type, match_var, multispan_sugg, qpath_res, snippet, snippet_with_applicability, snippet_with_macro_callsite,
9-
span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
8+
match_type, match_var, multispan_sugg, qpath_res, single_segment_path, snippet, snippet_with_applicability,
9+
snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then, sugg,
10+
SpanlessEq,
1011
};
1112
use if_chain::if_chain;
1213
use rustc_ast::ast;
@@ -452,6 +453,31 @@ declare_clippy_lint! {
452453
"the same item is pushed inside of a for loop"
453454
}
454455

456+
declare_clippy_lint! {
457+
/// **What it does:** Checks whether a for loop has a single element.
458+
///
459+
/// **Why is this bad?** There is no reason to have a loop of a
460+
/// single element.
461+
/// **Known problems:** None
462+
///
463+
/// **Example:**
464+
/// ```rust
465+
/// let item1 = 2;
466+
/// for item in &[item1] {
467+
/// println!("{}", item);
468+
/// }
469+
/// ```
470+
/// could be written as
471+
/// ```rust
472+
/// let item1 = 2;
473+
/// let item = &item1;
474+
/// println!("{}", item);
475+
/// ```
476+
pub SINGLE_ELEMENT_LOOP,
477+
complexity,
478+
"there is no reason to have a single element loop"
479+
}
480+
455481
declare_lint_pass!(Loops => [
456482
MANUAL_MEMCPY,
457483
NEEDLESS_RANGE_LOOP,
@@ -469,6 +495,7 @@ declare_lint_pass!(Loops => [
469495
MUT_RANGE_BOUND,
470496
WHILE_IMMUTABLE_CONDITION,
471497
SAME_ITEM_PUSH,
498+
SINGLE_ELEMENT_LOOP,
472499
]);
473500

474501
impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -776,6 +803,7 @@ fn check_for_loop<'tcx>(
776803
check_for_loop_arg(cx, pat, arg, expr);
777804
check_for_loop_over_map_kv(cx, pat, arg, body, expr);
778805
check_for_mut_range_bound(cx, arg, body);
806+
check_for_single_element_loop(cx, pat, arg);
779807
detect_same_item_push(cx, pat, arg, body, expr);
780808
}
781809

@@ -1865,6 +1893,29 @@ fn check_for_loop_over_map_kv<'tcx>(
18651893
}
18661894
}
18671895

1896+
fn check_for_single_element_loop<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, arg: &'tcx Expr<'_>) {
1897+
if_chain! {
1898+
if let ExprKind::AddrOf(BorrowKind::Ref, _, ref expr) = arg.kind;
1899+
if let PatKind::Binding(.., target, _) = pat.kind;
1900+
if let ExprKind::Array(ref expr_list) = expr.kind;
1901+
if expr_list.len() == 1;
1902+
if let ExprKind::Path(ref list_item) = expr_list[0].kind;
1903+
if let Some(list_tem_name) = single_segment_path(list_item).map(|ps| ps.ident.name);
1904+
1905+
then {
1906+
span_lint_and_sugg(
1907+
cx,
1908+
SINGLE_ELEMENT_LOOP,
1909+
expr.span,
1910+
"do not iterate over a single element array",
1911+
"try",
1912+
format!("let {} = &{};", target.name, list_tem_name),
1913+
Applicability::MachineApplicable
1914+
)
1915+
}
1916+
}
1917+
}
1918+
18681919
struct MutatePairDelegate<'a, 'tcx> {
18691920
cx: &'a LateContext<'tcx>,
18701921
hir_id_low: Option<HirId>,

src/lintlist/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2110,6 +2110,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
21102110
deprecation: None,
21112111
module: "single_component_path_imports",
21122112
},
2113+
Lint {
2114+
name: "single_element_loop",
2115+
group: "complexity",
2116+
desc: "there is no reason to have a single element loop",
2117+
deprecation: None,
2118+
module: "loops",
2119+
},
21132120
Lint {
21142121
name: "single_match",
21152122
group: "style",

tests/ui/single_element_loop.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Tests from for_loop.rs that don't have suggestions
2+
3+
#[warn(clippy::single_element_loop)]
4+
fn main() {
5+
let item1 = 2;
6+
for item in &[item1] {
7+
println!("{}", item);
8+
}
9+
}

tests/ui/single_element_loop.stderr

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: do not iterate over a single element array
2+
--> $DIR/single_element_loop.rs:6:18
3+
|
4+
LL | for item in &[item1] {
5+
| ^^^^^^^ help: try: `let item = &item1;`
6+
|
7+
= note: `-D clippy::single-element-loop` implied by `-D warnings`
8+
9+
error: aborting due to previous error
10+

0 commit comments

Comments
 (0)