user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(lines_filter_map_ok): move to under methods/ (#15925)

Browse source 
changelog: none
This commit is contained in:
Jason Newcomb 2025-10-26 22:04:43 +00:00 • committed by GitHub
commit 25cbcb4331
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 194 additions and 190 deletions
Download patch file Download diff file Expand all files Collapse all files

2
clippy_lints/src/declared_lints.rs
View file

@ -258,7 +258,6 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::lifetimes::ELIDABLE_LIFETIME_NAMES_INFO,
crate::lifetimes::EXTRA_UNUSED_LIFETIMES_INFO,
crate::lifetimes::NEEDLESS_LIFETIMES_INFO,
crate::lines_filter_map_ok::LINES_FILTER_MAP_OK_INFO,
crate::literal_representation::DECIMAL_LITERAL_REPRESENTATION_INFO,
crate::literal_representation::INCONSISTENT_DIGIT_GROUPING_INFO,
crate::literal_representation::LARGE_DIGIT_GROUPS_INFO,
@ -403,6 +402,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
crate::methods::ITER_SKIP_ZERO_INFO,
crate::methods::ITER_WITH_DRAIN_INFO,
crate::methods::JOIN_ABSOLUTE_PATHS_INFO,
crate::methods::LINES_FILTER_MAP_OK_INFO,
crate::methods::MANUAL_CONTAINS_INFO,
crate::methods::MANUAL_C_STR_LITERALS_INFO,
crate::methods::MANUAL_FILTER_MAP_INFO,

2
clippy_lints/src/lib.rs
View file

@ -191,7 +191,6 @@ mod let_if_seq;
mod let_underscore;
mod let_with_type_underscore;
mod lifetimes;
mod lines_filter_map_ok;
mod literal_representation;
mod literal_string_with_formatting_args;
mod loops;
@ -742,7 +741,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
store.register_late_pass(move |_| Box::new(manual_main_separator_str::ManualMainSeparatorStr::new(conf)));
store.register_late_pass(|_| Box::new(unnecessary_struct_initialization::UnnecessaryStruct));
store.register_late_pass(move |_| Box::new(unnecessary_box_returns::UnnecessaryBoxReturns::new(conf)));
store.register_late_pass(move |_| Box::new(lines_filter_map_ok::LinesFilterMapOk::new(conf)));
store.register_late_pass(|_| Box::new(tests_outside_test_module::TestsOutsideTestModule));
store.register_late_pass(|_| Box::new(manual_slice_size_calculation::ManualSliceSizeCalculation::new(conf)));
store.register_early_pass(move || Box::new(excessive_nesting::ExcessiveNesting::new(conf)));

141
clippy_lints/src/lines_filter_map_ok.rs
View file

@ -1,141 +0,0 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::res::{MaybeDef, MaybeResPath, MaybeTypeckRes};
use clippy_utils::sym;
use rustc_errors::Applicability;
use rustc_hir::{Body, Closure, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::impl_lint_pass;
use rustc_span::Symbol;
pub struct LinesFilterMapOk {
msrv: Msrv,
}
impl LinesFilterMapOk {
pub fn new(conf: &Conf) -> Self {
Self { msrv: conf.msrv }
}
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `lines.filter_map(Result::ok)` or `lines.flat_map(Result::ok)`
/// when `lines` has type `std::io::Lines`.
///
/// ### Why is this bad?
/// `Lines` instances might produce a never-ending stream of `Err`, in which case
/// `filter_map(Result::ok)` will enter an infinite loop while waiting for an
/// `Ok` variant. Calling `next()` once is sufficient to enter the infinite loop,
/// even in the absence of explicit loops in the user code.
///
/// This situation can arise when working with user-provided paths. On some platforms,
/// `std::fs::File::open(path)` might return `Ok(fs)` even when `path` is a directory,
/// but any later attempt to read from `fs` will return an error.
///
/// ### Known problems
/// This lint suggests replacing `filter_map()` or `flat_map()` applied to a `Lines`
/// instance in all cases. There are two cases where the suggestion might not be
/// appropriate or necessary:
///
/// - If the `Lines` instance can never produce any error, or if an error is produced
/// only once just before terminating the iterator, using `map_while()` is not
/// necessary but will not do any harm.
/// - If the `Lines` instance can produce intermittent errors then recover and produce
/// successful results, using `map_while()` would stop at the first error.
///
/// ### Example
/// ```no_run
/// # use std::{fs::File, io::{self, BufRead, BufReader}};
/// # let _ = || -> io::Result<()> {
/// let mut lines = BufReader::new(File::open("some-path")?).lines().filter_map(Result::ok);
/// // If "some-path" points to a directory, the next statement never terminates:
/// let first_line: Option<String> = lines.next();
/// # Ok(()) };
/// ```
/// Use instead:
/// ```no_run
/// # use std::{fs::File, io::{self, BufRead, BufReader}};
/// # let _ = || -> io::Result<()> {
/// let mut lines = BufReader::new(File::open("some-path")?).lines().map_while(Result::ok);
/// let first_line: Option<String> = lines.next();
/// # Ok(()) };
/// ```
#[clippy::version = "1.70.0"]
pub LINES_FILTER_MAP_OK,
suspicious,
"filtering `std::io::Lines` with `filter_map()`, `flat_map()`, or `flatten()` might cause an infinite loop"
}
impl_lint_pass!(LinesFilterMapOk => [LINES_FILTER_MAP_OK]);
impl LateLintPass<'_> for LinesFilterMapOk {
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
if let ExprKind::MethodCall(fm_method, fm_receiver, fm_args, fm_span) = expr.kind
&& cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator)
&& let fm_method_name = fm_method.ident.name
&& matches!(fm_method_name, sym::filter_map | sym::flat_map | sym::flatten)
&& cx
.typeck_results()
.expr_ty_adjusted(fm_receiver)
.is_diag_item(cx, sym::IoLines)
&& should_lint(cx, fm_args, fm_method_name)
&& self.msrv.meets(cx, msrvs::MAP_WHILE)
{
span_lint_and_then(
cx,
LINES_FILTER_MAP_OK,
fm_span,
format!("`{fm_method_name}()` will run forever if the iterator repeatedly produces an `Err`",),
|diag| {
diag.span_note(
fm_receiver.span,
"this expression returning a `std::io::Lines` may produce an infinite number of `Err` in case of a read error");
diag.span_suggestion(
fm_span,
"replace with",
"map_while(Result::ok)",
Applicability::MaybeIncorrect,
);
},
);
}
}
}
fn should_lint(cx: &LateContext<'_>, args: &[Expr<'_>], method_name: Symbol) -> bool {
match args {
[] => method_name == sym::flatten,
[fm_arg] => {
match &fm_arg.kind {
// Detect `Result::ok`
ExprKind::Path(qpath) => cx
.qpath_res(qpath, fm_arg.hir_id)
.opt_def_id()
.is_some_and(|did| cx.tcx.is_diagnostic_item(sym::result_ok_method, did)),
// Detect `|x| x.ok()`
ExprKind::Closure(Closure { body, .. }) => {
if let Body {
params: [param], value, ..
} = cx.tcx.hir_body(*body)
&& let ExprKind::MethodCall(method, receiver, [], _) = value.kind
{
method.ident.name == sym::ok
&& receiver.res_local_id() == Some(param.pat.hir_id)
&& cx
.typeck_results()
.type_dependent_def_id(value.hir_id)
.opt_parent(cx)
.opt_impl_ty(cx)
.is_diag_item(cx, sym::Result)
} else {
false
}
},
_ => false,
}
},
_ => false,
}
}

85
clippy_lints/src/methods/lines_filter_map_ok.rs Normal file
View file

@ -0,0 +1,85 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::res::{MaybeDef, MaybeResPath, MaybeTypeckRes};
use clippy_utils::sym;
use rustc_errors::Applicability;
use rustc_hir::{Body, Closure, Expr, ExprKind};
use rustc_lint::LateContext;
use rustc_span::Span;
use super::LINES_FILTER_MAP_OK;
pub(super) fn check_flatten(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, call_span: Span, msrv: Msrv) {
if cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator)
&& cx
.typeck_results()
.expr_ty_adjusted(recv)
.is_diag_item(cx, sym::IoLines)
&& msrv.meets(cx, msrvs::MAP_WHILE)
{
emit(cx, recv, "flatten", call_span);
}
}
pub(super) fn check_filter_or_flat_map(
cx: &LateContext<'_>,
expr: &Expr<'_>,
recv: &Expr<'_>,
method_name: &'static str,
method_arg: &Expr<'_>,
call_span: Span,
msrv: Msrv,
) {
if cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::Iterator)
&& cx
.typeck_results()
.expr_ty_adjusted(recv)
.is_diag_item(cx, sym::IoLines)
&& match method_arg.kind {
// Detect `Result::ok`
ExprKind::Path(ref qpath) => cx
.qpath_res(qpath, method_arg.hir_id)
.is_diag_item(cx, sym::result_ok_method),
// Detect `|x| x.ok()`
ExprKind::Closure(&Closure { body, .. }) => {
if let Body {
params: [param], value, ..
} = cx.tcx.hir_body(body)
&& let ExprKind::MethodCall(method, receiver, [], _) = value.kind
{
method.ident.name == sym::ok
&& receiver.res_local_id() == Some(param.pat.hir_id)
&& cx.ty_based_def(*value).is_diag_item(cx, sym::result_ok_method)
} else {
false
}
},
_ => false,
}
&& msrv.meets(cx, msrvs::MAP_WHILE)
{
emit(cx, recv, method_name, call_span);
}
}
fn emit(cx: &LateContext<'_>, recv: &Expr<'_>, method_name: &'static str, call_span: Span) {
span_lint_and_then(
cx,
LINES_FILTER_MAP_OK,
call_span,
format!("`{method_name}()` will run forever if the iterator repeatedly produces an `Err`"),
|diag| {
diag.span_note(
recv.span,
"this expression returning a `std::io::Lines` may produce \
an infinite number of `Err` in case of a read error",
);
diag.span_suggestion(
call_span,
"replace with",
"map_while(Result::ok)",
Applicability::MaybeIncorrect,
);
},
);
}

92
clippy_lints/src/methods/mod.rs
View file

@ -56,6 +56,7 @@ mod iter_with_drain;
mod iterator_step_by_zero;
mod join_absolute_paths;
mod lib;
mod lines_filter_map_ok;
mod manual_c_str_literals;
mod manual_contains;
mod manual_inspect;
@ -4665,6 +4666,55 @@ declare_clippy_lint! {
"making no use of the \"map closure\" when calling `.map_or_else(|| 2 * k, |n| n)`"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for usage of `lines.filter_map(Result::ok)` or `lines.flat_map(Result::ok)`
/// when `lines` has type `std::io::Lines`.
///
/// ### Why is this bad?
/// `Lines` instances might produce a never-ending stream of `Err`, in which case
/// `filter_map(Result::ok)` will enter an infinite loop while waiting for an
/// `Ok` variant. Calling `next()` once is sufficient to enter the infinite loop,
/// even in the absence of explicit loops in the user code.
///
/// This situation can arise when working with user-provided paths. On some platforms,
/// `std::fs::File::open(path)` might return `Ok(fs)` even when `path` is a directory,
/// but any later attempt to read from `fs` will return an error.
///
/// ### Known problems
/// This lint suggests replacing `filter_map()` or `flat_map()` applied to a `Lines`
/// instance in all cases. There are two cases where the suggestion might not be
/// appropriate or necessary:
///
/// - If the `Lines` instance can never produce any error, or if an error is produced
/// only once just before terminating the iterator, using `map_while()` is not
/// necessary but will not do any harm.
/// - If the `Lines` instance can produce intermittent errors then recover and produce
/// successful results, using `map_while()` would stop at the first error.
///
/// ### Example
/// ```no_run
/// # use std::{fs::File, io::{self, BufRead, BufReader}};
/// # let _ = || -> io::Result<()> {
/// let mut lines = BufReader::new(File::open("some-path")?).lines().filter_map(Result::ok);
/// // If "some-path" points to a directory, the next statement never terminates:
/// let first_line: Option<String> = lines.next();
/// # Ok(()) };
/// ```
/// Use instead:
/// ```no_run
/// # use std::{fs::File, io::{self, BufRead, BufReader}};
/// # let _ = || -> io::Result<()> {
/// let mut lines = BufReader::new(File::open("some-path")?).lines().map_while(Result::ok);
/// let first_line: Option<String> = lines.next();
/// # Ok(()) };
/// ```
#[clippy::version = "1.70.0"]
pub LINES_FILTER_MAP_OK,
suspicious,
"filtering `std::io::Lines` with `filter_map()`, `flat_map()`, or `flatten()` might cause an infinite loop"
}
#[expect(clippy::struct_excessive_bools)]
pub struct Methods {
avoid_breaking_exported_api: bool,
@ -4847,6 +4897,7 @@ impl_lint_pass!(Methods => [
IP_CONSTANT,
REDUNDANT_ITER_CLONED,
UNNECESSARY_OPTION_MAP_OR_ELSE,
LINES_FILTER_MAP_OK,
]);
/// Extracts a method call name, args, and `Span` of the method name.
@ -5169,6 +5220,15 @@ impl Methods {
unnecessary_filter_map::check(cx, expr, arg, name);
filter_map_bool_then::check(cx, expr, arg, call_span);
filter_map_identity::check(cx, expr, arg, span);
lines_filter_map_ok::check_filter_or_flat_map(
cx,
expr,
recv,
"filter_map",
arg,
call_span,
self.msrv,
);
},
(sym::find_map, [arg]) => {
unused_enumerate_index::check(cx, expr, recv, arg);
@ -5178,20 +5238,26 @@ impl Methods {
unused_enumerate_index::check(cx, expr, recv, arg);
flat_map_identity::check(cx, expr, arg, span);
flat_map_option::check(cx, expr, arg, span);
lines_filter_map_ok::check_filter_or_flat_map(
cx, expr, recv, "flat_map", arg, call_span, self.msrv,
);
},
(sym::flatten, []) => match method_call(recv) {
Some((sym::map, recv, [map_arg], map_span, _)) => {
map_flatten::check(cx, expr, recv, map_arg, map_span);
},
Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check(
cx,
expr,
recv,
recv2,
iter_overeager_cloned::Op::LaterCloned,
true,
),
_ => {},
(sym::flatten, []) => {
match method_call(recv) {
Some((sym::map, recv, [map_arg], map_span, _)) => {
map_flatten::check(cx, expr, recv, map_arg, map_span);
},
Some((sym::cloned, recv2, [], _, _)) => iter_overeager_cloned::check(
cx,
expr,
recv,
recv2,
iter_overeager_cloned::Op::LaterCloned,
true,
),
_ => {},
}
lines_filter_map_ok::check_flatten(cx, expr, recv, call_span, self.msrv);
},
(sym::fold, [init, acc]) => {
manual_try_fold::check(cx, expr, init, acc, call_span, self.msrv);

24
tests/ui/lines_filter_map_ok.fixed
View file

@ -1,39 +1,37 @@
#![allow(unused, clippy::map_identity)]
#![allow(clippy::map_identity)]
#![warn(clippy::lines_filter_map_ok)]
use std::io::{self, BufRead, BufReader};
fn main() -> io::Result<()> {
let f = std::fs::File::open("/")?;
// Lint
BufReader::new(f).lines().map_while(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
// Lint
// Lint:
let f = std::fs::File::open("/")?;
BufReader::new(f).lines().map_while(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
let f = std::fs::File::open("/")?;
BufReader::new(f).lines().map_while(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
// Lint
let f = std::fs::File::open("/")?;
BufReader::new(f).lines().map_while(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
let s = "foo\nbar\nbaz\n";
// Lint
io::stdin().lines().map_while(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
// Lint
io::stdin().lines().map_while(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
// Lint
io::stdin().lines().map_while(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
// Do not lint (not a `Lines` iterator)
// Do not lint:
// not a `Lines` iterator
io::stdin()
.lines()
.map(std::convert::identity)
.filter_map(|x| x.ok())
.for_each(|_| ());
// Do not lint (not a `Result::ok()` extractor)
// not a `Result::ok()` extractor
io::stdin().lines().filter_map(|x| x.err()).for_each(|_| ());
Ok(())
}

18
tests/ui/lines_filter_map_ok.rs
View file

@ -1,39 +1,37 @@
#![allow(unused, clippy::map_identity)]
#![allow(clippy::map_identity)]
#![warn(clippy::lines_filter_map_ok)]
use std::io::{self, BufRead, BufReader};
fn main() -> io::Result<()> {
// Lint:
let f = std::fs::File::open("/")?;
// Lint
BufReader::new(f).lines().filter_map(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
// Lint
let f = std::fs::File::open("/")?;
BufReader::new(f).lines().flat_map(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
// Lint
let f = std::fs::File::open("/")?;
BufReader::new(f).lines().flatten().for_each(|_| ());
//~^ lines_filter_map_ok
let s = "foo\nbar\nbaz\n";
// Lint
io::stdin().lines().filter_map(Result::ok).for_each(|_| ());
//~^ lines_filter_map_ok
// Lint
io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ());
//~^ lines_filter_map_ok
// Lint
io::stdin().lines().flatten().for_each(|_| ());
//~^ lines_filter_map_ok
// Do not lint (not a `Lines` iterator)
// Do not lint:
// not a `Lines` iterator
io::stdin()
.lines()
.map(std::convert::identity)
.filter_map(|x| x.ok())
.for_each(|_| ());
// Do not lint (not a `Result::ok()` extractor)
// not a `Result::ok()` extractor
io::stdin().lines().filter_map(|x| x.err()).for_each(|_| ());
Ok(())
}

20
tests/ui/lines_filter_map_ok.stderr
View file

@ -1,11 +1,11 @@
error: `filter_map()` will run forever if the iterator repeatedly produces an `Err`
--> tests/ui/lines_filter_map_ok.rs:9:31
--> tests/ui/lines_filter_map_ok.rs:10:31
|
LL | BufReader::new(f).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
--> tests/ui/lines_filter_map_ok.rs:9:5
--> tests/ui/lines_filter_map_ok.rs:10:5
|
LL | BufReader::new(f).lines().filter_map(Result::ok).for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^
@ -25,49 +25,49 @@ LL | BufReader::new(f).lines().flat_map(Result::ok).for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: `flatten()` will run forever if the iterator repeatedly produces an `Err`
--> tests/ui/lines_filter_map_ok.rs:17:31
--> tests/ui/lines_filter_map_ok.rs:16:31
|
LL | BufReader::new(f).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
--> tests/ui/lines_filter_map_ok.rs:17:5
--> tests/ui/lines_filter_map_ok.rs:16:5
|
LL | BufReader::new(f).lines().flatten().for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: `filter_map()` will run forever if the iterator repeatedly produces an `Err`
--> tests/ui/lines_filter_map_ok.rs:22:25
--> tests/ui/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
--> tests/ui/lines_filter_map_ok.rs:22:5
--> tests/ui/lines_filter_map_ok.rs:19:5
|
LL | io::stdin().lines().filter_map(Result::ok).for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^
error: `filter_map()` will run forever if the iterator repeatedly produces an `Err`
--> tests/ui/lines_filter_map_ok.rs:25:25
--> tests/ui/lines_filter_map_ok.rs:21:25
|
LL | io::stdin().lines().filter_map(|x| x.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
--> tests/ui/lines_filter_map_ok.rs:25:5
--> tests/ui/lines_filter_map_ok.rs:21:5
|
LL | io::stdin().lines().filter_map(|x| x.ok()).for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^
error: `flatten()` will run forever if the iterator repeatedly produces an `Err`
--> tests/ui/lines_filter_map_ok.rs:28:25
--> tests/ui/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
--> tests/ui/lines_filter_map_ok.rs:28:5
--> tests/ui/lines_filter_map_ok.rs:23:5
|
LL | io::stdin().lines().flatten().for_each(|_| ());
| ^^^^^^^^^^^^^^^^^^^