Skip to content

Commit a8fd2c1

Browse files
WeiTheShinobiWeiTheShinobi
committed
lint struct_fields_rest_default only detect derive Default now
1 parent 0dab1e3 commit a8fd2c1

File tree

3 files changed

+98
-28
lines changed

3 files changed

+98
-28
lines changed

‎clippy_lints/src/struct_fields_rest_default.rs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
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
@@ -14,6 +16,10 @@ declare_clippy_lint! {
1416
/// Using `..*::default()` can hide field initialization when new fields are added to structs,
1517
/// potentially leading to bugs where developers forget to explicitly set values for new fields.
1618
///
19+
/// ### Limitations
20+
/// Only check the derive `Default`, because when developer manually implement `Default` or use other base value,
21+
/// it mean they know what they are doing rather than just being lazy.
22+
///
1723
/// ### Example
1824
/// ```no_run
1925
/// #[derive(Default)]
@@ -70,16 +76,17 @@ declare_lint_pass!(StructFieldsDefault => [STRUCT_FIELDS_REST_DEFAULT]);
7076
impl<'tcx> LateLintPass<'tcx> for StructFieldsDefault {
7177
fn check_expr(&mut self, cx: &rustc_lint::LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx>) {
7278
if !expr.span.in_external_macro(cx.sess().source_map())
73-
&& let ExprKind::Struct(_, _, StructTailExpr::Base(base)) = &expr.kind
79+
&& let ExprKind::Struct(QPath::Resolved(_, _), _, StructTailExpr::Base(base)) = &expr.kind
7480
&& let ExprKind::Call(func, _) = base.kind
7581
&& is_trait_item(cx, func, sym::Default)
82+
&& is_struct_trait_from_derive(cx, expr, sym::Default)
7683
{
7784
span_lint_and_help(
7885
cx,
7986
STRUCT_FIELDS_REST_DEFAULT,
8087
base.span,
8188
format!(
82-
"should not use `..{}` to omit rest of struct field initialization",
89+
"usage of `..{}` to initialize struct fields",
8390
snippet(cx, base.span, "")
8491
),
8592
Some(expr.span),
@@ -88,3 +95,25 @@ impl<'tcx> LateLintPass<'tcx> for StructFieldsDefault {
8895
}
8996
}
9097
}
98+
99+
fn is_struct_trait_from_derive<'tcx>(
100+
cx: &rustc_lint::LateContext<'tcx>,
101+
expr: &'tcx rustc_hir::Expr<'tcx>,
102+
trait_item: Symbol,
103+
) -> bool {
104+
if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(trait_item) {
105+
let impls = cx.tcx.trait_impls_of(default_trait_id);
106+
for (impl_ty, impl_def_ids) in impls.non_blanket_impls() {
107+
if let Some(impl_struct_def_id) = impl_ty.def()
108+
&& let ty::Adt(curr_struct, _) = cx.typeck_results().expr_ty(expr).kind()
109+
&& curr_struct.did() == impl_struct_def_id
110+
{
111+
// we found the struct what we need, skip the rest.
112+
return impl_def_ids
113+
.iter()
114+
.any(|&impl_def_id| cx.tcx.is_automatically_derived(impl_def_id));
115+
}
116+
}
117+
}
118+
false
119+
}

‎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)