Skip to content

Commit b69304a

Browse files
committed
some code detection and suggestions
1 parent e64c5f4 commit b69304a

File tree

4 files changed

+246
-100
lines changed

4 files changed

+246
-100
lines changed

clippy_lints/src/new_without_default.rs

Lines changed: 78 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_hir_and_then;
2-
use clippy_utils::return_ty;
32
use clippy_utils::source::snippet;
43
use clippy_utils::sugg::DiagExt;
4+
use clippy_utils::{is_default_equivalent_call, return_ty};
55
use rustc_errors::Applicability;
66
use rustc_hir as hir;
77
use rustc_hir::HirIdMap;
@@ -134,7 +134,7 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
134134
if impl_item.span.in_external_macro(cx.sess().source_map()) {
135135
return;
136136
}
137-
if let hir::ImplItemKind::Fn(ref sig, _) = impl_item.kind {
137+
if let hir::ImplItemKind::Fn(ref sig, body_id) = impl_item.kind {
138138
let name = impl_item.ident.name;
139139
let id = impl_item.owner_id;
140140
if sig.header.is_unsafe() {
@@ -199,9 +199,31 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
199199
return;
200200
}
201201
suggest_new_without_default(cx, item, impl_item, id, self_ty, generics, impl_self_ty);
202-
} else {
202+
} else if let hir::ExprKind::Block(block, _) = cx.tcx.hir().body(body_id).value.kind {
203203
// this type has an automatically derived `Default` implementation
204204
// check if `new` and `default` are equivalent
205+
if !check_block_calls_default(cx, block) {
206+
let self_ty_fmt = self_ty.to_string();
207+
let self_type_snip = snippet(cx, impl_self_ty.span, &self_ty_fmt);
208+
span_lint_hir_and_then(
209+
cx,
210+
DEFAULT_MISMATCHES_NEW,
211+
id.into(),
212+
impl_item.span,
213+
format!(
214+
"you should consider delegating to the auto-derived `Default` for `{self_type_snip}`"
215+
),
216+
|diag| {
217+
diag.suggest_prepend_item(
218+
cx,
219+
block.span,
220+
"try using this",
221+
&"Self::default()",
222+
Applicability::MaybeIncorrect,
223+
);
224+
},
225+
);
226+
}
205227
}
206228
}
207229
}
@@ -211,14 +233,55 @@ impl<'tcx> LateLintPass<'tcx> for NewWithoutDefault {
211233
}
212234
}
213235

236+
/// Check if a block contains one of these:
237+
/// - Empty block with an expr (e.g., `{ Self::default() }`)
238+
/// - One statement (e.g., `{ return Self::default(); }`)
239+
fn check_block_calls_default(cx: &LateContext<'_>, block: &hir::Block<'_>) -> bool {
240+
if let Some(expr) = block.expr
241+
&& block.stmts.is_empty()
242+
{
243+
// Check the block's trailing expression (e.g., `Self::default()`)
244+
check_expr_call_default(cx, expr)
245+
} else if let [hir::Stmt { kind, .. }] = block.stmts
246+
&& let hir::StmtKind::Expr(expr) | hir::StmtKind::Semi(expr) = kind
247+
{
248+
// Check if the single statement is a return or expr
249+
match expr.kind {
250+
// Case 1: `return Self::default();`
251+
hir::ExprKind::Ret(Some(ret_expr)) => check_expr_call_default(cx, ret_expr),
252+
// Case 2: `Self::default()` (possibly inside a block)
253+
hir::ExprKind::Block(block, _) => check_block_calls_default(cx, block),
254+
// Case 3: Direct call (e.g., `Self::default()` as the entire body)
255+
_ => check_expr_call_default(cx, expr),
256+
}
257+
} else {
258+
false
259+
}
260+
}
261+
262+
/// Check for `Self::default()` call syntax or equivalent
263+
fn check_expr_call_default(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool {
264+
if let hir::ExprKind::Call(callee, args) = expr.kind
265+
&& args.is_empty()
266+
// FIXME: does this include `Self { }` style calls, which is equivalent,
267+
// but not the same as `Self::default()`?
268+
// FIXME: what should the whole_call_expr (3rd arg) be?
269+
&& is_default_equivalent_call(cx, callee, None)
270+
{
271+
true
272+
} else {
273+
false
274+
}
275+
}
276+
214277
fn suggest_new_without_default<'tcx>(
215278
cx: &LateContext<'tcx>,
216-
item: &rustc_hir::Item<'_>,
217-
impl_item: &rustc_hir::ImplItem<'_>,
218-
id: rustc_hir::OwnerId,
279+
item: &hir::Item<'_>,
280+
impl_item: &hir::ImplItem<'_>,
281+
id: hir::OwnerId,
219282
self_ty: Ty<'tcx>,
220-
generics: &rustc_hir::Generics<'_>,
221-
impl_self_ty: &rustc_hir::Ty<'_>,
283+
generics: &hir::Generics<'_>,
284+
impl_self_ty: &hir::Ty<'_>,
222285
) {
223286
let generics_sugg = snippet(cx, generics.span, "");
224287
let where_clause_sugg = if generics.has_where_clause_predicates {
@@ -239,23 +302,15 @@ fn suggest_new_without_default<'tcx>(
239302
cx,
240303
item.span,
241304
"try adding this",
242-
&create_new_without_default_suggest_msg(&self_type_snip, &generics_sugg, &where_clause_sugg),
305+
&format!(
306+
"impl{generics_sugg} Default for {self_type_snip}{where_clause_sugg} {{
307+
fn default() -> Self {{
308+
Self::new()
309+
}}
310+
}}"
311+
),
243312
Applicability::MachineApplicable,
244313
);
245314
},
246315
);
247316
}
248-
249-
fn create_new_without_default_suggest_msg(
250-
self_type_snip: &str,
251-
generics_sugg: &str,
252-
where_clause_sugg: &str,
253-
) -> String {
254-
#[rustfmt::skip]
255-
format!(
256-
"impl{generics_sugg} Default for {self_type_snip}{where_clause_sugg} {{
257-
fn default() -> Self {{
258-
Self::new()
259-
}}
260-
}}")
261-
}

tests/ui/default_mismatches_new.fixed

Lines changed: 0 additions & 46 deletions
This file was deleted.

tests/ui/default_mismatches_new.rs

Lines changed: 108 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,9 @@
1+
#![allow(clippy::needless_return)]
12
#![warn(clippy::default_mismatches_new)]
23

3-
#[derive(Default)]
4-
struct WithDeriveDefault(i32);
5-
impl WithDeriveDefault {
6-
pub fn new() -> Self {
7-
Self(42)
8-
}
9-
}
10-
4+
//
5+
// Nothing to change
6+
//
117
struct WithManualDefault(i32);
128
impl WithManualDefault {
139
pub fn new() -> Self {
@@ -20,21 +16,115 @@ impl Default for WithManualDefault {
2016
}
2117
}
2218

23-
fn main() {}
19+
#[derive(Default)]
20+
struct WithCallToDefaultDefault(i32);
21+
impl WithCallToDefaultDefault {
22+
pub fn new() -> Self {
23+
Default::default()
24+
}
25+
}
2426

25-
/*
27+
#[derive(Default)]
28+
struct WithCallToSelfDefault(i32);
29+
impl WithCallToSelfDefault {
30+
pub fn new() -> Self {
31+
Self::default()
32+
}
33+
}
2634

27-
DELETE THE FOLLOWING CODE.
35+
#[derive(Default)]
36+
struct WithCallToTypeDefault(i32);
37+
impl WithCallToTypeDefault {
38+
pub fn new() -> Self {
39+
WithCallToTypeDefault::default()
40+
}
41+
}
2842

29-
We only need it until the default_mismatches_new lint is implemented.
43+
#[derive(Default)]
44+
struct WithCallToFullTypeDefault(i32);
45+
impl WithCallToFullTypeDefault {
46+
pub fn new() -> Self {
47+
crate::WithCallToFullTypeDefault::default()
48+
}
49+
}
3050

31-
*/
32-
pub struct Foo;
51+
#[derive(Default)]
52+
struct WithReturnCallToSelfDefault(i32);
53+
impl WithReturnCallToSelfDefault {
54+
pub fn new() -> Self {
55+
return Self::default();
56+
}
57+
}
3358

34-
impl Foo {
35-
pub fn new() -> Foo {
36-
//~^ new_without_default
59+
//
60+
// Offer suggestions
61+
//
3762

38-
Foo
63+
#[derive(Default)]
64+
struct WithDeriveDefault;
65+
impl WithDeriveDefault {
66+
pub fn new() -> Self {
67+
//~^ default_mismatches_new
68+
Self
3969
}
4070
}
71+
72+
#[derive(Default)]
73+
struct WithDeriveTypeDefault;
74+
impl WithDeriveTypeDefault {
75+
pub fn new() -> Self {
76+
//~^ default_mismatches_new
77+
return crate::WithDeriveTypeDefault;
78+
}
79+
}
80+
81+
#[derive(Default)]
82+
struct WithDeriveIntDefault {
83+
value: i32,
84+
}
85+
impl WithDeriveIntDefault {
86+
pub fn new() -> Self {
87+
//~^ default_mismatches_new
88+
Self { value: 0 }
89+
}
90+
}
91+
92+
#[derive(Default)]
93+
struct WithDeriveTupleDefault(i32);
94+
impl WithDeriveTupleDefault {
95+
pub fn new() -> Self {
96+
//~^ default_mismatches_new
97+
Self(0)
98+
}
99+
}
100+
101+
#[derive(Default)]
102+
struct WithNonZeroDeriveDefault(i32);
103+
impl WithNonZeroDeriveDefault {
104+
pub fn new() -> Self {
105+
//~^ default_mismatches_new
106+
Self(42)
107+
}
108+
}
109+
110+
#[derive(Default)]
111+
struct WithExtraBlockDefault(i32);
112+
impl WithExtraBlockDefault {
113+
pub fn new() -> Self {
114+
//~^ default_mismatches_new
115+
{ Self::default() }
116+
}
117+
}
118+
119+
#[derive(Default)]
120+
struct WithExtraBlockRetDefault(i32);
121+
impl WithExtraBlockRetDefault {
122+
pub fn new() -> Self {
123+
//~^ default_mismatches_new
124+
{
125+
return Self::default();
126+
}
127+
}
128+
}
129+
130+
fn main() {}

0 commit comments

Comments
 (0)