Skip to content

Commit a143fb7

Browse files
botahamecbotahamec
committed
Avoid breaking exported API
1 parent 1b55c81 commit a143fb7

File tree

6 files changed

+68
-40
lines changed

6 files changed

+68
-40
lines changed

‎book/src/lint_configuration.md

+1
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ Suppress lints whenever the suggested change would cause breakage for other crat
130130
* [option_option](https://rust-lang.github.io/rust-clippy/master/index.html#option_option)
131131
* [linkedlist](https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist)
132132
* [rc_mutex](https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex)
133+
* [unnecessary_box_returns](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns)
133134

134135

135136
### msrv

‎clippy_lints/src/lib.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -941,7 +941,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
941941
store.register_late_pass(|_| Box::new(allow_attributes::AllowAttribute));
942942
store.register_late_pass(move |_| Box::new(manual_main_separator_str::ManualMainSeparatorStr::new(msrv())));
943943
store.register_late_pass(|_| Box::new(unnecessary_struct_initialization::UnnecessaryStruct));
944-
store.register_late_pass(|_| Box::new(unnecessary_box_returns::UnnecessaryBoxReturns));
944+
store.register_late_pass(move |_| {
945+
Box::new(unnecessary_box_returns::UnnecessaryBoxReturns::new(
946+
avoid_breaking_exported_api,
947+
))
948+
});
945949
// add lints here, do not remove this comment, it's used in `new_lint`
946950
}
947951

‎clippy_lints/src/unnecessary_box_returns.rs

+51-33
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
22
use rustc_errors::Applicability;
33
use rustc_hir::{def_id::LocalDefId, FnDecl, FnRetTy, ImplItemKind, Item, ItemKind, Node, TraitItem, TraitItemKind};
44
use rustc_lint::{LateContext, LateLintPass};
5-
use rustc_session::{declare_lint_pass, declare_tool_lint};
5+
use rustc_session::{declare_tool_lint, impl_lint_pass};
66

77
declare_clippy_lint! {
88
/// ### What it does
@@ -32,48 +32,66 @@ declare_clippy_lint! {
3232
pedantic,
3333
"Needlessly returning a Box"
3434
}
35-
declare_lint_pass!(UnnecessaryBoxReturns => [UNNECESSARY_BOX_RETURNS]);
3635

37-
fn check_fn_decl(cx: &LateContext<'_>, decl: &FnDecl<'_>, def_id: LocalDefId) {
38-
let FnRetTy::Return(return_ty_hir) = &decl.output else { return };
36+
pub struct UnnecessaryBoxReturns {
37+
avoid_breaking_exported_api: bool,
38+
}
3939

40-
let return_ty = cx
41-
.tcx
42-
.erase_late_bound_regions(cx.tcx.fn_sig(def_id).skip_binder())
43-
.output();
40+
impl_lint_pass!(UnnecessaryBoxReturns => [UNNECESSARY_BOX_RETURNS]);
4441

45-
if !return_ty.is_box() {
46-
return;
42+
impl UnnecessaryBoxReturns {
43+
pub fn new(avoid_breaking_exported_api: bool) -> Self {
44+
Self {
45+
avoid_breaking_exported_api,
46+
}
4747
}
4848

49-
let boxed_ty = return_ty.boxed_ty();
49+
fn check_fn_decl(&mut self, cx: &LateContext<'_>, decl: &FnDecl<'_>, def_id: LocalDefId) {
50+
// we don't want to tell someone to break an exported function if they ask us not to
51+
if self.avoid_breaking_exported_api && cx.effective_visibilities.is_exported(def_id) {
52+
return;
53+
}
54+
55+
let FnRetTy::Return(return_ty_hir) = &decl.output else { return };
56+
57+
let return_ty = cx
58+
.tcx
59+
.erase_late_bound_regions(cx.tcx.fn_sig(def_id).skip_binder())
60+
.output();
5061

51-
// it's sometimes useful to return Box<T> if T is unsized, so don't lint those
52-
if boxed_ty.is_sized(cx.tcx, cx.param_env) {
53-
span_lint_and_then(
54-
cx,
55-
UNNECESSARY_BOX_RETURNS,
56-
return_ty_hir.span,
57-
format!("boxed return of the sized type `{boxed_ty}`").as_str(),
58-
|diagnostic| {
59-
diagnostic.span_suggestion(
60-
return_ty_hir.span,
61-
"try",
62-
boxed_ty.to_string(),
63-
// the return value and function callers also needs to
64-
// be changed, so this can't be MachineApplicable
65-
Applicability::Unspecified,
66-
);
67-
diagnostic.help("changing this also requires a change to the return expressions in this function");
68-
},
69-
);
62+
if !return_ty.is_box() {
63+
return;
64+
}
65+
66+
let boxed_ty = return_ty.boxed_ty();
67+
68+
// it's sometimes useful to return Box<T> if T is unsized, so don't lint those
69+
if boxed_ty.is_sized(cx.tcx, cx.param_env) {
70+
span_lint_and_then(
71+
cx,
72+
UNNECESSARY_BOX_RETURNS,
73+
return_ty_hir.span,
74+
format!("boxed return of the sized type `{boxed_ty}`").as_str(),
75+
|diagnostic| {
76+
diagnostic.span_suggestion(
77+
return_ty_hir.span,
78+
"try",
79+
boxed_ty.to_string(),
80+
// the return value and function callers also needs to
81+
// be changed, so this can't be MachineApplicable
82+
Applicability::Unspecified,
83+
);
84+
diagnostic.help("changing this also requires a change to the return expressions in this function");
85+
},
86+
);
87+
}
7088
}
7189
}
7290

7391
impl LateLintPass<'_> for UnnecessaryBoxReturns {
7492
fn check_trait_item(&mut self, cx: &LateContext<'_>, item: &TraitItem<'_>) {
7593
let TraitItemKind::Fn(signature, _) = &item.kind else { return };
76-
check_fn_decl(cx, signature.decl, item.owner_id.def_id);
94+
self.check_fn_decl(cx, signature.decl, item.owner_id.def_id);
7795
}
7896

7997
fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::ImplItem<'_>) {
@@ -86,11 +104,11 @@ impl LateLintPass<'_> for UnnecessaryBoxReturns {
86104
}
87105

88106
let ImplItemKind::Fn(signature, ..) = &item.kind else { return };
89-
check_fn_decl(cx, signature.decl, item.owner_id.def_id);
107+
self.check_fn_decl(cx, signature.decl, item.owner_id.def_id);
90108
}
91109

92110
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
93111
let ItemKind::Fn(signature, ..) = &item.kind else { return };
94-
check_fn_decl(cx, signature.decl, item.owner_id.def_id);
112+
self.check_fn_decl(cx, signature.decl, item.owner_id.def_id);
95113
}
96114
}

‎clippy_lints/src/utils/conf.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ define_Conf! {
249249
/// arithmetic-side-effects-allowed-unary = ["SomeType", "AnotherType"]
250250
/// ```
251251
(arithmetic_side_effects_allowed_unary: rustc_data_structures::fx::FxHashSet<String> = <_>::default()),
252-
/// 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.
252+
/// 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.
253253
///
254254
/// Suppress lints whenever the suggested change would cause breakage for other crates.
255255
(avoid_breaking_exported_api: bool = true),

‎tests/ui/unnecessary_box_returns.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ trait Bar {
55
fn baz(&self) -> Box<usize>;
66
}
77

8-
struct Foo {}
8+
pub struct Foo {}
99

1010
impl Bar for Foo {
1111
// don't lint: this is a problem with the trait, not the implementation
@@ -27,7 +27,12 @@ fn boxed_usize() -> Box<usize> {
2727
}
2828

2929
// lint
30-
fn boxed_foo() -> Box<Foo> {
30+
fn _boxed_foo() -> Box<Foo> {
31+
Box::new(Foo {})
32+
}
33+
34+
// don't lint: this is exported
35+
pub fn boxed_foo() -> Box<Foo> {
3136
Box::new(Foo {})
3237
}
3338

‎tests/ui/unnecessary_box_returns.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ LL | fn boxed_usize() -> Box<usize> {
2424
= help: changing this also requires a change to the return expressions in this function
2525

2626
error: boxed return of the sized type `Foo`
27-
--> $DIR/unnecessary_box_returns.rs:30:19
27+
--> $DIR/unnecessary_box_returns.rs:30:20
2828
|
29-
LL | fn boxed_foo() -> Box<Foo> {
30-
| ^^^^^^^^ help: try: `Foo`
29+
LL | fn _boxed_foo() -> Box<Foo> {
30+
| ^^^^^^^^ help: try: `Foo`
3131
|
3232
= help: changing this also requires a change to the return expressions in this function
3333

0 commit comments

Comments
 (0)