Skip to content

Commit dbd19f9

Browse files
borsbors
committed
Auto merge of rust-lang#11691 - sjwang05:lines-filter-map-ok-fix, r=Centri3
Lint `flatten()` under `lines_filter_map_ok` Fixes rust-lang#11686 changelog: [`lines_filter_map_ok`]: Also lint calls to `flatten()`
2 parents 9263f80 + 4388158 commit dbd19f9

File tree

4 files changed

+74
-32
lines changed

4 files changed

+74
-32
lines changed

‎clippy_lints/src/lines_filter_map_ok.rs

+33-27
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,45 @@ declare_clippy_lint! {
5353
#[clippy::version = "1.70.0"]
5454
pub LINES_FILTER_MAP_OK,
5555
suspicious,
56-
"filtering `std::io::Lines` with `filter_map()` or `flat_map()` might cause an infinite loop"
56+
"filtering `std::io::Lines` with `filter_map()`, `flat_map()`, or `flatten()` might cause an infinite loop"
5757
}
5858
declare_lint_pass!(LinesFilterMapOk => [LINES_FILTER_MAP_OK]);
5959

6060
impl LateLintPass<'_> for LinesFilterMapOk {
6161
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
62-
if let ExprKind::MethodCall(fm_method, fm_receiver, [fm_arg], fm_span) = expr.kind
62+
if let ExprKind::MethodCall(fm_method, fm_receiver, fm_args, fm_span) = expr.kind
6363
&& is_trait_method(cx, expr, sym::Iterator)
64-
&& (fm_method.ident.as_str() == "filter_map" || fm_method.ident.as_str() == "flat_map")
64+
&& let fm_method_str = fm_method.ident.as_str()
65+
&& matches!(fm_method_str, "filter_map" | "flat_map" | "flatten")
6566
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty_adjusted(fm_receiver), sym::IoLines)
67+
&& should_lint(cx, fm_args, fm_method_str)
6668
{
67-
let lint = match &fm_arg.kind {
69+
span_lint_and_then(
70+
cx,
71+
LINES_FILTER_MAP_OK,
72+
fm_span,
73+
&format!("`{fm_method_str}()` will run forever if the iterator repeatedly produces an `Err`",),
74+
|diag| {
75+
diag.span_note(
76+
fm_receiver.span,
77+
"this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error");
78+
diag.span_suggestion(
79+
fm_span,
80+
"replace with",
81+
"map_while(Result::ok)",
82+
Applicability::MaybeIncorrect,
83+
);
84+
},
85+
);
86+
}
87+
}
88+
}
89+
90+
fn should_lint(cx: &LateContext<'_>, args: &[Expr<'_>], method_str: &str) -> bool {
91+
match args {
92+
[] => method_str == "flatten",
93+
[fm_arg] => {
94+
match &fm_arg.kind {
6895
// Detect `Result::ok`
6996
ExprKind::Path(qpath) => cx
7097
.qpath_res(qpath, fm_arg.hir_id)
@@ -86,29 +113,8 @@ impl LateLintPass<'_> for LinesFilterMapOk {
86113
}
87114
},
88115
_ => false,
89-
};
90-
if lint {
91-
span_lint_and_then(
92-
cx,
93-
LINES_FILTER_MAP_OK,
94-
fm_span,
95-
&format!(
96-
"`{}()` will run forever if the iterator repeatedly produces an `Err`",
97-
fm_method.ident
98-
),
99-
|diag| {
100-
diag.span_note(
101-
fm_receiver.span,
102-
"this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error");
103-
diag.span_suggestion(
104-
fm_span,
105-
"replace with",
106-
"map_while(Result::ok)",
107-
Applicability::MaybeIncorrect,
108-
);
109-
},
110-
);
111116
}
112-
}
117+
},
118+
_ => false,
113119
}
114120
}

‎tests/ui/lines_filter_map_ok.fixed

+6
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,17 @@ fn main() -> io::Result<()> {
1010
// Lint
1111
let f = std::fs::File::open("/")?;
1212
BufReader::new(f).lines().map_while(Result::ok).for_each(|_| ());
13+
// Lint
14+
let f = std::fs::File::open("/")?;
15+
BufReader::new(f).lines().map_while(Result::ok).for_each(|_| ());
16+
1317
let s = "foo\nbar\nbaz\n";
1418
// Lint
1519
io::stdin().lines().map_while(Result::ok).for_each(|_| ());
1620
// Lint
1721
io::stdin().lines().map_while(Result::ok).for_each(|_| ());
22+
// Lint
23+
io::stdin().lines().map_while(Result::ok).for_each(|_| ());
1824
// Do not lint (not a `Lines` iterator)
1925
io::stdin()
2026
.lines()

‎tests/ui/lines_filter_map_ok.rs

+6
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,17 @@ fn main() -> io::Result<()> {
1010
// Lint
1111
let f = std::fs::File::open("/")?;
1212
BufReader::new(f).lines().flat_map(Result::ok).for_each(|_| ());
13+
// Lint
14+
let f = std::fs::File::open("/")?;
15+
BufReader::new(f).lines().flatten().for_each(|_| ());
16+
1317
let s = "foo\nbar\nbaz\n";
1418
// Lint
1519
io::stdin().lines().filter_map(Result::ok).for_each(|_| ());
1620
// Lint
1721
io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ());
22+
// Lint
23+
io::stdin().lines().flatten().for_each(|_| ());
1824
// Do not lint (not a `Lines` iterator)
1925
io::stdin()
2026
.lines()

‎tests/ui/lines_filter_map_ok.stderr

+29-5
Original file line numberDiff line numberDiff line change
@@ -24,29 +24,53 @@ note: this expression returning a `std::io::Lines` may produce an infinite numbe
2424
LL | BufReader::new(f).lines().flat_map(Result::ok).for_each(|_| ());
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^
2626

27+
error: `flatten()` will run forever if the iterator repeatedly produces an `Err`
28+
--> $DIR/lines_filter_map_ok.rs:15:31
29+
|
30+
LL | BufReader::new(f).lines().flatten().for_each(|_| ());
31+
| ^^^^^^^^^ help: replace with: `map_while(Result::ok)`
32+
|
33+
note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error
34+
--> $DIR/lines_filter_map_ok.rs:15:5
35+
|
36+
LL | BufReader::new(f).lines().flatten().for_each(|_| ());
37+
| ^^^^^^^^^^^^^^^^^^^^^^^^^
38+
2739
error: `filter_map()` will run forever if the iterator repeatedly produces an `Err`
28-
--> $DIR/lines_filter_map_ok.rs:15:25
40+
--> $DIR/lines_filter_map_ok.rs:19:25
2941
|
3042
LL | io::stdin().lines().filter_map(Result::ok).for_each(|_| ());
3143
| ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `map_while(Result::ok)`
3244
|
3345
note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error
34-
--> $DIR/lines_filter_map_ok.rs:15:5
46+
--> $DIR/lines_filter_map_ok.rs:19:5
3547
|
3648
LL | io::stdin().lines().filter_map(Result::ok).for_each(|_| ());
3749
| ^^^^^^^^^^^^^^^^^^^
3850

3951
error: `filter_map()` will run forever if the iterator repeatedly produces an `Err`
40-
--> $DIR/lines_filter_map_ok.rs:17:25
52+
--> $DIR/lines_filter_map_ok.rs:21:25
4153
|
4254
LL | io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ());
4355
| ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `map_while(Result::ok)`
4456
|
4557
note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error
46-
--> $DIR/lines_filter_map_ok.rs:17:5
58+
--> $DIR/lines_filter_map_ok.rs:21:5
4759
|
4860
LL | io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ());
4961
| ^^^^^^^^^^^^^^^^^^^
5062

51-
error: aborting due to 4 previous errors
63+
error: `flatten()` will run forever if the iterator repeatedly produces an `Err`
64+
--> $DIR/lines_filter_map_ok.rs:23:25
65+
|
66+
LL | io::stdin().lines().flatten().for_each(|_| ());
67+
| ^^^^^^^^^ help: replace with: `map_while(Result::ok)`
68+
|
69+
note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error
70+
--> $DIR/lines_filter_map_ok.rs:23:5
71+
|
72+
LL | io::stdin().lines().flatten().for_each(|_| ());
73+
| ^^^^^^^^^^^^^^^^^^^
74+
75+
error: aborting due to 6 previous errors
5276

0 commit comments

Comments
 (0)