Add more cases to the useless_conversion lint (#13756)

The new cases are the application of `Into::into` or `From::from` through the following functions with no effect: - `Option::map()` - `Result::map()` and `Result::map_err()` - `ControlFlow::map_break()` and `ControlFlow::map_err()` - `Iterator::map()` changelog: [`useless_conversion`]: detect useless calls to `Into::into` and `From::from` application through `map*` methods
rust-lang · Dec 2, 2024 · 2ddfc92 · 2ddfc92
2 parents 0a07dc5 + ab5d55c
commit 2ddfc92
Show file tree

Hide file tree

Showing 5 changed files with 240 additions and 42 deletions.
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -4982,6 +4982,10 @@ impl Methods {
                     }
                     map_identity::check(cx, expr, recv, m_arg, name, span);
                     manual_inspect::check(cx, expr, m_arg, name, span, &self.msrv);
+                    crate::useless_conversion::check_function_application(cx, expr, recv, m_arg);
+                },
+                ("map_break" | "map_continue", [m_arg]) => {
+                    crate::useless_conversion::check_function_application(cx, expr, recv, m_arg);
                 },
                 ("map_or", [def, map]) => {
                     option_map_or_none::check(cx, expr, recv, def, map);

diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs
@@ -1,18 +1,20 @@
 use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context};
-use clippy_utils::sugg::Sugg;
+use clippy_utils::sugg::{DiagExt as _, Sugg};
 use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
-use clippy_utils::{get_parent_expr, is_trait_method, is_ty_alias, path_to_local};
+use clippy_utils::{
+    get_parent_expr, is_inherent_method_call, is_trait_item, is_trait_method, is_ty_alias, path_to_local,
+};
 use rustc_errors::Applicability;
 use rustc_hir::def_id::DefId;
 use rustc_hir::{BindingMode, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_infer::traits::Obligation;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::traits::ObligationCause;
-use rustc_middle::ty::{self, EarlyBinder, GenericArg, GenericArgsRef, Ty, TypeVisitableExt};
+use rustc_middle::ty::{self, AdtDef, EarlyBinder, GenericArg, GenericArgsRef, Ty, TypeVisitableExt};
 use rustc_session::impl_lint_pass;
-use rustc_span::{Span, sym};
+use rustc_span::{Span, Symbol, sym};
 use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt;
 
 declare_clippy_lint! {
@@ -382,3 +384,57 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
         }
     }
 }
+
+/// Check if `arg` is a `Into::into` or `From::from` applied to `receiver` to give `expr`, through a
+/// higher-order mapping function.
+pub fn check_function_application(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: &Expr<'_>) {
+    if has_eligible_receiver(cx, recv, expr)
+        && (is_trait_item(cx, arg, sym::Into) || is_trait_item(cx, arg, sym::From))
+        && let ty::FnDef(_, args) = cx.typeck_results().expr_ty(arg).kind()
+        && let &[from_ty, to_ty] = args.into_type_list(cx.tcx).as_slice()
+        && same_type_and_consts(from_ty, to_ty)
+    {
+        span_lint_and_then(
+            cx,
+            USELESS_CONVERSION,
+            expr.span.with_lo(recv.span.hi()),
+            format!("useless conversion to the same type: `{from_ty}`"),
+            |diag| {
+                diag.suggest_remove_item(
+                    cx,
+                    expr.span.with_lo(recv.span.hi()),
+                    "consider removing",
+                    Applicability::MachineApplicable,
+                );
+            },
+        );
+    }
+}
+
+fn has_eligible_receiver(cx: &LateContext<'_>, recv: &Expr<'_>, expr: &Expr<'_>) -> bool {
+    let recv_ty = cx.typeck_results().expr_ty(recv);
+    if is_inherent_method_call(cx, expr)
+        && let Some(recv_ty_defid) = recv_ty.ty_adt_def().map(AdtDef::did)
+    {
+        if let Some(diag_name) = cx.tcx.get_diagnostic_name(recv_ty_defid)
+            && matches!(diag_name, sym::Option | sym::Result)
+        {
+            return true;
+        }
+
+        // FIXME: Add ControlFlow diagnostic item
+        let def_path = cx.get_def_path(recv_ty_defid);
+        if def_path
+            .iter()
+            .map(Symbol::as_str)
+            .zip(["core", "ops", "control_flow", "ControlFlow"])
+            .all(|(sym, s)| sym == s)
+        {
+            return true;
+        }
+    }
+    if is_trait_method(cx, expr, sym::Iterator) {
+        return true;
+    }
+    false
+}
diff --git a/tests/ui/useless_conversion.fixed b/tests/ui/useless_conversion.fixed
@@ -3,6 +3,8 @@
 // FIXME(static_mut_refs): Do not allow `static_mut_refs` lint
 #![allow(static_mut_refs)]
 
+use std::ops::ControlFlow;
+
 fn test_generic<T: Copy>(val: T) -> T {
     let _ = val;
     val
@@ -297,3 +299,46 @@ impl From<Foo<'a'>> for Foo<'b'> {
         Foo
     }
 }
+
+fn direct_application() {
+    let _: Result<(), std::io::Error> = test_issue_3913();
+    //~^ useless_conversion
+    let _: Result<(), std::io::Error> = test_issue_3913();
+    //~^ useless_conversion
+    let _: Result<(), std::io::Error> = test_issue_3913();
+    //~^ useless_conversion
+    let _: Result<(), std::io::Error> = test_issue_3913();
+    //~^ useless_conversion
+
+    let c: ControlFlow<()> = ControlFlow::Continue(());
+    let _: ControlFlow<()> = c;
+    //~^ useless_conversion
+    let c: ControlFlow<()> = ControlFlow::Continue(());
+    let _: ControlFlow<()> = c;
+    //~^ useless_conversion
+
+    struct Absorb;
+    impl From<()> for Absorb {
+        fn from(_: ()) -> Self {
+            Self
+        }
+    }
+    impl From<std::io::Error> for Absorb {
+        fn from(_: std::io::Error) -> Self {
+            Self
+        }
+    }
+    let _: Vec<u32> = [1u32].into_iter().collect();
+    //~^ useless_conversion
+
+    // No lint for those
+    let _: Result<Absorb, std::io::Error> = test_issue_3913().map(Into::into);
+    let _: Result<(), Absorb> = test_issue_3913().map_err(Into::into);
+    let _: Result<Absorb, std::io::Error> = test_issue_3913().map(From::from);
+    let _: Result<(), Absorb> = test_issue_3913().map_err(From::from);
+}
+
+fn gen_identity<T>(x: [T; 3]) -> Vec<T> {
+    x.into_iter().collect()
+    //~^ useless_conversion
+}
diff --git a/tests/ui/useless_conversion.rs b/tests/ui/useless_conversion.rs
@@ -3,6 +3,8 @@
 // FIXME(static_mut_refs): Do not allow `static_mut_refs` lint
 #![allow(static_mut_refs)]
 
+use std::ops::ControlFlow;
+
 fn test_generic<T: Copy>(val: T) -> T {
     let _ = T::from(val);
     val.into()
@@ -297,3 +299,46 @@ impl From<Foo<'a'>> for Foo<'b'> {
         Foo
     }
 }
+
+fn direct_application() {
+    let _: Result<(), std::io::Error> = test_issue_3913().map(Into::into);
+    //~^ useless_conversion
+    let _: Result<(), std::io::Error> = test_issue_3913().map_err(Into::into);
+    //~^ useless_conversion
+    let _: Result<(), std::io::Error> = test_issue_3913().map(From::from);
+    //~^ useless_conversion
+    let _: Result<(), std::io::Error> = test_issue_3913().map_err(From::from);
+    //~^ useless_conversion
+
+    let c: ControlFlow<()> = ControlFlow::Continue(());
+    let _: ControlFlow<()> = c.map_break(Into::into);
+    //~^ useless_conversion
+    let c: ControlFlow<()> = ControlFlow::Continue(());
+    let _: ControlFlow<()> = c.map_continue(Into::into);
+    //~^ useless_conversion
+
+    struct Absorb;
+    impl From<()> for Absorb {
+        fn from(_: ()) -> Self {
+            Self
+        }
+    }
+    impl From<std::io::Error> for Absorb {
+        fn from(_: std::io::Error) -> Self {
+            Self
+        }
+    }
+    let _: Vec<u32> = [1u32].into_iter().map(Into::into).collect();
+    //~^ useless_conversion
+
+    // No lint for those
+    let _: Result<Absorb, std::io::Error> = test_issue_3913().map(Into::into);
+    let _: Result<(), Absorb> = test_issue_3913().map_err(Into::into);
+    let _: Result<Absorb, std::io::Error> = test_issue_3913().map(From::from);
+    let _: Result<(), Absorb> = test_issue_3913().map_err(From::from);
+}
+
+fn gen_identity<T>(x: [T; 3]) -> Vec<T> {
+    x.into_iter().map(Into::into).collect()
+    //~^ useless_conversion
+}