Add lint manual_inspect

CHANGELOG.md

@@ -5348,6 +5348,7 @@ Released 2018-09-13
 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
 [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
 [`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one
+[`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect
 [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
 [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
 [`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite

clippy_config/src/msrvs.rs

@@ -18,7 +18,7 @@ macro_rules! msrv_aliases {
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
  1,77,0 { C_STR_LITERALS }
- 1,76,0 { PTR_FROM_REF }
+ 1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
  1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
  1,70,0 { OPTION_RESULT_IS_VARIANT_AND, BINARY_HEAP_RETAIN }
  1,68,0 { PATH_MAIN_SEPARATOR_STR }

clippy_lints/src/declared_lints.rs

@@ -390,6 +390,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
  crate::methods::MANUAL_C_STR_LITERALS_INFO,
  crate::methods::MANUAL_FILTER_MAP_INFO,
  crate::methods::MANUAL_FIND_MAP_INFO,
+ crate::methods::MANUAL_INSPECT_INFO,
  crate::methods::MANUAL_IS_VARIANT_AND_INFO,
  crate::methods::MANUAL_NEXT_BACK_INFO,
  crate::methods::MANUAL_OK_OR_INFO,

clippy_lints/src/methods/manual_inspect.rs

@@ -0,0 +1,233 @@
+use clippy_config::msrvs::{self, Msrv};
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::{get_source_text, with_leading_whitespace, SpanRange};
+use clippy_utils::ty::get_field_by_name;
+use clippy_utils::visitors::{for_each_expr, for_each_expr_with_closures};
+use clippy_utils::{expr_use_ctxt, is_diag_item_method, is_diag_trait_item, path_to_local_id, ExprUseNode};
+use core::ops::ControlFlow;
+use rustc_errors::Applicability;
+use rustc_hir::{BindingAnnotation, BorrowKind, ByRef, ClosureKind, Expr, ExprKind, Mutability, Node, PatKind};
+use rustc_lint::LateContext;
+use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability};
+use rustc_span::{sym, BytePos, Span, Symbol, DUMMY_SP};
+
+use super::MANUAL_INSPECT;
+
+#[expect(clippy::too_many_lines)]
+pub(crate) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, arg: &Expr<'_>, name: &str, name_span: Span, msrv: &Msrv) {
+ if let ExprKind::Closure(c) = arg.kind
+ && matches!(c.kind, ClosureKind::Closure)
+ && let typeck = cx.typeck_results()
+ && let Some(fn_id) = typeck.type_dependent_def_id(expr.hir_id)
+ && (is_diag_trait_item(cx, fn_id, sym::Iterator)
+ || (msrv.meets(msrvs::OPTION_RESULT_INSPECT)
+ && (is_diag_item_method(cx, fn_id, sym::Option) || is_diag_item_method(cx, fn_id, sym::Result))))
+ && let body = cx.tcx.hir().body(c.body)
+ && let [param] = body.params
+ && let PatKind::Binding(BindingAnnotation(ByRef::No, Mutability::Not), arg_id, _, None) = param.pat.kind
+ && let arg_ty = typeck.node_type(arg_id)
+ && let ExprKind::Block(block, _) = body.value.kind
+ && let Some(final_expr) = block.expr
+ && !block.stmts.is_empty()
+ && path_to_local_id(final_expr, arg_id)
+ && typeck.expr_adjustments(final_expr).is_empty()
+ {
+ let mut requires_copy = false;
+ let mut requires_deref = false;
+
+ // The number of unprocessed return expressions.
+ let mut ret_count = 0u32;
+
+ // The uses for which processing is delayed until after the visitor.
+ let mut delayed = vec![];
+
+ let ctxt = arg.span.ctxt();
+ let can_lint = for_each_expr(block.stmts, |e| {
+ if let ExprKind::Closure(c) = e.kind {
+ // Nested closures don't need to treat returns specially.
+ let _: Option<!> = for_each_expr_with_closures(cx, cx.tcx.hir().body(c.body).value, |e| {
+ if path_to_local_id(e, arg_id) {
+ let (kind, same_ctxt) = check_use(cx, e);
+ match (kind, same_ctxt && e.span.ctxt() == ctxt) {
+ (_, false) | (UseKind::Deref | UseKind::Return(..), true) => {
+ requires_copy = true;
+ requires_deref = true;
+ },
+ (UseKind::AutoBorrowed, true) => {},
+ (UseKind::WillAutoDeref, true) => {
+ requires_copy = true;
+ },
+ (kind, true) => delayed.push(kind),
+ }
+ }
+ ControlFlow::Continue(())
+ });
+ } else if matches!(e.kind, ExprKind::Ret(_)) {
+ ret_count += 1;
+ } else if path_to_local_id(e, arg_id) {
+ let (kind, same_ctxt) = check_use(cx, e);
+ match (kind, same_ctxt && e.span.ctxt() == ctxt) {
+ (UseKind::Return(..), false) => {
+ return ControlFlow::Break(());
+ },
+ (_, false) | (UseKind::Deref, true) => {
+ requires_copy = true;
+ requires_deref = true;
+ },
+ (UseKind::AutoBorrowed, true) => {},
+ (UseKind::WillAutoDeref, true) => {
+ requires_copy = true;
+ },
+ (kind @ UseKind::Return(_), true) => {
+ ret_count -= 1;
+ delayed.push(kind);
+ },
+ (kind, true) => delayed.push(kind),
+ }
+ }
+ ControlFlow::Continue(())
+ })
+ .is_none();
+
+ if ret_count != 0 {
+ // A return expression that didn't return the original value was found.
+ return;
+ }
+
+ let mut edits = Vec::with_capacity(delayed.len() + 3);
+ let mut addr_of_edits = Vec::with_capacity(delayed.len());
+ for x in delayed {
+ match x {
+ UseKind::Return(s) => edits.push((with_leading_whitespace(cx, s).set_span_pos(s), String::new())),
+ UseKind::Borrowed(s) => {
+ if let Some(src) = get_source_text(cx, s)
+ && let Some(src) = src.as_str()
+ && let trim_src = src.trim_start_matches(|x| matches!(x, ' ' | '\t' | '\n' | '\r' | '('))
+ && trim_src.starts_with('&')
+ {
+ let range = s.into_range();
+ #[expect(clippy::cast_possible_truncation)]
+ let start = BytePos(range.start.0 + (src.len() - trim_src.len()) as u32);
+ addr_of_edits.push(((start..BytePos(start.0 + 1)).set_span_pos(s), String::new()));
+ } else {
+ requires_copy = true;
+ requires_deref = true;
+ }
+ },
+ UseKind::FieldAccess(name, e) => {
+ let Some(mut ty) = get_field_by_name(cx.tcx, arg_ty.peel_refs(), name) else {
+ requires_copy = true;
+ continue;
+ };
+ let mut prev_expr = e;
+
+ for (_, parent) in cx.tcx.hir().parent_iter(e.hir_id) {
+ if let Node::Expr(e) = parent {
+ match e.kind {
+ ExprKind::Field(_, name)
+ if let Some(fty) = get_field_by_name(cx.tcx, ty.peel_refs(), name.name) =>
+ {
+ ty = fty;
+ prev_expr = e;
+ continue;
+ },
+ ExprKind::AddrOf(BorrowKind::Ref, ..) => break,
+ _ if matches!(
+ typeck.expr_adjustments(prev_expr).first(),
+ Some(Adjustment {
+ kind: Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Not))
+ | Adjust::Deref(_),
+ ..
+ })
+ ) =>
+ {
+ break;
+ },
+ _ => {},
+ }
+ }
+ requires_copy |= !ty.is_copy_modulo_regions(cx.tcx, cx.param_env);
+ break;
+ }
+ },
+ // Already processed uses.
+ UseKind::AutoBorrowed | UseKind::WillAutoDeref | UseKind::Deref => {},
+ }
+ }
+
+ if can_lint
+ && (!requires_copy || arg_ty.is_copy_modulo_regions(cx.tcx, cx.param_env))
+ // This case could be handled, but a fair bit of care would need to be taken.
+ && (!requires_deref || arg_ty.is_freeze(cx.tcx, cx.param_env))
+ {
+ if requires_deref {
+ edits.push((param.span.shrink_to_lo(), "&".into()));
+ } else {
+ edits.extend(addr_of_edits);
+ }
+ edits.push((
+ name_span,
+ String::from(match name {
+ "map" => "inspect",
+ "map_err" => "inspect_err",
+ _ => return,
+ }),
+ ));
+ edits.push((
+ with_leading_whitespace(cx, final_expr.span).set_span_pos(final_expr.span),
+ String::new(),
+ ));
+ let app = if edits.iter().any(|(s, _)| s.from_expansion()) {
+ Applicability::MaybeIncorrect
+ } else {
+ Applicability::MachineApplicable
+ };
+ span_lint_and_then(cx, MANUAL_INSPECT, name_span, "", |diag| {
+ diag.multipart_suggestion("try", edits, app);
+ });
+ }
+ }
+}
+
+enum UseKind<'tcx> {
+ AutoBorrowed,
+ WillAutoDeref,
+ Deref,
+ Return(Span),
+ Borrowed(Span),
+ FieldAccess(Symbol, &'tcx Expr<'tcx>),
+}
+
+/// Checks how the value is used, and whether it was used in the same `SyntaxContext`.
+fn check_use<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> (UseKind<'tcx>, bool) {
+ let use_cx = expr_use_ctxt(cx, e);
+ if use_cx
+ .adjustments
+ .first()
+ .is_some_and(|a| matches!(a.kind, Adjust::Deref(_)))
+ {
+ return (UseKind::AutoBorrowed, use_cx.same_ctxt);
+ }
+ let res = match use_cx.use_node(cx) {
+ ExprUseNode::Return(_) => {
+ if let ExprKind::Ret(Some(e)) = use_cx.node.expect_expr().kind {
+ UseKind::Return(e.span)
+ } else {
+ return (UseKind::Return(DUMMY_SP), false);
+ }
+ },
+ ExprUseNode::FieldAccess(name) => UseKind::FieldAccess(name.name, use_cx.node.expect_expr()),
+ ExprUseNode::Callee | ExprUseNode::MethodArg(_, _, 0)
+ if use_cx
+ .adjustments
+ .first()
+ .is_some_and(|a| matches!(a.kind, Adjust::Borrow(AutoBorrow::Ref(_, AutoBorrowMutability::Not)))) =>
+ {
+ UseKind::AutoBorrowed
+ },
+ ExprUseNode::Callee | ExprUseNode::MethodArg(_, _, 0) => UseKind::WillAutoDeref,
+ ExprUseNode::AddrOf(BorrowKind::Ref, _) => UseKind::Borrowed(use_cx.node.expect_expr().span),
+ _ => UseKind::Deref,
+ };
+ (res, use_cx.same_ctxt)
+}

clippy_lints/src/methods/mod.rs

@@ -52,6 +52,7 @@ mod iter_with_drain;
 mod iterator_step_by_zero;
 mod join_absolute_paths;
 mod manual_c_str_literals;
+mod manual_inspect;
 mod manual_is_variant_and;
 mod manual_next_back;
 mod manual_ok_or;
@@ -4012,6 +4013,27 @@ declare_clippy_lint! {
  r#"creating a `CStr` through functions when `c""` literals can be used"#
 }
 
+declare_clippy_lint! {
+ /// ### What it does
+ /// Checks for uses of `map` which return the original item.
+ ///
+ /// ### Why is this bad?
+ /// `inspect` is both clearer in intent and shorter.
+ ///
+ /// ### Example
+ /// ```no_run
+ /// let x = Some(0).map(|x| { println!("{x}"); x });
+ /// ```
+ /// Use instead:
+ /// ```no_run
+ /// let x = Some(0).inspect(|x| println!("{x}"));
+ /// ```
+ #[clippy::version = "1.78.0"]
+ pub MANUAL_INSPECT,
+ complexity,
+ "use of `map` returning the original item"
+}
+
 pub struct Methods {
  avoid_breaking_exported_api: bool,
  msrv: Msrv,
@@ -4172,6 +4194,7 @@ impl_lint_pass!(Methods => [
  OPTION_AS_REF_CLONED,
  UNNECESSARY_RESULT_MAP_OR_ELSE,
  MANUAL_C_STR_LITERALS,
+ MANUAL_INSPECT,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4649,6 +4672,7 @@ impl Methods {
  }
  }
  map_identity::check(cx, expr, recv, m_arg, name, span);
+ manual_inspect::check(cx, expr, m_arg, name, span, &self.msrv);
  },
  ("map_or", [def, map]) => {
  option_map_or_none::check(cx, expr, recv, def, map);

clippy_utils/src/source.rs

@@ -13,8 +13,17 @@ use std::borrow::Cow;
 use std::ops::Range;
 
 /// A type which can be converted to the range portion of a `Span`.
-pub trait SpanRange {
+pub trait SpanRange: Sized {
  fn into_range(self) -> Range<BytePos>;
+ fn set_span_pos(self, sp: Span) -> Span {
+ let range = self.into_range();
+ SpanData {
+ lo: range.start,
+ hi: range.end,
+ ..sp.data()
+ }
+ .span()
+ }
 }
 impl SpanRange for Span {
  fn into_range(self) -> Range<BytePos> {
@@ -60,6 +69,22 @@ pub fn get_source_text(cx: &impl LintContext, sp: impl SpanRange) -> Option<Sour
  f(cx.sess().source_map(), sp.into_range())
 }
 
+pub fn with_leading_whitespace(cx: &impl LintContext, sp: impl SpanRange) -> Range<BytePos> {
+ #[expect(clippy::needless_pass_by_value, clippy::cast_possible_truncation)]
+ fn f(src: SourceFileRange, sp: Range<BytePos>) -> Range<BytePos> {
+ let Some(text) = &src.sf.src else {
+ return sp;
+ };
+ let len = src.range.start - text[..src.range.start].trim_end().len();
+ BytePos(sp.start.0 - len as u32)..sp.end
+ }
+ let sp = sp.into_range();
+ match get_source_text(cx, sp.clone()) {
+ Some(src) => f(src, sp),
+ None => sp,
+ }
+}
+
 /// Like `snippet_block`, but add braces if the expr is not an `ExprKind::Block`.
 pub fn expr_block<T: LintContext>(
  cx: &T,

clippy_utils/src/ty.rs

@@ -1280,3 +1280,17 @@ pub fn normalize_with_regions<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>
 pub fn is_manually_drop(ty: Ty<'_>) -> bool {
  ty.ty_adt_def().map_or(false, AdtDef::is_manually_drop)
 }
+
+/// Get's the type of a field by name.
+pub fn get_field_by_name<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, name: Symbol) -> Option<Ty<'tcx>> {
+ match *ty.kind() {
+ ty::Adt(def, args) if def.is_union() || def.is_struct() => def
+ .non_enum_variant()
+ .fields
+ .iter()
+ .find(|f| f.name == name)
+ .map(|f| f.ty(tcx, args)),
+ ty::Tuple(args) => name.as_str().parse::<usize>().ok().and_then(|i| args.get(i).copied()),
+ _ => None,
+ }
+}

tests/ui/copy_iterator.rs

@@ -1,4 +1,5 @@
 #![warn(clippy::copy_iterator)]
+#![allow(clippy::manual_inspect)]
 
 #[derive(Copy, Clone)]
 struct Countdown(u8);

tests/ui/copy_iterator.stderr

@@ -1,5 +1,5 @@
 error: you are implementing `Iterator` on a `Copy` type
- --> $DIR/copy_iterator.rs:6:1
+ --> $DIR/copy_iterator.rs:7:1
  |
 LL | / impl Iterator for Countdown {
 LL | |