Skip to content

Commit e0e617a

Browse files
borsbors
committed
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
2 parents a675778 + ba1ca19 commit e0e617a

9 files changed

+133
-16
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -1938,6 +1938,7 @@ Released 2018-09-13
19381938
[`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
19391939
[`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str
19401940
[`single_component_path_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports
1941+
[`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop
19411942
[`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
19421943
[`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
19431944
[`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next

‎clippy_lints/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
634634
&loops::NEEDLESS_RANGE_LOOP,
635635
&loops::NEVER_LOOP,
636636
&loops::SAME_ITEM_PUSH,
637+
&loops::SINGLE_ELEMENT_LOOP,
637638
&loops::WHILE_IMMUTABLE_CONDITION,
638639
&loops::WHILE_LET_LOOP,
639640
&loops::WHILE_LET_ON_ITERATOR,
@@ -1368,6 +1369,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13681369
LintId::of(&loops::NEEDLESS_RANGE_LOOP),
13691370
LintId::of(&loops::NEVER_LOOP),
13701371
LintId::of(&loops::SAME_ITEM_PUSH),
1372+
LintId::of(&loops::SINGLE_ELEMENT_LOOP),
13711373
LintId::of(&loops::WHILE_IMMUTABLE_CONDITION),
13721374
LintId::of(&loops::WHILE_LET_LOOP),
13731375
LintId::of(&loops::WHILE_LET_ON_ITERATOR),
@@ -1669,6 +1671,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16691671
LintId::of(&lifetimes::NEEDLESS_LIFETIMES),
16701672
LintId::of(&loops::EXPLICIT_COUNTER_LOOP),
16711673
LintId::of(&loops::MUT_RANGE_BOUND),
1674+
LintId::of(&loops::SINGLE_ELEMENT_LOOP),
16721675
LintId::of(&loops::WHILE_LET_LOOP),
16731676
LintId::of(&manual_strip::MANUAL_STRIP),
16741677
LintId::of(&manual_unwrap_or::MANUAL_UNWRAP_OR),

‎clippy_lints/src/loops.rs

+68-3
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@ use crate::utils::sugg::Sugg;
44
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,
7-
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,
7+
indent_of, is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment,
8+
match_trait_method, match_type, match_var, multispan_sugg, qpath_res, single_segment_path, snippet,
9+
snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
10+
span_lint_and_then, sugg, 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 {
@@ -777,6 +804,7 @@ fn check_for_loop<'tcx>(
777804
check_for_loop_arg(cx, pat, arg, expr);
778805
check_for_loop_over_map_kv(cx, pat, arg, body, expr);
779806
check_for_mut_range_bound(cx, arg, body);
807+
check_for_single_element_loop(cx, pat, arg, body, expr);
780808
detect_same_item_push(cx, pat, arg, body, expr);
781809
}
782810

@@ -1866,6 +1894,43 @@ fn check_for_loop_over_map_kv<'tcx>(
18661894
}
18671895
}
18681896

1897+
fn check_for_single_element_loop<'tcx>(
1898+
cx: &LateContext<'tcx>,
1899+
pat: &'tcx Pat<'_>,
1900+
arg: &'tcx Expr<'_>,
1901+
body: &'tcx Expr<'_>,
1902+
expr: &'tcx Expr<'_>,
1903+
) {
1904+
if_chain! {
1905+
if let ExprKind::AddrOf(BorrowKind::Ref, _, ref arg_expr) = arg.kind;
1906+
if let PatKind::Binding(.., target, _) = pat.kind;
1907+
if let ExprKind::Array(ref arg_expr_list) = arg_expr.kind;
1908+
if let [arg_expression] = arg_expr_list;
1909+
if let ExprKind::Path(ref list_item) = arg_expression.kind;
1910+
if let Some(list_item_name) = single_segment_path(list_item).map(|ps| ps.ident.name);
1911+
if let ExprKind::Block(ref block, _) = body.kind;
1912+
if !block.stmts.is_empty();
1913+
1914+
then {
1915+
let for_span = get_span_of_entire_for_loop(expr);
1916+
let mut block_str = snippet(cx, block.span, "..").into_owned();
1917+
block_str.remove(0);
1918+
block_str.pop();
1919+
1920+
1921+
span_lint_and_sugg(
1922+
cx,
1923+
SINGLE_ELEMENT_LOOP,
1924+
for_span,
1925+
"for loop over a single element",
1926+
"try",
1927+
format!("{{\n{}let {} = &{};{}}}", " ".repeat(indent_of(cx, block.stmts[0].span).unwrap_or(0)), target.name, list_item_name, block_str),
1928+
Applicability::MachineApplicable
1929+
)
1930+
}
1931+
}
1932+
}
1933+
18691934
struct MutatePairDelegate<'a, 'tcx> {
18701935
cx: &'a LateContext<'tcx>,
18711936
hir_id_low: Option<HirId>,

‎src/lintlist/mod.rs

+7
Original file line numberDiff line numberDiff line change
@@ -2139,6 +2139,13 @@ vec![
21392139
deprecation: None,
21402140
module: "single_component_path_imports",
21412141
},
2142+
Lint {
2143+
name: "single_element_loop",
2144+
group: "complexity",
2145+
desc: "there is no reason to have a single element loop",
2146+
deprecation: None,
2147+
module: "loops",
2148+
},
21422149
Lint {
21432150
name: "single_match",
21442151
group: "style",

‎tests/ui/if_same_then_else2.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
clippy::blacklisted_name,
44
clippy::collapsible_if,
55
clippy::ifs_same_cond,
6-
clippy::needless_return
6+
clippy::needless_return,
7+
clippy::single_element_loop
78
)]
89

910
fn if_same_then_else2() -> Result<&'static str, ()> {

‎tests/ui/if_same_then_else2.stderr

+12-12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `if` has identical blocks
2-
--> $DIR/if_same_then_else2.rs:19:12
2+
--> $DIR/if_same_then_else2.rs:20:12
33
|
44
LL | } else {
55
| ____________^
@@ -13,7 +13,7 @@ LL | | }
1313
|
1414
= note: `-D clippy::if-same-then-else` implied by `-D warnings`
1515
note: same as this
16-
--> $DIR/if_same_then_else2.rs:10:13
16+
--> $DIR/if_same_then_else2.rs:11:13
1717
|
1818
LL | if true {
1919
| _____________^
@@ -26,7 +26,7 @@ LL | | } else {
2626
| |_____^
2727

2828
error: this `if` has identical blocks
29-
--> $DIR/if_same_then_else2.rs:33:12
29+
--> $DIR/if_same_then_else2.rs:34:12
3030
|
3131
LL | } else {
3232
| ____________^
@@ -36,7 +36,7 @@ LL | | }
3636
| |_____^
3737
|
3838
note: same as this
39-
--> $DIR/if_same_then_else2.rs:31:13
39+
--> $DIR/if_same_then_else2.rs:32:13
4040
|
4141
LL | if true {
4242
| _____________^
@@ -45,7 +45,7 @@ LL | | } else {
4545
| |_____^
4646

4747
error: this `if` has identical blocks
48-
--> $DIR/if_same_then_else2.rs:40:12
48+
--> $DIR/if_same_then_else2.rs:41:12
4949
|
5050
LL | } else {
5151
| ____________^
@@ -55,7 +55,7 @@ LL | | }
5555
| |_____^
5656
|
5757
note: same as this
58-
--> $DIR/if_same_then_else2.rs:38:13
58+
--> $DIR/if_same_then_else2.rs:39:13
5959
|
6060
LL | if true {
6161
| _____________^
@@ -64,7 +64,7 @@ LL | | } else {
6464
| |_____^
6565

6666
error: this `if` has identical blocks
67-
--> $DIR/if_same_then_else2.rs:90:12
67+
--> $DIR/if_same_then_else2.rs:91:12
6868
|
6969
LL | } else {
7070
| ____________^
@@ -74,7 +74,7 @@ LL | | };
7474
| |_____^
7575
|
7676
note: same as this
77-
--> $DIR/if_same_then_else2.rs:88:21
77+
--> $DIR/if_same_then_else2.rs:89:21
7878
|
7979
LL | let _ = if true {
8080
| _____________________^
@@ -83,7 +83,7 @@ LL | | } else {
8383
| |_____^
8484

8585
error: this `if` has identical blocks
86-
--> $DIR/if_same_then_else2.rs:97:12
86+
--> $DIR/if_same_then_else2.rs:98:12
8787
|
8888
LL | } else {
8989
| ____________^
@@ -93,7 +93,7 @@ LL | | }
9393
| |_____^
9494
|
9595
note: same as this
96-
--> $DIR/if_same_then_else2.rs:95:13
96+
--> $DIR/if_same_then_else2.rs:96:13
9797
|
9898
LL | if true {
9999
| _____________^
@@ -102,7 +102,7 @@ LL | | } else {
102102
| |_____^
103103

104104
error: this `if` has identical blocks
105-
--> $DIR/if_same_then_else2.rs:122:12
105+
--> $DIR/if_same_then_else2.rs:123:12
106106
|
107107
LL | } else {
108108
| ____________^
@@ -112,7 +112,7 @@ LL | | }
112112
| |_____^
113113
|
114114
note: same as this
115-
--> $DIR/if_same_then_else2.rs:119:20
115+
--> $DIR/if_same_then_else2.rs:120:20
116116
|
117117
LL | } else if true {
118118
| ____________________^

‎tests/ui/single_element_loop.fixed

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

‎tests/ui/single_element_loop.rs

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

‎tests/ui/single_element_loop.stderr

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
error: for loop over a single element
2+
--> $DIR/single_element_loop.rs:7:5
3+
|
4+
LL | / for item in &[item1] {
5+
LL | | println!("{}", item);
6+
LL | | }
7+
| |_____^
8+
|
9+
= note: `-D clippy::single-element-loop` implied by `-D warnings`
10+
help: try
11+
|
12+
LL | {
13+
LL | let item = &item1;
14+
LL | println!("{}", item);
15+
LL | }
16+
|
17+
18+
error: aborting due to previous error
19+

0 commit comments

Comments
 (0)