Avoid breaking exported API

This commit is contained in:
Micha White 2023-02-26 13:17:34 -05:00
parent 1b55c81db5
commit a143fb7a11
No known key found for this signature in database
GPG Key ID: AED94BFA1C301389
6 changed files with 68 additions and 40 deletions

View File

@ -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

View File

@ -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`
}

View File

@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_then;
use rustc_errors::Applicability;
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_session::{declare_tool_lint, impl_lint_pass};
declare_clippy_lint! {
/// ### What it does
@ -32,48 +32,66 @@ declare_clippy_lint! {
pedantic,
"Needlessly returning a Box"
}
declare_lint_pass!(UnnecessaryBoxReturns => [UNNECESSARY_BOX_RETURNS]);
fn check_fn_decl(cx: &LateContext<'_>, decl: &FnDecl<'_>, def_id: LocalDefId) {
let FnRetTy::Return(return_ty_hir) = &decl.output else { return };
pub struct UnnecessaryBoxReturns {
avoid_breaking_exported_api: bool,
}
let return_ty = cx
.tcx
.erase_late_bound_regions(cx.tcx.fn_sig(def_id).skip_binder())
.output();
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();
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;
}
// 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");
},
);
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();
if !return_ty.is_box() {
return;
}
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);
self.check_fn_decl(cx, signature.decl, item.owner_id.def_id);
}
fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::ImplItem<'_>) {
@ -86,11 +104,11 @@ impl LateLintPass<'_> for UnnecessaryBoxReturns {
}
let ImplItemKind::Fn(signature, ..) = &item.kind else { return };
check_fn_decl(cx, signature.decl, item.owner_id.def_id);
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, item.owner_id.def_id);
self.check_fn_decl(cx, signature.decl, item.owner_id.def_id);
}
}

View File

@ -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),

View File

@ -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 {})
}

View File

@ -24,10 +24,10 @@ LL | fn boxed_usize() -> Box<usize> {
= help: changing this also requires a change to the return expressions in this function
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`
|
= help: changing this also requires a change to the return expressions in this function