Avoid breaking exported API

Author: botahamec

clippy_lints/src/lib.rs

@@ -928,6 +928,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(no_mangle_with_rust_abi::NoMangleWithRustAbi));
     store.register_late_pass(|_| Box::new(collection_is_never_read::CollectionIsNeverRead));
     store.register_late_pass(|_| Box::new(unnecessary_box_returns::UnnecessaryBoxReturns));
+    store.register_late_pass(move |_| {
+        Box::new(unnecessary_box_returns::UnnecessaryBoxReturns::new(
+            avoid_breaking_exported_api,
+        ))
+    });
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

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::{intravisit::FnKind, Body, FnDecl, FnRetTy, HirId, TraitItem, TraitItemKind};
+use rustc_hir::{def_id::LocalDefId, FnDecl, FnRetTy, Item, ItemKind, 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;
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -34,56 +33,62 @@ declare_clippy_lint! {
     nursery,
     "Needlessly returning a Box"
 }
-declare_lint_pass!(UnnecessaryBoxReturns => [UNNECESSARY_BOX_RETURNS]);
 
-fn check_fn_decl(cx: &LateContext<'_>, decl: &FnDecl<'_>) {
-    let FnRetTy::Return(return_ty_hir) = &decl.output else { return };
+pub struct UnnecessaryBoxReturns {
+    avoid_breaking_exported_api: bool,
+}
 
-    // this is safe, since we're not in a body
-    let return_ty = hir_ty_to_ty(cx.tcx, return_ty_hir);
+impl_lint_pass!(UnnecessaryBoxReturns => [UNNECESSARY_BOX_RETURNS]);
 
-    if !return_ty.is_box() {
-        return;
+impl UnnecessaryBoxReturns {
+    pub fn new(avoid_breaking_exported_api: bool) -> Self {
+        Self {
+            avoid_breaking_exported_api,
+        }
     }
 
-    let boxed_ty = return_ty.boxed_ty();
-    let Some(sized_trait) = cx.tcx.lang_items().sized_trait() else { return };
+    fn check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, def_id: LocalDefId) {
+        // we don't want to tell someone to break an exported function if they ask us not to
+        if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id) {
+            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);
 
-    // 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,
-        );
+        if !return_ty.is_box() {
+            return;
+        }
+
+        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);
+        self.check_fn_decl(cx, signature.decl, item.owner_id.def_id);
     }
 
-    fn check_fn(
-        &mut self,
-        cx: &LateContext<'_>,
-        fn_kind: FnKind<'_>,
-        decl: &FnDecl<'_>,
-        _: &Body<'_>,
-        _: Span,
-        _: HirId,
-    ) {
-        // It's unclear what part of a closure you would span, so for now it's ignored.
-        // Trait implementations should also not be linted.
-        // If this is changed, please also make sure not to call `hir_ty_to_ty` below.
-        let FnKind::ItemFn(..) = fn_kind else { return };
-
-        check_fn_decl(cx, decl);
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+        let ItemKind::Fn(signature, ..) = &item.kind else { return };
+        self.check_fn_decl(cx, signature.decl, item.owner_id.def_id);
     }
 }

tests/ui/unnecessary_box_returns.rs

@@ -5,7 +5,7 @@ trait Bar {
     fn baz(&self) -> Box<usize>;
 }
 
-struct Foo {}
+pub struct Foo {}
 
 impl Bar for Foo {
     // don't lint: this is a problem with the trait, not the implementation
@@ -20,7 +20,12 @@ fn boxed_usize() -> Box<usize> {
 }
 
 // lint
-fn boxed_foo() -> Box<Foo> {
+fn _boxed_foo() -> Box<Foo> {
+    Box::new(Foo {})
+}
+
+// don't lint: this is exported
+pub fn boxed_foo() -> Box<Foo> {
     Box::new(Foo {})
 }
 

tests/ui/unnecessary_box_returns.stderr

@@ -1,22 +1,22 @@
-error: function returns `Box<usize>` when `usize` implements `Sized`
+error: boxed return of the sized type `usize`
   --> $DIR/unnecessary_box_returns.rs:5:22
    |
 LL |     fn baz(&self) -> Box<usize>;
-   |                      ^^^^^^^^^^ help: change the return type to: `usize`
+   |                      ^^^^^^^^^^ help: try: `usize`
    |
    = note: `-D clippy::unnecessary-box-returns` implied by `-D warnings`
 
-error: function returns `Box<usize>` when `usize` implements `Sized`
+error: boxed return of the sized type `usize`
   --> $DIR/unnecessary_box_returns.rs:18:21
    |
 LL | fn boxed_usize() -> Box<usize> {
-   |                     ^^^^^^^^^^ help: change the return type to: `usize`
+   |                     ^^^^^^^^^^ help: try: `usize`
 
-error: function returns `Box<Foo>` when `Foo` implements `Sized`
-  --> $DIR/unnecessary_box_returns.rs:23:19
+error: boxed return of the sized type `Foo`
+  --> $DIR/unnecessary_box_returns.rs:23:20
    |
-LL | fn boxed_foo() -> Box<Foo> {
-   |                   ^^^^^^^^ help: change the return type to: `Foo`
+LL | fn _boxed_foo() -> Box<Foo> {
+   |                    ^^^^^^^^ help: try: `Foo`
 
 error: aborting due to 3 previous errors