Avoid breaking exported API

Author: botahamec

book/src/lint_configuration.md

@@ -130,6 +130,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
 * [option_option](https://rust-lang.github.io/rust-clippy/master/index.html#option_option)
 * [linkedlist](https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist)
 * [rc_mutex](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex)
+* [unnecessary_box_returns](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns)
 
 
 ### msrv

clippy_lints/src/lib.rs

@@ -941,7 +941,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|_| Box::new(allow_attributes::AllowAttribute));
     store.register_late_pass(move |_| Box::new(manual_main_separator_str::ManualMainSeparatorStr::new(msrv())));
     store.register_late_pass(|_| Box::new(unnecessary_struct_initialization::UnnecessaryStruct));
-    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,9 +1,9 @@
 use clippy_utils::{diagnostics::span_lint_and_sugg, ty::implements_trait};
 use rustc_errors::Applicability;
-use rustc_hir::{FnDecl, FnRetTy, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind};
+use rustc_hir::{def_id::LocalDefId, 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_session::{declare_tool_lint, impl_lint_pass};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -33,40 +33,58 @@ declare_clippy_lint! {
     pedantic,
     "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_impl_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::ImplItem<'_>) {
@@ -79,11 +97,11 @@ impl LateLintPass<'_> for UnnecessaryBoxReturns {
         }
 
         let ImplItemKind::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_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
         let ItemKind::Fn(signature, ..) = &item.kind else { return };
-        check_fn_decl(cx, signature.decl);
+        self.check_fn_decl(cx, signature.decl, item.owner_id.def_id);
     }
 }

clippy_lints/src/utils/conf.rs

@@ -249,7 +249,7 @@ define_Conf! {
     /// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
     /// ```
     (arithmetic_side_effects_allowed_unary: rustc_data_structures::fx::FxHashSet<String> = <_>::default()),
-    /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX.
+    /// Lint: ENUM_VARIANT_NAMES, LARGE_TYPES_PASSED_BY_VALUE, TRIVIALLY_COPY_PASS_BY_REF, UNNECESSARY_WRAPS, UNUSED_SELF, UPPER_CASE_ACRONYMS, WRONG_SELF_CONVENTION, BOX_COLLECTION, REDUNDANT_ALLOCATION, RC_BUFFER, VEC_BOX, OPTION_OPTION, LINKEDLIST, RC_MUTEX, UNNECESSARY_BOX_RETURNS.
     ///
     /// Suppress lints whenever the suggested change would cause breakage for other crates.
     (avoid_breaking_exported_api: bool = true),

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
@@ -27,7 +27,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

@@ -19,10 +19,10 @@ LL | fn boxed_usize() -> Box<usize> {
    |                     ^^^^^^^^^^ help: try: `usize`
 
 error: boxed return of the sized type `Foo`
-  --> $DIR/unnecessary_box_returns.rs:30:19
+  --> $DIR/unnecessary_box_returns.rs:30:20
    |
-LL | fn boxed_foo() -> Box<Foo> {
-   |                   ^^^^^^^^ help: try: `Foo`
+LL | fn _boxed_foo() -> Box<Foo> {
+   |                    ^^^^^^^^ help: try: `Foo`
 
 error: aborting due to 4 previous errors