Lint on trait declarations, not implementations

Author: botahamec

clippy_lints/src/unnecessary_box_returns.rs

@@ -1,10 +1,9 @@
 use clippy_utils::{diagnostics::span_lint_and_sugg, ty::implements_trait};
 use rustc_errors::Applicability;
-use rustc_hir::{def_id::LocalDefId, intravisit::FnKind, Body, FnDecl, FnRetTy};
+use rustc_hir::{FnDecl, FnRetTy, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind};
 use rustc_hir_analysis::hir_ty_to_ty;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::Span;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -36,46 +35,55 @@ 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,
-        _: 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<'_>) {
+    let FnRetTy::Return(return_ty_hir) = &decl.output else { return };
 
-        let FnRetTy::Return(return_ty_hir) = &decl.output else { return };
+    // this is safe, since we're not in a body
+    let return_ty = hir_ty_to_ty(cx.tcx, return_ty_hir);
 
-        // this is safe, since we're not in a body
-        let return_ty = hir_ty_to_ty(cx.tcx, return_ty_hir);
+    if !return_ty.is_box() {
+        return;
+    }
 
-        if !return_ty.is_box() {
+    let boxed_ty = return_ty.boxed_ty();
+    let Some(sized_trait) = cx.tcx.lang_items().sized_trait() else { return };
+
+    // it's sometimes useful to return Box<T> if T is unsized, so don't lint those
+    if implements_trait(cx, boxed_ty, sized_trait, &[]) {
+        span_lint_and_sugg(
+            cx,
+            UNNECESSARY_BOX_RETURNS,
+            return_ty_hir.span,
+            format!("boxed return of the sized type `{boxed_ty}`").as_str(),
+            "try",
+            boxed_ty.to_string(),
+            // the return value and function callers also needs to be changed, so this can't be MachineApplicable
+            Applicability::Unspecified,
+        );
+    }
+}
+
+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);
+    }
+
+    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 Some(sized_trait) = cx.tcx.lang_items().sized_trait() else { return };
+        let ImplItemKind::Fn(signature, ..) = &item.kind else { return };
+        check_fn_decl(cx, signature.decl);
+    }
 
-        // it's sometimes useful to return Box<T> if T is unsized, so don't lint those
-        if implements_trait(cx, boxed_ty, sized_trait, &[]) {
-            span_lint_and_sugg(
-                cx,
-                UNNECESSARY_BOX_RETURNS,
-                return_ty_hir.span,
-                format!("boxed return of the sized type `{boxed_ty}`").as_str(),
-                "try",
-                boxed_ty.to_string(),
-                // the return value and function callers also needs to be changed, so this can't be MachineApplicable
-                Applicability::Unspecified,
-            );
-        }
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+        let ItemKind::Fn(signature, ..) = &item.kind else { return };
+        check_fn_decl(cx, signature.decl);
     }
 }

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,16 +1,28 @@
 error: boxed return of the sized type `usize`
-  --> $DIR/unnecessary_box_returns.rs:6:21
+  --> $DIR/unnecessary_box_returns.rs:5:22
    |
-LL | fn boxed_usize() -> Box<usize> {
-   |                     ^^^^^^^^^^ help: try: `usize`
+LL |     fn baz(&self) -> Box<usize>;
+   |                      ^^^^^^^^^^ help: try: `usize`
    |
    = 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`
+
+error: boxed return of the sized type `usize`
+  --> $DIR/unnecessary_box_returns.rs:25:21
+   |
+LL | fn boxed_usize() -> Box<usize> {
+   |                     ^^^^^^^^^^ help: try: `usize`
+
 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`
 
-error: aborting due to 2 previous errors
+error: aborting due to 4 previous errors