Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)));
Expand Down
141 changes: 0 additions & 141 deletions clippy_lints/src/lines_filter_map_ok.rs

This file was deleted.

85 changes: 85 additions & 0 deletions clippy_lints/src/methods/lines_filter_map_ok.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 79 additions & 13 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -5165,6 +5216,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);
Expand All @@ -5174,20 +5234,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);
Expand Down
Loading