Skip to content

Commit 7f63851

Browse files
committed
lint struct_fields_rest_default only detect derive Default now
1 parent 0dab1e3 commit 7f63851

File tree

3 files changed

+101
-29
lines changed

3 files changed

+101
-29
lines changed

clippy_lints/src/struct_fields_rest_default.rs

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,27 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
22
use clippy_utils::is_trait_item;
33
use clippy_utils::source::snippet;
4-
use rustc_hir::{ExprKind, StructTailExpr};
4+
5+
use rustc_hir::{ExprKind, QPath, StructTailExpr};
56
use rustc_lint::{LateLintPass, LintContext};
7+
use rustc_middle::ty;
68
use rustc_session::declare_lint_pass;
7-
use rustc_span::sym;
9+
use rustc_span::{Symbol, sym};
810

911
declare_clippy_lint! {
1012
/// ### What it does
11-
/// Check struct initialization uses `..*::default()` pattern to skip rest of struct field initialization.
13+
/// Check if struct initialization uses derive `Default` with `..*::default()` pattern
14+
/// to skip rest of struct field initialization.
1215
///
1316
/// ### Why restrict this?
1417
/// Using `..*::default()` can hide field initialization when new fields are added to structs,
1518
/// potentially leading to bugs where developers forget to explicitly set values for new fields.
1619
///
20+
/// ### Limitations
21+
/// Only check the derive `Default` with `..*::default()` pattern,
22+
/// because when developer manually implements `Default` or uses other base value,
23+
/// it means that they know what they are doing rather than just being lazy.
24+
///
1725
/// ### Example
1826
/// ```no_run
1927
/// #[derive(Default)]
@@ -70,16 +78,17 @@ declare_lint_pass!(StructFieldsDefault => [STRUCT_FIELDS_REST_DEFAULT]);
7078
impl<'tcx> LateLintPass<'tcx> for StructFieldsDefault {
7179
fn check_expr(&mut self, cx: &rustc_lint::LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
7280
if !expr.span.in_external_macro(cx.sess().source_map())
73-
&& let ExprKind::Struct(_, _, StructTailExpr::Base(base)) = &expr.kind
81+
&& let ExprKind::Struct(QPath::Resolved(_, _), _, StructTailExpr::Base(base)) = &expr.kind
7482
&& let ExprKind::Call(func, _) = base.kind
7583
&& is_trait_item(cx, func, sym::Default)
84+
&& is_struct_trait_from_derive(cx, expr, sym::Default)
7685
{
7786
span_lint_and_help(
7887
cx,
7988
STRUCT_FIELDS_REST_DEFAULT,
8089
base.span,
8190
format!(
82-
"should not use `..{}` to omit rest of struct field initialization",
91+
"usage of `..{}` to initialize struct fields",
8392
snippet(cx, base.span, "")
8493
),
8594
Some(expr.span),
@@ -88,3 +97,25 @@ impl<'tcx> LateLintPass<'tcx> for StructFieldsDefault {
8897
}
8998
}
9099
}
100+
101+
fn is_struct_trait_from_derive<'tcx>(
102+
cx: &rustc_lint::LateContext<'tcx>,
103+
expr: &'tcx rustc_hir::Expr<'tcx>,
104+
trait_item: Symbol,
105+
) -> bool {
106+
if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(trait_item) {
107+
let impls = cx.tcx.trait_impls_of(default_trait_id);
108+
for (impl_ty, impl_def_ids) in impls.non_blanket_impls() {
109+
if let Some(impl_struct_def_id) = impl_ty.def()
110+
&& let ty::Adt(curr_struct, _) = cx.typeck_results().expr_ty(expr).kind()
111+
&& curr_struct.did() == impl_struct_def_id
112+
{
113+
// we found the struct what we need, skip the rest.
114+
return impl_def_ids
115+
.iter()
116+
.any(|&impl_def_id| cx.tcx.is_automatically_derived(impl_def_id));
117+
}
118+
}
119+
}
120+
false
121+
}

tests/ui/struct_fields_rest_default.rs

Lines changed: 54 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,56 +3,97 @@
33
extern crate proc_macros;
44

55
#[derive(Default)]
6-
struct Foo {
6+
struct DeriveCase {
77
a: i32,
88
b: i32,
99
c: i32,
1010
}
1111

12-
impl Foo {
13-
fn get_foo() -> Self {
14-
Foo { a: 0, b: 0, c: 0 }
12+
impl DeriveCase {
13+
fn get_derive_case() -> Self {
14+
DeriveCase { a: 0, b: 0, c: 0 }
15+
}
16+
}
17+
18+
struct ManuallyCase {
19+
a: i32,
20+
b: i32,
21+
c: i32,
22+
}
23+
24+
impl Default for ManuallyCase {
25+
fn default() -> Self {
26+
ManuallyCase { a: 10, b: 20, c: 30 }
1527
}
1628
}
1729

1830
fn main() {
1931
#[rustfmt::skip]
20-
let _ = Foo {
32+
let _ = DeriveCase {
2133
a: 10,
2234
..Default::default()
2335
//~^ struct_fields_rest_default
2436
};
2537

2638
#[rustfmt::skip]
27-
let _ = Foo {
39+
let _ = DeriveCase {
2840
a: 10,
29-
..Foo::default()
41+
..DeriveCase::default()
3042
//~^ struct_fields_rest_default
3143
};
3244

3345
// should not lint
34-
#[rustfmt::skip]
35-
let _ = Foo {
46+
let _ = DeriveCase {
47+
a: 10,
48+
..DeriveCase::get_derive_case()
49+
};
50+
51+
// ------- ManuallyCase -------
52+
// should not lint manually default
53+
let _ = ManuallyCase {
54+
a: 10,
55+
..Default::default()
56+
};
57+
58+
// should not lint manually default
59+
let _ = ManuallyCase {
3660
a: 10,
37-
..Foo::get_foo()
61+
..ManuallyCase::default()
3862
};
63+
// ----------------------------
3964

4065
// should not lint in external macro
4166
proc_macros::external! {
4267
#[derive(Default)]
43-
struct ExternalDefault {
68+
struct ExternalDeriveCase {
4469
a: i32,
4570
b: i32,
4671
}
4772

48-
let _ = ExternalDefault {
73+
let _ = ExternalDeriveCase {
74+
a: 10,
75+
..Default::default()
76+
};
77+
78+
struct ExternalManuallyCase {
79+
a: i32,
80+
b: i32,
81+
}
82+
83+
impl Default for ExternalManuallyCase {
84+
fn default() -> Self {
85+
ExternalManuallyCase { a: 10, b: 20 }
86+
}
87+
}
88+
89+
let _ = ExternalManuallyCase {
4990
a: 10,
5091
..Default::default()
5192
};
5293
}
5394

5495
// should not lint
55-
let _ = Foo {
96+
let _ = DeriveCase {
5697
a: Default::default(),
5798
b: Default::default(),
5899
c: Default::default(),

tests/ui/struct_fields_rest_default.stderr

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
error: should not use `..Default::default()` to omit rest of struct field initialization
2-
--> tests/ui/struct_fields_rest_default.rs:22:11
1+
error: usage of `..Default::default()` to initialize struct fields
2+
--> tests/ui/struct_fields_rest_default.rs:34:11
33
|
44
LL | ..Default::default()
55
| ^^^^^^^^^^^^^^^^^^
66
|
77
help: explicitly specify all fields or use other base value instead of `..*::default()`
8-
--> tests/ui/struct_fields_rest_default.rs:20:13
8+
--> tests/ui/struct_fields_rest_default.rs:32:13
99
|
10-
LL | let _ = Foo {
10+
LL | let _ = DeriveCase {
1111
| _____________^
1212
LL | | a: 10,
1313
LL | | ..Default::default()
@@ -17,19 +17,19 @@ LL | | };
1717
= note: `-D clippy::struct-fields-rest-default` implied by `-D warnings`
1818
= help: to override `-D warnings` add `#[allow(clippy::struct_fields_rest_default)]`
1919

20-
error: should not use `..Foo::default()` to omit rest of struct field initialization
21-
--> tests/ui/struct_fields_rest_default.rs:29:11
20+
error: usage of `..DeriveCase::default()` to initialize struct fields
21+
--> tests/ui/struct_fields_rest_default.rs:41:11
2222
|
23-
LL | ..Foo::default()
24-
| ^^^^^^^^^^^^^^
23+
LL | ..DeriveCase::default()
24+
| ^^^^^^^^^^^^^^^^^^^^^
2525
|
2626
help: explicitly specify all fields or use other base value instead of `..*::default()`
27-
--> tests/ui/struct_fields_rest_default.rs:27:13
27+
--> tests/ui/struct_fields_rest_default.rs:39:13
2828
|
29-
LL | let _ = Foo {
29+
LL | let _ = DeriveCase {
3030
| _____________^
3131
LL | | a: 10,
32-
LL | | ..Foo::default()
32+
LL | | ..DeriveCase::default()
3333
LL | |
3434
LL | | };
3535
| |_____^

0 commit comments

Comments
 (0)