[box_default
]: Preserve required path segments
When encountering code such as: ``` Box::new(outer::Inner::default()) ``` clippy would suggest replacing with `Box::<Inner>::default()`, dropping the `outer::` segment. This behavior is incorrect and that commit fixes it. What it does is it checks the contents of the `Box::new` and, if it is of the form `A::B::default`, does a text replacement, inserting `A::B` in the `Box`'s quickfix generic list. If the source does not match that pattern (including `Vec::from(..)` or other `T::new()` calls), we then fallback to the original code. Fixes #11927
This commit is contained in:
parent
b09fb8ab3b
commit
97dc4b22c6
@ -1,11 +1,12 @@
|
|||||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||||
use clippy_utils::macros::macro_backtrace;
|
use clippy_utils::macros::macro_backtrace;
|
||||||
|
use clippy_utils::source::snippet_opt;
|
||||||
use clippy_utils::ty::expr_sig;
|
use clippy_utils::ty::expr_sig;
|
||||||
use clippy_utils::{is_default_equivalent, path_def_id};
|
use clippy_utils::{is_default_equivalent, path_def_id};
|
||||||
use rustc_errors::Applicability;
|
use rustc_errors::Applicability;
|
||||||
use rustc_hir::def::Res;
|
use rustc_hir::def::Res;
|
||||||
use rustc_hir::intravisit::{walk_ty, Visitor};
|
use rustc_hir::intravisit::{walk_ty, Visitor};
|
||||||
use rustc_hir::{Block, Expr, ExprKind, Local, Node, QPath, TyKind};
|
use rustc_hir::{Block, Expr, ExprKind, Local, Node, QPath, Ty, TyKind};
|
||||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||||
use rustc_middle::lint::in_external_macro;
|
use rustc_middle::lint::in_external_macro;
|
||||||
use rustc_middle::ty::print::with_forced_trimmed_paths;
|
use rustc_middle::ty::print::with_forced_trimmed_paths;
|
||||||
@ -41,13 +42,24 @@
|
|||||||
|
|
||||||
impl LateLintPass<'_> for BoxDefault {
|
impl LateLintPass<'_> for BoxDefault {
|
||||||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
||||||
|
// If the expression is a call (`Box::new(...)`)
|
||||||
if let ExprKind::Call(box_new, [arg]) = expr.kind
|
if let ExprKind::Call(box_new, [arg]) = expr.kind
|
||||||
|
// And call is of the form `<T>::something`
|
||||||
|
// Here, it would be `<Box>::new`
|
||||||
&& let ExprKind::Path(QPath::TypeRelative(ty, seg)) = box_new.kind
|
&& let ExprKind::Path(QPath::TypeRelative(ty, seg)) = box_new.kind
|
||||||
&& let ExprKind::Call(arg_path, ..) = arg.kind
|
// And that method is `new`
|
||||||
&& !in_external_macro(cx.sess(), expr.span)
|
|
||||||
&& (expr.span.eq_ctxt(arg.span) || is_local_vec_expn(cx, arg, expr))
|
|
||||||
&& seg.ident.name == sym::new
|
&& seg.ident.name == sym::new
|
||||||
|
// And the call is that of a `Box` method
|
||||||
&& path_def_id(cx, ty).map_or(false, |id| Some(id) == cx.tcx.lang_items().owned_box())
|
&& path_def_id(cx, ty).map_or(false, |id| Some(id) == cx.tcx.lang_items().owned_box())
|
||||||
|
// And the single argument to the call is another function call
|
||||||
|
// This is the `T::default()` of `Box::new(T::default())`
|
||||||
|
&& let ExprKind::Call(arg_path, inner_call_args) = arg.kind
|
||||||
|
// And we are not in a foreign crate's macro
|
||||||
|
&& !in_external_macro(cx.sess(), expr.span)
|
||||||
|
// And the argument expression has the same context as the outer call expression
|
||||||
|
// or that we are inside a `vec!` macro expansion
|
||||||
|
&& (expr.span.eq_ctxt(arg.span) || is_local_vec_expn(cx, arg, expr))
|
||||||
|
// And the argument is equivalent to `Default::default()`
|
||||||
&& is_default_equivalent(cx, arg)
|
&& is_default_equivalent(cx, arg)
|
||||||
{
|
{
|
||||||
span_lint_and_sugg(
|
span_lint_and_sugg(
|
||||||
@ -59,7 +71,17 @@ fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
|||||||
if is_plain_default(cx, arg_path) || given_type(cx, expr) {
|
if is_plain_default(cx, arg_path) || given_type(cx, expr) {
|
||||||
"Box::default()".into()
|
"Box::default()".into()
|
||||||
} else if let Some(arg_ty) = cx.typeck_results().expr_ty(arg).make_suggestable(cx.tcx, true) {
|
} else if let Some(arg_ty) = cx.typeck_results().expr_ty(arg).make_suggestable(cx.tcx, true) {
|
||||||
with_forced_trimmed_paths!(format!("Box::<{arg_ty}>::default()"))
|
// Check if we can copy from the source expression in the replacement.
|
||||||
|
// We need the call to have no argument (see `explicit_default_type`).
|
||||||
|
if inner_call_args.is_empty()
|
||||||
|
&& let Some(ty) = explicit_default_type(arg_path)
|
||||||
|
&& let Some(s) = snippet_opt(cx, ty.span)
|
||||||
|
{
|
||||||
|
format!("Box::<{s}>::default()")
|
||||||
|
} else {
|
||||||
|
// Otherwise, use the inferred type's formatting.
|
||||||
|
with_forced_trimmed_paths!(format!("Box::<{arg_ty}>::default()"))
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
return;
|
return;
|
||||||
},
|
},
|
||||||
@ -81,6 +103,20 @@ fn is_plain_default(cx: &LateContext<'_>, arg_path: &Expr<'_>) -> bool {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Checks whether the call is of the form `A::B::f()`. Returns `A::B` if it is.
|
||||||
|
//
|
||||||
|
// In the event we have this kind of construct, it's easy to use `A::B` as a replacement in the
|
||||||
|
// quickfix. `f` must however have no parameter. Should `f` have some, then some of the type of
|
||||||
|
// `A::B` may be inferred from the arguments. This would be the case for `Vec::from([0; false])`,
|
||||||
|
// where the argument to `from` allows inferring this is a `Vec<bool>`
|
||||||
|
fn explicit_default_type<'a>(arg_path: &'a Expr<'_>) -> Option<&'a Ty<'a>> {
|
||||||
|
if let ExprKind::Path(QPath::TypeRelative(ty, _)) = &arg_path.kind {
|
||||||
|
Some(ty)
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn is_local_vec_expn(cx: &LateContext<'_>, expr: &Expr<'_>, ref_expr: &Expr<'_>) -> bool {
|
fn is_local_vec_expn(cx: &LateContext<'_>, expr: &Expr<'_>, ref_expr: &Expr<'_>) -> bool {
|
||||||
macro_backtrace(expr.span).next().map_or(false, |call| {
|
macro_backtrace(expr.span).next().map_or(false, |call| {
|
||||||
cx.tcx.is_diagnostic_item(sym::vec_macro, call.def_id) && call.span.eq_ctxt(ref_expr.span)
|
cx.tcx.is_diagnostic_item(sym::vec_macro, call.def_id) && call.span.eq_ctxt(ref_expr.span)
|
||||||
|
@ -21,7 +21,7 @@ macro_rules! outer {
|
|||||||
fn main() {
|
fn main() {
|
||||||
let _string: Box<String> = Box::default();
|
let _string: Box<String> = Box::default();
|
||||||
let _byte = Box::<u8>::default();
|
let _byte = Box::<u8>::default();
|
||||||
let _vec = Box::<Vec<u8>>::default();
|
let _vec = Box::<Vec::<u8>>::default();
|
||||||
let _impl = Box::<ImplementsDefault>::default();
|
let _impl = Box::<ImplementsDefault>::default();
|
||||||
let _impl2 = Box::<ImplementsDefault>::default();
|
let _impl2 = Box::<ImplementsDefault>::default();
|
||||||
let _impl3: Box<ImplementsDefault> = Box::default();
|
let _impl3: Box<ImplementsDefault> = Box::default();
|
||||||
@ -104,3 +104,19 @@ fn issue_11868() {
|
|||||||
foo(bar!(vec![]));
|
foo(bar!(vec![]));
|
||||||
foo(bar!(vec![1]));
|
foo(bar!(vec![1]));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Issue #11927: The quickfix for the `Box::new` suggests replacing with `Box::<Inner>::default()`,
|
||||||
|
// removing the `outer::` segment.
|
||||||
|
fn issue_11927() {
|
||||||
|
mod outer {
|
||||||
|
#[derive(Default)]
|
||||||
|
pub struct Inner {
|
||||||
|
_i: usize,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn foo() {
|
||||||
|
let _b = Box::<outer::Inner>::default();
|
||||||
|
let _b = Box::<std::collections::HashSet::<i32>>::default();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@ -104,3 +104,19 @@ macro_rules! bar {
|
|||||||
foo(bar!(vec![]));
|
foo(bar!(vec![]));
|
||||||
foo(bar!(vec![1]));
|
foo(bar!(vec![1]));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Issue #11927: The quickfix for the `Box::new` suggests replacing with `Box::<Inner>::default()`,
|
||||||
|
// removing the `outer::` segment.
|
||||||
|
fn issue_11927() {
|
||||||
|
mod outer {
|
||||||
|
#[derive(Default)]
|
||||||
|
pub struct Inner {
|
||||||
|
_i: usize,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn foo() {
|
||||||
|
let _b = Box::new(outer::Inner::default());
|
||||||
|
let _b = Box::new(std::collections::HashSet::<i32>::new());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
@ -17,7 +17,7 @@ error: `Box::new(_)` of default value
|
|||||||
--> tests/ui/box_default.rs:24:16
|
--> tests/ui/box_default.rs:24:16
|
||||||
|
|
|
|
||||||
LL | let _vec = Box::new(Vec::<u8>::new());
|
LL | let _vec = Box::new(Vec::<u8>::new());
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<Vec<u8>>::default()`
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<Vec::<u8>>::default()`
|
||||||
|
|
||||||
error: `Box::new(_)` of default value
|
error: `Box::new(_)` of default value
|
||||||
--> tests/ui/box_default.rs:25:17
|
--> tests/ui/box_default.rs:25:17
|
||||||
@ -97,5 +97,17 @@ error: `Box::new(_)` of default value
|
|||||||
LL | Some(Box::new(Foo::default()))
|
LL | Some(Box::new(Foo::default()))
|
||||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<Foo>::default()`
|
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<Foo>::default()`
|
||||||
|
|
||||||
error: aborting due to 16 previous errors
|
error: `Box::new(_)` of default value
|
||||||
|
--> tests/ui/box_default.rs:119:18
|
||||||
|
|
|
||||||
|
LL | let _b = Box::new(outer::Inner::default());
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<outer::Inner>::default()`
|
||||||
|
|
||||||
|
error: `Box::new(_)` of default value
|
||||||
|
--> tests/ui/box_default.rs:120:18
|
||||||
|
|
|
||||||
|
LL | let _b = Box::new(std::collections::HashSet::<i32>::new());
|
||||||
|
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::<std::collections::HashSet::<i32>>::default()`
|
||||||
|
|
||||||
|
error: aborting due to 18 previous errors
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user