Lint on trait declarations, not implementations

Author: botahamec

clippy_lints/src/unnecessary_box_returns.rs

@@ -1,9 +1,8 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use rustc_errors::Applicability;
-use rustc_hir::{def_id::LocalDefId, intravisit::FnKind, Body, FnDecl, FnRetTy};
+use rustc_hir::{def_id::LocalDefId, FnDecl, FnRetTy, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::Span;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -35,54 +34,63 @@ declare_clippy_lint! {
 }
 declare_lint_pass!(UnnecessaryBoxReturns => [UNNECESSARY_BOX_RETURNS]);
 
-impl LateLintPass<'_> for UnnecessaryBoxReturns {
-    fn check_fn(
-        &mut self,
-        cx: &LateContext<'_>,
-        fn_kind: FnKind<'_>,
-        decl: &FnDecl<'_>,
-        _: &Body<'_>,
-        _: Span,
-        def_id: LocalDefId,
-    ) {
-        // it's unclear what part of a closure you would span, so for now it's ignored
-        // if this is changed, please also make sure not to call `hir_ty_to_ty` below
-        if matches!(fn_kind, FnKind::Closure) {
-            return;
-        }
+fn check_fn_decl(cx: &LateContext<'_>, decl: &FnDecl<'_>, def_id: LocalDefId) {
+    let FnRetTy::Return(return_ty_hir) = &decl.output else { return };
 
-        let FnRetTy::Return(return_ty_hir) = &decl.output else { return };
+    let return_ty = cx
+        .tcx
+        .erase_late_bound_regions(cx.tcx.fn_sig(def_id).skip_binder())
+        .output();
 
-        let return_ty = cx
-            .tcx
-            .erase_late_bound_regions(cx.tcx.fn_sig(def_id).skip_binder())
-            .output();
+    if !return_ty.is_box() {
+        return;
+    }
 
-        if !return_ty.is_box() {
+    let boxed_ty = return_ty.boxed_ty();
+
+    // it's sometimes useful to return Box<T> if T is unsized, so don't lint those
+    if boxed_ty.is_sized(cx.tcx, cx.param_env) {
+        span_lint_and_then(
+            cx,
+            UNNECESSARY_BOX_RETURNS,
+            return_ty_hir.span,
+            format!("boxed return of the sized type `{boxed_ty}`").as_str(),
+            |diagnostic| {
+                diagnostic.span_suggestion(
+                    return_ty_hir.span,
+                    "try",
+                    boxed_ty.to_string(),
+                    // the return value and function callers also needs to
+                    // be changed, so this can't be MachineApplicable
+                    Applicability::Unspecified,
+                );
+                diagnostic.help("changing this also requires a change to the return expressions in this function");
+            },
+        );
+    }
+}
+
+impl LateLintPass<'_> for UnnecessaryBoxReturns {
+    fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) {
+        let TraitItemKind::Fn(signature, _) = &item.kind else { return };
+        check_fn_decl(cx, signature.decl, item.owner_id.def_id);
+    }
+
+    fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::ImplItem<'_>) {
+        // Ignore implementations of traits, because the lint should be on the
+        // trait, not on the implmentation of it.
+        let Node::Item(parent) = cx.tcx.hir().get_parent(item.hir_id()) else { return };
+        let ItemKind::Impl(parent) = parent.kind else { return };
+        if parent.of_trait.is_some() {
             return;
         }
 
-        let boxed_ty = return_ty.boxed_ty();
+        let ImplItemKind::Fn(signature, ..) = &item.kind else { return };
+        check_fn_decl(cx, signature.decl, item.owner_id.def_id);
+    }
 
-        // it's sometimes useful to return Box<T> if T is unsized, so don't lint those
-        if boxed_ty.is_sized(cx.tcx, cx.param_env) {
-            span_lint_and_then(
-                cx,
-                UNNECESSARY_BOX_RETURNS,
-                return_ty_hir.span,
-                format!("boxed return of the sized type `{boxed_ty}`").as_str(),
-                |diagnostic| {
-                    diagnostic.span_suggestion(
-                        return_ty_hir.span,
-                        "try",
-                        boxed_ty.to_string(),
-                        // the return value and function callers also needs to
-                        // be changed, so this can't be MachineApplicable
-                        Applicability::Unspecified,
-                    );
-                    diagnostic.help("changing this also requires a change to the return expressions in this function");
-                },
-            );
-        }
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+        let ItemKind::Fn(signature, ..) = &item.kind else { return };
+        check_fn_decl(cx, signature.decl, item.owner_id.def_id);
     }
 }

tests/ui/unnecessary_box_returns.rs

@@ -1,7 +1,26 @@
 #![warn(clippy::unnecessary_box_returns)]
 
+trait Bar {
+    // lint
+    fn baz(&self) -> Box<usize>;
+}
+
 struct Foo {}
 
+impl Bar for Foo {
+    // don't lint: this is a problem with the trait, not the implementation
+    fn baz(&self) -> Box<usize> {
+        Box::new(42)
+    }
+}
+
+impl Foo {
+    fn baz(&self) -> Box<usize> {
+        // lint
+        Box::new(13)
+    }
+}
+
 // lint
 fn boxed_usize() -> Box<usize> {
     Box::new(5)

tests/ui/unnecessary_box_returns.stderr

@@ -1,19 +1,35 @@
 error: boxed return of the sized type `usize`
-  --> $DIR/unnecessary_box_returns.rs:6:21
+  --> $DIR/unnecessary_box_returns.rs:5:22
+   |
+LL |     fn baz(&self) -> Box<usize>;
+   |                      ^^^^^^^^^^ help: try: `usize`
+   |
+   = help: changing this also requires a change to the return expressions in this function
+   = note: `-D clippy::unnecessary-box-returns` implied by `-D warnings`
+
+error: boxed return of the sized type `usize`
+  --> $DIR/unnecessary_box_returns.rs:18:22
+   |
+LL |     fn baz(&self) -> Box<usize> {
+   |                      ^^^^^^^^^^ help: try: `usize`
+   |
+   = help: changing this also requires a change to the return expressions in this function
+
+error: boxed return of the sized type `usize`
+  --> $DIR/unnecessary_box_returns.rs:25:21
    |
 LL | fn boxed_usize() -> Box<usize> {
    |                     ^^^^^^^^^^ help: try: `usize`
    |
    = help: changing this also requires a change to the return expressions in this function
-   = note: `-D clippy::unnecessary-box-returns` implied by `-D warnings`
 
 error: boxed return of the sized type `Foo`
-  --> $DIR/unnecessary_box_returns.rs:11:19
+  --> $DIR/unnecessary_box_returns.rs:30:19
    |
 LL | fn boxed_foo() -> Box<Foo> {
    |                   ^^^^^^^^ help: try: `Foo`
    |
    = help: changing this also requires a change to the return expressions in this function
 
-error: aborting due to 2 previous errors
+error: aborting due to 4 previous errors