diff --git a/CHANGELOG.md b/CHANGELOG.md index 5258cd9f41aa..c424d36e9de7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5199,6 +5199,7 @@ Released 2018-09-13 [`almost_complete_letter_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_letter_range [`almost_complete_range`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_complete_range [`almost_swapped`]: https://rust-lang.github.io/rust-clippy/master/index.html#almost_swapped +[`and_then_then_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#and_then_then_some [`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant [`arc_with_non_send_sync`]: https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync [`arithmetic_side_effects`]: https://rust-lang.github.io/rust-clippy/master/index.html#arithmetic_side_effects diff --git a/clippy_lints/src/and_then_then_some.rs b/clippy_lints/src/and_then_then_some.rs new file mode 100644 index 000000000000..819621fa0fa8 --- /dev/null +++ b/clippy_lints/src/and_then_then_some.rs @@ -0,0 +1,242 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::ty::is_copy; +use clippy_utils::visitors::for_each_expr; +use clippy_utils::{fn_def_id, is_from_proc_macro, match_def_path}; +use rustc_errors::Applicability; +use rustc_hir::def::Res; +use rustc_hir::{Block, Body, Closure, Expr, ExprKind, HirId, Node, Param, Pat, Path, QPath}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::lint::in_external_macro; +use rustc_session::declare_lint_pass; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// Detects usage of `Option::and_then` and `bool::then_some` that could + /// be replaced with `Option::filter`. + /// + /// ### Why is this bad? + /// Needless complexity, uses recent and uncommon stdlib functions instead of + /// one older function. + /// + /// ### Example + /// ```no_run + /// let x = Some("foo".to_string()); + /// let _y = x.clone().and_then(|v| v.starts_with('f').then_some(v)); + /// ``` + /// Use instead: + /// ```no_run + /// let x = Some("foo".to_string()); + /// let _y = x.clone().filter(|v| v.starts_with('f')); + /// ``` + #[clippy::version = "1.81.0"] + pub AND_THEN_THEN_SOME, + nursery, + "detects usage of `and_then` and `then_some` that can be replaced by `filter`" +} + +// note: `Option::filter` is older than `bool::then_some`, +// so no msrv check is required. +declare_lint_pass!(AndThenThenSome => [AND_THEN_THEN_SOME]); + +impl<'tcx> LateLintPass<'tcx> for AndThenThenSome { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if in_external_macro(cx.tcx.sess, expr.span) { + return; + } + match expr.kind { + ExprKind::MethodCall(_, recv_or_self, [arg], _) | ExprKind::Call(_, [recv_or_self, arg]) => { + // we could check if type of reciever is diagnostic item Option, + // but we don't technically need to, since we're checking the path. + if is_and_then(cx, expr) { + if let Some((closure_arg_span, closure_arg_hid, predicate)) = then_some_closure_arg(cx, arg) { + let add_ref_pattern; + // check if `closure_arg_hid` implements Copy, + // if so, show suggestion but with `&` added to the + // argument pattern. + if is_copy(cx, cx.typeck_results().node_type(closure_arg_hid)) { + add_ref_pattern = true; + } else if contains_only_autoref_of(cx, closure_arg_hid, predicate) { + add_ref_pattern = false; + } else { + return; + } + // this check is expensive, so we do it last. + if is_from_proc_macro(cx, expr) { + return; + } + show_sugg( + cx, + expr.span, + recv_or_self, + closure_arg_span, + predicate, + add_ref_pattern, + ); + } + } + }, + _ => {}, + } + } +} + +// This function returns the span and hir id of the closure arguments and the receiver of +// `then_some` (usually `bool`) if the expression passed is a closure whose single expression is a +// call to `then_some`. +fn then_some_closure_arg<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) -> Option<(Span, HirId, &'tcx Expr<'tcx>)> { + match expr.kind { + ExprKind::Closure(Closure { body, .. }) => { + if let Node::Expr(expr) = cx.tcx.hir_node(body.hir_id) + && let Body { + params: + [ + Param { + hir_id: arg_id, + pat: Pat { span, .. }, + .. + }, + ], + .. + } = cx.tcx.hir().body(*body) + { + (peel_closure_body(cx, expr, *arg_id)).map(|body| (*span, *arg_id, body)) + } else { + None + } + }, + _ => None, + } +} + +fn peel_closure_body<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + closure_arg_id: HirId, +) -> Option<&'tcx Expr<'tcx>> { + match expr.kind { + ExprKind::Ret(Some(wrapped_expr)) => + // duplicated blocks because 2023 reference statements are awkward. + // "&" peels multiple layers of indirection instead of just one like we want. + { + peel_closure_body(cx, wrapped_expr, closure_arg_id) + }, + // it would be nice if we could lift { x; y.a() } into { x; y }.a() + ExprKind::Block( + Block { + stmts: [], + expr: Some(wrapped_expr), + .. + }, + _, + ) => peel_closure_body(cx, wrapped_expr, closure_arg_id), + ExprKind::MethodCall(_, pred, [arg], _) | ExprKind::Call(_, [pred, arg]) => { + if is_then_some(cx, expr) && is_local_defined_at(cx, arg, closure_arg_id) { + Some(pred) + } else { + None + } + }, + _ => None, + } +} + +fn get_pat_hid(node: Node<'_>) -> Option { + match node { + Node::Param(Param { + pat: Pat { hir_id, .. }, + .. + }) + | Node::Pat(Pat { hir_id, .. }) => Some(*hir_id), + _ => None, + } +} + +fn is_local_defined_at(cx: &LateContext<'_>, local: &Expr<'_>, arg_hid: HirId) -> bool { + match local.kind { + ExprKind::Path(QPath::Resolved( + _, + Path { + res: Res::Local(local_hid), + .. + }, + )) => { + let local_pat_id = get_pat_hid(cx.tcx.hir_node(*local_hid)); + local_pat_id.is_some() && local_pat_id == get_pat_hid(cx.tcx.hir_node(arg_hid)) + }, + // is not local at all, so definitly isn't a local defined at the given position + _ => false, + } +} + +fn show_sugg( + cx: &LateContext<'_>, + span: Span, + selfarg: &Expr<'_>, + closure_args: Span, + predicate: &Expr<'_>, + add_ref_pattern: bool, +) { + let mut appl = Applicability::MachineApplicable; + let prefix = if add_ref_pattern { "&" } else { "" }; + let sugg = format!( + "{}.filter(|{prefix}{}| {})", + snippet_with_applicability(cx, selfarg.span, "