Lint flatten() under lines_filter_map_ok

This commit is contained in:
sjwang05 2023-10-20 10:19:46 -07:00
parent 902c79c654
commit 39eded7b05
No known key found for this signature in database
GPG Key ID: AB262FD6FFBFCFFE
4 changed files with 71 additions and 34 deletions

View File

@ -53,40 +53,41 @@ declare_clippy_lint! {
#[clippy::version = "1.70.0"] #[clippy::version = "1.70.0"]
pub LINES_FILTER_MAP_OK, pub LINES_FILTER_MAP_OK,
suspicious, suspicious,
"filtering `std::io::Lines` with `filter_map()` or `flat_map()` might cause an infinite loop" "filtering `std::io::Lines` with `filter_map()`, `flat_map()`, or `flatten()` might cause an infinite loop"
} }
declare_lint_pass!(LinesFilterMapOk => [LINES_FILTER_MAP_OK]); declare_lint_pass!(LinesFilterMapOk => [LINES_FILTER_MAP_OK]);
impl LateLintPass<'_> for LinesFilterMapOk { impl LateLintPass<'_> for LinesFilterMapOk {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
if let ExprKind::MethodCall(fm_method, fm_receiver, [fm_arg], fm_span) = expr.kind if let ExprKind::MethodCall(fm_method, fm_receiver, fm_args, fm_span) = expr.kind &&
&& is_trait_method(cx, expr, sym::Iterator) is_trait_method(cx, expr, sym::Iterator) &&
&& (fm_method.ident.as_str() == "filter_map" || fm_method.ident.as_str() == "flat_map") let fm_method_str = fm_method.ident.as_str() &&
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty_adjusted(fm_receiver), sym::IoLines) matches!(fm_method_str, "filter_map" | "flat_map" | "flatten") &&
is_type_diagnostic_item(cx, cx.typeck_results().expr_ty_adjusted(fm_receiver), sym::IoLines)
{ {
let lint = match &fm_arg.kind { let lint = if let [fm_arg] = fm_args {
match &fm_arg.kind {
// Detect `Result::ok` // Detect `Result::ok`
ExprKind::Path(qpath) => cx ExprKind::Path(qpath) =>
.qpath_res(qpath, fm_arg.hir_id) cx.qpath_res(qpath, fm_arg.hir_id).opt_def_id().map(|did|
.opt_def_id() match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD)).unwrap_or_default(),
.map(|did| match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD))
.unwrap_or_default(),
// Detect `|x| x.ok()` // Detect `|x| x.ok()`
ExprKind::Closure(Closure { body, .. }) => { ExprKind::Closure(Closure { body, .. }) =>
if let Body { if let Body { params: [param], value, .. } = cx.tcx.hir().body(*body) &&
params: [param], value, .. let ExprKind::MethodCall(method, receiver, [], _) = value.kind &&
} = cx.tcx.hir().body(*body) path_to_local_id(receiver, param.pat.hir_id) &&
&& let ExprKind::MethodCall(method, receiver, [], _) = value.kind let Some(method_did) = cx.typeck_results().type_dependent_def_id(value.hir_id)
&& path_to_local_id(receiver, param.pat.hir_id)
&& let Some(method_did) = cx.typeck_results().type_dependent_def_id(value.hir_id)
{ {
is_diag_item_method(cx, method_did, sym::Result) && method.ident.as_str() == "ok" is_diag_item_method(cx, method_did, sym::Result) && method.ident.as_str() == "ok"
} else { } else {
false false
}
}, },
_ => false, _ => false,
}
} else {
fm_method_str == "flatten"
}; };
if lint { if lint {
span_lint_and_then( span_lint_and_then(
cx, cx,

View File

@ -10,11 +10,17 @@ fn main() -> io::Result<()> {
// Lint // Lint
let f = std::fs::File::open("/")?; let f = std::fs::File::open("/")?;
BufReader::new(f).lines().map_while(Result::ok).for_each(|_| ()); BufReader::new(f).lines().map_while(Result::ok).for_each(|_| ());
// Lint
let f = std::fs::File::open("/")?;
BufReader::new(f).lines().map_while(Result::ok).for_each(|_| ());
let s = "foo\nbar\nbaz\n"; let s = "foo\nbar\nbaz\n";
// Lint // Lint
io::stdin().lines().map_while(Result::ok).for_each(|_| ()); io::stdin().lines().map_while(Result::ok).for_each(|_| ());
// Lint // Lint
io::stdin().lines().map_while(Result::ok).for_each(|_| ()); io::stdin().lines().map_while(Result::ok).for_each(|_| ());
// Lint
io::stdin().lines().map_while(Result::ok).for_each(|_| ());
// Do not lint (not a `Lines` iterator) // Do not lint (not a `Lines` iterator)
io::stdin() io::stdin()
.lines() .lines()

View File

@ -10,11 +10,17 @@ fn main() -> io::Result<()> {
// Lint // Lint
let f = std::fs::File::open("/")?; let f = std::fs::File::open("/")?;
BufReader::new(f).lines().flat_map(Result::ok).for_each(|_| ()); BufReader::new(f).lines().flat_map(Result::ok).for_each(|_| ());
// Lint
let f = std::fs::File::open("/")?;
BufReader::new(f).lines().flatten().for_each(|_| ());
let s = "foo\nbar\nbaz\n"; let s = "foo\nbar\nbaz\n";
// Lint // Lint
io::stdin().lines().filter_map(Result::ok).for_each(|_| ()); io::stdin().lines().filter_map(Result::ok).for_each(|_| ());
// Lint // Lint
io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ()); io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ());
// Lint
io::stdin().lines().flatten().for_each(|_| ());
// Do not lint (not a `Lines` iterator) // Do not lint (not a `Lines` iterator)
io::stdin() io::stdin()
.lines() .lines()

View File

@ -24,29 +24,53 @@ note: this expression returning a `std::io::Lines` may produce an infinite numbe
LL | BufReader::new(f).lines().flat_map(Result::ok).for_each(|_| ()); LL | BufReader::new(f).lines().flat_map(Result::ok).for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^
error: `filter_map()` will run forever if the iterator repeatedly produces an `Err` error: `flatten()` will run forever if the iterator repeatedly produces an `Err`
--> $DIR/lines_filter_map_ok.rs:15:25 --> $DIR/lines_filter_map_ok.rs:15:31
| |
LL | io::stdin().lines().filter_map(Result::ok).for_each(|_| ()); LL | BufReader::new(f).lines().flatten().for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `map_while(Result::ok)` | ^^^^^^^^^ help: replace with: `map_while(Result::ok)`
| |
note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error
--> $DIR/lines_filter_map_ok.rs:15:5 --> $DIR/lines_filter_map_ok.rs:15:5
| |
LL | BufReader::new(f).lines().flatten().for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: `filter_map()` will run forever if the iterator repeatedly produces an `Err`
--> $DIR/lines_filter_map_ok.rs:19:25
|
LL | io::stdin().lines().filter_map(Result::ok).for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `map_while(Result::ok)`
|
note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error
--> $DIR/lines_filter_map_ok.rs:19:5
|
LL | io::stdin().lines().filter_map(Result::ok).for_each(|_| ()); LL | io::stdin().lines().filter_map(Result::ok).for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^
error: `filter_map()` will run forever if the iterator repeatedly produces an `Err` error: `filter_map()` will run forever if the iterator repeatedly produces an `Err`
--> $DIR/lines_filter_map_ok.rs:17:25 --> $DIR/lines_filter_map_ok.rs:21:25
| |
LL | io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ()); LL | io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `map_while(Result::ok)` | ^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `map_while(Result::ok)`
| |
note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error
--> $DIR/lines_filter_map_ok.rs:17:5 --> $DIR/lines_filter_map_ok.rs:21:5
| |
LL | io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ()); LL | io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^
error: aborting due to 4 previous errors error: `flatten()` will run forever if the iterator repeatedly produces an `Err`
--> $DIR/lines_filter_map_ok.rs:23:25
|
LL | io::stdin().lines().flatten().for_each(|_| ());
| ^^^^^^^^^ help: replace with: `map_while(Result::ok)`
|
note: this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error
--> $DIR/lines_filter_map_ok.rs:23:5
|
LL | io::stdin().lines().flatten().for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^
error: aborting due to 6 previous errors