diff --git a/clippy_lints/src/box_default.rs b/clippy_lints/src/box_default.rs index d9e63f0d0..779ae03c4 100644 --- a/clippy_lints/src/box_default.rs +++ b/clippy_lints/src/box_default.rs @@ -1,11 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::macros::macro_backtrace; +use clippy_utils::source::snippet_opt; use clippy_utils::ty::expr_sig; use clippy_utils::{is_default_equivalent, path_def_id}; use rustc_errors::Applicability; use rustc_hir::def::Res; 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_middle::lint::in_external_macro; use rustc_middle::ty::print::with_forced_trimmed_paths; @@ -41,13 +42,24 @@ declare_lint_pass!(BoxDefault => [BOX_DEFAULT]); impl LateLintPass<'_> for BoxDefault { 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 + // And call is of the form `::something` + // Here, it would be `::new` && let ExprKind::Path(QPath::TypeRelative(ty, seg)) = box_new.kind - && let ExprKind::Call(arg_path, ..) = arg.kind - && !in_external_macro(cx.sess(), expr.span) - && (expr.span.eq_ctxt(arg.span) || is_local_vec_expn(cx, arg, expr)) + // And that method is `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()) + // 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) { span_lint_and_sugg( @@ -59,7 +71,17 @@ impl LateLintPass<'_> for BoxDefault { if is_plain_default(cx, arg_path) || given_type(cx, expr) { "Box::default()".into() } 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 { 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` +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 { 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) diff --git a/tests/ui/box_default.fixed b/tests/ui/box_default.fixed index 48408e191..fea7405c6 100644 --- a/tests/ui/box_default.fixed +++ b/tests/ui/box_default.fixed @@ -21,7 +21,7 @@ macro_rules! outer { fn main() { let _string: Box = Box::default(); let _byte = Box::::default(); - let _vec = Box::>::default(); + let _vec = Box::>::default(); let _impl = Box::::default(); let _impl2 = Box::::default(); let _impl3: Box = Box::default(); @@ -104,3 +104,19 @@ fn issue_11868() { foo(bar!(vec![])); foo(bar!(vec![1])); } + +// Issue #11927: The quickfix for the `Box::new` suggests replacing with `Box::::default()`, +// removing the `outer::` segment. +fn issue_11927() { + mod outer { + #[derive(Default)] + pub struct Inner { + _i: usize, + } + } + + fn foo() { + let _b = Box::::default(); + let _b = Box::>::default(); + } +} diff --git a/tests/ui/box_default.rs b/tests/ui/box_default.rs index 58b912707..eecba7464 100644 --- a/tests/ui/box_default.rs +++ b/tests/ui/box_default.rs @@ -104,3 +104,19 @@ fn issue_11868() { foo(bar!(vec![])); foo(bar!(vec![1])); } + +// Issue #11927: The quickfix for the `Box::new` suggests replacing with `Box::::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::::new()); + } +} diff --git a/tests/ui/box_default.stderr b/tests/ui/box_default.stderr index f727b3773..8bb5917a6 100644 --- a/tests/ui/box_default.stderr +++ b/tests/ui/box_default.stderr @@ -17,7 +17,7 @@ error: `Box::new(_)` of default value --> tests/ui/box_default.rs:24:16 | LL | let _vec = Box::new(Vec::::new()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::>::default()` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::>::default()` error: `Box::new(_)` of default value --> tests/ui/box_default.rs:25:17 @@ -97,5 +97,17 @@ error: `Box::new(_)` of default value LL | Some(Box::new(Foo::default())) | ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::::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::::default()` + +error: `Box::new(_)` of default value + --> tests/ui/box_default.rs:120:18 + | +LL | let _b = Box::new(std::collections::HashSet::::new()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Box::>::default()` + +error: aborting due to 18 previous errors